Fix tile selection bug, add some helpful tools for debugging

CHANGES.md

@@ -9,12 +9,16 @@
 - Added `value_type` typedef to `AccessorWriter`.
 - Added `InstanceAttributeSemantics` to `CesiumGltf`.
 - Added `VertexAttributeSemantics::FEATURE_ID_n`.
+- Added a `const` version of `Tileset::forEachLoadedTile`.
+- Added `DebugTileStateDatabase`, which provides tools for debugging the tile selection algorithm using SQLite.
+- Added `CesiumAsync::SqliteHelper`, containing functions for working with SQLite.
 - Updates generated classes for `EXT_structural_metadata`. See https://github.com/CesiumGS/glTF/pull/71.
 
 ##### Fixes :wrench:
 
 - Fixed a bug in `thenPassThrough` that caused a compiler error when given a value by r-value refrence.
 - Fixed a bug in  `SubtreeFileReader::loadBinary` that prevented valid subtrees from loading if they did not contain binary data.
+- Fixed a bug in the `Tileset` selection algorithm that could cause detail to disappear during load in some cases.
 
 ### v0.42.0 - 2024-12-02
 

Cesium3DTilesSelection/include/Cesium3DTilesSelection/DebugTileStateDatabase.h

@@ -0,0 +1,48 @@
+#pragma once
+
+#include <memory>
+#include <string>
+
+namespace Cesium3DTilesSelection {
+
+class Tile;
+class Tileset;
+
+/**
+ * @brief Helps debug the tile selection algorithm by recording the state of
+ * tiles each frame to a SQLite database.
+ */
+class DebugTileStateDatabase {
+public:
+  /**
+   * @brief Creates a new instance.
+   *
+   * @param databaseFilename The full path and filename of the output SQLite
+   * database.
+   */
+  DebugTileStateDatabase(const std::string& databaseFilename);
+  ~DebugTileStateDatabase() noexcept;
+
+  /**
+   * @brief Records the state of all tiles that are currently loaded by the
+   * given tileset.
+   *
+   * @param frameNumber The current frame number.
+   * @param tileset The tileset.
+   */
+  void recordAllTileStates(int32_t frameNumber, const Tileset& tileset);
+
+  /**
+   * @brief Records the state of a given tile.
+   *
+   * @param frameNumber The current frame number.
+   * @param tile The tile.
+   */
+  void recordTileState(int32_t frameNumber, const Tile& tile);
+
+private:
+  struct Impl;
+  std::unique_ptr<Impl> _pImpl;
+};
+
+} // namespace Cesium3DTilesSelection

Cesium3DTilesSelection/include/Cesium3DTilesSelection/Tileset.h

@@ -235,6 +235,14 @@ class CESIUM3DTILESSELECTION_API Tileset final {
    */
   void forEachLoadedTile(const std::function<void(Tile& tile)>& callback);
 
+  /**
+   * @brief Invokes a function for each tile that is currently loaded.
+   *
+   * @param callback The function to invoke.
+   */
+  void forEachLoadedTile(
+      const std::function<void(const Tile& tile)>& callback) const;
+
   /**
    * @brief Gets the total number of bytes of tile and raster overlay data that
    * are currently loaded.

Cesium3DTilesSelection/src/DebugTileStateDatabase.cpp

@@ -0,0 +1,267 @@
+#include <Cesium3DTilesSelection/DebugTileStateDatabase.h>
+#include <Cesium3DTilesSelection/Tileset.h>
+#include <CesiumAsync/SqliteHelper.h>
+
+#include <sqlite3.h>
+
+using namespace CesiumAsync;
+
+namespace {
+
+const std::string DROP_STATE_TABLE_SQL = "DROP TABLE IF EXISTS TileStates";
+const std::string CREATE_STATE_TABLE_SQL =
+    "CREATE TABLE IF NOT EXISTS TileSelectionStates ("
+    " Value INTEGER PRIMARY KEY,"
+    " Name TEXT"
+    ");"
+    "CREATE TABLE IF NOT EXISTS IsRenderableStates ("
+    " Value INTEGER PRIMARY KEY, "
+    " Name TEXT"
+    ");"
+    "CREATE TABLE IF NOT EXISTS TileSelectionFrames ("
+    " Pointer INTEGER NOT NULL,"
+    " FrameNumber INTEGER NOT NULL,"
+    " TileID TEXT,"
+    " SelectionStateFrameNumber INTEGER,"
+    " SelectionState INTEGER,"
+    " IsRenderable BOOLEAN,"
+    " PRIMARY KEY (Pointer, FrameNumber),"
+    " FOREIGN KEY (SelectionState) REFERENCES TileSelectionStates(Value),"
+    " FOREIGN KEY (IsRenderable) REFERENCES IsRenderableStates(Value)"
+    ")";
+const std::string PRAGMA_WAL_SQL = "PRAGMA journal_mode=WAL";
+const std::string PRAGMA_SYNC_SQL = "PRAGMA synchronous=OFF";
+const std::string WRITE_TILE_SELECTION_SQL =
+    "REPLACE INTO TileSelectionFrames ("
+    " Pointer,"
+    " FrameNumber,"
+    " TileID,"
+    " SelectionStateFrameNumber,"
+    " SelectionState,"
+    " IsRenderable"
+    ") VALUES ("
+    " ?, ?, ?, ?, ?, ?"
+    ")";
+
+} // namespace
+
+namespace Cesium3DTilesSelection {
+
+struct DebugTileStateDatabase::Impl {
+  CesiumAsync::SqliteConnectionPtr pConnection;
+  CesiumAsync::SqliteStatementPtr writeTileState;
+};
+
+DebugTileStateDatabase::DebugTileStateDatabase(
+    const std::string& databaseFilename)
+    : _pImpl(std::make_unique<Impl>()) {
+  CESIUM_SQLITE(sqlite3*) pConnection;
+  int status =
+      CESIUM_SQLITE(sqlite3_open)(databaseFilename.c_str(), &pConnection);
+  if (status != SQLITE_OK) {
+    throw std::runtime_error(CESIUM_SQLITE(sqlite3_errstr)(status));
+  }
+
+  this->_pImpl->pConnection =
+      std::unique_ptr<CESIUM_SQLITE(sqlite3), DeleteSqliteConnection>(
+          pConnection);
+
+  // Recreate state database
+  char* createTableError = nullptr;
+
+  status = CESIUM_SQLITE(sqlite3_exec)(
+      this->_pImpl->pConnection.get(),
+      DROP_STATE_TABLE_SQL.c_str(),
+      nullptr,
+      nullptr,
+      &createTableError);
+  if (status != SQLITE_OK) {
+    std::string errorStr(createTableError);
+    CESIUM_SQLITE(sqlite3_free)(createTableError);
+    throw std::runtime_error(errorStr);
+  }
+
+  status = CESIUM_SQLITE(sqlite3_exec)(
+      this->_pImpl->pConnection.get(),
+      CREATE_STATE_TABLE_SQL.c_str(),
+      nullptr,
+      nullptr,
+      &createTableError);
+  if (status != SQLITE_OK) {
+    std::string errorStr(createTableError);
+    CESIUM_SQLITE(sqlite3_free)(createTableError);
+    throw std::runtime_error(errorStr);
+  }
+
+  // turn on WAL mode
+  char* walError = nullptr;
+  status = CESIUM_SQLITE(sqlite3_exec)(
+      this->_pImpl->pConnection.get(),
+      PRAGMA_WAL_SQL.c_str(),
+      nullptr,
+      nullptr,
+      &walError);
+  if (status != SQLITE_OK) {
+    std::string errorStr(walError);
+    CESIUM_SQLITE(sqlite3_free)(walError);
+    throw std::runtime_error(errorStr);
+  }
+
+  // turn off synchronous mode
+  char* syncError = nullptr;
+  status = CESIUM_SQLITE(sqlite3_exec)(
+      this->_pImpl->pConnection.get(),
+      PRAGMA_SYNC_SQL.c_str(),
+      nullptr,
+      nullptr,
+      &syncError);
+  if (status != SQLITE_OK) {
+    std::string errorStr(syncError);
+    CESIUM_SQLITE(sqlite3_free)(syncError);
+    throw std::runtime_error(errorStr);
+  }
+
+  this->_pImpl->writeTileState = SqliteHelper::prepareStatement(
+      this->_pImpl->pConnection,
+      WRITE_TILE_SELECTION_SQL);
+
+  std::vector<std::pair<TileSelectionState::Result, std::string>>
+      tileSelectionStates = {
+          {TileSelectionState::Result::None, "None"},
+          {TileSelectionState::Result::Culled, "Culled"},
+          {TileSelectionState::Result::Rendered, "Rendered"},
+          {TileSelectionState::Result::Refined, "Refined"},
+          {TileSelectionState::Result::RenderedAndKicked, "RenderedAndKicked"},
+          {TileSelectionState::Result::RefinedAndKicked, "RefinedAndKicked"}};
+
+  for (const auto& state : tileSelectionStates) {
+    std::string sql = fmt::format(
+        "REPLACE INTO TileSelectionStates (Value, Name) VALUES ({}, '{}');",
+        static_cast<int32_t>(state.first),
+        state.second.c_str());
+    status = CESIUM_SQLITE(sqlite3_exec)(
+        this->_pImpl->pConnection.get(),
+        sql.c_str(),
+        nullptr,
+        nullptr,
+        nullptr);
+  }
+
+  std::vector<std::pair<int32_t, std::string>> isRenderableStates = {
+      {0, "Not Renderable"},
+      {1, "Renderable"}};
+
+  for (const auto& state : isRenderableStates) {
+    std::string sql = fmt::format(
+        "REPLACE INTO IsRenderableStates (Value, Name) VALUES ({}, '{}');",
+        state.first,
+        state.second.c_str());
+    status = CESIUM_SQLITE(sqlite3_exec)(
+        this->_pImpl->pConnection.get(),
+        sql.c_str(),
+        nullptr,
+        nullptr,
+        nullptr);
+  }
+}
+
+DebugTileStateDatabase::~DebugTileStateDatabase() noexcept = default;
+
+void DebugTileStateDatabase::recordAllTileStates(
+    int32_t frameNumber,
+    const Tileset& tileset) {
+  int status = CESIUM_SQLITE(sqlite3_exec)(
+      this->_pImpl->pConnection.get(),
+      "BEGIN TRANSACTION",
+      nullptr,
+      nullptr,
+      nullptr);
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  tileset.forEachLoadedTile([frameNumber, this](const Tile& tile) {
+    this->recordTileState(frameNumber, tile);
+  });
+
+  status = CESIUM_SQLITE(sqlite3_exec)(
+      this->_pImpl->pConnection.get(),
+      "COMMIT TRANSACTION",
+      nullptr,
+      nullptr,
+      nullptr);
+}
+
+void DebugTileStateDatabase::recordTileState(
+    int32_t frameNumber,
+    const Tile& tile) {
+  int status = CESIUM_SQLITE(sqlite3_reset)(this->_pImpl->writeTileState.get());
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  status =
+      CESIUM_SQLITE(sqlite3_clear_bindings)(this->_pImpl->writeTileState.get());
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  status = CESIUM_SQLITE(sqlite3_bind_int64)(
+      this->_pImpl->writeTileState.get(),
+      1, // Pointer
+      reinterpret_cast<int64_t>(&tile));
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  status = CESIUM_SQLITE(sqlite3_bind_int)(
+      this->_pImpl->writeTileState.get(),
+      2, // FrameNumber
+      frameNumber);
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  std::string id = TileIdUtilities::createTileIdString(tile.getTileID());
+  status = CESIUM_SQLITE(sqlite3_bind_text)(
+      this->_pImpl->writeTileState.get(),
+      3, // TileID
+      id.c_str(),
+      static_cast<int>(id.size()),
+      SQLITE_STATIC);
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  status = CESIUM_SQLITE(sqlite3_bind_int)(
+      this->_pImpl->writeTileState.get(),
+      4, // SelectionStateFrameNumber
+      tile.getLastSelectionState().getFrameNumber());
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  status = CESIUM_SQLITE(sqlite3_bind_int)(
+      this->_pImpl->writeTileState.get(),
+      5, // SelectionState
+      static_cast<int>(tile.getLastSelectionState().getResult(
+          tile.getLastSelectionState().getFrameNumber())));
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  status = CESIUM_SQLITE(sqlite3_bind_int)(
+      this->_pImpl->writeTileState.get(),
+      6, // IsRenderable
+      static_cast<int>(tile.isRenderable()));
+  if (status != SQLITE_OK) {
+    return;
+  }
+
+  status = CESIUM_SQLITE(sqlite3_step)(this->_pImpl->writeTileState.get());
+  if (status != SQLITE_DONE) {
+    return;
+  }
+}
+
+} // namespace Cesium3DTilesSelection

Cesium3DTilesSelection/src/Tileset.cpp

@@ -506,6 +506,16 @@ void Tileset::forEachLoadedTile(
   }
 }
 
+void Tileset::forEachLoadedTile(
+    const std::function<void(const Tile& tile)>& callback) const {
+  const Tile* pCurrent = this->_loadedTiles.head();
+  while (pCurrent) {
+    const Tile* pNext = this->_loadedTiles.next(pCurrent);
+    callback(*pCurrent);
+    pCurrent = pNext;
+  }
+}
+
 int64_t Tileset::getTotalDataBytes() const noexcept {
   return this->_pTilesetContentManager->getTotalDataUsed();
 }
@@ -1647,10 +1657,30 @@ Tileset::TraversalDetails Tileset::createTraversalDetailsForSingleTile(
   TileSelectionState::Result lastFrameResult =
       lastFrameSelectionState.getResult(frameState.lastFrameNumber);
   bool isRenderable = tile.isRenderable();
+
   bool wasRenderedLastFrame =
-      lastFrameResult == TileSelectionState::Result::Rendered ||
-      (tile.getRefine() == TileRefine::Add &&
-       lastFrameResult == TileSelectionState::Result::Refined);
+      lastFrameResult == TileSelectionState::Result::Rendered;
+  if (!wasRenderedLastFrame &&
+      lastFrameResult == TileSelectionState::Result::Refined) {
+    if (tile.getRefine() == TileRefine::Add) {
+      // An additive-refined tile that was refined was also rendered.
+      wasRenderedLastFrame = true;
+    } else {
+      // With replace-refinement, if any of this refined tile's children were
+      // rendered last frame, but are no longer rendered because this tile is
+      // loaded and has sufficient detail, we must treat this tile as rendered
+      // last frame, too. This is necessary to prevent this tile from being
+      // kicked just because _it_ wasn't rendered last frame (which could cause
+      // a new hole to appear).
+      for (const Tile& child : tile.getChildren()) {
+        TraversalDetails childDetails = createTraversalDetailsForSingleTile(
+            frameState,
+            child,
+            child.getLastSelectionState());
+        wasRenderedLastFrame |= childDetails.anyWereRenderedLastFrame;
+      }
+    }
+  }
 
   TraversalDetails traversalDetails;
   traversalDetails.allAreRenderable = isRenderable;