Merge pull request #8820 from CesiumGS/outline-32

Fix problem switching from Uint16 to Uint32 indices for outlining.

Author: emackey

CHANGES.md

@@ -8,6 +8,7 @@
 
 ##### Fixes :wrench:
 
+- Fixed a bug that could cause rendering of a glTF model to become corrupt when switching from a Uint16 to a Uint32 index buffer to accomodate new vertices added for edge outlining. [#8820](https://github.com/CesiumGS/cesium/pull/8820)
 - This fixes a bug where a removed billboard can prevent changing of the terrainProvider [#8766](https://github.com/CesiumGS/cesium/pull/8766)
 
 ### 1.69.0 - 2020-05-01

Source/Scene/ModelOutlineLoader.js

@@ -8,6 +8,10 @@ import TextureMinificationFilter from "../Renderer/TextureMinificationFilter.js"
 import TextureWrap from "../Renderer/TextureWrap.js";
 import ForEach from "../ThirdParty/GltfPipeline/ForEach.js";
 
+// glTF does not allow an index value of 65535 because this is the primitive
+// restart value in some APIs.
+var MAX_GLTF_UINT16_INDEX = 65534;
+
 /**
  * Creates face outlines for glTF primitives with the `CESIUM_primitive_outline` extension.
  * @private
@@ -273,7 +277,10 @@ function addOutline(
         vertexCopies[unmatchableVertexIndex] = copy;
       }
 
-      if (copy >= 65536 && triangleIndices instanceof Uint16Array) {
+      if (
+        copy > MAX_GLTF_UINT16_INDEX &&
+        triangleIndices instanceof Uint16Array
+      ) {
         // We outgrew a 16-bit index buffer, switch to 32-bit.
         triangleIndices = new Uint32Array(triangleIndices);
         triangleIndexAccessorGltf.componentType = 5125; // UNSIGNED_INT
@@ -295,6 +302,15 @@ function addOutline(
           0,
           triangleIndices.byteLength
         );
+
+        // The index componentType is also squirreled away in ModelLoadResources.
+        // Hackily update it, or else we'll end up creating the wrong type
+        // of index buffer later.
+        loadResources.indexBuffersToCreate._array.forEach(function (toCreate) {
+          if (toCreate.id === triangleIndexAccessorGltf.bufferView) {
+            toCreate.componentType = triangleIndexAccessorGltf.componentType;
+          }
+        });
       }
 
       if (unmatchableVertexIndex === i0) {

Specs/Scene/ModelOutlineLoaderSpec.js

@@ -499,13 +499,21 @@ describe(
       });
     });
 
-    it("switches to 32-bit indices if more than 65536 vertices are required", function () {
+    it("switches to 32-bit indices if more than 65535 vertices are required", function () {
+      if (!scene.context.elementIndexUint) {
+        // This extension is supported everywhere these days, except possibly
+        // in our mocked WebGL context used in the tests on Travis. Consistent
+        // with the approach in ModelSpec.js, `loads a gltf with uint32 indices`,
+        // we'll just give this test a pass if uint indices aren't supported.
+        return;
+      }
+
       var vertices = [];
       var indices = [];
       var edges = [];
 
       // Tricky model is 9 vertices. Add copies of it until we're just under 65636 vertices.
-      for (var i = 0; vertices.length / 7 + 9 <= 65536; ++i) {
+      for (var i = 0; vertices.length / 7 + 9 <= 65535; ++i) {
         createTrickyModel(vertices, indices, edges, 2, true, true, true);
       }
 
@@ -575,6 +583,10 @@ describe(
           }
         }
 
+        var rendererIndexBuffer =
+          model._rendererResources.buffers[triangleIndexAccessor.bufferView];
+        expect(rendererIndexBuffer.bytesPerIndex).toBe(4);
+
         builder.destroy();
       });
     });