Merge pull request #5032 from AnalyticalGraphicsInc/fix-normal-genera…

…tion Normal generation errors
CesiumGS · Feb 24, 2017 · 416cfc2 · 416cfc2
2 parents e63e6a9 + b33ff44
commit 416cfc2
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 9 deletions.
diff --git a/Source/Core/GeometryPipeline.js b/Source/Core/GeometryPipeline.js
@@ -1173,20 +1173,28 @@ define([
         for (i = 0; i < numVertices; i++) {
             var i3 = i * 3;
             vertexNormalData = normalsPerVertex[i];
+            Cartesian3.clone(Cartesian3.ZERO, normal);
             if (vertexNormalData.count > 0) {
-                Cartesian3.clone(Cartesian3.ZERO, normal);
                 for (j = 0; j < vertexNormalData.count; j++) {
                     Cartesian3.add(normal, normalsPerTriangle[normalIndices[vertexNormalData.indexOffset + j]], normal);
                 }
-                Cartesian3.normalize(normal, normal);
-                normalValues[i3] = normal.x;
-                normalValues[i3 + 1] = normal.y;
-                normalValues[i3 + 2] = normal.z;
-            } else {
-                normalValues[i3] = 0.0;
-                normalValues[i3 + 1] = 0.0;
-                normalValues[i3 + 2] = 1.0;
+
+                // We can run into an issue where a vertex is used with 2 primitives that have opposite winding order.
+                if (Cartesian3.equalsEpsilon(Cartesian3.ZERO, normal, CesiumMath.EPSILON10)) {
+                    Cartesian3.clone(normalsPerTriangle[normalIndices[vertexNormalData.indexOffset]], normal);
+                }
             }
+
+            // We end up with a zero vector probably because of a degenerate triangle
+            if (Cartesian3.equalsEpsilon(Cartesian3.ZERO, normal, CesiumMath.EPSILON10)) {
+                // Default to (0,0,1)
+                normal.z = 1.0;
+            }
+
+            Cartesian3.normalize(normal, normal);
+            normalValues[i3] = normal.x;
+            normalValues[i3 + 1] = normal.y;
+            normalValues[i3 + 2] = normal.z;
         }
 
         geometry.attributes.normal = new GeometryAttribute({

diff --git a/Specs/Core/GeometryPipelineSpec.js b/Specs/Core/GeometryPipelineSpec.js
@@ -1410,6 +1410,50 @@ defineSuite([
         expect(Cartesian3.fromArray(normals, 18)).toEqualEpsilon(Cartesian3.negate(Cartesian3.UNIT_Z, new Cartesian3()), CesiumMath.EPSILON7);
     });
 
+    it('computeNormal computes normal of (0,0,1) for a degenerate triangle', function() {
+        var geometry = new Geometry({
+            attributes: {
+                position: new GeometryAttribute({
+                    values: [0, 0, 0, 1, 0, 0],
+                    componentsPerAttribute: 3,
+                    componentDatatype : ComponentDatatype.FLOAT
+                })
+            },
+            indices : [0, 1, 0],
+            primitiveType: PrimitiveType.TRIANGLES
+        });
+
+        geometry = GeometryPipeline.computeNormal(geometry);
+
+        expect(geometry.attributes.normal.values.length).toEqual(2*3);
+        expect(geometry.attributes.normal.values).toEqual([0, 0, 1, 0, 0, 1]);
+    });
+
+    it('computeNormal takes first normal for two coplanar triangles with opposite winding orders', function() {
+        var geometry = new Geometry({
+            attributes: {
+                position: new GeometryAttribute({
+                    values: [0, 0, 0, 1, 0, 1, 1, 1, 1],
+                    componentsPerAttribute: 3,
+                    componentDatatype : ComponentDatatype.FLOAT
+                })
+            },
+            indices : [0, 1, 2, 2, 1, 0],
+            primitiveType: PrimitiveType.TRIANGLES
+        });
+
+        geometry = GeometryPipeline.computeNormal(geometry);
+
+        var normals = geometry.attributes.normal.values;
+        expect(normals.length).toEqual(3*3);
+
+        var a = Cartesian3.normalize(new Cartesian3(-1, 0, 1), new Cartesian3());
+
+        expect(Cartesian3.fromArray(normals, 0)).toEqualEpsilon(a, CesiumMath.EPSILON7);
+        expect(Cartesian3.fromArray(normals, 3)).toEqualEpsilon(a, CesiumMath.EPSILON7);
+        expect(Cartesian3.fromArray(normals, 6)).toEqualEpsilon(a, CesiumMath.EPSILON7);
+    });
+
     it('computeTangentAndBitangent throws when geometry is undefined', function() {
         expect(function() {
             GeometryPipeline.computeTangentAndBitangent();