Merge pull request #12893 from CesiumGS/fix-pnts-draco-loading

Fix pnts draco loading

Author: ggetz

CHANGES.md

@@ -11,6 +11,7 @@
 - Fix flickering artifact in Gaussian splat models caused by incorrect sorting results. [#12662](https://github.com/CesiumGS/cesium/issues/12662)
 - Improved performance and reduced memory usage of `Event` class. [#12896](https://github.com/CesiumGS/cesium/pull/12896)
 - Fixes vertical misalignment of glyphs in labels with small fonts [#8474](https://github.com/CesiumGS/cesium/issues/8474)
+- Prevent runtime errors for certain forms of invalid PNTS files [#12872](https://github.com/CesiumGS/cesium/issues/12872)
 
 #### Additions :tada:
 

Specs/Data/Cesium3DTiles/PointCloud/PointCloudDracoInvalid/README.md

@@ -0,0 +1,14 @@
+# PointCloudDracoInvalid
+
+A test related to https://github.com/CesiumGS/cesium/issues/12872 :
+
+For a certain period of time (2 years), and under certain conditions, the
+point cloud tiler generated invalid PNTS files when draco compression
+was enabled. The batch table could define certain properties as binary
+body references, without a batch table binary being present, and without
+the `3DTILES_draco_point_compression` extension object being present
+in the batch table JSON.
+
+The PNTS file here is one PNTS file from the data set that was linked
+in the issue (with details in the issue description). This is to
+ensure that CesiumJS gracefully handles this case, without crashing.

Specs/Data/Cesium3DTiles/PointCloud/PointCloudDracoInvalid/pointCloudDracoInvalid.pnts

@@ -0,0 +1,16 @@
+{
+  "asset": {
+    "version": "1.0"
+  },
+  "extensionsRequired": ["3DTILES_draco_point_compression"],
+  "extensionsUsed": ["3DTILES_draco_point_compression"],
+  "geometricError": 1.7320508075688772,
+  "root": {
+    "boundingVolume": {
+      "box": [0.5, 0.5, 0.5, 0.5, 0.0, 0.0, 0.0, 0.5, 0.0, 0.0, 0.0, 0.5]
+    },
+    "content": { "uri": "pointCloudDracoInvalid.pnts" },
+    "geometricError": 0.0,
+    "refine": "ADD"
+  }
+}

Specs/Data/Cesium3DTiles/PointCloud/PointCloudDracoInvalid/tileset.json

@@ -6,7 +6,7 @@ import ComponentDatatype from "../Core/ComponentDatatype.js";
 import defined from "../Core/defined.js";
 import DeveloperError from "../Core/DeveloperError.js";
 import getBinaryAccessor from "./getBinaryAccessor.js";
-import RuntimeError from "../Core/RuntimeError.js";
+import Cesium3DTileBatchTable from "./Cesium3DTileBatchTable.js";
 
 /**
  * Object for handling the <code>3DTILES_batch_table_hierarchy</code> extension
@@ -127,7 +127,7 @@ function initialize(hierarchy, hierarchyJson, binaryBody) {
   for (i = 0; i < classesLength; ++i) {
     const classInstancesLength = classes[i].length;
     const properties = classes[i].instances;
-    const binaryProperties = getBinaryProperties(
+    const binaryProperties = Cesium3DTileBatchTable.getBinaryProperties(
       classInstancesLength,
       properties,
       binaryBody,
@@ -154,54 +154,6 @@ function initialize(hierarchy, hierarchyJson, binaryBody) {
   hierarchy._byteLength = byteLength;
 }
 
-function getBinaryProperties(featuresLength, properties, binaryBody) {
-  let binaryProperties;
-  for (const name in properties) {
-    if (properties.hasOwnProperty(name)) {
-      const property = properties[name];
-      const byteOffset = property.byteOffset;
-      if (defined(byteOffset)) {
-        // This is a binary property
-        const componentType = property.componentType;
-        const type = property.type;
-        if (!defined(componentType)) {
-          throw new RuntimeError("componentType is required.");
-        }
-        if (!defined(type)) {
-          throw new RuntimeError("type is required.");
-        }
-        if (!defined(binaryBody)) {
-          throw new RuntimeError(
-            `Property ${name} requires a batch table binary.`,
-          );
-        }
-
-        const binaryAccessor = getBinaryAccessor(property);
-        const componentCount = binaryAccessor.componentsPerAttribute;
-        const classType = binaryAccessor.classType;
-        const typedArray = binaryAccessor.createArrayBufferView(
-          binaryBody.buffer,
-          binaryBody.byteOffset + byteOffset,
-          featuresLength,
-        );
-
-        if (!defined(binaryProperties)) {
-          binaryProperties = {};
-        }
-
-        // Store any information needed to access the binary data, including the typed array,
-        // componentCount (e.g. a VEC4 would be 4), and the type used to pack and unpack (e.g. Cartesian4).
-        binaryProperties[name] = {
-          typedArray: typedArray,
-          componentCount: componentCount,
-          type: classType,
-        };
-      }
-    }
-  }
-  return binaryProperties;
-}
-
 function countBinaryPropertyMemory(binaryProperties) {
   let byteLength = 0;
   for (const name in binaryProperties) {

packages/engine/Source/Scene/BatchTableHierarchy.js

@@ -5,6 +5,7 @@ import combine from "../Core/combine.js";
 import ComponentDatatype from "../Core/ComponentDatatype.js";
 import defined from "../Core/defined.js";
 import getJsonFromTypedArray from "../Core/getJsonFromTypedArray.js";
+import oneTimeWarning from "../Core/oneTimeWarning.js";
 import RuntimeError from "../Core/RuntimeError.js";
 import AttributeType from "./AttributeType.js";
 import Cesium3DTileFeatureTable from "./Cesium3DTileFeatureTable.js";
@@ -185,9 +186,73 @@ PntsParser.parse = function (arrayBuffer, byteOffset) {
     parsedContent.batchTableBinary = batchTableBinary;
   }
 
+  // Handle the case that binary body references are contained in the
+  // batch table without a batch table binary being present
+  removeInvalidBinaryBodyReferences(parsedContent);
+
   return parsedContent;
 };
 
+/**
+ * Remove all invalid binary body references from the batch table
+ * JSON of the given parsed content.
+ *
+ * This is a workaround for gracefully handling the invalid PNTS
+ * files that may have been created by the point cloud tiler.
+ * See https://github.com/CesiumGS/cesium/issues/12872
+ *
+ * When the batch table JSON is undefined, nothing will be done.
+ * When the batch table binary is defined, nothing will be done
+ * (assuming that any binary body references are valid - this is
+ * not checked here).
+ *
+ * Otherwise, this will remove all binary body references from the
+ * batch table JSON that are not resolved from draco via the
+ * `parsedContent.draco.batchTableProperties`.
+ *
+ * If any (invalid) binary body reference is found (and removed),
+ * a one-time warning will be printed.
+ *
+ * @param {object} parsedContent The parsed content
+ */
+function removeInvalidBinaryBodyReferences(parsedContent) {
+  const batchTableJson = parsedContent.batchTableJson;
+  if (!defined(batchTableJson)) {
+    return;
+  }
+  const batchTableBinary = parsedContent.batchTableBinary;
+  if (defined(batchTableBinary)) {
+    return;
+  }
+  const dracoBatchTablePropertyNames = Object.keys(
+    parsedContent.draco?.batchTableProperties ?? {},
+  );
+
+  // Collect the names of all binary body references (identified
+  // by the property having a `byteOffset`) that have not been
+  // resolved via the parsedContent.draco.batchTableProperties
+  const invalidBinaryBodyReferenceNames = [];
+  for (const name of Object.keys(batchTableJson)) {
+    const property = batchTableJson[name];
+    const byteOffset = property.byteOffset;
+    if (defined(byteOffset)) {
+      if (!dracoBatchTablePropertyNames.includes(name)) {
+        invalidBinaryBodyReferenceNames.push(name);
+      }
+    }
+  }
+
+  // If there have been invalid binary body references, print
+  // a one-time warning for each of them, and delete them.
+  for (const name of invalidBinaryBodyReferenceNames) {
+    oneTimeWarning(
+      `PntsParser-invalidBinaryBodyReference`,
+      `The point cloud data contained a binary property ${name} that could not be resolved - skipping`,
+    );
+    delete batchTableJson[name];
+  }
+}
+
 function parseDracoProperties(featureTable, batchTableJson) {
   const featureTableJson = featureTable.json;
   let dracoBuffer;

packages/engine/Source/Scene/PntsParser.js

@@ -48,6 +48,8 @@ describe(
       "./Data/Cesium3DTiles/PointCloud/PointCloudDracoPartial/pointCloudDracoPartial.pnts";
     const pointCloudDracoBatchedUrl =
       "./Data/Cesium3DTiles/PointCloud/PointCloudDracoBatched/pointCloudDracoBatched.pnts";
+    const pointCloudDracoInvalidUrl =
+      "./Data/Cesium3DTiles/PointCloud/PointCloudDracoInvalid/pointCloudDracoInvalid.pnts";
     const pointCloudWGS84Url =
       "./Data/Cesium3DTiles/PointCloud/PointCloudWGS84/pointCloudWGS84.pnts";
     const pointCloudBatchedUrl =
@@ -538,6 +540,18 @@ describe(
       });
     });
 
+    it("loads PointCloudDracoInvalid without crashing", async function () {
+      // Test for https://github.com/CesiumGS/cesium/issues/12872:
+      // PNTS files that are invalid due to a missing batch table
+      // binary and draco compression extension object should
+      // load. (The metadata is expected to be empty here)
+      const loader = await loadPnts(pointCloudDracoInvalidUrl);
+      const components = loader.components;
+      expect(components).toBeDefined();
+      const isBatched = false;
+      expectMetadata(components.structuralMetadata, {}, isBatched);
+    });
+
     it("loads PointCloudWGS84", function () {
       return loadPnts(pointCloudWGS84Url).then(function (loader) {
         const components = loader.components;