BUG: Improve segment deep copy to separate shared labelmaps

When copying labelmap representations between segments, all values in the labelmap were previously copied.
This commit changes vtkSegment::DeepCopy to only copy the label value of the source segment.

Reimplements r28672 with some additional changes to ensure that DeepCopy is not used in vtkSegmentationHistory.
This ensures that Undo/Redo behavior is not affected.

git-svn-id: http://svn.slicer.org/Slicer4/trunk@28687 3bd1e089-480b-0410-8dfb-8563597acbee

Libs/vtkSegmentationCore/vtkSegment.cxx

@@ -21,13 +21,15 @@
 // SegmentationCore includes
 #include "vtkSegment.h"
 
+#include "vtkSegmentationConverter.h"
 #include "vtkSegmentationConverterFactory.h"
 #include "vtkOrientedImageData.h"
 #include "vtkOrientedImageDataResample.h"
 
 // VTK includes
 #include <vtkBoundingBox.h>
 #include <vtkImageData.h>
+#include <vtkImageThreshold.h>
 #include <vtkMatrix4x4.h>
 #include <vtkNew.h>
 #include <vtkObjectFactory.h>
@@ -172,6 +174,7 @@ void vtkSegment::DeepCopy(vtkSegment* source)
     }
 
   this->DeepCopyMetadata(source);
+  this->SetLabelValue(source->GetLabelValue());
 
   // Deep copy representations
   std::set<std::string> representationNamesToKeep;
@@ -185,7 +188,20 @@ void vtkSegment::DeepCopy(vtkSegment* source)
       vtkErrorMacro("DeepCopy: Unable to construct representation type class '" << reprIt->second->GetClassName() << "'");
       continue;
       }
+
     representationCopy->DeepCopy(reprIt->second);
+    // Binary labelmap is a special case, as it may contain multiple segment representations
+    if (reprIt->first == vtkSegmentationConverter::GetBinaryLabelmapRepresentationName())
+      {
+      vtkNew<vtkImageThreshold> threshold;
+      threshold->SetInputData(representationCopy);
+      threshold->ThresholdBetween(source->LabelValue, source->LabelValue);
+      threshold->SetInValue(source->LabelValue);
+      threshold->SetOutValue(0);
+      threshold->Update();
+      representationCopy->DeepCopy(threshold->GetOutput());
+      }
+
     this->AddRepresentation(reprIt->first, representationCopy);
     representationCopy->Delete(); // this representation is now owned by the segment
     representationNamesToKeep.insert(reprIt->first);

Libs/vtkSegmentationCore/vtkSegmentationHistory.cxx

@@ -142,21 +142,9 @@ bool vtkSegmentationHistory::SaveState()
         baselineSegment = baselineSegmentIt->second.GetPointer();
         }
       }
+
     vtkSmartPointer<vtkSegment> segmentClone = vtkSmartPointer<vtkSegment>::New();
-    // If the same object (i.e. shared labelmap) has already been copied into previous segmentation, then point to that
-    // object instead.
-    vtkDataObject* masterRepresentation = segment->GetRepresentation(this->Segmentation->GetMasterRepresentationName());
-    if (savedObjects.find(masterRepresentation) == savedObjects.end())
-      {
-      this->CopySegment(segmentClone, segment, baselineSegment, std::vector<std::string>());
-      savedObjects[masterRepresentation] = segmentClone->GetRepresentation(this->Segmentation->GetMasterRepresentationName());
-      }
-    else
-      {
-      std::vector<std::string> representationsToIgnore = { this->Segmentation->GetMasterRepresentationName() };
-      this->CopySegment(segmentClone, segment, baselineSegment, representationsToIgnore);
-      segmentClone->AddRepresentation(this->Segmentation->GetMasterRepresentationName(), savedObjects[masterRepresentation]);
-      }
+    this->CopySegment(segmentClone, segment, baselineSegment, savedObjects);
     newSegmentationState.Segments[*segmentIDIt] = segmentClone;
     }
   this->SegmentationStates.push_back(newSegmentationState);
@@ -171,7 +159,7 @@ bool vtkSegmentationHistory::SaveState()
 
 //---------------------------------------------------------------------------
 void vtkSegmentationHistory::CopySegment(vtkSegment* destination, vtkSegment* source, vtkSegment* baseline,
-  std::vector<std::string> representationsToIgnore/*std::vector<std::string>()*/)
+  std::map<vtkDataObject*, vtkDataObject*>& cachedRepresentations/*=std::map<vtkDataObject*, vtkDataObject*>()*/)
 {
   destination->RemoveAllRepresentations();
   destination->DeepCopyMetadata(source);
@@ -182,12 +170,15 @@ void vtkSegmentationHistory::CopySegment(vtkSegment* destination, vtkSegment* so
   for (std::vector<std::string>::iterator representationNameIt = representationNames.begin();
     representationNameIt != representationNames.end(); ++representationNameIt)
     {
-    if (std::find(representationsToIgnore.begin(), representationsToIgnore.end(), *representationNameIt) != representationsToIgnore.end())
+    vtkDataObject* sourceRepresentation = source->GetRepresentation(*representationNameIt);
+    if (cachedRepresentations.find(sourceRepresentation) != cachedRepresentations.end())
       {
+      // If the same object (i.e. shared labelmap) has already been cached from a previous segment, then point to that
+      // object instead. No need to perform copy.
+      destination->AddRepresentation(*representationNameIt, cachedRepresentations[sourceRepresentation]);
       continue;
       }
 
-    vtkDataObject* sourceRepresentation = source->GetRepresentation(*representationNameIt);
     vtkDataObject* baselineRepresentation = nullptr;
     if (baseline)
       {
@@ -199,6 +190,7 @@ void vtkSegmentationHistory::CopySegment(vtkSegment* destination, vtkSegment* so
       {
       // we already have an up-to-date copy in the baseline, so reuse that
       destination->AddRepresentation(*representationNameIt, baselineRepresentation);
+      cachedRepresentations[sourceRepresentation] = baselineRepresentation;
       }
     else
       {
@@ -211,6 +203,7 @@ void vtkSegmentationHistory::CopySegment(vtkSegment* destination, vtkSegment* so
         }
       representationCopy->DeepCopy(sourceRepresentation);
       destination->AddRepresentation(*representationNameIt, representationCopy);
+      cachedRepresentations[sourceRepresentation] = representationCopy;
       representationCopy->Delete(); // this representation is now owned by the segment
       }
     }
@@ -271,10 +264,11 @@ bool vtkSegmentationHistory::RestoreState(unsigned int stateIndex)
   SegmentationState restoredState = this->SegmentationStates[stateIndex];
 
   std::set<std::string> segmentIDsToKeep;
-  std::map<vtkDataObject*, vtkDataObject*> restoredDataObjects;
+  std::map<vtkDataObject*, vtkDataObject*> restoredRepresentations;
   for (SegmentsMap::iterator restoredSegmentsIt = restoredState.Segments.begin();
     restoredSegmentsIt != restoredState.Segments.end(); ++restoredSegmentsIt)
     {
+    vtkSegment* segmentToRestore = restoredSegmentsIt->second;
     segmentIDsToKeep.insert(restoredSegmentsIt->first);
     vtkSmartPointer<vtkSegment> segment = this->Segmentation->GetSegment(restoredSegmentsIt->first);
     if (segment == nullptr)
@@ -283,15 +277,19 @@ bool vtkSegmentationHistory::RestoreState(unsigned int stateIndex)
       this->Segmentation->AddSegment(segment, restoredSegmentsIt->first);
       }
 
-    vtkDataObject* restoredRepresentation = restoredSegmentsIt->second->GetRepresentation(this->Segmentation->GetMasterRepresentationName());
-    segment->DeepCopy(restoredSegmentsIt->second);
-    if (restoredDataObjects.find(restoredRepresentation) == restoredDataObjects.end())
-      {
-      restoredDataObjects[restoredRepresentation] = segment->GetRepresentation(this->Segmentation->GetMasterRepresentationName());
-      }
-    else
+    this->CopySegment(segment, segmentToRestore, nullptr, restoredRepresentations);
+
+    std::vector<std::string> restoredRepresentationNames;
+    segmentToRestore->GetContainedRepresentationNames(restoredRepresentationNames);
+    std::vector<std::string> currentRepresentationNames;
+    segment->GetContainedRepresentationNames(currentRepresentationNames);
+    // Remove representations that are not in the restoring segment
+    for (std::string representationName : currentRepresentationNames)
       {
-      segment->AddRepresentation(this->Segmentation->GetMasterRepresentationName(), restoredDataObjects[restoredRepresentation]);
+      if (std::find(restoredRepresentationNames.begin(), restoredRepresentationNames.end(), representationName) == restoredRepresentationNames.end())
+        {
+        segment->RemoveRepresentation(representationName);
+        }
       }
     }
 

Libs/vtkSegmentationCore/vtkSegmentationHistory.h

@@ -33,6 +33,7 @@
 #include "vtkSegmentationCoreConfigure.h"
 
 class vtkCallbackCommand;
+class vtkDataObject;
 class vtkSegment;
 class vtkSegmentation;
 
@@ -103,7 +104,8 @@ class vtkSegmentationCore_EXPORT vtkSegmentationHistory : public vtkObject
 
   /// Deep copies source segment to destination segment. If the same representation is found in baseline
   /// with up-to-date timestamp then the representation is reused from baseline.
-  void CopySegment(vtkSegment* destination, vtkSegment* source, vtkSegment* baseline, std::vector<std::string> representationsToIgnore);
+  void CopySegment(vtkSegment* destination, vtkSegment* source, vtkSegment* baseline,
+    std::map<vtkDataObject*, vtkDataObject*>& cachedRepresentations=std::map<vtkDataObject*, vtkDataObject*>());
 
 protected:  /// Container type for segments. Maps segment IDs to segment objects
   typedef std::map<std::string, vtkSmartPointer<vtkSegment> > SegmentsMap;