address comments and remove SetRenderRequestCallback

Medyan-Naser · Medyan-Naser · commit 41d8d309c24f · 2026-01-29T18:19:34.000-05:00
diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt
@@ -6,7 +6,6 @@ set(F3D_VTK_MODULES
   VTK::CommonColor
   VTK::IOImage
   VTK::InteractionWidgets
-  f3d::vtkext
 )
 
 # Check modules
@@ -151,7 +150,7 @@ target_include_directories(libf3d
   $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
   )
 
-vtk_module_autoinit(TARGETS libf3d MODULES ${F3D_VTK_MODULES} f3d::vtkextPrivate)
+vtk_module_autoinit(TARGETS libf3d MODULES ${F3D_VTK_MODULES})
 
 # The initialize function pointer of the static plugins must be added to the map of static plugins
 # in the factory constructor. When loading a plugin, there's a lookup into this map.
@@ -174,7 +173,7 @@ list(TRANSFORM F3D_STATIC_PLUGIN_TARGETS PREPEND "f3d-plugin-")
 
 target_link_libraries(libf3d
   PUBLIC ${F3D_VTK_MODULES}
-  PRIVATE f3d::vtkextPrivate ${F3D_STATIC_PLUGIN_TARGETS})
+  PRIVATE f3d::vtkext f3d::vtkextPrivate ${F3D_STATIC_PLUGIN_TARGETS})
 
 target_compile_options(libf3d PUBLIC ${f3d_compile_options_public} PRIVATE ${f3d_compile_options_private})
 target_link_options(libf3d PUBLIC ${f3d_link_options_public})
diff --git a/library/private/scene_impl.h b/library/private/scene_impl.h
@@ -12,8 +12,6 @@
 #include "log.h"
 #include "scene.h"
 
-#include "F3DNodeInfo.h"
-
 #include <memory>
 
 namespace f3d
diff --git a/library/public/F3DNodeInfo.h b/library/public/F3DNodeInfo.h
diff --git a/library/public/scene.h b/library/public/scene.h
@@ -12,6 +12,8 @@
 #include <vector>
 /// @endcond
 
+#include "F3DNodeInfo.h"
+
 namespace f3d
 {
 /**
diff --git a/library/src/scene_impl.cxx b/library/src/scene_impl.cxx
@@ -362,7 +362,7 @@ scene& scene_impl::add(std::byte* buffer, std::size_t size)
   }
 
   log::debug("\nLoading stream");
-  this->Internals->Load({ importer });
+  this->Internals->Load({ importer }, this);
   return *this;
 }
 
diff --git a/library/src/window_impl.cxx b/library/src/window_impl.cxx
@@ -760,16 +760,6 @@ void window_impl::SetCachePath(const fs::path& cachePath)
 void window_impl::SetInteractor(interactor_impl* interactor)
 {
   this->Internals->Interactor = interactor;
-  
-  // Set up render request callback on the UI actor
-  // This allows the UI to safely request renders from within the render loop
-  if (interactor)
-  {
-    this->Internals->Renderer->SetRenderRequestCallback([interactor]()
-    {
-      interactor->requestRender();
-    });
-  }
 }
 
 //----------------------------------------------------------------------------
diff --git a/vtkext/private/module/CMakeLists.txt b/vtkext/private/module/CMakeLists.txt
@@ -138,6 +138,10 @@ vtk_module_add_module(f3d::vtkextPrivate
   PRIVATE_HEADERS ${private_headers}
   )
 
+vtk_module_include(f3d::vtkextPrivate
+  PUBLIC
+    "${F3D_SOURCE_DIR}/library/public")
+
 if(MSVC)
   vtk_module_definitions(f3d::vtkextPrivate PRIVATE _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS _CRT_SECURE_NO_WARNINGS)
 endif()
diff --git a/vtkext/private/module/vtkF3DImguiActor.cxx b/vtkext/private/module/vtkF3DImguiActor.cxx
@@ -8,8 +8,6 @@
 #include "vtkF3DImguiVS.h"
 
 #include <vtkImageData.h>
-#include <vtkInformation.h>
-#include <vtkInformationIntegerKey.h>
 #include <vtkObjectFactory.h>
 #include <vtkOpenGLBufferObject.h>
 #include <vtkOpenGLRenderWindow.h>
@@ -45,8 +43,6 @@
 #include <sstream>
 #include <string>
 
-vtkInformationKeyMacro(vtkF3DImguiActor, USER_VISIBILITY, Integer);
-
 namespace
 {
 constexpr float LOGO_DISPLAY_WIDTH = 256.f;
@@ -419,19 +415,6 @@ void vtkF3DImguiActor::RenderNode(NodeInfo* node)
       NodeVisibilityState[node->prop] = visible;
       node->prop->SetVisibility(visible ? 1 : 0);
       node->prop->Modified();
-
-      // Mark this prop as user-controlled
-      vtkInformation* info = node->prop->GetPropertyKeys();
-      if (!info)
-      {
-        info = vtkInformation::New();
-        node->prop->SetPropertyKeys(info);
-        info->Delete();
-      }
-      info->Set(vtkF3DImguiActor::USER_VISIBILITY(), visible ? 1 : 0);
-
-      // Request a render to update the scene
-      this->VisibilityChangedThisFrame = true;
     }
 
     ImGui::PopID();
@@ -1005,9 +988,6 @@ void vtkF3DImguiActor::StartFrame(vtkOpenGLRenderWindow* renWin)
   io.DisplaySize = ImVec2(static_cast<float>(size[0]), static_cast<float>(size[1]));
 
   this->Pimpl->Initialize(renWin);
-  
-  // Reset the visibility change flag at the start of each frame
-  this->VisibilityChangedThisFrame = false;
 
   ImGui::NewFrame();
 }
@@ -1017,12 +997,6 @@ void vtkF3DImguiActor::EndFrame(vtkOpenGLRenderWindow* renWin)
 {
   ImGui::Render();
   this->Pimpl->RenderDrawData(renWin, ImGui::GetDrawData());
-  
-  // If visibility changed, request a render to update the scene
-  if (this->VisibilityChangedThisFrame && this->RenderRequestCallback)
-  {
-    this->RenderRequestCallback();
-  }
 }
 
 
diff --git a/vtkext/private/module/vtkF3DImguiActor.h b/vtkext/private/module/vtkF3DImguiActor.h
@@ -16,16 +16,11 @@
 class vtkOpenGLRenderWindow;
 class vtkWindow;
 
-class vtkInformationIntegerKey;
-
 class vtkF3DImguiActor : public vtkF3DUIActor
 {
 public:
   static vtkF3DImguiActor* New();
   vtkTypeMacro(vtkF3DImguiActor, vtkF3DUIActor);
-  
-  // Information key to track user-controlled visibility
-  static vtkInformationIntegerKey* USER_VISIBILITY();
 
   /**
    * Initialize the UI actor resources
@@ -53,9 +48,6 @@ class vtkF3DImguiActor : public vtkF3DUIActor
   // Track visibility state for each node
   std::unordered_map<vtkProp*, bool> NodeVisibilityState;
 
-  // Flag to trigger render when visibility changes
-  bool VisibilityChangedThisFrame = false;
-
   /**
    * Called at the beginning of the rendering step
    * Initialize the imgui context if needed and setup a new frame
diff --git a/vtkext/private/module/vtkF3DMetaImporter.cxx b/vtkext/private/module/vtkF3DMetaImporter.cxx
@@ -719,15 +719,15 @@ std::vector<NodeInfo> vtkF3DMetaImporter::GetActorHierarchy()
 {
   std::vector<NodeInfo> nodes;
 
-  // Get scene hierarchy for GLTF files (where names are stored)
+  // Get scene hierarchy from importers that support it
   vtkDataAssembly* hierarchy = this->GetSceneHierarchy();
   std::map<int, std::string> actorIndexNameMap;
   
-  // If we have a scene hierarchy (GLTF file), extract names from it
+  // If we have a scene hierarchy, extract names from it
   if (hierarchy)
   {
     // Build a map of actor indices to their names from the hierarchy
-    // Since GLTF doesn't store explicit actor_id attributes, we map leaf mesh nodes sequentially
+    // Map leaf mesh nodes sequentially to actor indices
     int meshCounter = 0;
     std::function<void(int)> traverseHierarchy = [&](int nodeId) {
       const char* nodeName = hierarchy->GetNodeName(nodeId);
@@ -770,10 +770,10 @@ std::vector<NodeInfo> vtkF3DMetaImporter::GetActorHierarchy()
     node.prop = actor;
 
     // Try to get a meaningful name for the actor
-    // Priority: DataAssembly (GLTF hierarchy) > Actor ObjectName > Mapper input name > Generic
+    // Priority: DataAssembly hierarchy > Actor ObjectName > Mapper input name > Generic
     std::string actorName;
 
-    // First, check if we have a name from the GLTF scene hierarchy
+    // First, check if we have a name from the scene hierarchy
     auto it = actorIndexNameMap.find(actorIndex);
     if (it != actorIndexNameMap.end())
     {
diff --git a/vtkext/private/module/vtkF3DMetaImporter.h b/vtkext/private/module/vtkF3DMetaImporter.h
@@ -11,6 +11,7 @@
 
 #include <vtkActor.h>
 #include <vtkBoundingBox.h>
+#include <vtkDataAssembly.h>
 #include <vtkPointGaussianMapper.h>
 #include <vtkProperty.h>
 #include <vtkSmartVolumeMapper.h>
@@ -170,10 +171,10 @@ class vtkF3DMetaImporter : public vtkF3DImporter
   vtkMTimeType GetUpdateMTime();
 
   /**
-  * Returns the scene hierarchy if any of the importers is a GLTF importer.
-  * Returns nullptr if no GLTF importer was added.
+  * Returns the scene hierarchy if any of the importers supports it.
+  * Returns nullptr if no importer with hierarchy support was added.
   */
-  vtkDataAssembly* GetSceneHierarchy() override;
+  vtkDataAssembly* GetSceneHierarchy();
   std::vector<NodeInfo> GetActorHierarchy();
 
 private:
diff --git a/vtkext/private/module/vtkF3DRenderer.cxx b/vtkext/private/module/vtkF3DRenderer.cxx
@@ -17,7 +17,7 @@
 #include "vtkF3DUserRenderPass.h"
 
 #if F3D_MODULE_UI
-#include "vtkF3DImguiActor.h"
+#include "vtkF3DUIActor.h"
 #endif
 
 #include <vtkAxesActor.h>
@@ -2903,26 +2903,6 @@ void vtkF3DRenderer::SetComponentForColoring(int component)
   }
 }
 
-//----------------------------------------------------------------------------
-namespace
-{
-/**
- * Helper to get user-controlled visibility from actor's property keys
- * Returns the user-set visibility if available, otherwise returns defaultValue
- */
-bool GetUserVisibility(vtkProp* prop, bool defaultValue)
-{
-#if F3D_MODULE_UI
-  vtkInformation* info = prop->GetPropertyKeys();
-  if (info && info->Has(vtkF3DImguiActor::USER_VISIBILITY()))
-  {
-    return info->Get(vtkF3DImguiActor::USER_VISIBILITY()) != 0;
-  }
-#endif
-  return defaultValue;
-}
-}
-
 //----------------------------------------------------------------------------
 void vtkF3DRenderer::ConfigureColoring()
 {
@@ -2961,16 +2941,13 @@ void vtkF3DRenderer::ConfigureColoring()
         }
       }
       
-      // Apply visibility, respecting user overrides
-      visible = GetUserVisibility(coloring.Actor, visible);
       coloring.Actor->SetVisibility(visible);
-      coloring.OriginalActor->SetVisibility(GetUserVisibility(coloring.OriginalActor, !visible));
+      coloring.OriginalActor->SetVisibility(!visible);
     }
     else
     {
-      // When geometries not visible, hide both unless user-controlled
-      coloring.Actor->SetVisibility(GetUserVisibility(coloring.Actor, false));
-      coloring.OriginalActor->SetVisibility(GetUserVisibility(coloring.OriginalActor, false));
+      coloring.Actor->SetVisibility(false);
+      coloring.OriginalActor->SetVisibility(false);
     }
   }
   if (geometriesVisible)
@@ -3497,8 +3474,3 @@ void vtkF3DRenderer::SetConsoleBadgeEnabled(bool enabled)
   this->UIActor->SetConsoleBadgeEnabled(enabled);
 }
 
-//----------------------------------------------------------------------------
-void vtkF3DRenderer::SetRenderRequestCallback(std::function<void()> callback)
-{
-  this->UIActor->SetRenderRequestCallback(std::move(callback));
-}
diff --git a/vtkext/private/module/vtkF3DRenderer.h b/vtkext/private/module/vtkF3DRenderer.h
@@ -494,11 +494,6 @@ class vtkF3DRenderer : public vtkOpenGLRenderer
    */
   void SetConsoleBadgeEnabled(bool enabled);
 
-  /**
-   * Set a callback to request a render safely from within the render loop
-   */
-  void SetRenderRequestCallback(std::function<void()> callback);
-
 private:
   vtkF3DRenderer();
   ~vtkF3DRenderer() override;
diff --git a/vtkext/private/module/vtkF3DUIActor.h b/vtkext/private/module/vtkF3DUIActor.h
@@ -193,15 +193,6 @@ class vtkF3DUIActor : public vtkProp
   {
   }
 
-  /**
-   * Set a callback to request a render outside the render loop
-   * This is safe to call from within rendering code
-   */
-  void SetRenderRequestCallback(std::function<void()> callback)
-  {
-    this->RenderRequestCallback = std::move(callback);
-  }
-
 protected:
   vtkF3DUIActor();
   ~vtkF3DUIActor() override;
@@ -326,9 +317,6 @@ class vtkF3DUIActor : public vtkProp
   double BackdropOpacity = 0.9;
   std::vector<NodeInfo> HierarchyNodes;
 
-  // Callback to request a render safely
-  std::function<void()> RenderRequestCallback;
-
 private:
   vtkF3DUIActor(const vtkF3DUIActor&) = delete;
   void operator=(const vtkF3DUIActor&) = delete;

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,8 @@`
`12`	`12`	`#include <vector>`
`13`	`13`	`/// @endcond`
`14`	`14`
	`15`	`+#include "F3DNodeInfo.h"`
	`16`	`+`
`15`	`17`	`namespace f3d`
`16`	`18`	`{`
`17`	`19`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -362,7 +362,7 @@ scene& scene_impl::add(std::byte* buffer, std::size_t size)`
`362`	`362`	`}`
`363`	`363`
`364`	`364`	`log::debug("\nLoading stream");`
`365`		`- this->Internals->Load({ importer });`
	`365`	`+ this->Internals->Load({ importer }, this);`
`366`	`366`	`return *this;`
`367`	`367`	`}`
`368`	`368`