From 6ba5c53030c55cec8947698e717e0e5a4dfc9fde Mon Sep 17 00:00:00 2001 From: lassoan Date: Tue, 17 Dec 2019 05:34:01 +0000 Subject: [PATCH] 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 | 16 ++++++ .../vtkSegmentationHistory.cxx | 50 +++++++++---------- .../vtkSegmentationHistory.h | 4 +- 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/Libs/vtkSegmentationCore/vtkSegment.cxx b/Libs/vtkSegmentationCore/vtkSegment.cxx index 6bed1839309..ac517fea91b 100644 --- a/Libs/vtkSegmentationCore/vtkSegment.cxx +++ b/Libs/vtkSegmentationCore/vtkSegment.cxx @@ -21,6 +21,7 @@ // SegmentationCore includes #include "vtkSegment.h" +#include "vtkSegmentationConverter.h" #include "vtkSegmentationConverterFactory.h" #include "vtkOrientedImageData.h" #include "vtkOrientedImageDataResample.h" @@ -28,6 +29,7 @@ // VTK includes #include #include +#include #include #include #include @@ -172,6 +174,7 @@ void vtkSegment::DeepCopy(vtkSegment* source) } this->DeepCopyMetadata(source); + this->SetLabelValue(source->GetLabelValue()); // Deep copy representations std::set 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 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); diff --git a/Libs/vtkSegmentationCore/vtkSegmentationHistory.cxx b/Libs/vtkSegmentationCore/vtkSegmentationHistory.cxx index 8201ed62af6..29835e65159 100644 --- a/Libs/vtkSegmentationCore/vtkSegmentationHistory.cxx +++ b/Libs/vtkSegmentationCore/vtkSegmentationHistory.cxx @@ -142,21 +142,9 @@ bool vtkSegmentationHistory::SaveState() baselineSegment = baselineSegmentIt->second.GetPointer(); } } + vtkSmartPointer segmentClone = vtkSmartPointer::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()); - savedObjects[masterRepresentation] = segmentClone->GetRepresentation(this->Segmentation->GetMasterRepresentationName()); - } - else - { - std::vector 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 representationsToIgnore/*std::vector()*/) + std::map& cachedRepresentations/*=std::map()*/) { destination->RemoveAllRepresentations(); destination->DeepCopyMetadata(source); @@ -182,12 +170,15 @@ void vtkSegmentationHistory::CopySegment(vtkSegment* destination, vtkSegment* so for (std::vector::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 segmentIDsToKeep; - std::map restoredDataObjects; + std::map restoredRepresentations; for (SegmentsMap::iterator restoredSegmentsIt = restoredState.Segments.begin(); restoredSegmentsIt != restoredState.Segments.end(); ++restoredSegmentsIt) { + vtkSegment* segmentToRestore = restoredSegmentsIt->second; segmentIDsToKeep.insert(restoredSegmentsIt->first); vtkSmartPointer 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 restoredRepresentationNames; + segmentToRestore->GetContainedRepresentationNames(restoredRepresentationNames); + std::vector 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); + } } } diff --git a/Libs/vtkSegmentationCore/vtkSegmentationHistory.h b/Libs/vtkSegmentationCore/vtkSegmentationHistory.h index 2953824d3e6..58f03d570df 100644 --- a/Libs/vtkSegmentationCore/vtkSegmentationHistory.h +++ b/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 representationsToIgnore); + void CopySegment(vtkSegment* destination, vtkSegment* source, vtkSegment* baseline, + std::map& cachedRepresentations=std::map()); protected: /// Container type for segments. Maps segment IDs to segment objects typedef std::map > SegmentsMap;