8
\$\begingroup\$

I'm currently writing a mesh generator class, and I'm making a method that generates vertices, normals and texcoords for a cube. But, as you can see, it's incredibly complicated and hard to follow. Is anyone willing to help me making this shorter?

public static void GenerateCube(float size, out Vector3[] vertices, out Vector2[] texCoords, out Vector3[] normals)
{
    List<Vector3> vertexList = new List<Vector3>();
    List<Vector2> texCoordList = new List<Vector2>();
    List<Vector3> normalList = new List<Vector3>();

    for (int i = 0; i <= 2; i++)
    {
        Vector3 normal = i == 0 ? Vector3.UnitZ : (i == 1 ? -Vector3.UnitY : Vector3.UnitX);

        float ninety = (float)(Math.PI / 2);
        Matrix4 matrix = i == 0 ? Matrix4.Identity : (i == 1 ? Matrix4.CreateRotationX(ninety) : Matrix4.CreateRotationY(ninety));

        Vector3 vertex1a = new Vector3(-1, -1, 1);
        Vector3 vertex2a = new Vector3(-1, 1, 1);
        Vector3 vertex3a = new Vector3(1, 1, 1);
        Vector3 vertex4a = new Vector3(1, -1, 1);

        Vector3 vertex1b = new Vector3(-1, -1, 1);
        Vector3 vertex2b = new Vector3(1, -1, 1);
        Vector3 vertex3b = new Vector3(1, 1, 1);
        Vector3 vertex4b = new Vector3(-1, 1, 1);

        vertex1a = Vector3.Transform(vertex1a, matrix);
        vertex2a = Vector3.Transform(vertex2a, matrix);
        vertex3a = Vector3.Transform(vertex3a, matrix);
        vertex4a = Vector3.Transform(vertex4a, matrix);

        vertex1b = Vector3.Transform(vertex1b, matrix);
        vertex2b = Vector3.Transform(vertex2b, matrix);
        vertex3b = Vector3.Transform(vertex3b, matrix);
        vertex4b = Vector3.Transform(vertex4b, matrix);

        for (int j = -1; j <= 1; j += 2)
        {
            Vector3 vertex1, vertex2, vertex3, vertex4;
            if (j == -1)
            {
                vertex1 = vertex1a;
                vertex2 = vertex2a;
                vertex3 = vertex3a;
                vertex4 = vertex4a;
            }
            else
            {
                vertex1 = vertex1b;
                vertex2 = vertex2b;
                vertex3 = vertex3b;
                vertex4 = vertex4b;
            }

            //Triangle 1
            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(0, 0));
            vertexList.Add(vertex1 * j);

            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(0, 1));
            vertexList.Add(vertex2 * j);

            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(1, 1));
            vertexList.Add(vertex3 * j);

            //Triangle 2
            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(0, 0));
            vertexList.Add(vertex1 * j);

            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(1, 1));
            vertexList.Add(vertex3 * j);

            normalList.Add(normal * j);
            texCoordList.Add(new Vector2(1, 0));
            vertexList.Add(vertex4 * j);
        }
    }

    vertices = vertexList.ToArray();
    texCoords = texCoordList.ToArray();
    normals = normalList.ToArray();

    for (int i = 0; i < vertices.Length; i++)
        vertices[i] *= size;
}
\$\endgroup\$
3
  • 3
    \$\begingroup\$ Well any time you see repeated code like that, it almost always means that you should throw that in a function. \$\endgroup\$ Oct 1 '11 at 23:34
  • \$\begingroup\$ p.s., What exactly is a Matrix4? I'm assuming this is an XNA project and Matrix4 is an alias for the Matrix type. \$\endgroup\$ Oct 2 '11 at 0:25
  • \$\begingroup\$ It's an OpenTK project, a Matrix4 is simply a class holding 16 floats in a 4x4 matrix. \$\endgroup\$
    – Dlaor
    Oct 2 '11 at 14:37
3
\$\begingroup\$

Im not familiar with XNA or have any experience working with matrixes, vectors and that kind of stuff. But i've made som basic refactorings to make it abit easier to read and maintain.

public static void GenerateCube(float size, out Vector3[] vertices, out Vector2[] texCoords, out Vector3[] normals)
{
        var vertexList   = new List<Vector3>();
        var texCoordList = new List<Vector2>();
        var normalList   = new List<Vector3>();

        const float ninety = (float)(Math.PI / 2);

        for (var i = 0; i <= 2; i++)
        {
            var normal = CreateVector(i);
            var matrix = CreateMatrix(i, ninety);

            var vectorsA = CreateVectorsA();
            var vectorsB = CreateVectorsB();

            vectorsA.ForEach(x => Vector3.Transform(x, matrix));
            vectorsB.ForEach(x => Vector3.Transform(x, matrix));

            for (var j = -1; j <= 1; j += 2)
            {
                var vectors = j == -1 ? vectorsA.ToArray() : vectorsB.ToArray();

                var someCalculations = CalculateSomething(j, normal);
                normalList.AddRange(someCalculations);

                AddTextureCoordinates(texCoordList);
                AddVertexes(j, vertexList, vectors);
            }
        }

        vertices  = vertexList.ToArray();
        texCoords = texCoordList.ToArray();
        normals   = normalList.ToArray();

        SetVectorSize(vertices, size);
    }

    private static void SetVectorSize(IList<Vector3> vertices, float size)
    {
        for (var i = 0; i < vertices.Count; i++)
        {
            vertices[i] *= size;
        }
    }

    private static Vector3 CreateVector(int i)
    {
        return i == 0 ? Vector3.UnitZ : (i == 1 ? Vector3.UnitY : Vector3.UnitX);
    }

    private static Matrix4 CreateMatrix(int i, float ninety)
    {
        return i == 0 ? Matrix4.Identity : (i == 1 ? Matrix4.CreateRotationX(ninety) : Matrix4.CreateRotationY(ninety));
    }

    private static void AddVertexes(int j, ICollection<Vector3> vertexList, IList<Vector3> vectorsArray)
    {
        //Triangle 1
        vertexList.Add(vectorsArray[0] * j);
        vertexList.Add(vectorsArray[1] * j);
        vertexList.Add(vectorsArray[2] * j);

        //Triangle 2
        vertexList.Add(vectorsArray[0] * j);
        vertexList.Add(vectorsArray[2] * j);
        vertexList.Add(vectorsArray[3] * j);
    }

    private static void AddTextureCoordinates(ICollection<Vector2> texCoordList)
    {
        //Triangle 1
        texCoordList.Add(new Vector2(0, 0));
        texCoordList.Add(new Vector2(0, 1));
        texCoordList.Add(new Vector2(1, 1));

        //Triangle 2
        texCoordList.Add(new Vector2(0, 0));
        texCoordList.Add(new Vector2(1, 1));
        texCoordList.Add(new Vector2(1, 0));
    }

    private static IEnumerable<Vector3> CalculateSomething(int j, Vector3 normal)
    {
        var someCalculation = normal * j;
        return Enumerable.Repeat(someCalculation, 6);
    }

    private static List<Vector3> CreateVectorsB()
    {
        return new List<Vector3>
                   {
                       new Vector3(-1, -1, 1),
                       new Vector3(1, -1, 1),
                       new Vector3(1, 1, 1),
                       new Vector3(-1, 1, 1)
                   };
    }

    private static List<Vector3> CreateVectorsA()
    {
        return new List<Vector3>
                   {
                       new Vector3(-1, -1, 1),
                       new Vector3(-1, 1, 1),
                       new Vector3(1, 1, 1),
                       new Vector3(1, -1, 1)
                   };
    }
\$\endgroup\$
1
  • \$\begingroup\$ I'd like to make a suggestion that you use the rule of three. The var j=-1 .. j+=2 could be written flat with some comments. It only iterates twice and (IMHO) seems unnecessary. +1 still great refactoring here. \$\endgroup\$ Oct 4 '11 at 6:07
1
\$\begingroup\$

You should consider creating a Triangle3D and Rectangle3D classes with associated operations like .Transform(Matrix4 matrix). Work with these higher level classes.

ninety should be a global member of the class and a const.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.