Clarify recorder scene graph deletion.

facebookresearch · Dec 22, 2022 · 6ee6be6 · 6ee6be6
1 parent 364075c
commit 6ee6be6
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 16 deletions.
diff --git a/src/esp/gfx/replay/Recorder.cpp b/src/esp/gfx/replay/Recorder.cpp
@@ -78,7  78,7 @@ void Recorder::onCreateRenderAssetInstance(
       node, instanceKey, Corrade::Containers::NullOpt, deletionHelper});
 }
 
-void Recorder::onRemoveSceneGraph(const esp::scene::SceneGraph& sceneGraph) {
 void Recorder::onDeleteSceneGraph(const esp::scene::SceneGraph& sceneGraph) {
   auto& root = sceneGraph.getRootNode();
   scene::preOrderTraversalWithCallback(root,
                                        [this](const scene::SceneNode& node) {
@@ -158,12  158,13 @@ void Recorder::checkAndAddDeletion(Keyframe* keyframe,
 
 void Recorder::onDeleteRenderAssetInstance(const scene::SceneNode* node) {
   int index = findInstance(node);
   CORRADE_INTERNAL_ASSERT(index != ID_UNDEFINED);
 
-  if (index != ID_UNDEFINED) {
-    auto instanceKey = instanceRecords_[index].instanceKey;
-    checkAndAddDeletion(&getKeyframe(), instanceKey);
-    instanceRecords_.erase(instanceRecords_.begin()   index);
-  }
   auto instanceKey = instanceRecords_[index].instanceKey;
 
   checkAndAddDeletion(&getKeyframe(), instanceKey);
 
   instanceRecords_.erase(instanceRecords_.begin()   index);
 }
 
 Keyframe& Recorder::getKeyframe() {

diff --git a/src/esp/gfx/replay/Recorder.h b/src/esp/gfx/replay/Recorder.h
@@ -59,13  59,13 @@ class Recorder {
   void onLoadRenderAsset(const esp::assets::AssetInfo& assetInfo);
 
   /**
-   * @brief Remove all graph nodes from the current keyframe.
-   * Because scene graphs are currently leaked when scenes changes, we cannot
-   * rely on node deletion to issue deletion entries. This function allows to
-   * circumvent this issue.
-   * @param sceneGraph The scene graph from which deletion entries are issued.
    * @brief Record deletion of all render instances in a scene graph.
    * Because scene graphs are currently leaked when the active scene changes, we
    * cannot rely on node deletion to issue gfx-replay deletion entries. This
    * function allows to circumvent this issue.
    * @param sceneGraph The scene graph to delete.
    */
-  void onRemoveSceneGraph(const esp::scene::SceneGraph& sceneGraph);
   void onDeleteSceneGraph(const esp::scene::SceneGraph& sceneGraph);
 
   /**
    * @brief Save/capture a render keyframe (a visual snapshot of the scene).

diff --git a/src/esp/sim/Simulator.cpp b/src/esp/sim/Simulator.cpp
@@ -254,13  254,13 @@ bool Simulator::createSceneInstance(const std::string& activeSceneName) {
   // before anything else.
   seed(config_.randomSeed);
 
-  // This code removes the previously loaded scene from gfx-replay. Because of
-  // the issue below, scene graphs are leaked, so we cannot rely on node
-  // deletion to issue gfx-replay deletion entries.
   // This code deletes the instances in the previously loaded scene from
   // gfx-replay. Because of the issue below, scene graphs are leaked, so we
   // cannot rely on node deletion to issue gfx-replay deletion entries.
   auto recorder = gfxReplayMgr_->getRecorder();
   if (recorder && activeSceneID_ >= 0 &&
       activeSceneID_ < sceneManager_->getSceneGraphCount()) {
-    recorder->onRemoveSceneGraph(sceneManager_->getSceneGraph(activeSceneID_));
     recorder->onDeleteSceneGraph(sceneManager_->getSceneGraph(activeSceneID_));
   }
 
   // initialize scene graph CAREFUL! previous scene graph is not deleted!