Adding new method variants should not be difficult.
Current non-factory methods usually start with
Get... and take the output object pointer as an argument.
In VTK, a public method is factory method if an only if its name starts with
Create.... (There are just a handful of exceptions:
New... is used for iterators and
Make... for keys, transform, camera, and render window interactor). So, name of any new factory method in VTK classes in Slicer should start with
Since the old methods names start with
Get... and have a different argument list, the method signature will be different, so there would be no backward compatibility issues.
The new method names could be
Create...Copy when we want to get an object but it is only feasible to get a copy (because we don’t want to expose an internal object or the object is not stored internally). For example:
int vtkMRMLTransformNode::GetMatrixTransformToParent(vtkMatrix4x4* matrix)
we could use:
VTK_NEWINSTANCE vtkMatrix4x4* vtkMRMLTransformNode::CreateMatrixTransformToParentCopy()
bool vtkMRMLSegmentationNode::GetBinaryLabelmapRepresentation(const std::string segmentId, vtkOrientedImageData* outputBinaryLabelmap);
we could use:
VTK_NEWINSTANCE vtkOrientedImageData* vtkMRMLSegmentationNode::CreateBinaryLabelmapRepresentationCopy(const std::string segmentId);
A complication is that in Slicer some factory methods don’t start with
Create.... For these we could invent a new name and deprecate the old one. However, this can be quite difficult.
Current non-conform factory method name:
nodes = slicer.mrmlScene.GetNodesByClass('vtkMRMLScalarVolumeNode')
What would be the factory method name?
nodes = slicer.mrmlScene.CreateNodeCollectionByClass('vtkMRMLScalarVolumeNode')
nodes = slicer.mrmlScene.CreateNodeListByClass('vtkMRMLScalarVolumeNode')
nodes = slicer.mrmlScene.CreateCollectionOfNodesByClass('vtkMRMLScalarVolumeNode')
Yes, ideally we should add this to all current factory methods. The issue is that currently when a factory method is called from Python then
Unregister is called, which would crash Slicer after if we simply add the VTK_NEWINSTANCE hint.
Maybe we could do it in phases:
CreateAndTake... method variant (only expose in Python)
node = slicer.mrmlScene.CreateAndTakeNodeByClass('vtkMRMLMultiVolumeNode')
- Remove Python wrapping from the original
In this phase Python code that is not updated to
CreateAndTake... would fail. This should force every maintained extensions to switch to
Re-enable Python wrapping of
Create..., but now with VTK_NEWINSTANCE hint enabled.
If we don’t want to rush developers then we would need to spend at least 6 months in each phase.