Fix the normal matrix when det < 0.

This was tested by replacing the node 0 scale in BusterDrone with
[-1, 1, 1].

For future reference, commit f728776 shows when we switched from
transpose(inverse()) to cof().

Fixes #3001.

filament/src/Scene.cpp

@@ -199,6 +199,12 @@ void FScene::updateUBOs(utils::Range<uint32_t> visibleRenderables, backend::Hand
         mat3f m = mat3f::getTransformForNormals(model.upperLeft());
         m *= mat3f(1.0f / std::sqrt(max(float3{length2(m[0]), length2(m[1]), length2(m[2])})));
 
+        // Since our "getTransformForNormals" matrix omits the divide-by-determinant step,
+        // it has the wrong sign when the determinant is negative.
+        if (sceneData.elementAt<REVERSED_WINDING_ORDER>(i)) {
+            m = -m;
+        }
+
         UniformBuffer::setUniform(buffer,
                 offset + offsetof(PerRenderableUib, worldFromModelNormalMatrix), m);
 

libs/math/include/math/mat3.h

@@ -260,6 +260,23 @@ class MATH_EMPTY_BASES TMat33 :
     /**
      * Returns a matrix suitable for transforming normals
      *
+     * The inverse-transpose of a matrix is equal to its cofactor matrix divided by its determinant:
+     *
+     *     transpose(inverse(M)) = cof(M) / det(M)
+     *
+     * Since we normalize normals anyway, there's no need to divide by the determinant.
+     * Clients should be aware of this when handling "mirror" transformations (det < 0).
+     *
+     * Some excellent references from Dale Weiler and Eric Lengyel:
+     *   - https://github.com/graphitemaster/normals_revisited
+     *   - FGED Volume 1, section 1.7.5 "Inverses of Small Matrices"
+     *   - FGED Volume 1, section 3.2.2 "Transforming Normal Vectors"
+     *
+     * At the bottom of pg 105, Lengyel notes that there are two types of transformed normals: one
+     * that uses the transposed adjugate (aka cofactor matrix) and one that uses the transposed
+     * inverse. He goes on to say that this difference is inconsequential, except when mirroring
+     * is involved.
+     *
      * @param m the transform applied to vertices
      * @return a matrix to apply to normals
      *

shaders/src/main.vs

@@ -33,7 +33,7 @@ void main() {
 
         // We don't need to normalize here, even if there's a scale in the matrix
         // because we ensure the worldFromModelNormalMatrix pre-scales the normal such that
-        // all its components are < 1.0. This precents the bitangent to exceed the range of fp16
+        // all its components are < 1.0. This prevents the bitangent to exceed the range of fp16
         // in the fragment shader, where we renormalize after interpolation
         vertex_worldTangent.xyz = objectUniforms.worldFromModelNormalMatrix * vertex_worldTangent.xyz;
         vertex_worldTangent.w = mesh_tangents.w;