Avoid holes when frustum culling is disabled, even with unconditionally-refined tiles

CHANGES.md

@@ -12,6 +12,10 @@
 - Added support for loading tilesets with `pnts` content. Point clouds are converted to `glTF`s with a single `POINTS` primitive, while batch tables are converted to `EXT_feature_metadata`.
 - Added `createTranslationRotationScaleMatrix` and `computeTranslationRotationScaleFromMatrix` methods to `CesiumGeometry::Transforms`.
 
+##### Fixes :wrench:
+
+- Fixed a bug that could cause holes to appear in a tileset, even with frustum culling disabled, when the tileset includes some empty tiles with a geometric error greater than their parent's.
+
 ### v0.21.3 - 2023-02-01
 
 ##### Fixes :wrench:

Cesium3DTilesSelection/src/Tile.cpp

@@ -188,7 +188,7 @@ bool Tile::isRenderable() const noexcept {
   }
 
   if (getState() == TileLoadState::Done) {
-    if (!isExternalContent()) {
+    if (!getUnconditionallyRefine()) {
       return std::all_of(
           this->_rasterTiles.begin(),
           this->_rasterTiles.end(),

Cesium3DTilesSelection/src/Tileset.cpp

@@ -840,31 +840,35 @@ bool Tileset::_queueLoadOfChildrenRequiredForForbidHoles(
   for (Tile& child : children) {
     this->_markTileVisited(child);
 
-    if (!child.isRenderable() && !child.isExternalContent()) {
-      waitingForChildren = true;
-
-      // While we are waiting for the child to load, we need to push along the
-      // tile and raster loading by continuing to update it.
-      this->_pTilesetContentManager->updateTileContent(
-          child,
-          tilePriority,
-          _options);
-
-      // We're using the distance to the parent tile to compute the load
-      // priority. This is fine because the relative priority of the children is
-      // irrelevant; we can't display any of them until all are loaded, anyway.
-      addTileToLoadQueue(this->_loadQueueMedium, child, tilePriority);
-    } else if (child.getUnconditionallyRefine()) {
-      // This child tile is set to unconditionally refine. That means refining
-      // _to_ it will immediately refine _through_ it. So we need to make sure
-      // its children are renderable, too.
-      // The distances are not correct for the child's children, but once again
-      // we don't care because all tiles must be loaded before we can render any
-      // of them, so their relative priority doesn't matter.
-      waitingForChildren |= this->_queueLoadOfChildrenRequiredForForbidHoles(
-          frameState,
-          child,
-          tilePriority);
+    // While we are waiting for the child to load, we need to push along the
+    // tile and raster loading by continuing to update it. This will do
+    // nothing if the tile is already loaded.
+    this->_pTilesetContentManager->updateTileContent(
+        child,
+        tilePriority,
+        _options);
+
+    // We're using the distance to the parent tile to compute the load
+    // priority. This is fine because the relative priority of the children is
+    // irrelevant; we can't display any of them until all are loaded, anyway.
+    // This will also do nothing if the tile is already loaded.
+    addTileToLoadQueue(this->_loadQueueMedium, child, tilePriority);
+
+    if (!child.isRenderable()) {
+      if (child.getUnconditionallyRefine()) {
+        // This child tile is set to unconditionally refine. That means refining
+        // _to_ it will immediately refine _through_ it. So we need to make sure
+        // its children are renderable, too.
+        // The distances are not correct for the child's children, but once
+        // again we don't care because all tiles must be loaded before we can
+        // render any of them, so their relative priority doesn't matter.
+        waitingForChildren |= this->_queueLoadOfChildrenRequiredForForbidHoles(
+            frameState,
+            child,
+            tilePriority);
+      } else {
+        waitingForChildren = true;
+      }
     }
   }
 

Cesium3DTilesSelection/test/TestTilesetSelectionAlgorithm.cpp

@@ -1090,3 +1090,149 @@ TEST_CASE("Can load example tileset.json from 3DTILES_bounding_volume_S2 "
 
   REQUIRE(pGreatGrandchild->getChildren().empty());
 }
+
+namespace {
+
+void runUnconditionallyRefinedTestCase(const TilesetOptions& options) {
+  class CustomContentLoader : public TilesetContentLoader {
+  public:
+    Tile* _pRootTile = nullptr;
+    std::optional<Promise<TileLoadResult>> _grandchildPromise;
+
+    std::unique_ptr<Tile> createRootTile() {
+      auto pRootTile = std::make_unique<Tile>(this);
+      this->_pRootTile = pRootTile.get();
+
+      pRootTile->setTileID(CesiumGeometry::QuadtreeTileID(0, 0, 0));
+
+      Cartographic center = Cartographic::fromDegrees(118.0, 32.0, 0.0);
+      pRootTile->setBoundingVolume(CesiumGeospatial::BoundingRegion(
+          GlobeRectangle(
+              center.longitude - 0.001,
+              center.latitude - 0.001,
+              center.longitude + 0.001,
+              center.latitude + 0.001),
+          0.0,
+          10.0));
+      pRootTile->setGeometricError(100000000000.0);
+
+      Tile child(this);
+      child.setTileID(CesiumGeometry::QuadtreeTileID(1, 0, 0));
+      child.setBoundingVolume(pRootTile->getBoundingVolume());
+      child.setGeometricError(1e100);
+
+      std::vector<Tile> children;
+      children.emplace_back(std::move(child));
+
+      pRootTile->createChildTiles(std::move(children));
+
+      Tile grandchild(this);
+      grandchild.setTileID(CesiumGeometry::QuadtreeTileID(1, 0, 0));
+      grandchild.setBoundingVolume(pRootTile->getBoundingVolume());
+      grandchild.setGeometricError(0.1);
+
+      std::vector<Tile> grandchildren;
+      grandchildren.emplace_back(std::move(grandchild));
+      pRootTile->getChildren()[0].createChildTiles(std::move(grandchildren));
+
+      return pRootTile;
+    }
+
+    virtual CesiumAsync::Future<TileLoadResult>
+    loadTileContent(const TileLoadInput& input) override {
+      if (&input.tile == this->_pRootTile) {
+        TileLoadResult result{};
+        result.contentKind = CesiumGltf::Model();
+        return input.asyncSystem.createResolvedFuture(std::move(result));
+      } else if (input.tile.getParent() == this->_pRootTile) {
+        TileLoadResult result{};
+        result.contentKind = TileEmptyContent();
+        return input.asyncSystem.createResolvedFuture(std::move(result));
+      } else if (
+          input.tile.getParent() != nullptr &&
+          input.tile.getParent()->getParent() == this->_pRootTile) {
+        this->_grandchildPromise =
+            input.asyncSystem.createPromise<TileLoadResult>();
+        return this->_grandchildPromise->getFuture();
+      }
+
+      return input.asyncSystem.createResolvedFuture(
+          TileLoadResult::createFailedResult(nullptr));
+    }
+
+    virtual TileChildrenResult createTileChildren(const Tile&) override {
+      return TileChildrenResult{{}, TileLoadResultState::Failed};
+    }
+  };
+
+  TilesetExternals tilesetExternals{
+      nullptr,
+      std::make_shared<SimplePrepareRendererResource>(),
+      AsyncSystem(std::make_shared<SimpleTaskProcessor>()),
+      nullptr};
+
+  auto pCustomLoader = std::make_unique<CustomContentLoader>();
+  CustomContentLoader* pRawLoader = pCustomLoader.get();
+
+  Tileset tileset(
+      tilesetExternals,
+      std::move(pCustomLoader),
+      pRawLoader->createRootTile(),
+      options);
+
+  // On the first update, we should render the root tile, even though nothing is
+  // loaded yet.
+  initializeTileset(tileset);
+  CHECK(
+      tileset.getRootTile()->getLastSelectionState().getResult(
+          tileset.getRootTile()->getLastSelectionState().getFrameNumber()) ==
+      TileSelectionState::Result::Rendered);
+
+  // On the third update, the grandchild load will still be pending.
+  // But the child is unconditionally refined, so we should render the root
+  // instead of the child.
+  initializeTileset(tileset);
+  initializeTileset(tileset);
+  const Tile& child = tileset.getRootTile()->getChildren()[0];
+  const Tile& grandchild = child.getChildren()[0];
+  CHECK(
+      tileset.getRootTile()->getLastSelectionState().getResult(
+          tileset.getRootTile()->getLastSelectionState().getFrameNumber()) ==
+      TileSelectionState::Result::Rendered);
+  CHECK(
+      child.getLastSelectionState().getResult(
+          child.getLastSelectionState().getFrameNumber()) !=
+      TileSelectionState::Result::Rendered);
+  CHECK(
+      grandchild.getLastSelectionState().getResult(
+          grandchild.getLastSelectionState().getFrameNumber()) !=
+      TileSelectionState::Result::Rendered);
+
+  REQUIRE(pRawLoader->_grandchildPromise);
+
+  // Once the grandchild is loaded, it should be rendered instead.
+  TileLoadResult result{};
+  result.contentKind = CesiumGltf::Model();
+  pRawLoader->_grandchildPromise->resolve(std::move(result));
+
+  initializeTileset(tileset);
+
+  CHECK(
+      grandchild.getLastSelectionState().getResult(
+          grandchild.getLastSelectionState().getFrameNumber()) ==
+      TileSelectionState::Result::Rendered);
+}
+
+} // namespace
+
+TEST_CASE("An unconditionally-refined tile is not rendered") {
+  SECTION("With default settings") {
+    runUnconditionallyRefinedTestCase(TilesetOptions());
+  }
+
+  SECTION("With forbidHoles enabled") {
+    TilesetOptions options{};
+    options.forbidHoles = true;
+    runUnconditionallyRefinedTestCase(options);
+  }
+}