ENH: Improve rendering updates

- Update pipelines on render window updates
- Automatically request render on pipeline reset
- Add utility function to take screenshots from RW to
  avoid unwanted behavior with existing VTK vtkWindowToImageFilter
- Add blockers to request render only once for large pipeline
  updates
- Add tests for camera clipping range updates

Author: Thibault-Pelletier

LayerDM/MRMLDM/vtkMRMLLayerDMCameraSynchronizer.cxx

@@ -16,11 +16,15 @@
 // STL includes
 #include <array>
 
+/// \brief Abstract class for the camera strategies.
+/// Implements only the reset camera clipping range logic for the layer cameras.
+/// Other methods are expected to be implemented by deriving classes.
 class CameraSynchronizeStrategy
 {
 public:
-  explicit CameraSynchronizeStrategy(const vtkSmartPointer<vtkCamera>& camera)
+  explicit CameraSynchronizeStrategy(const vtkSmartPointer<vtkCamera>& camera, std::function<void()> invokeModifiedEvent)
     : m_camera(camera)
+    , m_invokeModifiedEvent{ std::move(invokeModifiedEvent) }
   {
   }
   virtual ~CameraSynchronizeStrategy() = default;
@@ -29,26 +33,25 @@ class CameraSynchronizeStrategy
 protected:
   vtkSmartPointer<vtkCamera> m_camera;
   vtkNew<vtkMRMLLayerDMObjectEventObserver> m_eventObserver;
+  std::function<void()> m_invokeModifiedEvent;
 };
 
+/// Default camera synchronization consists in updating the camera when the first renderer active camera is updated.
 class DefaultCameraSynchronizeStrategy : public CameraSynchronizeStrategy
 {
 public:
-  explicit DefaultCameraSynchronizeStrategy(const vtkSmartPointer<vtkCamera>& camera, vtkRenderer* renderer)
-    : CameraSynchronizeStrategy(camera)
+  explicit DefaultCameraSynchronizeStrategy(const vtkSmartPointer<vtkCamera>& camera, vtkRenderer* renderer, std::function<void()> invokeModifiedEvent)
+    : CameraSynchronizeStrategy(camera, std::move(invokeModifiedEvent))
     , m_renderer(renderer)
   {
     this->m_eventObserver->SetUpdateCallback(
       [this](vtkObject* object)
       {
-        if (object == this->m_observedCamera)
-        {
-          this->UpdateCamera();
-        }
         if (object == this->m_renderer)
         {
           this->ObserveActiveCamera();
         }
+        this->UpdateCamera();
       });
 
     this->m_eventObserver->UpdateObserver(nullptr, this->m_renderer, vtkCommand::ActiveCameraEvent);
@@ -57,11 +60,17 @@ class DefaultCameraSynchronizeStrategy : public CameraSynchronizeStrategy
 
   void UpdateCamera() override
   {
-    if (this->m_observedCamera)
+    if (!this->m_observedCamera)
     {
-      this->m_camera->DeepCopy(this->m_observedCamera);
-      this->m_camera->Modified();
+      return;
     }
+
+    // Update camera and preserve clipping range
+    double clippingRange[2];
+    this->m_camera->GetClippingRange(clippingRange);
+    this->m_camera->DeepCopy(this->m_observedCamera);
+    this->m_camera->SetClippingRange(clippingRange);
+    this->m_invokeModifiedEvent();
   }
 
 private:
@@ -76,18 +85,26 @@ class DefaultCameraSynchronizeStrategy : public CameraSynchronizeStrategy
 
     this->m_eventObserver->UpdateObserver(this->m_observedCamera, camera);
     this->m_observedCamera = camera;
-    this->UpdateCamera();
   }
 
   vtkWeakPointer<vtkRenderer> m_renderer;
   vtkWeakPointer<vtkCamera> m_observedCamera;
 };
 
+/// Synchronizes the default camera to the current slice node view configuration.
+/// The Slice renderer 0 camera is not configured nor modified during camera changes.
+/// All of its actors are set to render in 2D.
+///
+/// To simplify adding new pipelines, the sync adjusts the default camera to render in parallel projection in the correct
+/// orientation with respect to the current node configuration.
+///
+/// Clipping range is configured to show all actors attached to the default camera.
+/// If clipping is required, then it should be done inside the specific pipeline.
 class SliceViewCameraSynchronizeStrategy : public CameraSynchronizeStrategy
 {
 public:
-  explicit SliceViewCameraSynchronizeStrategy(const vtkSmartPointer<vtkCamera>& camera, vtkMRMLSliceNode* sliceNode)
-    : CameraSynchronizeStrategy(camera)
+  explicit SliceViewCameraSynchronizeStrategy(const vtkSmartPointer<vtkCamera>& camera, vtkMRMLSliceNode* sliceNode, std::function<void()> invokeModifiedEvent)
+    : CameraSynchronizeStrategy(camera, std::move(invokeModifiedEvent))
     , m_sliceNode{ sliceNode }
   {
     this->m_eventObserver->SetUpdateCallback(
@@ -142,6 +159,7 @@ class SliceViewCameraSynchronizeStrategy : public CameraSynchronizeStrategy
     vtkMath::Cross(vRight.data(), vUp.data(), normal.data());
     double position[3] = { viewCenterRAS[0] + normal[0] * d, viewCenterRAS[1] + normal[1] * d, viewCenterRAS[2] + normal[2] * d };
     this->m_camera->SetPosition(position);
+    this->m_invokeModifiedEvent();
   }
 
 private:
@@ -199,13 +217,29 @@ void vtkMRMLLayerDMCameraSynchronizer::UpdateStrategy()
     return;
   }
 
+  const auto invokeModifiedEvent = [this]
+  {
+    if (this->m_isBlocked)
+    {
+      return;
+    }
+    this->Modified();
+  };
+
   if (auto sliceNode = vtkMRMLSliceNode::SafeDownCast(this->m_viewNode))
   {
-    this->m_syncStrategy = std::make_unique<SliceViewCameraSynchronizeStrategy>(this->m_defaultCamera, sliceNode);
+    this->m_syncStrategy = std::make_unique<SliceViewCameraSynchronizeStrategy>(this->m_defaultCamera, sliceNode, invokeModifiedEvent);
   }
   else
   {
-    this->m_syncStrategy = std::make_unique<DefaultCameraSynchronizeStrategy>(this->m_defaultCamera, this->m_renderer);
+    this->m_syncStrategy = std::make_unique<DefaultCameraSynchronizeStrategy>(this->m_defaultCamera, this->m_renderer, invokeModifiedEvent);
   }
   this->m_syncStrategy->UpdateCamera();
 }
+
+bool vtkMRMLLayerDMCameraSynchronizer::BlockModified(bool isBlocked)
+{
+  bool wasBlocked = this->m_isBlocked;
+  m_isBlocked = isBlocked;
+  return wasBlocked;
+}

LayerDM/MRMLDM/vtkMRMLLayerDMCameraSynchronizer.h

@@ -38,6 +38,10 @@ class VTK_SLICER_LAYERDM_MODULE_MRMLDISPLAYABLEMANAGER_EXPORT vtkMRMLLayerDMCame
   /// The renderer active camera will be monitored for change when applicable (for instance 3D views).
   void SetRenderer(vtkRenderer* renderer);
 
+  /// Blocks modified invocation. Doesn't block updates on the underlying camera changes.
+  /// @return previous blocked state.
+  bool BlockModified(bool isBlocked);
+
 protected:
   vtkMRMLLayerDMCameraSynchronizer();
   ~vtkMRMLLayerDMCameraSynchronizer() override;
@@ -50,4 +54,5 @@ class VTK_SLICER_LAYERDM_MODULE_MRMLDISPLAYABLEMANAGER_EXPORT vtkMRMLLayerDMCame
   vtkWeakPointer<vtkRenderer> m_renderer;
   vtkWeakPointer<vtkMRMLAbstractViewNode> m_viewNode;
   std::unique_ptr<CameraSynchronizeStrategy> m_syncStrategy;
+  bool m_isBlocked{ false };
 };

LayerDM/MRMLDM/vtkMRMLLayerDMLayerManager.cxx

@@ -77,7 +77,7 @@ void vtkMRMLLayerDMLayerManager::RemovePipeline(vtkMRMLLayerDMPipelineI* pipelin
 void vtkMRMLLayerDMLayerManager::ResetCameraClippingRange() const
 {
   // Reset first renderer clipping range
-  if (auto defaultRenderer = this->GetDefaultRenderer())
+  if (const auto defaultRenderer = this->GetDefaultRenderer())
   {
     defaultRenderer->ResetCameraClippingRange();
   }
@@ -107,6 +107,7 @@ void vtkMRMLLayerDMLayerManager::SetDefaultCamera(const vtkSmartPointer<vtkCamer
   {
     return;
   }
+
   this->m_defaultCamera = camera;
   this->UpdateLayers();
 }
@@ -191,8 +192,7 @@ std::uintptr_t vtkMRMLLayerDMLayerManager::GetCameraId(vtkCamera* camera)
 
 vtkCamera* vtkMRMLLayerDMLayerManager::GetCameraForLayer(const LayerKey& key, const std::set<vtkWeakPointer<vtkMRMLLayerDMPipelineI>>& pipelines) const
 {
-  auto cameraId = std::get<1>(key);
-  if (cameraId == 0)
+  if (const auto cameraId = std::get<1>(key); cameraId == 0)
   {
     return this->m_defaultCamera;
   }
@@ -298,7 +298,7 @@ void vtkMRMLLayerDMLayerManager::ResetRenderersCameraClippingRange(const std::se
     {
       continue;
     }
-    renderer->ResetCameraClippingRange(bounds.data());
+    renderer->ResetCameraClippingRange(const_cast<double*>(bounds.data()));
   }
 }
 
@@ -349,19 +349,19 @@ void vtkMRMLLayerDMLayerManager::UpdateLayers()
   this->RemoveOutdatedPipelines();
   this->RemoveOutdatedLayers();
   this->AddMissingLayers();
-  this->UpdateRenderWindowLayerOrdering();
+  this->UpdateRendererLayerOrdering();
   this->UpdateRendererCamera();
   this->SynchronizePipelineRenderers();
+  this->UpdateRenderWindowNumberOfLayers();
 }
 
-void vtkMRMLLayerDMLayerManager::UpdateRenderWindowLayerOrdering() const
+void vtkMRMLLayerDMLayerManager::UpdateRendererLayerOrdering() const
 {
   // Managed layers are always ordered from layer 1 to the number of managed renderers
   for (int iRenderer = 0; iRenderer < this->GetNumberOfRenderers(); iRenderer++)
   {
     this->m_renderers[iRenderer]->SetLayer(iRenderer + 1);
   }
-  this->UpdateRenderWindowNumberOfLayers();
 }
 
 void vtkMRMLLayerDMLayerManager::UpdateRendererCamera()

LayerDM/MRMLDM/vtkMRMLLayerDMLayerManager.h

@@ -11,7 +11,6 @@
 
 // STL includes
 #include <array>
-#include <cstdint>
 #include <map>
 #include <set>
 
@@ -39,14 +38,31 @@ class VTK_SLICER_LAYERDM_MODULE_MRMLDISPLAYABLEMANAGER_EXPORT vtkMRMLLayerDMLaye
   static vtkMRMLLayerDMLayerManager* New();
   vtkTypeMacro(vtkMRMLLayerDMLayerManager, vtkObject);
 
+  /// Adds the pipeline to the layers.
+  /// May change an update of the layer ordering.
+  /// Will trigger the SetRenderer call on the pipeline when it's added to its layer.
   void AddPipeline(vtkMRMLLayerDMPipelineI* pipeline);
+
   static LayerKey GetPipelineLayerKey(vtkMRMLLayerDMPipelineI* pipeline);
+
   int GetNumberOfDistinctLayers() const;
   int GetNumberOfManagedLayers() const;
+
+  /// Returns the current number of managed renderers in the render window.
   int GetNumberOfRenderers() const;
+
+  /// Removes the pipeline from the layers.
+  /// May change the layer ordering if pipeline was the last one of its current renderer.
   void RemovePipeline(vtkMRMLLayerDMPipelineI* pipeline);
+
+  /// Iterates over the renderers and resets their clipping range to visible bounds
   void ResetCameraClippingRange() const;
+
+  /// Changes the render window managed by the layer manager.
+  /// Will trigger a removal of all managed layers and creation of new layers if the render window is not null.
   void SetRenderWindow(vtkRenderWindow* renderWindow);
+
+  /// If the default camera has changed, update the layers with ne new camera
   void SetDefaultCamera(const vtkSmartPointer<vtkCamera>& camera);
 
 protected:
@@ -73,7 +89,7 @@ class VTK_SLICER_LAYERDM_MODULE_MRMLDISPLAYABLEMANAGER_EXPORT vtkMRMLLayerDMLaye
   void SynchronizePipelineRenderers();
   void UpdateRenderWindowNumberOfLayers() const;
   void UpdateLayers();
-  void UpdateRenderWindowLayerOrdering() const;
+  void UpdateRendererLayerOrdering() const;
   void UpdateRendererCamera();
 
   // Map of pipeline layers ordered by ascending <layer value, camera synchronization mode>

LayerDM/MRMLDM/vtkMRMLLayerDMPipelineFactory.cxx

@@ -98,10 +98,10 @@ vtkMRMLLayerDMPipelineI* vtkMRMLLayerDMPipelineFactory::GetLastPipeline() const
 }
 
 vtkMRMLLayerDMPipelineFactory::vtkMRMLLayerDMPipelineFactory()
-  : m_lastView(nullptr)
+  : m_obs(vtkSmartPointer<vtkMRMLLayerDMObjectEventObserver>::New())
+  , m_lastView(nullptr)
   , m_lastNode(nullptr)
   , m_lastPipeline(nullptr)
-  , m_obs(vtkSmartPointer<vtkMRMLLayerDMObjectEventObserver>::New())
 {
   m_obs->SetUpdateCallback([this](vtkObject* node) { this->SortPipelineCreators(); });
 }

LayerDM/MRMLDM/vtkMRMLLayerDMPipelineFactory.h

@@ -6,6 +6,7 @@
 #include <vtkCommand.h>
 #include <vtkObject.h>
 #include <vtkSmartPointer.h>
+#include <vtkWeakPointer.h>
 
 // STL includes
 #include <functional>
@@ -83,8 +84,8 @@ class VTK_SLICER_LAYERDM_MODULE_MRMLDISPLAYABLEMANAGER_EXPORT vtkMRMLLayerDMPipe
   void SortPipelineCreators();
 
   std::vector<vtkSmartPointer<vtkMRMLLayerDMPipelineCreatorI>> m_pipelineCreators;
-  vtkMRMLAbstractViewNode* m_lastView;
-  vtkMRMLNode* m_lastNode;
-  vtkMRMLLayerDMPipelineI* m_lastPipeline;
   vtkSmartPointer<vtkMRMLLayerDMObjectEventObserver> m_obs;
+  vtkWeakPointer<vtkMRMLAbstractViewNode> m_lastView;
+  vtkWeakPointer<vtkMRMLNode> m_lastNode;
+  vtkWeakPointer<vtkMRMLLayerDMPipelineI> m_lastPipeline;
 };

LayerDM/MRMLDM/vtkMRMLLayerDMPipelineI.cxx

@@ -31,7 +31,11 @@ void vtkMRMLLayerDMPipelineI::ResetDisplay()
     return;
   }
 
+  // Make sure to avoid looping reset display during processing
+  this->BlockResetDisplay(true);
   this->UpdatePipeline();
+  this->RequestRender();
+  this->BlockResetDisplay(false);
 }
 
 void vtkMRMLLayerDMPipelineI::SetViewNode(vtkMRMLAbstractViewNode* viewNode)

LayerDM/MRMLDM/vtkMRMLLayerDMPipelineI.h

@@ -156,9 +156,11 @@ class VTK_SLICER_LAYERDM_MODULE_MRMLDISPLAYABLEMANAGER_EXPORT vtkMRMLLayerDMPipe
 
   /// Request rendering and camera clipping reset.
   /// Calls are delegated to \sa vtkMRMLLayerDMPipelineManager::RequestRender.
+  ///
+  /// \sa ResetDisplay
   void RequestRender() const;
 
-  /// Resets the pipeline display.
+  /// Resets the pipeline display and request a new render \sa RequestRender.
   /// Delegates actual work to \sa UpdatePipeline.
   /// Called the first time after pipeline initialization.
   void ResetDisplay();