2
\$\begingroup\$

It does generate pretty decent normals but I'm not sure if I could improve it even more without losing performance.

You can find the whole code here:

What do you think about this one? Other than declaring an Epsilon constant, of course.

void CalcNormals( const Vector3* pVertices, const GND_TILE_INDICES* pTileIndices,
                             const GND_TILE_DATA* pTileData, uint32_t nWidth, uint32_t nDepth, 
                             float fHeightScale, Vector3 *out_pNormals )
{
    float fScale = -.1f;
    if( fHeightScale != 0.0f )
            fScale = -1.0f/fHeightScale;
    uint32_t i,nIndexX = 0, nIndexZ = 0, nTotalTiles = nWidth*nDepth;
    const GND_TILE_INDICES* pIdxCurrent=0, * pIdxFront=0, * pIdxRight=0, * pIdxFrontRight=0;
    const GND_TILE_DATA* pTileCurrent=0, * pTileFront=0, * pTileRight=0, * pTileFrontRight=0;

    Vector3 BlendNormalFinal;
    Vector3 BlendNormal0;
    Vector3 BlendNormal1;
    Vector3 BlendNormal2;
    Vector3 BlendNormal3;

    // first pass builds triangle normals independently (no normal smoothing)
    for( i=0; i<nTotalTiles; i++ )
    {
            pIdxCurrent             = &pTileIndices[i];
            pTileCurrent    = &pTileData[i];
            if( -1 == pIdxCurrent->VerticesTop[0] )
                    continue;
            ////-------------------------------
            //BlendNormal0 = ( (pVertices[pIdxCurrent->VerticesTop[2]] - pVertices[pIdxCurrent->VerticesTop[0]]) )
            //      .Cross( (pVertices[pIdxCurrent->VerticesTop[1]] - pVertices[pIdxCurrent->VerticesTop[0]]) );

            //BlendNormal1 = ( (pVertices[pIdxCurrent->VerticesTop[3]] - pVertices[pIdxCurrent->VerticesTop[1]]) )
            //      .Cross( -(pVertices[pIdxCurrent->VerticesTop[0]] - pVertices[pIdxCurrent->VerticesTop[1]]) );

            //BlendNormal2 = ( (pVertices[pIdxCurrent->VerticesTop[3]] - pVertices[pIdxCurrent->VerticesTop[2]]) )
            //      .Cross( (pVertices[pIdxCurrent->VerticesTop[0]] - pVertices[pIdxCurrent->VerticesTop[2]]) );

            //BlendNormal3 = ( (pVertices[pIdxCurrent->VerticesTop[2]] - pVertices[pIdxCurrent->VerticesTop[3]]) )
            //      .Cross( -(pVertices[pIdxCurrent->VerticesTop[1]] - pVertices[pIdxCurrent->VerticesTop[3]]) );

            BlendNormal0 = ( (Vector3(0.0f, pTileCurrent->fHeight[2]*fScale, 1.0f) - Vector3(0.0f, pTileCurrent->fHeight[0]*fScale, 0.0f)) )
                    .Cross( (Vector3(1.0f, pTileCurrent->fHeight[1]*fScale, 0.0f) - Vector3(0.0f, pTileCurrent->fHeight[0]*fScale, 0.0f)) );

            BlendNormal1 = ( (Vector3(1.0f, pTileCurrent->fHeight[3]*fScale, 1.0f) - Vector3(1.0f, pTileCurrent->fHeight[1]*fScale, 0.0f)) )
                    .Cross( -(Vector3(0.0f, pTileCurrent->fHeight[0]*fScale, 0.0f) - Vector3(1.0f, pTileCurrent->fHeight[1]*fScale, 0.0f)) );

            BlendNormal2 = ( (Vector3(1.0f, pTileCurrent->fHeight[3]*fScale, 1.0f) - Vector3(0.0f, pTileCurrent->fHeight[2]*fScale, 1.0f)) )
                    .Cross( (Vector3(0.0f, pTileCurrent->fHeight[0]*fScale, 0.0f) - Vector3(0.0f, pTileCurrent->fHeight[2]*fScale, 1.0f)) );

            BlendNormal3 = ( (Vector3(0.0f, pTileCurrent->fHeight[2]*fScale, 1.0f) - Vector3(1.0f, pTileCurrent->fHeight[3]*fScale, 1.0f)) )
                    .Cross( -(Vector3(1.0f, pTileCurrent->fHeight[1]*fScale, 0.0f) - Vector3(1.0f, pTileCurrent->fHeight[3]*fScale, 1.0f)) );

            out_pNormals[pIdxCurrent->VerticesTop[0]] = //(BlendNormal0+BlendNormal1+BlendNormal2+BlendNormal3).Normalize();
            out_pNormals[pIdxCurrent->VerticesTop[1]] = //(BlendNormal1+BlendNormal0+BlendNormal3+BlendNormal2).Normalize();
            out_pNormals[pIdxCurrent->VerticesTop[2]] = //(BlendNormal2+BlendNormal0+BlendNormal3+BlendNormal1).Normalize();
            out_pNormals[pIdxCurrent->VerticesTop[3]] = (BlendNormal3+BlendNormal1+BlendNormal2+BlendNormal0).Normalize();
    }

    Vector3 preBlendFrontRight; 
    // iterate again to perform the normal blending
    for( i=0; i<nTotalTiles; i++ )
    {
            pIdxCurrent             = &pTileIndices[i];
            pTileCurrent    = &pTileData[i];
            if( -1 == pIdxCurrent->VerticesTop[0] )
                    continue;

            if( i < (nTotalTiles-1) )
            {
                    pIdxFront       = &pTileIndices[i+1];
                    pTileFront      = &pTileData[i+1];
                    if( -1 == pIdxFront->VerticesTop[0] )
                    { 
                            pTileFront = 0; 
                            pIdxFront = 0;
                    }

                    if( i < (nTotalTiles-nWidth) )
                    {
                            pIdxRight               = &pTileIndices[i+nWidth];
                            pTileRight              = &pTileData[i+nWidth];
                            if( -1 == pIdxRight->VerticesTop[0] )
                            { 
                                    pTileRight = 0; 
                                    pIdxRight = 0;
                            }

                            if( i < (nTotalTiles-nWidth-1) )
                            {
                                    pIdxFrontRight  = &pTileIndices[i+nWidth+1];
                                    pTileFrontRight = &pTileData[i+nWidth+1];
                                    if( -1 == pIdxFrontRight->VerticesTop[0] )
                                    { 
                                            pTileFrontRight = 0; 
                                            pIdxFrontRight = 0;
                                    }
                            }
                            else
                            {
                                    pIdxFrontRight  = 0;
                                    pTileFrontRight = 0;
                            }
                    }
                    else
                    {
                            pIdxRight               = 0;
                            pIdxFrontRight  = 0;
                            pTileRight              = 0;
                            pTileFrontRight = 0;
                    };
            }
            else
            {
                    pIdxFront               = 0;
                    pIdxRight               = 0;
                    pIdxFrontRight  = 0;
                    pTileFront              = 0;
                    pTileRight              = 0;
                    pTileFrontRight = 0;
            };


            if( -1 != pIdxCurrent->VerticesTop[0] )
            {
                    preBlendFrontRight = out_pNormals[pIdxCurrent->VerticesTop[3]]; // *.5f;
                    preBlendFrontRight += out_pNormals[pIdxCurrent->VerticesTop[2]]; // *.5f;
                    preBlendFrontRight += out_pNormals[pIdxCurrent->VerticesTop[1]]; // *.5f;
                    preBlendFrontRight.Normalize();
            }
            else
                    preBlendFrontRight.x = preBlendFrontRight.y = preBlendFrontRight.z = 0;

            if( pIdxFront 
                    && (fabs(pTileCurrent->fHeight[3] - pTileFront->fHeight[2]) < 0.0001f) )
            {
                    preBlendFrontRight += out_pNormals[pIdxFront->VerticesTop[2]];
                    preBlendFrontRight += out_pNormals[pIdxFront->VerticesTop[3]];
                    preBlendFrontRight += out_pNormals[pIdxFront->VerticesTop[1]];
            }
            if( pIdxRight 
                    && (fabs(pTileCurrent->fHeight[3] - pTileRight->fHeight[1]) < 0.0001f) )
            {
                    preBlendFrontRight += out_pNormals[pIdxRight->VerticesTop[1]];
                    preBlendFrontRight += out_pNormals[pIdxRight->VerticesTop[2]];
                    preBlendFrontRight += out_pNormals[pIdxRight->VerticesTop[3]];
            }
            if( pIdxFrontRight 
                    && (fabs(pTileCurrent->fHeight[3] - pTileFrontRight->fHeight[0]) < 0.0001f) )
            {
                    preBlendFrontRight += out_pNormals[pIdxFrontRight->VerticesTop[0]];
                    preBlendFrontRight += out_pNormals[pIdxFrontRight->VerticesTop[1]];
                    preBlendFrontRight += out_pNormals[pIdxFrontRight->VerticesTop[2]];
            }
            preBlendFrontRight.Normalize();

            // --------------------------------- Front Right!! --------------------------
            out_pNormals[pIdxCurrent->VerticesTop[3]]                       = (preBlendFrontRight);
            out_pNormals[pIdxCurrent->VerticesTop[3]].Normalize();
            if( pIdxFront 
                    && (fabs(pTileCurrent->fHeight[3] - pTileFront->fHeight[2]) < 0.0001f) )
            {
                    out_pNormals[pIdxFront->VerticesTop[2]]                 = (preBlendFrontRight);
                    out_pNormals[pIdxFront->VerticesTop[2]].Normalize();
            }
            if( pIdxRight 
                    && (fabs(pTileCurrent->fHeight[3] - pTileRight->fHeight[1]) < 0.0001f) )
            {
                    out_pNormals[pIdxRight->VerticesTop[1]]                 = (preBlendFrontRight);
                    out_pNormals[pIdxRight->VerticesTop[1]].Normalize();
            }
            if( pIdxFrontRight 
                    && (fabs(pTileCurrent->fHeight[3] - pTileFrontRight->fHeight[0]) < 0.0001f) )
            {
                    out_pNormals[pIdxFrontRight->VerticesTop[0]]    = (preBlendFrontRight);
                    out_pNormals[pIdxFrontRight->VerticesTop[0]].Normalize();
            }

    };

};
\$\endgroup\$
3
  • \$\begingroup\$ 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 in only 2 different lines. \$\endgroup\$ Mar 4, 2012 at 21:35
  • 1
    \$\begingroup\$ I added computational-geometry, which is more general than "terrain", and could be applied to more questions. \$\endgroup\$ Mar 5, 2012 at 7:29
  • \$\begingroup\$ This is C++ or "C with Classes" code: the type Vector3 is used like a function, so that must be a C++ constructor call. \$\endgroup\$
    – aschepler
    Mar 6, 2012 at 3:39

2 Answers 2

3
\$\begingroup\$

What I take to be the problematic portion of this code is the portion that assigns pIdxFront and friends.

What you need to do is convert your related variables into arrays and then do things in a loop.

enum Directions
{
    Front,
    Top,
    TopFront

    DIRECTION_COUNT
};

GND_TILE_INDICES * pIdx[DIRECTION_COUNT]; 
GND_TILE_DATA * pTile[DIRECTION_COUNT];
for(Direction direction = 0; direction < DIRECTION_COUNT; direction++)
{
    int index;
    switch(direction)
    { 
        case Front:
            index = i + 1;
            break;
        case Top:
            index = i + mWidth;
            break;
        case FrontTop:
            index = i + mWidth + 1;
            break;
    }

    pIdx[direction] = pTileIndices[i + 1];
    pTile[direction] = pTileData[i + 1];
    if(-1 == pIdx[direction]->VerticesTop[0])
    {
        pIdx[direction] = 0;
        pTile[direction] = 0;
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Thanks! It seems so obvious now you say it, but I really lost that one. \$\endgroup\$ Mar 6, 2012 at 4:38
2
\$\begingroup\$

Your code is hard to review, since one would need to exactly understand the underlying mathematics, then understand your code, then propose meaningful abstractions. I do hope someone will do it, but I can't. I also believe that for complicated code like that, it's OK to repeat yourself, since it's often hard to encapsulate and to modify without having to understand the whole thing.

What I can do is provide some comments on the code itself:

  1. You can do better than declaring an epsilon constant: encapsulate the equality test! It's an implementation detail and easy to get wrong.
  2. You should care more about spacing: at least make it consistent, eg. avoid this:

                   pIdxRight               = 0;
                   pIdxFrontRight  = 0;
    

    I think the way SE displays the code doesn't help.

  3. This one is borderline code obfuscation. It's better in your repository (without the comments). :) Consider using a temporary variable.

    out_pNormals[pIdxCurrent->VerticesTop[0]] = //(BlendNormal0+BlendNormal1+BlendNormal2+BlendNormal3).Normalize();
    out_pNormals[pIdxCurrent->VerticesTop[1]] = //(BlendNormal1+BlendNormal0+BlendNormal3+BlendNormal2).Normalize();
    out_pNormals[pIdxCurrent->VerticesTop[2]] = //(BlendNormal2+BlendNormal0+BlendNormal3+BlendNormal1).Normalize();
    out_pNormals[pIdxCurrent->VerticesTop[3]] = (BlendNormal3+BlendNormal1+BlendNormal2+BlendNormal0).Normalize();
    
  4. I didn't know you could end loops and functions with };. Deleting the ; would prevent confusion with the syntax for classes.
\$\endgroup\$
1
  • \$\begingroup\$ Thanks, you got it right, one of the problems I do find with these kind of programming is to encapsulate without losing track ie sometimes it's better to have the code right there inlined instead of having a separate function and have to switch to understand what's going on. Most people would give me the advice to encapsulate when I actually do inline on purpose because it's easier for me to work on it. \$\endgroup\$ Mar 5, 2012 at 15:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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