Merge pull request #490 from CesiumGS/techniques-fix

Fix techniques with same uniform name but different parameter
CesiumGS · Mar 30, 2020 · 91abb76 · 91abb76
2 parents 0f74cdf + 0180b89
commit 91abb76
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 17 deletions.
diff --git a/lib/moveTechniquesToExtension.js b/lib/moveTechniquesToExtension.js
@@ -22,6 +22,7 @@ function moveTechniquesToExtension(gltf) {
     const techniquesLegacy = gltf.techniques;
     const mappedUniforms = {};
     const updatedTechniqueIndices = {};
+    const seenPrograms = {};
     if (defined(techniquesLegacy)) {
         const extension = {
             programs: [],
@@ -33,7 +34,7 @@ function moveTechniquesToExtension(gltf) {
         const glExtensions = gltf.glExtensionsUsed;
         delete gltf.glExtensionsUsed;
 
-        ForEach.technique(gltf, function (techniqueLegacy, techniqueIndex) {
+        ForEach.technique(gltf, function (techniqueLegacy, techniqueId) {
             const technique = {
                 name: techniqueLegacy.name,
                 program: undefined,
@@ -60,27 +61,36 @@ function moveTechniquesToExtension(gltf) {
                 };
 
                 // Store the name of the uniform to update material values.
-                mappedUniforms[parameterName] = uniformName;
+                if (!defined(mappedUniforms[techniqueId])) {
+                    mappedUniforms[techniqueId] = {};
+                }
+                mappedUniforms[techniqueId][parameterName] = uniformName;
             });
 
-            const programLegacy = gltf.programs[techniqueLegacy.program];
-            const program = {
-                name: programLegacy.name,
-                fragmentShader: undefined,
-                vertexShader: undefined,
-                glExtensions: glExtensions
-            };
+            if (!defined(seenPrograms[techniqueLegacy.program])) {
+                const programLegacy = gltf.programs[techniqueLegacy.program];
 
-            const fs = gltf.shaders[programLegacy.fragmentShader];
-            program.fragmentShader = addToArray(extension.shaders, fs, true);
+                const program = {
+                    name: programLegacy.name,
+                    fragmentShader: undefined,
+                    vertexShader: undefined,
+                    glExtensions: glExtensions
+                };
 
-            const vs = gltf.shaders[programLegacy.vertexShader];
-            program.vertexShader = addToArray(extension.shaders, vs, true);
+                const fs = gltf.shaders[programLegacy.fragmentShader];
+                program.fragmentShader = addToArray(extension.shaders, fs, true);
 
-            technique.program = addToArray(extension.programs, program);
+                const vs = gltf.shaders[programLegacy.vertexShader];
+                program.vertexShader = addToArray(extension.shaders, vs, true);
+
+                technique.program = addToArray(extension.programs, program);
+                seenPrograms[techniqueLegacy.program] = technique.program;
+            } else {
+                technique.program = seenPrograms[techniqueLegacy.program];
+            }
 
             // Store the index of the new technique to reference instead.
-            updatedTechniqueIndices[techniqueIndex] = addToArray(extension.techniques, technique);
+            updatedTechniqueIndices[techniqueId] = addToArray(extension.techniques, technique);
         });
 
         if (extension.techniques.length > 0) {
@@ -105,7 +115,7 @@ function moveTechniquesToExtension(gltf) {
                     materialExtension.values = {};
                 }
 
-                const uniformName = mappedUniforms[parameterName];
+                const uniformName = mappedUniforms[material.technique][parameterName];
                 materialExtension.values[uniformName] = value;
             });
 

diff --git a/specs/lib/moveTechniquesToExtensionSpec.js b/specs/lib/moveTechniquesToExtensionSpec.js
@@ -100,6 +100,17 @@ describe('moveTechniquesToExtension', () => {
                 },
                 technique1: {
                     program: 'program_1'
+                },
+                technique2: {
+                    parameters: {
+                        diffuse: {
+                            type: WebGLConstants.FLOAT_VEC4
+                        }
+                    },
+                    program: 'program_0',
+                    uniforms: {
+                        u_diffuse: 'diffuse'
+                    }
                 }
             },
             materials: [
@@ -116,6 +127,18 @@ describe('moveTechniquesToExtension', () => {
                             1
                         ]
                     }
+                },
+                {
+                    name: 'Color',
+                    technique: 'technique2',
+                    values: {
+                        diffuse: [
+                            0.2,
+                            0.2,
+                            0.2,
+                            1
+                        ]
+                    }
                 }
             ]
         };
@@ -124,7 +147,7 @@ describe('moveTechniquesToExtension', () => {
         expect(gltfWithTechniquesWebgl.extensions).toBeDefined();
         const techniques = gltfWithTechniquesWebgl.extensions.KHR_techniques_webgl;
         expect(techniques).toBeDefined();
-        expect(techniques.techniques.length).toBe(2);
+        expect(techniques.techniques.length).toBe(3);
 
         const technique = techniques.techniques[0];
         const attributes = technique.attributes;
@@ -158,6 +181,7 @@ describe('moveTechniquesToExtension', () => {
         expect(materialTechniques).toBeDefined();
         expect(materialTechniques.technique).toBe(0);
         expect(materialTechniques.values.u_shininess).toBe(256);
+        expect(materialTechniques.values.u_diffuse).toBe('texture_Image0001');
 
         expect(material.technique).toBeUndefined();
         expect(material.values).toBeUndefined();
@@ -166,5 +190,10 @@ describe('moveTechniquesToExtension', () => {
         const program2 = techniques.programs[technique2.program];
         expect(program2.vertexShader).toBe(program.vertexShader);
         expect(program2.fragmentShader).not.toBe(program.fragmentShader);
+
+        const technique3 = techniques.techniques[2];
+        expect(technique3.program).toBe(0);
+        expect(technique3.uniforms.u_diffuse.type).toBe(WebGLConstants.FLOAT_VEC4);
+        expect(gltf.materials[1].extensions.KHR_techniques_webgl.values.u_diffuse).toEqual([0.2, 0.2, 0.2, 1.0]);
     });
 });