Some people in my team would like to get rid of vtkSmartPointer and use the more classic std::unique_ptr instead. My initial thought was : “Hell no !” but their implementation does not seem too bad.
I’m wondering if it is a good idea to leverage the std::unique_ptr from STL to use with any VTK object. We test for now with vtkPolyData.
The idea is to re-implement for some VTK objects default_delete function of std::unique_ptr to use the Delete() function inside.
Is there any specific situation where we may find difficulties? I think using Register method may be one.
Yes, VTK uses intrusive reference counting (the refcount is stored in the object itself). A std::unique_ptr is going to cause issues because if you pass the pointer to a filter or something, it can increase the refcount and expect it to hold it. You’ll need to manually do uptr.release() once this happens. Unfortunately, VTK is not that great at documenting when this happens.
In addition, the dtor for VTK objects tends to be protected, so you can’t access it anyways.
I have the same reaction. Why? The rational is to say that the std::unique_ptr interface is known by everyone, which is not the case of vtkSmartPointer. And some std functions like std::move are not yet available in VTK before version 9.
@ben.boeckel regarding the destructor, this is the purpose of re-implementing default-delete function of std::unique_ptr. You will be able to call the VTK “Delete()” function which is public.
@lassoan I don’t think it simplifies the VTK syntax in the case of vtkNew. In the case of vtkSmartPointer, you don’t have to call TakeReference. But the win is very limited.
I have also struggled a bit using vtkSmartPointer as a localized unique_ptr. It help a bit if it were move assignable, and had a simple method to clear its reference. Ideally would like one (or both) of these:
vtkSmartPointer<vtkSomething> localObject(...);
// sometime later
localObject.reset(nullptr);
// Or
localObject = nullptr;
Is that something that can be done in one place for all VTK objects, needs to be done for each type in its header, or on each declaration of a std::unique_ptr. I’ll also note that std::make_unique isn’t going to work either due to the ::New() construction convention.
Getting move semantics into VTK objects would be welcome, but I don’t think it’s going to be easy or gain as much as one might like since VTK likes slinging pointers rather than values around. It would at least help with these wrapper types though.
The implementation the team did is based on a macro you call for each type you want to use std::unique_ptr. That means you have to include the header file where you have the macro. The macro is used to re-implement the default-delete function.
I think trying to go outside of the classic usage of VTK object is dangerous since we are not controlling everything happening in VTK.
vtkNew already has move semantics - see vtkNew and vtkSmartPointer, so you don’t get simplified syntax with std::unique_ptr.
I understand that developers don’t want to learn new things if it is not necessary. However, in this case the slightly different syntax is used for a good reason: to clearly differentiate objects that use internal reference counting. Trying to cover this up to ease the burden on developers seems to be a step in the wrong direction, as developers may make wrong decisions if they are not aware how things actually work.
and I gave up partly down the rabbit hole of vtkSmartPointer::CheckTypes. Don’t really see how the std::nullptr_t gets promoted to the correct type so instead it appears that the implicit constructor from a raw pointer might instead be involved and that that is assigned in the operator=.
Here’s my two cents: I tend to use each API’s smart pointer. If it’s name is verborragic so be it. Better have a Java-like code than wasting time with mysterious bugs and crashes later. The APIs have different designs and manage their objects differently. I use STL smart pointers only with objects of my classes.
In my opinion, as VTK user, is that vtkSmartPointer is alike std::shared_ptr rather than std::unique_ptr.
vtkSmartPointer has a lot of capabilities out of the box, like being compatible with code expecting raw pointers.
It is also good to reflect that vtkObjects follow a pattern without calling c’tors/d’tors directly, so there is no side effects. The same API is shared among defunct TCL, Python and Java.
Despite I tend to prefer standard things, on this case the benefits outlast this preference.