Merge pull request #9173 from CesiumGS/unlit-model-normals

Allow normals to be used when model material is unlit
CesiumGS · Nov 1, 2020 · e3acb52 · e3acb52
2 parents b0846bc + d113713
commit e3acb52
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 22 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -2,6 +2,9 @@
 
 ### 1.75 - 2020-11-02
 
+##### Fixes :wrench:
+
+- Fixed an issue in the PBR material where models with the `KHR_materials_unlit` extension had the normal attribute disabled. [#9173](https://github.com/CesiumGS/cesium/pull/9173).
 - Fixed JSDoc and TypeScript type definitions for `writeTextToCanvas` which listed incorrect return type. [#9196](https://github.com/CesiumGS/cesium/pull/9196)
 - Fixed JSDoc and TypeScript type definitions for `Viewer.globe` constructor option to allow disabling the globe on startup. [#9063](https://github.com/CesiumGS/cesium/pull/9063)
 

diff --git a/Source/Scene/Model.js b/Source/Scene/Model.js
@@ -3003,12 +3003,10 @@ function getAttributeLocations(model, primitive) {
 
   var attributes = technique.attributes;
   var program = model._rendererResources.programs[technique.program];
-  var programVertexAttributes = program.vertexAttributes;
   var programAttributeLocations = program._attributeLocations;
 
-  // Note: WebGL shader compiler may have optimized and removed some attributes from programVertexAttributes
-  for (location in programVertexAttributes) {
-    if (programVertexAttributes.hasOwnProperty(location)) {
+  for (location in programAttributeLocations) {
+    if (programAttributeLocations.hasOwnProperty(location)) {
       var attribute = attributes[location];
       if (defined(attribute)) {
         index = programAttributeLocations[location];
@@ -3017,11 +3015,7 @@ function getAttributeLocations(model, primitive) {
     }
   }
 
-  // Always add pre-created attributes.
-  // Some pre-created attributes, like per-instance pickIds, may be compiled out of the draw program
-  // but should be included in the list of attribute locations for the pick program.
-  // This is safe to do since programVertexAttributes and programAttributeLocations are equivalent except
-  // that programVertexAttributes optimizes out unused attributes.
+  // Add pre-created attributes.
   var precreatedAttributes = model._precreatedAttributes;
   if (defined(precreatedAttributes)) {
     for (location in precreatedAttributes) {

diff --git a/Source/Scene/processPbrMaterials.js b/Source/Scene/processPbrMaterials.js
@@ -289,8 +289,6 @@ function generateTechnique(
     defined(material.extensions.KHR_materials_unlit)
   ) {
     isUnlit = true;
-    hasNormals = false;
-    hasTangents = false;
   }
 
   if (hasNormals) {
@@ -480,15 +478,16 @@ function generateTechnique(
       semantic: "NORMAL",
     };
     vertexShader += "attribute vec3 a_normal;\n";
-    vertexShader += "varying vec3 v_normal;\n";
-    if (hasSkinning) {
-      vertexShaderMain +=
-        "    v_normal = u_normalMatrix * mat3(skinMatrix) * weightedNormal;\n";
-    } else {
-      vertexShaderMain += "    v_normal = u_normalMatrix * weightedNormal;\n";
+    if (!isUnlit) {
+      vertexShader += "varying vec3 v_normal;\n";
+      if (hasSkinning) {
+        vertexShaderMain +=
+          "    v_normal = u_normalMatrix * mat3(skinMatrix) * weightedNormal;\n";
+      } else {
+        vertexShaderMain += "    v_normal = u_normalMatrix * weightedNormal;\n";
+      }
+      fragmentShader += "varying vec3 v_normal;\n";
     }
-
-    fragmentShader += "varying vec3 v_normal;\n";
     fragmentShader += "varying vec3 v_positionEC;\n";
   }
 
@@ -638,7 +637,7 @@ function generateTechnique(
   vertexShader += "}\n";
 
   // Fragment shader lighting
-  if (hasNormals) {
+  if (hasNormals && !isUnlit) {
     fragmentShader += "const float M_PI = 3.141592653589793;\n";
 
     fragmentShader +=
@@ -737,7 +736,7 @@ function generateTechnique(
   fragmentShader += fragmentShaderMain;
 
   // Add normal mapping to fragment shader
-  if (hasNormals) {
+  if (hasNormals && !isUnlit) {
     fragmentShader += "    vec3 ng = normalize(v_normal);\n";
     fragmentShader +=
       "    vec3 positionWC = vec3(czm_inverseView * vec4(v_positionEC, 1.0));\n";
@@ -814,7 +813,7 @@ function generateTechnique(
 
   fragmentShader += "    vec3 baseColor = baseColorWithAlpha.rgb;\n";
 
-  if (hasNormals) {
+  if (hasNormals && !isUnlit) {
     if (useSpecGloss) {
       if (defined(generatedMaterialValues.u_specularGlossinessTexture)) {
         fragmentShader +=