To start, rework the examples and the inner workings of classes. Many users/developers learn from these, and copy code fragments from these, and we could do better at setting a good example. The API will have to evolve if we decide to move in that direction.
Will,
The examples in the VTK Examples allude either vtkNew or vtkSmartPointer. I notice that most users posting examples do the same.
Bill
Excellent on the Examples thatās a credit to you, Andrew and many others who are keeping these polished. Grepping the internals of classes *.cxx for ::New() and doing a quick word count I get over 8500 cases and I suspect I am responsible for many :-). Some I assume may be necessary but there is certainly room for improvement.
@will.schroeder This is interesting, using something like:
find . -name *.cxx -exec grep -Hn '>::New()' {} + | wc
or
find . -name *.cxx -exec grep -Hn 'vtkNew<' {} + | wc
I get for the VTKExamples:
- vtkSmartPointer : 10256
- vtkNew : 283
- vtkNew/vtkSmartPointer: 0.03
For VTK:
- vtkSmartPointer : 6285
- vtkNew : 8150
- vtkNew/vtkSmartPointer: 1.30
A lot of the VTK examples were written before vtkNew was introduced . @lorensen and I also converted many of the older ones to using vtkSmartPointer. This explains why the ratio is much lower. However, if you look at the examples, the majority can use vtkNew in place of vtkSmartPointer.
A quick look at VTK shows that thare are still areas where vtkNew or vtkSMartPointer can replace the old
vtkObject *Obj = vtkObject::New();`
//
Obj->Delete();
pattern. These will most likely change in the future as the code evolves.
I should add that the VTK Examples are keeping up with the VTK master changes as I do a build on Windows and Linux on a mostly daily basis.
Also see my thread about refactoring examples to use more vtkNew in June 2017: https://vtk.org/pipermail/vtk-developers/2017-June/035145.html
(That was right before vtkNew got the implicit conversion to raw pointer, so my suggestion was met with some scepticism because of all the .Get() it would add)
I use a mix of both forms, for different purposes.
For ānormalā use, managing a pointer:
auto mapper = vtkSmartPointer<vtkPolyDataMapper>::New();
But also use without a managing pointer:
vtkSmartPointer<vtkRenderer> renderer;
...
if (some_condition)
{
renderer = vtkSmartPointer<vtkRenderer>::New();
syncRenderers->SetRenderer(renderer);
}
This allows some processes to have a VTK renderer. The ones without have a null pointer, but in all cases I donāt have to fiddle with a Delete anywhere, since the vtkSmartPointer takes care of that for me.
/mark
Iāve started using auto in new examples:
auto actor = vtkSmartPointer::New();
Personal preference.
I prefer vtkNew when it is a local declaration as even with auto it is more compact, and more clearly conveys what I am doing:
vtkNew<vtkChartXY> chart;
versus
auto chart = vtkSmartPointer<vtkChartXY>::New()
I will admit to some bias, but think that it is both shorter and more expressive to use vtkNew in that context. If I am storing an instance I get, or working with it from a factory etc I would prefer vtkSmartPointer, i.e.
vtkSmartPointer<vtkChartXY> chart = chartFactory->createLineChart();
That is where I wonder if the vtkNew name is its downfall when using move semantics. We named it quite narrowly, and calling
vtkNew<vtkChartXY> chart = chartFactory->createLineChart();
Seems a little muddy, whereas vtkUniquePointer would be clearer in intent. I guess people can learn the nuance of what happened with vtkNew, but before it becomes too widespread it is worth considering whether the addition of something like vtkUniquePointer as a preferred factory method would be preferable to generalizing vtkNew.
When I proposed it and added it I wanted a simple equivalent to declaring a local instance.
vtkChartXY chart;
vtkNew<vtkChartXY> chart;
It was not named in the best way, or intended to fill the emerging use of unique_ptr with move semantics, factory method creation etc.
@allison.vacanti is it possible that some casts work on Windows while throw an error on Linux/Mac?
For example,
vtkWeakPointer<...> b;
ā¦
vtkSmartPointer<ā¦ > a = b;
Seems to work fine on Windows but fails to build on Linux/Mac (https://github.com/Slicer/Slicer/commit/8fdb84d8ca10fee270688ec3a7aa10407c02b418#commitcomment-33060893). Either behavior would be OK, but it should be consistent across major compiler versions.
Iām on leave for the next 6 weeks recovering from surgery and donāt have access to my dev machine, but from what I remember, all of the smart pointers are convertible from vtkNew, but other smart pointer types need an explicit Get() to convert e.g. weak to smart. I can double check this when I get back. Iām not opposed to allowing those conversions, IIRC it was tricky to get the dependencies right while avoiding circular includes so I held off on that feature until I had more time to dig in.
As for the platform-specific behavior, I donāt think we have a lot of control over keeping things consistent across compilers for now, it sounds like some are just more permissive than others when it comes to allowing conversions.
As you pointed out, this only works in Windows MSVC:
auto pd = vtkSmartPointer<vtkPolyData>::New();
vtkWeakPointer<vtkPolyData> wppd = pd;
std::cout << wppd->GetReferenceCount() << std::endl;
vtkSmartPointer<vtkPolyData> pd1 = wppd;
std::cout << wppd->GetReferenceCount() << std::endl;
This works in Linux and MSVC:
auto pd = vtkSmartPointer<vtkPolyData>::New();
vtkWeakPointer<vtkPolyData> wppd(pd);
std::cout << wppd->GetReferenceCount() << std::endl;
vtkSmartPointer<vtkPolyData> pd1(wppd);
std::cout << wppd->GetReferenceCount() << std::endl;
A quick scan of the tests for vtkWeakPointer also shows that a constructor not assignment is used.
I am new user in VTK i have readed your discussion,it is helpful! but i can`t understand all of them .
So in brief ,if i want to create a smart pointer , use the vtknew or vtkSmartPoiner to create is the same thing?
Thereās at least in most case no big difference between them ļ¼
Use vtkNew<> whenever you create a new object.
If you need to keep a pointer to an existing object then you can use:
-
vtkSmartPointer<>- it prevents automatic deletion of the referenced object -
vtkWeakPointer<>- it does not prevent automatic deletion, but when the referenced object is deleted then the smart pointer is set to nullptr - simple object pointer - it does not prevent automatic deletion and when the referenced object is deleted then it will point to invalid memory area and crashes the application if dereferenced, so it has to be avoided (except special highly performance-critical parts of your code)
2 Likes
Thatās the answer I want to know!
Thank you so much for your reply!!! 
1 Like