glTF modifier: fixed case of raster overlays

Cesium3DTilesSelection/include/Cesium3DTilesSelection/TileContent.h

@@ -251,13 +251,32 @@ class CESIUM3DTILESSELECTION_API TileRenderContent {
    */
   void replaceWithModifiedModel() noexcept;
 
+  /**
+   * @brief Returns whether this tile is currently being up-sampled.
+   * It should only be called by the main thread.
+   */
+  bool isBeingUpSampled() const noexcept;
+
+  /**
+   * @brief Increment the current up-sampling task count.
+   * It should only be called by the main thread.
+   */
+  void incrementUpSamplingTaskCount() const noexcept;
+
+  /**
+   * @brief Decrement the current up-sampling task count.
+   * It should only be called by the main thread.
+   */
+  void decrementUpSamplingTaskCount() const noexcept;
+
 private:
   CesiumGltf::Model _model;
   void* _pRenderResources;
 
   GltfModifierState _modifierState;
   std::optional<CesiumGltf::Model> _modifiedModel;
   void* _pModifiedRenderResources;
+  mutable int32_t _activeUpSamplingTaskCount;
 
   CesiumRasterOverlays::RasterOverlayDetails _rasterOverlayDetails;
   std::vector<CesiumUtility::Credit> _credits;

Cesium3DTilesSelection/src/RasterMappedTo3DTile.cpp

@@ -88,6 +88,13 @@ RasterOverlayTile::MoreDetailAvailable RasterMappedTo3DTile::update(
                : RasterOverlayTile::MoreDetailAvailable::No;
   }
 
+  auto const isTileUpToDate = [](Tile const& testedTile) {
+    const TileRenderContent* pRenderContent =
+        testedTile.getContent().getRenderContent();
+    return testedTile.getState() == TileLoadState::Done && pRenderContent &&
+           pRenderContent->getGltfModifierState() == GltfModifierState::Idle;
+  };
+
   // If the loading tile has failed, try its parent's loading tile.
   Tile* pTile = &tile;
   while (this->_pLoadingTile &&
@@ -143,14 +150,16 @@ RasterOverlayTile::MoreDetailAvailable RasterMappedTo3DTile::update(
           *pTile,
           this->_pLoadingTile->getTileProvider().getOwner());
       if (pCandidate &&
-          pCandidate->getState() >= RasterOverlayTile::LoadState::Loaded) {
+          pCandidate->getState() >= RasterOverlayTile::LoadState::Loaded &&
+          isTileUpToDate(*pTile)) {
         break;
       }
       pTile = pTile->getParent();
     }
 
     if (pCandidate &&
         pCandidate->getState() >= RasterOverlayTile::LoadState::Loaded &&
+        isTileUpToDate(*pTile) &&
         this->_pReadyTile != pCandidate) {
       if (this->getState() != AttachmentState::Unattached) {
         prepareRendererResources.detachRasterInMainThread(
@@ -180,7 +189,8 @@ RasterOverlayTile::MoreDetailAvailable RasterMappedTo3DTile::update(
 
   // Attach the ready tile if it's not already attached.
   if (this->_pReadyTile &&
-      this->getState() == RasterMappedTo3DTile::AttachmentState::Unattached) {
+      this->getState() == RasterMappedTo3DTile::AttachmentState::Unattached &&
+      isTileUpToDate(tile)) {
     this->_pReadyTile->loadInMainThread();
 
     prepareRendererResources.attachRasterInMainThread(

Cesium3DTilesSelection/src/RasterOverlayUpsampler.cpp

@@ -58,9 +58,21 @@ RasterOverlayUpsampler::loadTileContent(const TileLoadInput& loadInput) {
         TileLoadResult::createFailedResult(loadInput.pAssetAccessor, nullptr));
   }
 
+  if (pParentRenderContent->getGltfModifierState() ==
+      GltfModifierState::WorkerRunning) {
+    // Parent is currently being modified, so it would be useless to upsample
+    // the version about to be replaced - also, its rasterOverlayProjections
+    // may have been emptied in order to be recomputed as well.
+    return loadInput.asyncSystem.createResolvedFuture(
+        TileLoadResult::createRetryLaterResult(
+            loadInput.pAssetAccessor,
+            nullptr));
+  }
+
   size_t index = 0;
   const std::vector<CesiumGeospatial::Projection>& parentProjections =
       pParentRenderContent->getRasterOverlayDetails().rasterOverlayProjections;
+  CESIUM_ASSERT(!parentProjections.empty());
   for (const RasterMappedTo3DTile& mapped : pParent->getMappedRasterTiles()) {
     if (mapped.isMoreDetailAvailable()) {
       const CesiumGeospatial::Projection& projection =
@@ -79,13 +91,24 @@ RasterOverlayUpsampler::loadTileContent(const TileLoadInput& loadInput) {
       getProjectionEllipsoid(projection);
 
   const CesiumGltf::Model& parentModel = pParentRenderContent->getModel();
-  return loadInput.asyncSystem.runInWorkerThread(
-      [&parentModel,
-       ellipsoid,
-       transform = loadInput.tile.getTransform(),
-       textureCoordinateIndex = index,
-       tileID = *pTileID,
-       pAssetAccessor = loadInput.pAssetAccessor]() mutable {
+
+  pParentRenderContent->incrementUpSamplingTaskCount();
+  return loadInput.asyncSystem
+      .runInWorkerThread([&parentModel,
+                          pParentRenderContent,
+                          ellipsoid,
+                          transform = loadInput.tile.getTransform(),
+                          textureCoordinateIndex = index,
+                          tileID = *pTileID,
+                          pAssetAccessor = loadInput.pAssetAccessor]() mutable {
+        if (pParentRenderContent->getGltfModifierState() ==
+            GltfModifierState::WorkerRunning) {
+          // Parent tile is being modified, no need to spend time upsampling an
+          // obsolete version.
+          return TileLoadResult::createRetryLaterResult(
+              pAssetAccessor,
+              nullptr);
+        }
         auto model = RasterOverlayUtilities::upsampleGltfForRasterOverlays(
             parentModel,
             tileID,
@@ -119,6 +142,11 @@ RasterOverlayUpsampler::loadTileContent(const TileLoadInput& loadInput) {
             {},
             TileLoadResultState::Success,
             ellipsoid};
+      })
+      .thenInMainThread([pParentRenderContent](TileLoadResult&& result) {
+        CESIUM_ASSERT(pParentRenderContent->isBeingUpSampled());
+        pParentRenderContent->decrementUpSamplingTaskCount();
+        return std::move(result);
       });
 }
 

Cesium3DTilesSelection/src/TileContent.cpp

@@ -20,6 +20,7 @@ TileRenderContent::TileRenderContent(CesiumGltf::Model&& model)
       _modifierState{GltfModifierState::Idle},
       _modifiedModel{},
       _pModifiedRenderResources(nullptr),
+      _activeUpSamplingTaskCount(0),
       _rasterOverlayDetails{},
       _credits{},
       _lodTransitionFadePercentage{0.0f} {}
@@ -72,6 +73,7 @@ void TileRenderContent::resetModifiedModelAndRenderResources() noexcept {
 
 void TileRenderContent::replaceWithModifiedModel() noexcept {
   CESIUM_ASSERT(this->_modifiedModel);
+  CESIUM_ASSERT(!this->isBeingUpSampled());
   if (this->_modifiedModel) {
     this->_model = std::move(*this->_modifiedModel);
     // reset after move because this is tested for nullopt in
@@ -82,6 +84,19 @@ void TileRenderContent::replaceWithModifiedModel() noexcept {
   }
 }
 
+bool TileRenderContent::isBeingUpSampled() const noexcept {
+  return this->_activeUpSamplingTaskCount > 0;
+}
+
+void TileRenderContent::incrementUpSamplingTaskCount() const noexcept {
+  this->_activeUpSamplingTaskCount++;
+}
+
+void TileRenderContent::decrementUpSamplingTaskCount() const noexcept {
+  CESIUM_ASSERT(this->_activeUpSamplingTaskCount > 0);
+  this->_activeUpSamplingTaskCount--;
+}
+
 const RasterOverlayDetails&
 TileRenderContent::getRasterOverlayDetails() const noexcept {
   return this->_rasterOverlayDetails;

Cesium3DTilesSelection/src/TilesetContentManager.cpp

@@ -567,15 +567,21 @@ postProcessContentInWorkerThread(
 
   std::optional<int64_t> version =
       pGltfModifier ? pGltfModifier->getCurrentVersion() : std::nullopt;
+  const bool needsApplyGltfModifier = (pGltfModifier && version);
 
   auto asyncSystem = tileLoadInfo.asyncSystem;
 
   result.initialBoundingVolume = tileLoadInfo.tileBoundingVolume;
   result.initialContentBoundingVolume = tileLoadInfo.tileContentBoundingVolume;
 
-  postProcessGltfInWorkerThread(result, std::move(projections), tileLoadInfo);
+  // Important: post-process must be performed *after* GltfModifier, as it does
+  // add attribute _CESIUMOVERLAY_xxx, which is required for raster overlays.
+  if (!needsApplyGltfModifier) {
+    postProcessGltfInWorkerThread(result, std::move(projections), tileLoadInfo);
+  }
 
-  auto applyGltfModifier = [&]() {
+  auto applyGltfModifier = [&](std::vector<CesiumGeospatial::Projection>&&
+                                   projections) {
     // Apply the glTF modifier right away, otherwise it will be
     // triggered immediately after the renderer-side resources
     // have been created, which is both inefficient and a cause
@@ -593,7 +599,10 @@ postProcessContentInWorkerThread(
             .tileTransform = tileLoadInfo.tileTransform})
         .thenInWorkerThread(
             [result = std::move(result),
-             version](std::optional<GltfModifierOutput>&& modified) mutable {
+             version,
+             projections = std::move(projections),
+             tileLoadInfo = std::move(tileLoadInfo)](
+                std::optional<GltfModifierOutput>&& modified) mutable {
               if (modified) {
                 result.contentKind = std::move(modified->modifiedModel);
               }
@@ -604,14 +613,22 @@ postProcessContentInWorkerThread(
                 GltfModifierVersionExtension::setVersion(*pModel, *version);
               }
 
-              return result;
-            })
-        .thenPassThrough(std::move(tileLoadInfo));
+              // Important: post-process must be performed *after* the model
+              // is modified.
+              postProcessGltfInWorkerThread(
+                  result,
+                  std::move(projections),
+                  tileLoadInfo);
+
+              return std::make_tuple(
+                  std::move(tileLoadInfo),
+                  std::move(result));
+            });
   };
 
   CesiumAsync::Future<std::tuple<TileContentLoadInfo, TileLoadResult>> future =
-      pGltfModifier && version
-          ? applyGltfModifier()
+      needsApplyGltfModifier
+          ? applyGltfModifier(std::move(projections))
           : tileLoadInfo.asyncSystem.createResolvedFuture(std::move(result))
                 .thenPassThrough(std::move(tileLoadInfo));
 
@@ -991,6 +1008,25 @@ void TilesetContentManager::reapplyGltfModifier(
   CESIUM_ASSERT(externals.pGltfModifier->getCurrentVersion());
   int64_t version = externals.pGltfModifier->getCurrentVersion().value_or(-1);
 
+  // Ensure raster overlays will be recomputed for this tile, BUT restore
+  // overlay details afterwards, or the tile could be unloaded in
+  // #updateDoneState (see test on status.firstIndexWithMissingProjection).
+  auto const rasterOverlayDetails =
+      pRenderContent->getRasterOverlayDetails();
+  pRenderContent->setRasterOverlayDetails({});
+  std::vector<CesiumGeospatial::Projection> projections =
+      this->_overlayCollection.addTileOverlays(tile, tilesetOptions);
+  pRenderContent->setRasterOverlayDetails(rasterOverlayDetails);
+
+  TileContentLoadInfo tileLoadInfo{
+      this->_externals.asyncSystem,
+      this->_externals.pAssetAccessor,
+      this->_externals.pPrepareRendererResources,
+      this->_externals.pLogger,
+      this->_pSharedAssetSystem,
+      tilesetOptions.contentOptions,
+      tile};
+
   // It is safe to capture the TilesetExternals and Model by reference because
   // the TilesetContentManager guarantees both will continue to exist and are
   // immutable while modification is in progress.
@@ -1022,13 +1058,15 @@ void TilesetContentManager::reapplyGltfModifier(
                            tileBoundingVolume = tile.getBoundingVolume(),
                            tileContentBoundingVolume =
                                tile.getContentBoundingVolume(),
-                           rendererOptions = tilesetOptions.rendererOptions](
-                              std::optional<GltfModifierOutput>&& modified) {
+                           rendererOptions = tilesetOptions.rendererOptions,
+                           tileLoadInfo = std::move(tileLoadInfo),
+                           ellipsoid = tilesetOptions.ellipsoid,
+                           projections = std::move(projections)](
+                              std::optional<GltfModifierOutput>&&
+                                  modified) mutable {
         TileLoadResult tileLoadResult;
         tileLoadResult.state = TileLoadResultState::Success;
         tileLoadResult.pAssetAccessor = externals.pAssetAccessor;
-        tileLoadResult.rasterOverlayDetails =
-            pRenderContent->getRasterOverlayDetails();
         tileLoadResult.initialBoundingVolume = tileBoundingVolume;
         tileLoadResult.initialContentBoundingVolume = tileContentBoundingVolume;
 
@@ -1042,10 +1080,24 @@ void TilesetContentManager::reapplyGltfModifier(
           }
         }
 
+        tileLoadResult.ellipsoid = ellipsoid;
         if (modified) {
           tileLoadResult.contentKind = std::move(modified->modifiedModel);
+
+          // Apply post-process to the modified model.
+          postProcessGltfInWorkerThread(
+              tileLoadResult,
+              std::move(projections),
+              tileLoadInfo);
+          if (tileLoadResult.rasterOverlayDetails) {
+            pRenderContent->setRasterOverlayDetails(
+                *tileLoadResult.rasterOverlayDetails);
+          }
         } else {
+          // No modification could be reapplied => we'll keep previous model and
+          // render resources.
           tileLoadResult.contentKind = previousModel;
+          tileLoadResult.state = TileLoadResultState::Failed;
         }
 
         if (modified && externals.pPrepareRendererResources) {
@@ -1111,15 +1163,18 @@ void TilesetContentManager::reapplyGltfModifier(
           pRenderContent->setGltfModifierState(GltfModifierState::WorkerDone);
 
           // The modified model is up-to-date with the version that triggered
-          // this run.
+          // this run (even though the modification did nothing or failed, as it
+          // can happen for upsampling typically).
           CesiumGltf::Model& modifiedModel =
               std::get<CesiumGltf::Model>(pair.result.contentKind);
           GltfModifierVersionExtension::setVersion(modifiedModel, version);
 
-          pRenderContent->setModifiedModelAndRenderResources(
-              std::move(modifiedModel),
-              pair.pRenderResources);
-          this->_externals.pGltfModifier->onWorkerThreadApplyComplete(*pTile);
+          if (pair.result.state == TileLoadResultState::Success) {
+            pRenderContent->setModifiedModelAndRenderResources(
+                std::move(modifiedModel),
+                pair.pRenderResources);
+            this->_externals.pGltfModifier->onWorkerThreadApplyComplete(*pTile);
+          }
         }
       })
       .catchInMainThread(
@@ -1601,6 +1656,14 @@ void TilesetContentManager::finishLoading(
 
   if (this->_externals.pGltfModifier &&
       this->_externals.pGltfModifier->needsMainThreadModification(tile)) {
+
+    if (pRenderContent->isBeingUpSampled()) {
+      // If this tile is currently being up-sampled in a worker thread, we
+      // cannot replace its model. Return so that we do not block the main
+      // thread (finishLoading will be called again later).
+      return;
+    }
+
     // Free outdated render resources before replacing them.
     if (this->_externals.pPrepareRendererResources) {
       this->_externals.pPrepareRendererResources->free(

CesiumRasterOverlays/src/RasterOverlayUtilities.cpp

@@ -594,8 +594,8 @@ RasterOverlayUtilities::upsampleGltfForRasterOverlays(
         mesh.primitives.erase(mesh.primitives.begin() + ptrdiff_t(i));
         --i;
       }
-      containsPrimitives |= !mesh.primitives.empty();
     }
+    containsPrimitives |= !mesh.primitives.empty();
   }
 
   return containsPrimitives ? std::make_optional<Model>(std::move(result))