2
\$\begingroup\$

This code is a mess due the complex nature of the job, so I'd like to know your opinions. The concept is very simple if you know what this is about, but there are some things I'm not being able to refactor..

Examples of what I want as an answer: Macroing of repetitive tasks? Declaring pointers in stack? But I also want to keep it as clear as possible, and I don't think want to declare a macro or a pointer just for using it only 2 times.

The major problem I found for refactoring (as muchs as I'd like to) is that even being so similiar the code for each tile orientation, they are a still different.

int32_t ropp::BuildTerrainGeometryFromGND( const GND_TILE_DATA* lstTileData, uint32_t nTileCount,
            const GND_ATTRIBUTE_DATA* pAttributeData, int32_t nGNDAttributeCount, uint32_t nTextureCount,
            float fHeightScale,
            uint32_t nColumnSubgrids, uint32_t nRowSubgrids,
            uint32_t nSubgridColumnTiles, uint32_t nSubgridRowTiles, 
            Vector3* out_pVertices, uint32_t* out_nVertexCount, void* out_pIndices, bool* b32Bit,
            uint32_t* out_nIndexCount, Vector2* out_pTexCoord, Vector3* out_pNormals, 
            uint32_t* out_pAttributes, uint32_t* out_nMeshAttributeCount, 
            MESH_ATTRIBUTE_DATA* out_pAttributeData, GND_TILE_INDICES* out_lstTileIndices,
            MESH_NODE_DATA* out_pNodeData, uint32_t* nTopTileCount, 
            uint32_t* nFrontTileCount, uint32_t* nRightTileCount )// node count is nRowSubgrids*nColumnSubgrids
{
    const GND_TILE_DATA * pTile = 0, * pTileF = 0, * pTileR = 0;
    const GND_ATTRIBUTE_DATA* pAttr=0;

    GND_TILE_INDICES* pTileIndices=0;

    if( 0 != out_pNormals )
    {
        if( 0 != out_lstTileIndices )
            pTileIndices = out_lstTileIndices;
        else
            pTileIndices = (GND_TILE_INDICES*)malloc( sizeof( GND_TILE_INDICES )*nTileCount );
        if( pTileIndices )
            memset( pTileIndices, 0, sizeof(GND_TILE_INDICES)*nTileCount );
    }

    uint32_t nTotalWidth = nColumnSubgrids*nSubgridColumnTiles;
    uint32_t nTotalDepth = nRowSubgrids*nSubgridRowTiles;

    bool bCountOnly = false;
    if( 0 == out_pVertices )
        bCountOnly = true;
    if( false == bCountOnly )
    {
        if( 0 == out_pIndices )
            return -1;
    }
    if( 0 == nRowSubgrids || 0 == nColumnSubgrids || 0 == nSubgridColumnTiles || 0 == nSubgridRowTiles )
        return -1;
    if( 0 == nTextureCount || 0 == pAttributeData || 0 == lstTileData || 0 == nTileCount )
        return -1;

    uint32_t nTileOffsetX, nTileOffsetZ;
    // i: local row, j: local column, k: TextureIndex, rs: row subgrid, cs: column subgrid
    uint32_t i=0, j=0, k=0, cs=0, rs=0, nGlobalTileIndex=0;
    uint32_t nVertexCount=0, nIndexCount=0, nTriangleCount=0, nAttributeCount=0;
    uint32_t nAttrID, nVertexStart, nAttrVertexCount, nFaceStart, nFaceCount;
    if( out_pNormals )
        memset( out_pNormals, 0, sizeof( Vector3 )*(*out_nVertexCount) );
    uint32_t nGlobalTileX, nGlobalTileZ;

    Vector3 vFinalOffset = Vector3( nTotalWidth*fHeightScale*-.5f, 0.0f, nTotalDepth*fHeightScale*-.5f ); 
    uint32_t nSubgridIndex = 0;

    Vector3 * out_Vert0, * out_Vert1, * out_Vert2, * out_Vert3;
    Vector3 * pvMax, * pvMin;
    for( rs=0; rs<nRowSubgrids; rs++ )
    {
        for( cs=0; cs<nColumnSubgrids; cs++ )
        {
            nSubgridIndex = (rs*nColumnSubgrids)+cs;
            nTileOffsetX = (nSubgridColumnTiles)*cs;
            nTileOffsetZ = (nSubgridRowTiles)*rs;
            if( out_pNodeData )
            {
                pvMax = &out_pNodeData[nSubgridIndex].BoundingVolume.vMax;
                pvMin = &out_pNodeData[nSubgridIndex].BoundingVolume.vMin;
                pvMin->x = (nTileOffsetX)*fHeightScale;
                pvMin->z = (nTileOffsetZ)*fHeightScale;
                pvMax->x = (nTileOffsetX+nSubgridColumnTiles)*fHeightScale;
                pvMax->z = (nTileOffsetZ+nSubgridRowTiles)*fHeightScale;
            }
            for( k=0; k<nTextureCount; k++ )
            {
                if( out_pAttributeData )
                {
                    if( nAttributeCount < *out_nMeshAttributeCount )
                    {
                        out_pAttributeData[nAttributeCount].nAttributeID    = nAttributeCount;
                        out_pAttributeData[nAttributeCount].nTextureIndex      = k;
                        out_pAttributeData[nAttributeCount].nVertexStart    = nVertexCount;
                        out_pAttributeData[nAttributeCount].nTriangleStart    = nTriangleCount;
                    }
                }
                else
                {
                    nAttrID      = nAttributeCount;
                    nFaceStart        = nTriangleCount;
                    nVertexStart    = nVertexCount;
                }

                for( i=0; i<nSubgridRowTiles; i++ )
                {
                    for( j=0; j<nSubgridColumnTiles; j++ )
                    {
                        nGlobalTileIndex    = (nTileOffsetZ+i)*nTotalWidth+j+nTileOffsetX;;
                        nGlobalTileX        = (j + nTileOffsetX);
                        nGlobalTileZ        = (i + nTileOffsetZ);
                        //nGlobalTileX  = nGlobalTileIndex % nTotalWidth;
                        //nGlobalTileZ  = nGlobalTileIndex / nTotalWidth;

                        pTile = &lstTileData[nGlobalTileIndex];
                        pTileF = GetFrontTile( lstTileData, nGlobalTileIndex, nTotalWidth, nTotalDepth );
                        pTileR = GetRightTile( lstTileData, nGlobalTileIndex, nTotalWidth, nTotalDepth );

                        if( ( pTile->TopAttributeIndex >= 0 ) 
                            &&  ( pTile->TopAttributeIndex < nGNDAttributeCount ) 
                            && k == pAttributeData[pTile->TopAttributeIndex].nTextureIndex )
                        {
                            if( false == bCountOnly )
                            {
                                pAttr = &pAttributeData[pTile->TopAttributeIndex];
                                out_Vert0 = &out_pVertices[nVertexCount+0];
                                out_Vert1 = &out_pVertices[nVertexCount+1];
                                out_Vert2 = &out_pVertices[nVertexCount+2];
                                out_Vert3 = &out_pVertices[nVertexCount+3];
                                (*out_Vert0) = Vector3( (nGlobalTileX)*fHeightScale,    -pTile->fHeight[0], (nGlobalTileZ)*fHeightScale );
                                (*out_Vert1) = Vector3( (nGlobalTileX+1)*fHeightScale,  -pTile->fHeight[1], (nGlobalTileZ)*fHeightScale );
                                (*out_Vert2) = Vector3( (nGlobalTileX)*fHeightScale,    -pTile->fHeight[2], (nGlobalTileZ+1)*fHeightScale );
                                (*out_Vert3) = Vector3( (nGlobalTileX+1)*fHeightScale,  -pTile->fHeight[3], (nGlobalTileZ+1)*fHeightScale );
                                (*out_Vert0) += vFinalOffset;
                                (*out_Vert1) += vFinalOffset;
                                (*out_Vert2) += vFinalOffset;
                                (*out_Vert3) += vFinalOffset;
                                if( out_pNodeData )
                                {
                                    pvMax->y = max( pvMax->y, out_Vert0->y ); pvMin->y = min( pvMin->y, out_Vert0->y );
                                    pvMax->y = max( pvMax->y, out_Vert1->y ); pvMin->y = min( pvMin->y, out_Vert1->y );
                                    pvMax->y = max( pvMax->y, out_Vert2->y ); pvMin->y = min( pvMin->y, out_Vert2->y );
                                    pvMax->y = max( pvMax->y, out_Vert3->y ); pvMin->y = min( pvMin->y, out_Vert3->y );
                                }
                                AssignIndices( nIndexCount, nVertexCount, out_pIndices, *b32Bit );
                                AssignTexCoord( nVertexCount, pAttr, out_pTexCoord );

                                if( out_pNormals )
                                {
                                    out_pNormals[nVertexCount+0] = 
                                    out_pNormals[nVertexCount+1] = 
                                    out_pNormals[nVertexCount+2] = 
                                    out_pNormals[nVertexCount+3] = Vector3( 0.0f, 1.0f, 0.0f );
                                }
                                if( out_pNormals || out_lstTileIndices )
                                {
                                    pTileIndices[nGlobalTileIndex].VerticesTop[0] = nVertexCount+0;
                                    pTileIndices[nGlobalTileIndex].VerticesTop[1] = nVertexCount+1;
                                    pTileIndices[nGlobalTileIndex].VerticesTop[2] = nVertexCount+2;
                                    pTileIndices[nGlobalTileIndex].VerticesTop[3] = nVertexCount+3;
                                }
                                // set attribute
                                if( out_pAttributes )
                                {
                                    out_pAttributes[nTriangleCount]  = nAttributeCount;
                                    out_pAttributes[nTriangleCount+1]      = nAttributeCount;
                                }
                            }
                            nVertexCount    += 4;
                            nIndexCount      += 6;
                            nTriangleCount  += 2;
                            if( 0 != nTopTileCount )
                                (*nTopTileCount)++;
                        }
                        else if( -1 == pTile->TopAttributeIndex
                            && ( out_pNormals || out_lstTileIndices ) )
                        {
                            pTileIndices[nGlobalTileIndex].VerticesTop[0] = 
                            pTileIndices[nGlobalTileIndex].VerticesTop[1] = 
                            pTileIndices[nGlobalTileIndex].VerticesTop[2] = 
                            pTileIndices[nGlobalTileIndex].VerticesTop[3] = -1;
                        }

                        if( ( pTile->FrontAttributeIndex >= 0 ) 
                            &&  ( pTile->FrontAttributeIndex < nGNDAttributeCount ) 
                            && k == pAttributeData[pTile->FrontAttributeIndex].nTextureIndex )
                        {
                            if( false == bCountOnly )
                            {
                                pAttr = &pAttributeData[pTile->FrontAttributeIndex];
                                out_Vert0 = &out_pVertices[nVertexCount+0];
                                out_Vert1 = &out_pVertices[nVertexCount+1];
                                out_Vert2 = &out_pVertices[nVertexCount+2];
                                out_Vert3 = &out_pVertices[nVertexCount+3];                              
                                (*out_Vert0) = Vector3( (nGlobalTileX+1)*fHeightScale, -pTile->fHeight[3], (nGlobalTileZ+1)*fHeightScale );
                                (*out_Vert1) = Vector3( (nGlobalTileX+1)*fHeightScale, -pTile->fHeight[1], (nGlobalTileZ)*fHeightScale );
                                if( pTileF )
                                {
                                    (*out_Vert2) = Vector3( (nGlobalTileX+1)*fHeightScale, -pTileF->fHeight[2], (nGlobalTileZ+1)*fHeightScale );
                                    (*out_Vert3) = Vector3( (nGlobalTileX+1)*fHeightScale, -pTileF->fHeight[0], (nGlobalTileZ)*fHeightScale );
                                }
                                else
                                {
                                    (*out_Vert2) = Vector3( (nGlobalTileX+1)*fHeightScale, -0, (nGlobalTileZ+1)*fHeightScale );
                                    (*out_Vert3) = Vector3( (nGlobalTileX+1)*fHeightScale, -0, (nGlobalTileZ)*fHeightScale );
                                }
                                (*out_Vert0) += vFinalOffset;
                                (*out_Vert1) += vFinalOffset;
                                (*out_Vert2) += vFinalOffset;
                                (*out_Vert3) += vFinalOffset;

                                AssignIndices( nIndexCount, nVertexCount, out_pIndices, *b32Bit );
                                AssignTexCoord( nVertexCount, pAttr, out_pTexCoord );
                                if( out_pNormals )
                                {
                                    if( (pTile->fHeight[1]*fHeightScale) 
                                        > ( (0 != pTileF) ? (pTileF->fHeight[0]*fHeightScale) : 0 ) 
                                        )
                                    {
                                        out_pNormals[nVertexCount+0] = 
                                        out_pNormals[nVertexCount+1] = 
                                        out_pNormals[nVertexCount+2] = 
                                        out_pNormals[nVertexCount+3] = Vector3( -1.0f, 0.0f, 0.0f );
                                    }
                                    else
                                    {
                                        out_pNormals[nVertexCount+0] = 
                                        out_pNormals[nVertexCount+1] = 
                                        out_pNormals[nVertexCount+2] = 
                                        out_pNormals[nVertexCount+3] = Vector3( 1.0f, 0.0f, 0.0f );
                                    }
                                }

                                if( out_lstTileIndices )
                                {
                                    pTileIndices[nGlobalTileIndex].VerticesFront[0] = nVertexCount+0;
                                    pTileIndices[nGlobalTileIndex].VerticesFront[1] = nVertexCount+1;
                                    pTileIndices[nGlobalTileIndex].VerticesFront[2] = nVertexCount+2;
                                    pTileIndices[nGlobalTileIndex].VerticesFront[3] = nVertexCount+3;
                                }

                                // set attribute
                                if( out_pAttributes )
                                {
                                    out_pAttributes[nTriangleCount]  = nAttributeCount;
                                    out_pAttributes[nTriangleCount+1]      = nAttributeCount;
                                }
                            }
                            nVertexCount    += 4;
                            nIndexCount      += 6;
                            nTriangleCount  += 2;
                            if( 0 != nFrontTileCount )
                                (*nFrontTileCount)++;
                        }
                        else if( -1 == pTile->FrontAttributeIndex )
                        {
                            if( out_lstTileIndices )
                            {
                                pTileIndices[nGlobalTileIndex].VerticesFront[0] = 
                                pTileIndices[nGlobalTileIndex].VerticesFront[1] = 
                                pTileIndices[nGlobalTileIndex].VerticesFront[2] = 
                                pTileIndices[nGlobalTileIndex].VerticesFront[3] = -1;
                            }
                        }
                        if( ( pTile->RightAttributeIndex >= 0 ) 
                            &&  ( pTile->RightAttributeIndex < nGNDAttributeCount ) 
                            && k == pAttributeData[pTile->RightAttributeIndex].nTextureIndex )
                        {
                            if( false == bCountOnly )
                            {
                                pAttr = &pAttributeData[pTile->RightAttributeIndex];
                                out_Vert0 = &out_pVertices[nVertexCount+0];
                                out_Vert1 = &out_pVertices[nVertexCount+1];
                                out_Vert2 = &out_pVertices[nVertexCount+2];
                                out_Vert3 = &out_pVertices[nVertexCount+3];
                                (*out_Vert0) = Vector3( (nGlobalTileX)*fHeightScale,  -pTile->fHeight[2], (nGlobalTileZ+1)*fHeightScale );
                                (*out_Vert1) = Vector3( (nGlobalTileX+1)*fHeightScale,-pTile->fHeight[3], (nGlobalTileZ+1)*fHeightScale );
                                if( pTileR )
                                {
                                    (*out_Vert2) = Vector3( (nGlobalTileX)*fHeightScale,  -pTileR->fHeight[0], (nGlobalTileZ+1)*fHeightScale );
                                    (*out_Vert3) = Vector3( (nGlobalTileX+1)*fHeightScale,-pTileR->fHeight[1], (nGlobalTileZ+1)*fHeightScale );
                                }
                                else
                                {
                                    (*out_Vert2) = Vector3( (nGlobalTileX)*fHeightScale,    -0, (nGlobalTileZ+1)*fHeightScale );
                                    (*out_Vert3) = Vector3( (nGlobalTileX+1)*fHeightScale,  -0, (nGlobalTileZ+1)*fHeightScale );
                                }
                                (*out_Vert0) += vFinalOffset;
                                (*out_Vert1) += vFinalOffset;
                                (*out_Vert2) += vFinalOffset;
                                (*out_Vert3) += vFinalOffset;

                                AssignIndices( nIndexCount, nVertexCount, out_pIndices, *b32Bit );
                                AssignTexCoord( nVertexCount, pAttr, out_pTexCoord );

                                if( out_pNormals )
                                {
                                    if( (pTile->fHeight[2]) 
                                        > ( (0 != pTileR) ? (pTileR->fHeight[0]) : 0)
                                        )
                                    {
                                        out_pNormals[nVertexCount+0] = 
                                        out_pNormals[nVertexCount+1] = 
                                        out_pNormals[nVertexCount+2] = 
                                        out_pNormals[nVertexCount+3] = Vector3( 0.0f, 0.0f, -1.0f );
                                    }
                                    else
                                    {
                                        out_pNormals[nVertexCount+0] = 
                                        out_pNormals[nVertexCount+1] = 
                                        out_pNormals[nVertexCount+2] = 
                                        out_pNormals[nVertexCount+3] = Vector3( 0.0f, 0.0f, 1.0f );
                                    }
                                }
                                if( out_lstTileIndices )
                                {
                                    pTileIndices[nGlobalTileIndex].VerticesRight[0] = nVertexCount+0;
                                    pTileIndices[nGlobalTileIndex].VerticesRight[1] = nVertexCount+1;
                                    pTileIndices[nGlobalTileIndex].VerticesRight[2] = nVertexCount+2;
                                    pTileIndices[nGlobalTileIndex].VerticesRight[3] = nVertexCount+3;
                                }

                                // set attribute
                                if( out_pAttributes )
                                {
                                    out_pAttributes[nTriangleCount]  = nAttributeCount;
                                    out_pAttributes[nTriangleCount+1]      = nAttributeCount;
                                }
                            }
                            nVertexCount    += 4;
                            nIndexCount      += 6;
                            nTriangleCount  += 2;
                            if( 0 != nRightTileCount )
                                (*nRightTileCount)++;
                        } // 
                        else if( -1 == pTile->RightAttributeIndex )
                        {
                            if( out_lstTileIndices )
                            {
                                pTileIndices[nGlobalTileIndex].VerticesRight[0] = 
                                pTileIndices[nGlobalTileIndex].VerticesRight[1] = 
                                pTileIndices[nGlobalTileIndex].VerticesRight[2] = 
                                pTileIndices[nGlobalTileIndex].VerticesRight[3] = -1;
                            }
                        }
                    } // j=0;
                } // i=0;
                if( out_pAttributeData )
                {
                    if( nAttributeCount < *out_nMeshAttributeCount )
                    {
                        out_pAttributeData[nAttributeCount].nVertexCount = nVertexCount - out_pAttributeData[nAttributeCount].nVertexStart;
                        out_pAttributeData[nAttributeCount].nTriangleCount = nTriangleCount - out_pAttributeData[nAttributeCount].nTriangleStart;
                        if( out_pAttributeData[nAttributeCount].nVertexCount > 0 )
                        {
                            if( out_pNodeData )
                            {
                                out_pNodeData[nSubgridIndex].lstAttributeIndices
                                    [out_pNodeData[nSubgridIndex].nAttributeCount] = nAttributeCount;
                                out_pNodeData[nSubgridIndex].nAttributeCount++;
                                if( out_pNodeData[nSubgridIndex].nAttributeCount == 64 )
                                    throw( "" );
                            }
                            nAttributeCount++;
                        }
                        // we skip empty attributes
                    }
                }
                else
                {
                    nFaceCount = nTriangleCount - nFaceStart;
                    nAttrVertexCount = nVertexCount - nVertexStart;
                    if( nAttrVertexCount > 0 )
                        nAttributeCount++;
                }
            } // k=0
            if( false == bCountOnly && out_pNodeData )
            {
                out_pNodeData[nSubgridIndex].BoundingVolume.vCenter = (*pvMin)+( ((*pvMax)-(*pvMin))*.5f );
            }
        } // cs=0;
    }// rs=0;
    if( nVertexCount > 0xFFFF )
        *b32Bit = true;
    else 
        *b32Bit = false;

    if( (false == bCountOnly) && out_pNormals )
        CalcNormals( out_pVertices, pTileIndices, lstTileData, nTotalWidth, nTotalDepth, fHeightScale, out_pNormals );

    *out_nVertexCount      = nVertexCount;
    *out_nIndexCount    = nIndexCount;
    if( out_nMeshAttributeCount )
        *out_nMeshAttributeCount = nAttributeCount;

    if( 0 != out_pNormals && 0 == out_lstTileIndices )
        free( pTileIndices );

    return 0;
};
\$\endgroup\$
  • \$\begingroup\$ So what is the question? "The concept is very simple if you know what it is about."..so, what is it about? \$\endgroup\$ – Bart Mar 4 '12 at 21:26
  • \$\begingroup\$ Building 3d geometry from tile description, creating VBOs on-the-fly \$\endgroup\$ – Pablo Ariel Mar 4 '12 at 21:28
  • 1
    \$\begingroup\$ What is a 'tile description'? Perhaps you could post an image of what the end result is so people can draw on their experiences with similar problems. \$\endgroup\$ – Marm0t Mar 4 '12 at 22:46
  • 1
    \$\begingroup\$ @PabloAriel, if your concern is that the code is too long to post (fair enough), then let's narrow it down. Pick a specific part of the code you want a review on. Is the particular part of the code that you really want review on? For example, you could get a review just on the CalcNormals function. If there is another part you are more interested in, then go with that. \$\endgroup\$ – Winston Ewert Mar 5 '12 at 23:46
  • 2
    \$\begingroup\$ I'm taking a look at your code. It appears to be in C++ but mostly using C idioms. Is this a deliberate choice?, if so: why? Your code can be much simplified by using some C++ idioms and the standard template library, but I need to know if you've got some good reason to avoid them. \$\endgroup\$ – Winston Ewert Mar 6 '12 at 2:07
6
\$\begingroup\$

Turn on your compiler warnings: (Fix this first)

Even at the most basic warning level I get a whole bunch of warnings. Personally I compile at a much higher warning level then basic and then I get two pages of warning messages.

They may be called warnings but really they are logical errors in your code. You really should fix them all (or at least address them to make sure the code works correctly). Personally I always tell the compiler to treat warnings as errors thus it will fail to compile unless I fix them.

Major Comments on code: (Fix this second)

Refactor the code so you do not have functions that are 500 lines long.
Ideally (not always achievable) one screen is a good rule of thumb. Then you should be able to see the whole function in a glance.

Second Major comment: (Fix this third)

You tagged your question as C but your file is *.cpp (which implies C++) and you are using namespace.

using namespace ropp;

So please pick a language and stick to it. Intermixing the two is really bad idea. Looking at your code you are definitely still writing C you just happen to be using some C++ features which makes it impossible to compile with a C compiler and completely horrible C++. This is commonly refereed to as C with Classes (a zone of badly written C++).

  1. Learn to write only C
  2. Or. Learn how to use C++ which probably means learning OO and the idioms associated with it.

Normal Comments: (Now start looking at these).

This is a bad idea.

using namespace ropp;

You are polluting the global namespace. It is best to prefix types and object with their appropriate namespace or selectively bring into the current scope just the bits you need.

Code that look like this:

    if( <TEST> )
            return <VALUE-1>;
    return <Value-2>;

In my opinion is easier to read and write as:

    return <TEST> ? <Value-1> : <Value-2>;

I could go on. But I think your first task to better organize your code so that it is readable. After that we can go into how it to make it better but you are a long long way from being able to do anything useful.

Yoda conditionals:

if( false == bCountOnly )

These were in style about 10 years ago. They are no longer in style (thank god) and some (like me (though not everybody)) consider them bad practice. I think it makes the code harder to read. They may have given you a slight protection against accidental assignment but the compiler already does that. Just make sure your code compiler with no warnings and you will have better code anyway.

\$\endgroup\$
  • \$\begingroup\$ Yea I could easily do that with python/java then my realtime app will easily take an hour to load a map chunk which my implementation does instantly. I could easily rename the variables so people who read the code don't understand which index references which vector. I managed to cut in half the character count that way, but it became unreadable. Your answer is so useless that I can't believe how you got that rep. I've been refactoring for long, that's why I'm here, and I'm downvoting you ASAP. \$\endgroup\$ – Pablo Ariel Mar 4 '12 at 21:44
  • \$\begingroup\$ @PabloAriel: I think I will survive you down vote. But I stand by answer. The best thing you can do for your code is refactor to make it readable (in my opinion its practically unmaintainable). You asked for a review of you code; that's what I gave you, do not shoot the messenger because you don't like the advice. PS. I am not sure how the first 3/4 of your comment is relevant or even meaningful (seems like meaningless trolling). \$\endgroup\$ – Martin York Mar 4 '12 at 21:55
  • \$\begingroup\$ I don't think you're understanding, again, I'm asking how to refactor this more than I already did. \$\endgroup\$ – Pablo Ariel Mar 4 '12 at 21:56
  • \$\begingroup\$ Without losing performance! If you don't understand the meaning then you don't have a clue about performance coding I suppose. \$\endgroup\$ – Pablo Ariel Mar 4 '12 at 21:57
  • 2
    \$\begingroup\$ You might want to try being less abrasive. \$\endgroup\$ – Bart Mar 4 '12 at 22:00
4
\$\begingroup\$

The first thing you need is a enum to specify the different direction

enum TileOrientation
{
    TOP,
    FRONT,
    RIGHT,

    ORIENTATION_COUNT
}

Right now you have code like:

pTile->FrontAttributeIndex

Change your Tile struct so that you can do

pTile->AttributeIndex[FRONT]

Create a function that configures that out parameters. In fact everywhere you have different variables for the different orientations, make them into an array.

void generateVertices(int nGlobalTileX, int nGlobalTileZ, float fHeightScale, GND_TILE_DATA ** pTile, Vector3 * pVertex, TileOrientation orientation)
{
    switch(orientation)
    {
        case TOP:
            pVertex[0] = Vector3( (nGlobalTileX)*fHeightScale,    -pTile[TOP]->fHeight[0], (nGlobalTileZ)*fHeightScale );
            pVertex[1] = Vector3( (nGlobalTileX+1)*fHeightScale,  -pTile[TOP]->fHeight[1], (nGlobalTileZ)*fHeightScale );
            pVertex[2] = Vector3( (nGlobalTileX)*fHeightScale,    -pTile[TOP]->fHeight[2], (nGlobalTileZ+1)*fHeightScale );
            pVertex[3] = Vector3( (nGlobalTileX+1)*fHeightScale,  -pTile[TOP]->fHeight[3], (nGlobalTileZ+1)*fHeightScale );
            break;
        case FRONT:
            pVertex[0] = Vector3( (nGlobalTileX+1)*fHeightScale, -pTile[TOP]->fHeight[3], (nGlobalTileZ+1)*fHeightScale );
            pVertex[1] = Vector3( (nGlobalTileX+1)*fHeightScale, -pTile[TOP]->fHeight[1], (nGlobalTileZ)*fHeightScale );
            if( pTileF )
            {
                pVertex[2] = Vector3( (nGlobalTileX+1)*fHeightScale, -pTile[FRONT]->fHeight[2], (nGlobalTileZ+1)*fHeightScale );
                pVertex[3] = Vector3( (nGlobalTileX+1)*fHeightScale, -pTile[FRONT]->fHeight[0], (nGlobalTileZ)*fHeightScale );
            }
            else
            {
                pVertex[2] = Vector3( (nGlobalTileX+1)*fHeightScale, -0, (nGlobalTileZ+1)*fHeightScale );
                pVertex[3] = Vector3( (nGlobalTileX+1)*fHeightScale, -0, (nGlobalTileZ)*fHeightScale );
            }
            break;
        case RIGHT:
            // something similiar
            break;
    }
}

You should be able to write a similiar function to handle the normals. Then you should be able to write a good chunk of your function as something like

for(TileOrientation orientation = 0; orientation < ORIENTATION_COUNT; orientation)
{
    if( pTile[TOP]->OrientationIndex[orientation] >= 0)
    {
         generateVerticies(nGlobalTileX, nGlobalTileZ, pTile, pVertex + nVertexCount);
         generateNormals(...)
         // do other stuff
         nVertexCount += 4;
         nTriangleCount += 2;
    }
}

That should deal with your biggest problem, repeating sections which are almost exactly the same. Your code could use cleanup in a variety of other ways as well, but that really needs to be fixed first.

\$\endgroup\$
  • \$\begingroup\$ This is exactly what I needed, thank you very much, I'll leave the question open for another day just in case someone else want to add something more, but this already helps a lot. Thanks again. \$\endgroup\$ – Pablo Ariel Mar 6 '12 at 4:43

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.