ands / lightmapper

A C/C++ single-file library for drop-in lightmap baking. Just use your existing OpenGL renderer to bounce light!
1.44k stars 134 forks source link

Normals get transformed twice #27

Open alektron opened 10 months ago

alektron commented 10 months ago

When calling lmSetGeometry there is an option to pass in vertex normal data. If no normal data is passed (type is set to LM_NONE), the normals will internally be calculated from the triangle vertices.

However the normal seems to get calculated from the triangle vertices AFTER the triangle was already transformed by the model matrix. So the normals are automatically in world space. They then get transformed again by the normal matrix (inverse transpose of the model matrix).

ctx->meshPosition.triangle.p[i] = lm_transformPosition(ctx->mesh.modelMatrix, p);

// ...
// decode uv coordinates
// ...

lm_vec3 flatNormal = lm_cross3(
    lm_sub3(ctx->meshPosition.triangle.p[1], ctx->meshPosition.triangle.p[0]),
    lm_sub3(ctx->meshPosition.triangle.p[2], ctx->meshPosition.triangle.p[0]));

for (int i = 0; i < 3; i++)
{
    // decode and pre-transform vertex normal
    const void *nPtr = ctx->mesh.normals + vIndices[i] * ctx->mesh.normalsStride;
    lm_vec3 n;
    switch (ctx->mesh.normalsType)
    {
        // TODO: signed formats
    case LM_FLOAT: {
        n = *(const lm_vec3*)nPtr;
    } break;
    case LM_NONE: {
        n = flatNormal;
    } break;
    default: {
        assert(LM_FALSE);
    } break;
    }
    ctx->meshPosition.triangle.n[i] = lm_normalize3(lm_transformNormal(ctx->mesh.normalMatrix, n));
}

It would seem to me that the transformation with the normal matrix should only be done if the normalsType is not LM_NONE. If I could get some confirmation that this is indeed a bug, I'd be glad to make a pull request.

Note: Great library. Small, yet super effective. It does a great job!