Merge pull request #287 from AnalyticalGraphicsInc/fix-remove-normals

Fix remove normals
CesiumGS · Jun 6, 2017 · d64e179 · d64e179
2 parents 6a8239e + 8484f9b
commit d64e179
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 29 deletions.
diff --git a/lib/Pipeline.js b/lib/Pipeline.js
@@ -104,16 +104,17 @@ Pipeline.processJSONWithExtras  = function(gltfWithExtras, options) {
     var shouldRemoveNormals = defaultValue(options.removeNormals, false);
     var shouldPreserve = defaultValue(options.preserve, false);
     if (shouldRemoveNormals) {
-        removeNormals(gltfWithExtras);
+        removeNormals(gltfWithExtras, options);
     }
 
-    RemoveUnusedProperties.removeAll(gltfWithExtras);
     var smoothNormals = defaultValue(options.smoothNormals, false);
     var faceNormals = defaultValue(options.faceNormals, false);
     if (smoothNormals || faceNormals) {
         generateNormals(gltfWithExtras, options);
     }
 
+    RemoveUnusedProperties.removeAll(gltfWithExtras);
+
     var mergeVertices = defaultValue(options.mergeVertices, false);
     if (!shouldPreserve) {
         if (mergeVertices) {

diff --git a/lib/removeNormals.js b/lib/removeNormals.js
@@ -1,25 +1,30 @@
 'use strict';
 var Cesium = require('cesium');
+var clone = require('clone');
+var addExtensionsUsed = require('./addExtensionsUsed');
 var getPrimitiveAttributeSemantics = require('./getPrimitiveAttributeSemantics');
+var getUniqueId = require('./getUniqueId');
+var processModelMaterialsCommon = require('./processModelMaterialsCommon');
 
+var defaultValue = Cesium.defaultValue;
 var defined = Cesium.defined;
 
 module.exports = removeNormals;
 
 /**
- * Removes normals from primitives. The technique is stripped off of the primitive's material so that
- * it can be regenerated.
+ * Removes normals from primitives. The material is regenerated to use constant lighting.
  *
  * The glTF asset must be initialized for the pipeline.
  *
  * @param {Object} gltf A javascript object containing a glTF asset.
+ * @param {Object} [options] Options passed to processModelMaterialsCommon.
  * @returns {Object} The glTF asset with removed normals.
  *
  * @see addPipelineExtras
  * @see loadGltfUris
  */
-function removeNormals(gltf) {
-    var materials = gltf.materials;
+function removeNormals(gltf, options) {
+    var generatedMaterials = {};
     var meshes = gltf.meshes;
     for (var meshId in meshes) {
         if (meshes.hasOwnProperty(meshId)) {
@@ -29,17 +34,14 @@ function removeNormals(gltf) {
             for (var i = 0; i < primitivesLength; i++) {
                 var primitive = primitives[i];
                 if (removePrimitiveNormals(primitive)) {
-                    var materialId = primitive.material;
-                    if (defined(materialId)) {
-                        var material = materials[materialId];
-                        if (defined(material.technique)) {
-                            delete material.technique;
-                        }
-                    }
+                    generateMaterial(gltf, primitive, generatedMaterials);
                 }
             }
         }
     }
+    if (Object.keys(generatedMaterials).length > 0) {
+        processModelMaterialsCommon(gltf, options);
+    }
 }
 
 function removePrimitiveNormals(primitive) {
@@ -54,4 +56,39 @@ function removePrimitiveNormals(primitive) {
         }
     }
     return removedNormals;
-}
+}
+
+function generateMaterial(gltf, primitive, generatedMaterials) {
+    gltf.materials = defaultValue(gltf.materials, {});
+    var materials = gltf.materials;
+    var materialId = primitive.material;
+    var material = materials[materialId];
+
+    var generatedMaterialId = generatedMaterials[materialId];
+    if (defined(generatedMaterialId)) {
+        primitive.material = generatedMaterialId;
+        return;
+    }
+
+    addExtensionsUsed(gltf, 'KHR_materials_common');
+    generatedMaterialId = getUniqueId(gltf, materialId);
+
+    var values = [];
+    if (defined(material)) {
+        values = defaultValue(clone(material.values), []);
+    }
+
+    gltf.materials[generatedMaterialId] = {
+        extensions : {
+            KHR_materials_common : {
+                technique : 'CONSTANT',
+                values : values,
+                extras: {
+                    _pipeline: {}
+                }
+            }
+        }
+    };
+    primitive.material = generatedMaterialId;
+    generatedMaterials[materialId] = generatedMaterialId;
+}
diff --git a/specs/lib/removeNormalsSpec.js b/specs/lib/removeNormalsSpec.js
@@ -1,16 +1,19 @@
 'use strict';
 
 var removeNormals = require('../../lib/removeNormals');
+var techniqueParameterForSemantic = require('../../lib/techniqueParameterForSemantic');
 
 describe('removeNormals', function() {
-    it('removes normals and strips the technique from the material', function() {
+    it('removes normals and generates constant technique', function() {
         var gltf = {
+            asset : {},
             meshes : {
                 mesh: {
                     primitives: [
                         {
                             attributes: {
-                                NORMAL: 'some_accessor'
+                                POSITION : 'position_accessor',
+                                NORMAL: 'normal_accessor'
                             },
                             material: 'material'
                         }
@@ -27,12 +30,18 @@ describe('removeNormals', function() {
             }
         };
         removeNormals(gltf);
-        expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).not.toBeDefined();
-        expect(gltf.materials.material.technique).not.toBeDefined();
+        var primitive = gltf.meshes.mesh.primitives[0];
+        var material = gltf.materials[primitive.material];
+        var technique = gltf.techniques[material.technique];
+        expect(primitive.attributes.NORMAL).toBeUndefined();
+        expect(technique).toBeDefined();
+        var parameter = techniqueParameterForSemantic(technique, 'NORMAL');
+        expect(parameter).toBeUndefined(); // Technique should not reference normals
     });
 
     it('handles undefined technique', function() {
         var gltf = {
+            asset : {},
             meshes : {
                 mesh: {
                     primitives: [
@@ -50,11 +59,12 @@ describe('removeNormals', function() {
             }
         };
         removeNormals(gltf);
-        expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).not.toBeDefined();
+        expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).toBeUndefined();
     });
 
     it('handles undefined material', function() {
         var gltf = {
+            asset : {},
             meshes : {
                 mesh: {
                     primitives: [
@@ -68,11 +78,12 @@ describe('removeNormals', function() {
             }
         };
         removeNormals(gltf);
-        expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).not.toBeDefined();
+        expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).toBeUndefined();
     });
 
     it('only deletes normals and handles subscripts', function() {
         var gltf = {
+            asset : {},
             meshes : {
                 mesh: {
                     primitives: [{
@@ -100,19 +111,16 @@ describe('removeNormals', function() {
                 }
             },
             techniques : {
-                technique : {}
+                technique : {},
+                technique2 : {}
             }
         };
         removeNormals(gltf);
         var primitives = gltf.meshes.mesh.primitives;
-        expect(primitives[0].attributes.NORMAL).not.toBeDefined();
+        expect(primitives[0].attributes.NORMAL).toBeUndefined();
         expect(primitives[0].attributes.POSITION).toBeDefined();
-        expect(primitives[0].material).toBeDefined();
-        expect(gltf.materials.material.technique).not.toBeDefined();
-        expect(primitives[1].attributes.NORMAL).not.toBeDefined();
-        expect(primitives[1].attributes.NORMAL_0).not.toBeDefined();
+        expect(primitives[1].attributes.NORMAL).toBeUndefined();
+        expect(primitives[1].attributes.NORMAL_0).toBeUndefined();
         expect(primitives[1].attributes.POSITION).toBeDefined();
-        expect(primitives[1].material).toBeDefined();
-        expect(gltf.materials.material2.technique).not.toBeDefined();
     });
-});
+});