BUG: Standardize fit slice to volume behavior

"Fit to slice" function has several slightly different implementations (FitSliceToFirst, FitSliceToBackground, FitSliceToAll) and they were used inconsistently in different places in the code (clicking fit to slice in slice view controller, fitting to slice view via view linking, using subject hierarchy "show" function, after loading a volume, after executing a CLI module).

Since ClipToBackgroundVolume property was added to the slice composite node, the most appropriate method to fit volumes into a slice view is to fit to all volumes (FitSliceToAll) if ClipToBackgroundVolume=false because then all volumes are visible in the slice view. If ClipToBackgroundVolume=true then the view is fit to the first visible volume (FitSliceToFirst) because that clips other layers. This behavior is implemented in FitSliceToBackground and this method is now called consistently everywhere.

Applications/SlicerApp/Testing/Python/SliceLinkLogic.py

@@ -185,13 +185,13 @@ def test_SliceLinkLogic1(self):
         # Set the orientation to axial
         logic.StartSliceNodeInteraction(12)  # OrientationFlag & ResetFieldOfViewFlag
         compareNode.SetOrientation("Axial")
-        logic.FitSliceToAll()
+        logic.FitSliceToBackground()
         compareNode.UpdateMatrices()
         logic.EndSliceNodeInteraction()
 
         # Reset the field of view
         logic.StartSliceNodeInteraction(8)  # ResetFieldOfViewFlag
-        logic.FitSliceToAll()
+        logic.FitSliceToBackground()
         compareNode.UpdateMatrices()
         logic.EndSliceNodeInteraction()
         # Note: we validate on fov[1] when resetting the field of view (fov[0] can
@@ -209,7 +209,7 @@ def test_SliceLinkLogic1(self):
         # Changed the number of lightboxes
         ln.SetNumberOfCompareViewLightboxColumns(6)
         logic.StartSliceNodeInteraction(8)  # ResetFieldOfViewFlag
-        logic.FitSliceToAll()
+        logic.FitSliceToBackground()
         compareNode.UpdateMatrices()
         logic.EndSliceNodeInteraction()
 
@@ -275,7 +275,7 @@ def test_SliceLinkLogic1(self):
         # Change the orientation
         logic.StartSliceNodeInteraction(12)  # OrientationFlag & ResetFieldOfViewFlag
         compareNode.SetOrientation("Sagittal")
-        logic.FitSliceToAll()
+        logic.FitSliceToBackground()
         compareNode.UpdateMatrices()
         logic.EndSliceNodeInteraction()
         self.delayDisplay("Broadcasted a change in slice orientation to all Compare Views")

Base/Python/slicer/util.py

@@ -581,7 +581,7 @@ def _nodeID(nodeOrID):
             for i in range(sliceLogics.GetNumberOfItems()):
                 sliceLogic = sliceLogics.GetItemAsObject(i)
                 if sliceLogic:
-                    sliceLogic.FitSliceToAll()
+                    sliceLogic.FitSliceToBackground()
 
 
 def setToolbarsVisible(visible, ignore=None):

Libs/MRML/DisplayableManager/vtkMRMLSliceIntersectionWidget.cxx

@@ -565,7 +565,7 @@ bool vtkMRMLSliceIntersectionWidget::ProcessInteractionEvent(vtkMRMLInteractionE
     {
       this->SliceLogic->GetMRMLScene()->SaveStateForUndo();
       this->SliceLogic->StartSliceNodeInteraction(vtkMRMLSliceNode::ResetFieldOfViewFlag);
-      this->SliceLogic->FitSliceToAll();
+      this->SliceLogic->FitSliceToBackground();
       this->GetSliceNode()->UpdateMatrices();
       this->SliceLogic->EndSliceNodeInteraction();
     }

Libs/MRML/Logic/vtkMRMLApplicationLogic.cxx

@@ -153,7 +153,7 @@ void vtkMRMLApplicationLogic::vtkInternal::PropagateVolumeSelection(int layer, i
   }
   if (fit)
   {
-    this->External->FitSliceToAll(true);
+    this->External->FitSliceToBackground(true);
   }
 }
 //----------------------------------------------------------------------------
@@ -524,7 +524,7 @@ void vtkMRMLApplicationLogic::PropagatePlotChartSelection()
 }
 
 //----------------------------------------------------------------------------
-void vtkMRMLApplicationLogic::FitSliceToAll(bool onlyIfPropagateVolumeSelectionAllowed /* =false */, bool resetOrientation /* =true */)
+void vtkMRMLApplicationLogic::FitSliceToContent(bool all, bool onlyIfPropagateVolumeSelectionAllowed /* =false */, bool resetOrientation /* =true */)
 {
   if (this->Internal->SliceLogics.GetPointer() == nullptr)
   {
@@ -554,11 +554,31 @@ void vtkMRMLApplicationLogic::FitSliceToAll(bool onlyIfPropagateVolumeSelectionA
       sliceLogic->RotateSliceToLowestVolumeAxes(false);
     }
     int* dims = sliceNode->GetDimensions();
-    sliceLogic->FitSliceToAll(dims[0], dims[1]);
+    if (all)
+    {
+      sliceLogic->FitSliceToAll(dims[0], dims[1]);
+    }
+    else
+    {
+      sliceLogic->FitSliceToBackground(dims[0], dims[1]);
+    }
+
     sliceLogic->SnapSliceOffsetToIJK();
   }
 }
 
+//----------------------------------------------------------------------------
+void vtkMRMLApplicationLogic::FitSliceToBackground(bool onlyIfPropagateVolumeSelectionAllowed /* =false */, bool resetOrientation /* =true */)
+{
+  this->FitSliceToContent(false, onlyIfPropagateVolumeSelectionAllowed, resetOrientation);
+}
+
+//----------------------------------------------------------------------------
+void vtkMRMLApplicationLogic::FitSliceToAll(bool onlyIfPropagateVolumeSelectionAllowed /* =false */, bool resetOrientation /* =true */)
+{
+  this->FitSliceToContent(true, onlyIfPropagateVolumeSelectionAllowed, resetOrientation);
+}
+
 //----------------------------------------------------------------------------
 bool vtkMRMLApplicationLogic::Zip(const char* zipFileName, const char* directoryToZip)
 {

Libs/MRML/Logic/vtkMRMLApplicationLogic.h

@@ -87,29 +87,29 @@ class VTK_MRML_LOGIC_EXPORT vtkMRMLApplicationLogic
   /// Perform the default behavior related to selecting a volume
   /// (in this case, making it the background for all SliceCompositeNodes)
   /// \sa vtkInternal::PropagateVolumeSelection()
-  /// \sa FitSliceToAll()
+  /// \sa FitSliceToBackground()
   /// \sa vtkMRMLSelectionNode::SetActiveVolumeID()
   /// \sa vtkMRMLSelectionNode::SetSecondaryVolumeID()
   /// \sa vtkMRMLSelectionNode::SetActiveLabelVolumeID()
   void PropagateVolumeSelection(int fit = 1);
 
   /// Propagate only active background volume in the SelectionNode to slice composite
   /// nodes
-  /// \sa FitSliceToAll()
+  /// \sa FitSliceToBackground()
   /// \sa vtkMRMLSelectionNode::SetActiveVolumeID()
   /// \sa Layers::BackgroundLayer
   void PropagateBackgroundVolumeSelection(int fit = 1);
 
   /// Propagate only active foreground volume in the SelectionNode to slice composite
   /// nodes
-  /// \sa FitSliceToAll()
+  /// \sa FitSliceToBackground()
   /// \sa vtkMRMLSelectionNode::SetSecondaryVolumeID()
   /// \sa Layers::ForegroundLayer
   void PropagateForegroundVolumeSelection(int fit = 1);
 
   /// Propagate only active label volume in the SelectionNode to slice composite
   /// nodes
-  /// \sa FitSliceToAll()
+  /// \sa FitSliceToBackground()
   /// \sa vtkMRMLSelectionNode::SetActiveLabelVolumeID()
   /// \sa Layers::LabelLayer
   void PropagateLabelVolumeSelection(int fit = 1);
@@ -133,6 +133,15 @@ class VTK_MRML_LOGIC_EXPORT vtkMRMLApplicationLogic
   /// If resetOrientation is true then slice orientation can be modified during function call
   void FitSliceToAll(bool onlyIfPropagateVolumeSelectionAllowed=false, bool resetOrientation=true);
 
+  /// Fit all the visible volumes into their views.
+  /// This is a more advanced version of FitSliceToAll, which takes into account that in case of
+  /// ClipToBackgroundVolume is enabled for the slice then all layers above the background volume
+  /// will be clipped to the background volume's extents.
+  /// If onlyIfPropagateVolumeSelectionAllowed is true then field of view will be reset on
+  /// only those slices where propagate volume selection is allowed
+  /// If resetOrientation is true then slice orientation can be modified during function call
+  void FitSliceToBackground(bool onlyIfPropagateVolumeSelectionAllowed=false, bool resetOrientation=true);
+
   /// Propagate selected table in the SelectionNode to table view nodes.
   void PropagateTableSelection();
 
@@ -302,6 +311,8 @@ class VTK_MRML_LOGIC_EXPORT vtkMRMLApplicationLogic
 
   void ProcessMRMLNodesEvents(vtkObject* caller, unsigned long event, void* callData) override;
 
+  void FitSliceToContent(bool all, bool onlyIfPropagateVolumeSelectionAllowed=false, bool resetOrientation=true);
+
   void OnMRMLSceneStartBatchProcess() override;
   void OnMRMLSceneEndBatchProcess() override;
   void OnMRMLSceneStartImport() override;

Libs/MRML/Logic/vtkMRMLSliceLinkLogic.cxx

@@ -461,7 +461,7 @@ void vtkMRMLSliceLinkLogic::BroadcastSliceNodeEvent(vtkMRMLSliceNode *sliceNode)
         vtkMRMLSliceLogic* logic = this->GetMRMLApplicationLogic()->GetSliceLogic(sNode);
         if (logic)
         {
-          logic->FitSliceToAll();
+          logic->FitSliceToBackground();
           sNode->UpdateMatrices();
         }
       }

Libs/MRML/Logic/vtkMRMLSliceLogic.cxx

@@ -1809,24 +1809,18 @@ void vtkMRMLSliceLogic::FitSliceToFirst(int width, int height)
 // adjust the node's field of view to match the extent of current background volume
 void vtkMRMLSliceLogic::FitSliceToBackground(int width, int height)
 {
-  // Use SliceNode dimensions if width and height parameters are omitted
-  if (width < 0 || height < 0)
+  if (!this->SliceCompositeNode)
   {
-    int* dimensions = this->SliceNode->GetDimensions();
-    width = dimensions ? dimensions[0] : -1;
-    height = dimensions ? dimensions[1] : -1;
+    return;
   }
-
-  if (width < 0 || height < 0)
+  if (this->SliceCompositeNode->GetClipToBackgroundVolume())
   {
-    vtkErrorMacro(<< __FUNCTION__ << "- Invalid size:" << width
-                  << "x" << height);
-    return;
+    this->FitSliceToFirst(width, height);
+  }
+  else
+  {
+    this->FitSliceToAll(width, height);
   }
-
-  vtkMRMLVolumeNode *backgroundNode = nullptr;
-  backgroundNode = this->GetLayerVolumeNode(0);
-  this->FitSliceToVolume(backgroundNode, width, height);
 }
 
 //----------------------------------------------------------------------------

Libs/MRML/Logic/vtkMRMLSliceLogic.h

@@ -260,7 +260,10 @@ class VTK_MRML_LOGIC_EXPORT vtkMRMLSliceLogic : public vtkMRMLAbstractLogic
   void FitSliceToFirst(int width = -1, int height = -1);
 
   ///
-  /// adjust the node's field of view to match the extent of current background volume
+  /// Adjust the node's field of view to match the extent of the volume visible in the slice's background.
+  /// This is a more advanced version of FitSliceToAll, which takes into account that in case of
+  /// ClipToBackgroundVolume is enabled then all layers above the background volume
+  /// will be clipped to the background volume's extents.
   void FitSliceToBackground(int width = -1, int height = -1);
 
   ///

Libs/MRML/Widgets/qMRMLLayoutManager.h

@@ -247,7 +247,7 @@ public slots:
   void resetThreeDViews();
 
   /// Reset focal view around volumes
-  /// \sa qMRMLSliceControllerWidget::fitSliceToBackground(), vtkMRMLSliceLogic::FitSliceToAll()
+  /// \sa qMRMLSliceControllerWidget::fitSliceToBackground(), vtkMRMLSliceLogic::FitSliceToBackground()
   void resetSliceViews();
 
   /// Calls setPauseRender(pause) on all slice and 3D views

Libs/MRML/Widgets/qMRMLSliceControllerWidget.cxx

@@ -2049,15 +2049,7 @@ void qMRMLSliceControllerWidget::fitSliceToBackground()
   // This can be done by changing the interaction flag to
   // vtkMRMLSliceNode::FieldOfViewFlag
   d->SliceLogic->StartSliceNodeInteraction(vtkMRMLSliceNode::ResetFieldOfViewFlag);
-  if (d->MRMLSliceCompositeNode->GetClipToBackgroundVolume())
-  {
-    d->SliceLogic->FitSliceToFirst();
-  }
-  else
-  {
-    d->SliceLogic->FitSliceToAll();
-  }
-
+  d->SliceLogic->FitSliceToBackground();
   this->mrmlSliceNode()->UpdateMatrices();
   d->SliceLogic->EndSliceNodeInteraction();
 }

Modules/Loadable/Markups/Testing/Python/MarkupsInCompareViewersSelfTest.py

@@ -147,7 +147,7 @@ def run(self):
         compareLogic1.StartSliceCompositeNodeInteraction(1)
         compareCompositeNode1.SetBackgroundVolumeID(mrHeadVolume.GetID())
         compareLogic1.EndSliceCompositeNodeInteraction()
-        compareLogic1.FitSliceToAll()
+        compareLogic1.FitSliceToBackground()
         # make it visible in 3D
         compareLogic1.GetSliceNode().SetSliceVisible(1)
 
@@ -173,7 +173,7 @@ def run(self):
 
         # switch to compare grid
         lm.setLayout(23)
-        compareLogic1.FitSliceToAll()
+        compareLogic1.FitSliceToBackground()
         slicer.util.delayDisplay("Switched to Compare grid")
 
         # switch back to conventional

Modules/Loadable/Markups/Testing/Python/NeurosurgicalPlanningTutorialMarkupsSelfTest.py

@@ -202,7 +202,7 @@ def test_NeurosurgicalPlanningTutorialMarkupsSelfTest1(self):
         sliceLogic.StartSliceCompositeNodeInteraction(1)
         compositeNode.SetBackgroundVolumeID(baselineVolume.GetID())
         slicer.app.processEvents()
-        sliceLogic.FitSliceToAll()
+        sliceLogic.FitSliceToBackground()
         sliceLogic.EndSliceCompositeNodeInteraction()
         self.takeScreenshot("NeurosurgicalPlanning-Baseline", "Baseline in background")
 
@@ -221,7 +221,7 @@ def test_NeurosurgicalPlanningTutorialMarkupsSelfTest1(self):
         #
         lm.setLayout(slicer.vtkMRMLLayoutNode.SlicerLayoutOneUpRedSliceView)
         slicer.app.processEvents()
-        sliceLogic.FitSliceToAll()
+        sliceLogic.FitSliceToBackground()
         self.takeScreenshot("NeurosurgicalPlanning-RedSliceOnly", "Set layout to Red Slice only")
 
         #

Modules/Loadable/Segmentations/Widgets/qMRMLSegmentEditorWidget.cxx

@@ -2472,7 +2472,7 @@ void qMRMLSegmentEditorWidget::showSourceVolumeInSliceViewers(bool forceShowInBa
     }
     if (fitSlice)
     {
-      sliceLogic->FitSliceToAll(true);
+      sliceLogic->FitSliceToBackground(true);
     }
   }
 }

Modules/Loadable/Sequences/qSlicerSequencesReader.cxx

@@ -245,7 +245,7 @@ bool qSlicerSequencesReader::load(const IOProperties& properties)
         }
         if (appLogic)
         {
-          appLogic->PropagateVolumeSelection(); // includes FitSliceToAll by default
+          appLogic->PropagateVolumeSelection(); // includes FitSliceToBackground by default
         }
       }
     }

Modules/Loadable/Volumes/SubjectHierarchyPlugins/qSlicerSubjectHierarchyVolumesPlugin.cxx

@@ -695,7 +695,7 @@ void qSlicerSubjectHierarchyVolumesPlugin::showVolumeInAllViews(
         }
         if (resetFov)
         {
-          sliceLogic->FitSliceToAll();
+          sliceLogic->FitSliceToBackground();
         }
       }
     }
@@ -1278,7 +1278,7 @@ bool qSlicerSubjectHierarchyVolumesPlugin::showItemInView(vtkIdType itemID, vtkM
     }
     if (d->resetFieldOfViewOnShow())
     {
-      sliceLogic->FitSliceToAll();
+      sliceLogic->FitSliceToBackground();
     }
     sliceLogic->EndSliceNodeInteraction();
   }

Modules/Loadable/Volumes/qSlicerVolumesReader.cxx

@@ -202,7 +202,7 @@ bool qSlicerVolumesReader::load(const IOProperties& properties)
         }
         if (appLogic)
         {
-          appLogic->PropagateVolumeSelection(); // includes FitSliceToAll by default
+          appLogic->PropagateVolumeSelection(); // includes FitSliceToBackground by default
         }
       }
     }

Modules/Scripted/DICOMPlugins/DICOMImageSequencePlugin.py

@@ -280,7 +280,7 @@ def addSequenceBrowserNode(self, name, outputSequenceNodes, playbackRateFps, loa
                 selNode = appLogic.GetSelectionNode()
                 selNode.SetReferenceActiveVolumeID(proxyVolumeNode.GetID())
                 appLogic.PropagateVolumeSelection()
-                appLogic.FitSliceToAll()
+                appLogic.FitSliceToBackground()
                 slicer.modules.sequences.setToolBarActiveBrowserNode(outputSequenceBrowserNode)
 
         # Show sequence browser toolbar

Modules/Scripted/WebServer/WebServerLib/SlicerRequestHandler.py

@@ -958,7 +958,7 @@ def slice(self, request):
             if orientation.lower() == "coronal":
                 sliceNode.SetOrientationToCoronal()
             if orientation.lower() != previousOrientation:
-                sliceLogic.FitSliceToAll()
+                sliceLogic.FitSliceToBackground()
 
         imageData = sliceLogic.GetBlend().Update(0)
         imageData = sliceLogic.GetBlend().GetOutputDataObject(0)