Fix review issues

CesiumGS · Dec 20, 2024 · 1b7b8ce · 1b7b8ce
1 parent 61c1fa5
commit 1b7b8ce
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 20 deletions.
diff --git a/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp b/Cesium3DTilesContent/src/I3dmToGltfConverter.cpp
@@ -17,7 +17,6 @@
 #include <CesiumGltf/Model.h>
 #include <CesiumGltfContent/GltfUtilities.h>
 #include <CesiumGltfReader/GltfReader.h>
-#include <CesiumUtility/Assert.h>
 #include <CesiumUtility/AttributeCompression.h>
 #include <CesiumUtility/Math.h>
 #include <CesiumUtility/Uri.h>
@@ -354,7 +353,7 @@ CesiumAsync::Future<ConvertedI3dm> convertI3dmContent(
   if (!parsedJsonResult) {
     return finishEarly();
   }
-  const I3dmContent& parsedContent = *parsedJsonResult;
+  I3dmContent& parsedContent = *parsedJsonResult;
   decodedInstances.rtcCenter = parsedContent.rtcCenter;
   decodedInstances.rotationENU = parsedContent.eastNorthUp;
 
@@ -370,8 +369,7 @@ CesiumAsync::Future<ConvertedI3dm> convertI3dmContent(
             pBinaryData + *parsedContent.position),
         numInstances);
     decodedInstances.positions.assign(rawPositions.begin(), rawPositions.end());
-  } else {
-    CESIUM_ASSERT(parsedContent.positionQuantized.has_value());
+  } else if (parsedContent.positionQuantized) {
     std::span<const uint16_t[3]> rawQuantizedPositions(
         reinterpret_cast<const uint16_t(*)[3]>(
             pBinaryData + *parsedContent.positionQuantized),
@@ -390,7 +388,11 @@ CesiumAsync::Future<ConvertedI3dm> convertI3dmContent(
           }
           return position;
         });
+  } else {
+    parsedContent.errors.emplaceError(
+        "Missing position or positionQuantized in parsed content");
   }
+
   decodedInstances.rotations.resize(
       numInstances,
       glm::quat(1.0f, 0.0f, 0.0f, 0.0f));

diff --git a/Cesium3DTilesContent/src/PntsToGltfConverter.cpp b/Cesium3DTilesContent/src/PntsToGltfConverter.cpp
@@ -1138,7 +1138,8 @@ void parsePositionsFromFeatureTableBinary(
       reinterpret_cast<glm::vec3*>(positionData.data()),
       pointsLength);
 
-  if (parsedContent.positionQuantized) {
+  if (parsedContent.positionQuantized && parsedContent.quantizedVolumeScale &&
+      parsedContent.quantizedVolumeOffset) {
     // PERFORMANCE_IDEA: In the future, it might be more performant to detect
     // if the recipient engine can handle dequantization itself, and if so, use
     // the KHR_mesh_quantization extension to avoid dequantizing here.
@@ -1147,10 +1148,8 @@ void parsePositionsFromFeatureTableBinary(
             featureTableBinaryData.data() + parsedContent.position.byteOffset),
         pointsLength);
 
-    CESIUM_ASSERT(parsedContent.quantizedVolumeScale.has_value());
     const glm::vec3 quantizedVolumeScale(
         parsedContent.quantizedVolumeScale.value());
-    CESIUM_ASSERT(parsedContent.quantizedVolumeOffset.has_value());
     const glm::vec3 quantizedVolumeOffset(
         parsedContent.quantizedVolumeOffset.value());
 
@@ -1170,6 +1169,10 @@ void parsePositionsFromFeatureTableBinary(
       parsedContent.positionMax =
           glm::max(parsedContent.positionMax, dequantizedPosition);
     }
+  } else if (parsedContent.positionQuantized) {
+    parsedContent.errors.emplaceError(
+        "Missing quantizedVolumeScale or quantizedVolumeOffset in parsed "
+        "content");
   } else {
     // The position accessor min / max is required by the glTF spec, so
     // use a for loop instead of std::memcpy.
@@ -1665,13 +1668,13 @@ void convertPntsContentToGltf(
           std::span<const std::byte>(parsedContent.dracoBatchTableBinary);
     }
 
-    CESIUM_ASSERT(result.model.has_value());
-
-    result.errors.merge(BatchTableToGltfStructuralMetadata::convertFromPnts(
-        featureTableJson,
-        batchTableJson,
-        batchTableBinaryData,
-        result.model.value()));
+    if (result.model) {
+      result.errors.merge(BatchTableToGltfStructuralMetadata::convertFromPnts(
+          featureTableJson,
+          batchTableJson,
+          batchTableBinaryData,
+          result.model.value()));
+    }
   }
 }
 } // namespace

diff --git a/Cesium3DTilesSelection/include/Cesium3DTilesSelection/TilesetOptions.h b/Cesium3DTilesSelection/include/Cesium3DTilesSelection/TilesetOptions.h
@@ -4,7 +4,7 @@
 #include <CesiumGeospatial/Ellipsoid.h>
 #include <CesiumGltf/Ktx2TranscodeTargets.h>
 
-#include <cstddef>
+#include <any>
 #include <functional>
 #include <memory>
 #include <optional>
@@ -204,7 +204,7 @@ struct CESIUM3DTILESSELECTION_API TilesetOptions {
    * unloaded until the total is under this number or until only required tiles
    * remain, whichever comes first.
    */
-  int64_t maximumCachedBytes = static_cast<int64_t>(512 * 1024 * 1024);
+  int64_t maximumCachedBytes = 512LL * 1024 * 1024;
 
   /**
    * @brief A table that maps the camera height above the ellipsoid to a fog

diff --git a/Cesium3DTilesSelection/src/EllipsoidTilesetLoader.cpp b/Cesium3DTilesSelection/src/EllipsoidTilesetLoader.cpp
@@ -174,12 +174,12 @@ BoundingRegion EllipsoidTilesetLoader::createBoundingRegion(
 
 EllipsoidTilesetLoader::Geometry
 EllipsoidTilesetLoader::createGeometry(const Tile& tile) const {
-  static constexpr uint16_t resolution = 24;
+  static constexpr size_t resolution = 24;
 
   std::vector<uint16_t> indices;
-  indices.reserve(static_cast<size_t>(6 * (resolution - 1) * (resolution - 1)));
+  indices.reserve(6 * (resolution - 1) * (resolution - 1));
 
-  std::vector<glm::vec3> vertices(static_cast<size_t>(resolution * resolution));
+  std::vector<glm::vec3> vertices(resolution * resolution);
   std::vector<glm::vec3> normals(vertices.size());
 
   const Ellipsoid& ellipsoid = _projection.getEllipsoid();

diff --git a/Cesium3DTilesSelection/src/TilesetJsonLoader.cpp b/Cesium3DTilesSelection/src/TilesetJsonLoader.cpp
@@ -996,7 +996,8 @@ TilesetJsonLoader::loadTileContent(const TileLoadInput& loadInput) {
                        upAxis,
                        tileUrl,
                        pAssetAccessor,
-                       pCompletedRequest = std::move(pCompletedRequest)](GltfConverterResult&& result) mutable {
+                       pCompletedRequest = std::move(pCompletedRequest)](
+                          GltfConverterResult&& result) mutable {
                         logTileLoadResult(pLogger, tileUrl, result.errors);
                         if (result.errors) {
                           return TileLoadResult::createFailedResult(