Simplifying the use of vtkNew

For C++ developers only. We had a discussion on the VTK mailing list arguing that vtkNew should be as simple to use as vtkSmartPointer or not. Currently, when an object is created by vtkNew, you need to call .GetPointer() to get the raw pointer.

Current usage:

someFilter->SetInputData(imageData.GetPointer());

I recommended to add an implicit converter to raw pointer, so that we can simply write:

someFilter->SetInputData(imageData);

If you agree that it would be better to simplify the syntax, please mark your preference in this poll - takes two clicks. If you think that requiring adding .GetPointer() is safer, fill the form accordingly, too.

It that the main use case? Maybe the vtkSetObjectMacro should be polymorphic and apply GetPointer when needed.

I think we could add implicit conversion for existing classes at Slicer level, but that would mean that VTK code that works in Slicer may not work (or work slightly differently) in a plain VTK environment. So far 75% of poll respondents support this simplification, so hopefully this can be changed in VTK.

@lassoan I thought we were getting rid of GetPointer, but it’s still needed, at least on my gcc 7.3 ubuntu 18.04 builds. I have been just fixing them but they keep popping up. Can we come up with some way to get rid of them all or if not can we explicitly always use them.

[ 11%] Building CXX object Libs/MRML/Core/CMakeFiles/MRMLCore.dir/vtkMRMLSegmentationNode.cxx.o
/home/pieper/slicer4/latest/Slicer/Libs/MRML/Core/vtkMRMLSegmentationNode.cxx: In member function ‘virtual bool vtkMRMLSegmentationNode::GenerateEditMask(vtkOrientedImageData*, int, vtkOrientedImageData*, std::__cxx11::string, std::__cxx11::string, vtkOrientedImageData*, double*, vtkMRMLSegmentationDisplayNode*)’:
/home/pieper/slicer4/latest/Slicer/Libs/MRML/Core/vtkMRMLSegmentationNode.cxx:782:63: error: no matching function for call to ‘vtkOrientedImageData::SetImageToWorldMatrix(vtkNew<vtkMatrix4x4>&)’
   maskImage->SetImageToWorldMatrix(referenceImageToWorldMatrix);

After the next patch release, we should be able to remove the explicit use of GeTPointer()

at least on my gcc 7.3 ubuntu 18.04 builds

Could you confirm that the build is done with VTK8 and c++11 ?

@pieper I don’t know about any other developer using qt4/vtk7, so it would be great if you could keep fixing these. I try to pay attention to adding those GetPointer() calls but since the compiler does not complain, it us easy to miss them. After releasing 4.10.1, qt4/vtk7 will not be supported, so this will not be problem anymore.

I added the extra GetPointer calls and also version check ifdef on one vtk method that was added after vtk7.

This was actually a vtk7/Qt5.8 build. I’ve stopped doing Qt4 builds and have no reason to keep a vtk7 build (it just happened that I updated an old build tree). So if we can’t be sure these work we should explicitly stop supporting them.

We’ll explicitly stop supporting it in Slicer-5, which I think should be now or within a few days.

3 Likes