Skip to content

Commit

Permalink
BUG: Make volume rendering preset registration more robust
Browse files Browse the repository at this point in the history 
When a volume rendering preset was added to the preset scene, the input preset node was moved to that scene, thereby making the scene that contained the input preset node unusable.
Fixed by making a copy of the input preset node instead of moving it.
  • Loading branch information
@lassoan
lassoan committed Feb 29, 2024
1 parent 1dd8730 commit c7fe865
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 24 deletions.
89 changes: 72 additions & 17 deletions Modules/Loadable/VolumeRendering/Logic/Testing/Cxx/vtkSlicerVolumeRenderingLogicTest.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

// MRML includes
#include <vtkMRMLCoreTestingMacros.h>
#include <vtkMRMLScalarVolumeNode.h>
#include <vtkMRMLScene.h>
#include <vtkMRMLVolumeNode.h>

Expand Down Expand Up @@ -63,7 +64,7 @@ int testDefaultRenderingMethod(const std::string& moduleShareDirectory)
CHECK_NULL(displayNode);

vtkNew<vtkMRMLScene> scene;
logic->SetMRMLScene(scene.GetPointer());
logic->SetMRMLScene(scene);
displayNode = logic->CreateVolumeRenderingDisplayNode();
CHECK_NOT_NULL(displayNode);
CHECK_BOOL(displayNode->IsA("vtkMRMLGPURayCastVolumeRenderingDisplayNode"), true);
Expand All @@ -84,25 +85,79 @@ int testPresets(const std::string& moduleShareDirectory)
vtkNew<vtkSlicerVolumeRenderingLogic> logic;
logic->SetModuleShareDirectory(moduleShareDirectory);

// Default presets
CHECK_NOT_NULL(logic->GetPresetsScene());
CHECK_NOT_NULL(logic->GetPresetByName("MR-Default"));

vtkNew<vtkMRMLVolumePropertyNode> newPreset;
newPreset->SetName("MyNewPreset");
CHECK_NULL(logic->GetPresetByName("MyNewPreset"));
logic->AddPreset(newPreset.GetPointer());
CHECK_NOT_NULL(logic->GetPresetByName("MyNewPreset"));

vtkNew<vtkImageData> iconImage;
vtkNew<vtkMRMLVolumePropertyNode> newPresetWithIcon;
newPresetWithIcon->SetName("MyNewPresetWithIcon");
logic->AddPreset(newPresetWithIcon.GetPointer(), iconImage.GetPointer());
vtkMRMLNode* newPresetWithIcon2 = logic->GetPresetByName("MyNewPresetWithIcon");
CHECK_NOT_NULL(newPresetWithIcon2);
vtkMRMLVolumeNode* iconNode = vtkMRMLVolumeNode::SafeDownCast(
newPresetWithIcon2->GetNodeReference(vtkSlicerVolumeRenderingLogic::GetIconVolumeReferenceRole()));
CHECK_NOT_NULL(iconNode);
CHECK_POINTER(iconNode->GetImageData(), iconImage.GetPointer());
// Add new preset
{
CHECK_NULL(logic->GetPresetByName("MyNewPreset"));
vtkNew<vtkMRMLVolumePropertyNode> newPreset;
newPreset->SetName("MyNewPreset");
CHECK_NOT_NULL(logic->AddPreset(newPreset));
CHECK_NOT_NULL(logic->GetPresetByName("MyNewPreset"));
}

// Add new preset with icon
{
vtkNew<vtkImageData> iconImage;
iconImage->SetDimensions(128, 128, 1);
iconImage->AllocateScalars(VTK_UNSIGNED_CHAR, 3);
vtkNew<vtkMRMLVolumePropertyNode> newPresetWithIcon;
newPresetWithIcon->SetName("MyNewPresetWithIcon");
CHECK_NOT_NULL(logic->AddPreset(newPresetWithIcon, iconImage));
// Check the added preset
vtkMRMLNode* addedPresetWithIcon = logic->GetPresetByName("MyNewPresetWithIcon");
CHECK_NOT_NULL(addedPresetWithIcon);
vtkMRMLVolumeNode* iconNode = vtkMRMLVolumeNode::SafeDownCast(
addedPresetWithIcon->GetNodeReference(vtkSlicerVolumeRenderingLogic::GetIconVolumeReferenceRole()));
CHECK_NOT_NULL(iconNode);
CHECK_POINTER_DIFFERENT(iconNode->GetImageData(), iconImage);
// Check that the icon has the same content
int* dimensions = iconNode->GetImageData()->GetDimensions();
CHECK_INT(dimensions[0], 128);
CHECK_INT(dimensions[1], 128);
CHECK_INT(dimensions[2], 1);
}

// Add new preset with icon from a custom presets scene
{
// Create custom presets scene (it is normally read from a scene file)
vtkNew<vtkMRMLScene> customPresetsScene;
vtkNew<vtkMRMLVolumePropertyNode> newPresetWithIcon;
newPresetWithIcon->SetName("MyNewPresetWithIcon");
customPresetsScene->AddNode(newPresetWithIcon);
vtkNew<vtkMRMLScalarVolumeNode> iconNode;
vtkNew<vtkImageData> iconImage;
iconImage->SetDimensions(128, 128, 1);
iconImage->AllocateScalars(VTK_UNSIGNED_CHAR, 3);
iconNode->SetAndObserveImageData(iconImage);
customPresetsScene->AddNode(iconNode);
newPresetWithIcon->SetNodeReferenceID(vtkSlicerVolumeRenderingLogic::GetIconVolumeReferenceRole(), iconNode->GetID());

// Add the preset
CHECK_NOT_NULL(logic->AddPreset(newPresetWithIcon));
// Check hat the preset is added
vtkMRMLNode* addedPresetWithIcon = logic->GetPresetByName("MyNewPresetWithIcon");
CHECK_NOT_NULL(addedPresetWithIcon);
// Check that the input preset node is still in the input scene
CHECK_POINTER(newPresetWithIcon->GetScene(), customPresetsScene);

// Check if icon is added and it is a different instance with the same content
vtkMRMLVolumeNode* addedIconNode = vtkMRMLVolumeNode::SafeDownCast(
addedPresetWithIcon->GetNodeReference(vtkSlicerVolumeRenderingLogic::GetIconVolumeReferenceRole()));
// Check that the icon is valid
CHECK_NOT_NULL(addedIconNode);
CHECK_NOT_NULL(addedIconNode->GetImageData());
// Check that the icon has the same content
int* dimensions = addedIconNode->GetImageData()->GetDimensions();
CHECK_INT(dimensions[0], 128);
CHECK_INT(dimensions[1], 128);
CHECK_INT(dimensions[2], 1);
// Check that the icon is an independent copy (not the same instance as the input)
CHECK_POINTER_DIFFERENT(addedIconNode, iconNode);
CHECK_POINTER_DIFFERENT(addedIconNode->GetImageData(), iconImage);
}

return EXIT_SUCCESS;
}
24 changes: 18 additions & 6 deletions Modules/Loadable/VolumeRendering/Logic/vtkSlicerVolumeRenderingLogic.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1416,12 +1416,13 @@ bool vtkSlicerVolumeRenderingLogic::IsDifferentFunction(vtkColorTransferFunction
}

//---------------------------------------------------------------------------
void vtkSlicerVolumeRenderingLogic::AddPreset(vtkMRMLVolumePropertyNode* preset, vtkImageData* icon/*=nullptr*/, bool appendToEnd/*=false*/)
vtkMRMLVolumePropertyNode* vtkSlicerVolumeRenderingLogic::AddPreset(
vtkMRMLVolumePropertyNode* preset, vtkImageData* icon/*=nullptr*/, bool appendToEnd/*=false*/)
{
if (preset == nullptr)
{
vtkErrorMacro("vtkSlicerVolumeRenderingLogic::AddPreset failed: preset is invalid");
return;
return nullptr;
}
if (icon == nullptr)
{
Expand All @@ -1434,24 +1435,35 @@ void vtkSlicerVolumeRenderingLogic::AddPreset(vtkMRMLVolumePropertyNode* preset,
}
}
vtkMRMLScene* presetScene = this->GetPresetsScene();
// Create a copy of the preset node because adding the node to the presetScene would remove the
// preset node from its current scene (where the icon volume is stored).
vtkNew<vtkMRMLVolumePropertyNode> presetToAdd;
presetToAdd->CopyContent(preset);
// Need to copy the node name explicitly, as CopyContent does not copy node name
presetToAdd->SetName(preset->GetName());
if (icon != nullptr)
{
// Create and independent copy of input icon
vtkNew<vtkImageData> iconCopy;
iconCopy->DeepCopy(icon);
// vector volume is chosen because usually icons are RGB color images
vtkNew<vtkMRMLVectorVolumeNode> iconNode;
iconNode->SetAndObserveImageData(icon);
iconNode->SetAndObserveImageData(iconCopy);
vtkMRMLNode* addedIconNode = presetScene->AddNode(iconNode.GetPointer());
// Need to set the node reference before adding the node to the scene to make sure the icon
// is available immediately when the node is added (otherwise widgets may add the item without an icon)
preset->SetNodeReferenceID(vtkSlicerVolumeRenderingLogic::GetIconVolumeReferenceRole(), addedIconNode->GetID());
presetToAdd->SetNodeReferenceID(vtkSlicerVolumeRenderingLogic::GetIconVolumeReferenceRole(), addedIconNode->GetID());
}
vtkMRMLNode* nodeAdded = nullptr;
if (appendToEnd || presetScene->GetNumberOfNodes() == 0)
{
presetScene->AddNode(preset);
nodeAdded = presetScene->AddNode(presetToAdd);
}
else
{
presetScene->InsertBeforeNode(presetScene->GetNthNode(0), preset);
nodeAdded = presetScene->InsertBeforeNode(presetScene->GetNthNode(0), presetToAdd);
}
return vtkMRMLVolumePropertyNode::SafeDownCast(nodeAdded);
}

//---------------------------------------------------------------------------
Expand Down
9 changes: 8 additions & 1 deletion Modules/Loadable/VolumeRendering/Logic/vtkSlicerVolumeRenderingLogic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,16 @@ class VTK_SLICER_VOLUMERENDERING_MODULE_LOGIC_EXPORT vtkSlicerVolumeRenderingLog
/// If the optional icon image is specified then that will be used to
/// in preset selector widgets. The icon is stored as a volume node
/// in the preset scene.
/// \param preset input node that contains specification of the volume rendering preset.
/// This node is not modified, but its content is copied into a new node in the preset scene.
/// \param icon specifies an icon image for the preset. If this icon is not specified then
/// the image node referenced by the input preset node is used as icon (using
/// vtkSlicerVolumeRenderingLogic::GetIconVolumeReferenceRole() node reference role).
/// In either case a copy of the icon image is made and that is stored in the preset scene.
/// \param appendToEnd controls if the preset is added before or after existing presets.
/// \sa GetPresetsScene(), GetIconVolumeReferenceRole()
void AddPreset(vtkMRMLVolumePropertyNode* preset, vtkImageData* icon = nullptr, bool appendToEnd=false);
/// \return Copy of the preset node that is added to the preset scene, nullptr on failure.
vtkMRMLVolumePropertyNode* AddPreset(vtkMRMLVolumePropertyNode* preset, vtkImageData* icon = nullptr, bool appendToEnd=false);

/// Removes a preset and its associated icon (if specified) from the preset scene.
/// \sa GetPresetsScene(), GetIconVolumeReferenceRole()
Expand Down

0 comments on commit c7fe865

Please sign in to comment.