[vtk-developers] vtk-developers Digest, Vol 158, Issue 21

Elvis Stansvik elvis.stansvik at orexplore.com
Sun Jun 25 04:43:42 EDT 2017


But, going back to my original proposal now.

1. Using auto -> not a good idea since people may want to compile with
older compilers
2. Using vtkNew -> not a good idea since all the .Get() / .GetPointer() are ugly
3. Tidying up variable names (e.g. "iren", "renWin", ...) -> I got
some opposition to that too

So I guess I'll hold off on doing anything for now.

Though re. 1, what compilers don't support C++11 nowadays? I
personally think that would be sufficiently rare at this point that
those users could adapt the examples to fit their old compiler.

But I guess the bigger question is whether the examples should be
curated at all, or always left intact, to not discourage contribution?

Elvis

2017-06-25 10:27 GMT+02:00 Elvis Stansvik <elvis.stansvik at orexplore.com>:
> 2017-06-24 22:07 GMT+02:00 Andras Lasso <lasso at queensu.ca>:
>> It is great that you've brought this up! There is a very important, fundamental difference. Unlike std::shared_ptr or QSharedPointer, VTK smart pointer does NOT need clear distinction between the pointer object and the raw pointer it holds, because reference counting is implemented in the managed object itself.
>>
>> See in std::shared_ptr documentation: "Constructing a new shared_ptr using the raw underlying pointer owned by another shared_ptr leads to undefined behavior." (http://en.cppreference.com/w/cpp/memory/shared_ptr)
>>
>> In VTK, if you assign a vtkSmartPointer to another, internally the raw pointer is retrieved and used (Common/Core/vtkSmartPointer.h):
>>
>>   vtkSmartPointer& operator=(const vtkSmartPointer<U>& r)
>>   {
>>     this->vtkSmartPointerBase::operator=(CheckType(r.GetPointer()));
>>     return *this;
>>   }
>>
>> VTK smart pointers work differently from STL and Qt, so it is reasonable to use a different API that reflects this difference. As VTK allows safe use of a simpler API, we should take advantage of that and use it.
>
> That's a good point, and an important difference.
>
> Though, it's still unsafe (of course) in VTK to do say (pseudo):
>
> struct MyClass {
>     void setArray(vtkDataArray *arr) { a = arr; }
>     void doSomething() { /* Use the a member here */ }
>
>     vtkDataArray *a;
> }
>
> and then
>
> MyClass c;
>
> { // some scope somewhere
>     vtkNew<vtkDataArray> a;
>     ...
>     c.func(a);
> } // a dies (internal pointer freed), c survives
>
> c.doSomething(); // oops
>
> But I guess this should be filed in the 'so stupid the user should not
> be "protected" from it by a slightly awkward .Get()' category.
>
> I think you have my vote to add implicit conversion as well.
>
> Elvis
>
>>
>> Andras
>>
>> -----Original Message-----
>> From: Elvis Stansvik [mailto:elvis.stansvik at orexplore.com]
>> Sent: Saturday, June 24, 2017 12:36 PM
>> To: Andras Lasso <lasso at queensu.ca>
>> Cc: David Lonie <david.lonie at kitware.com>; Shawn Waldon <shawn.waldon at kitware.com>; VTK Developers <vtk-developers at vtk.org>; Andrew Maclean <andrew.amaclean at gmail.com>
>> Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21
>>
>> 2017-06-24 15:53 GMT+02:00 Andras Lasso <lasso at queensu.ca>:
>>> Based on the feedbacks and the poll results (11 out of 13 votes for
>>> changing current behavior), it seems there is a strong consensus for adding implicit conversion.
>>
>> I was away and missed the vote, but frankly I'm not sure what I would have voted.
>>
>> Looking at what others do, I guess vtkNew is comparable to std::unique_ptr or Qt's QScopedPointer, none of which do implicit conversion to the underlying pointer (they have .get() and .data() respectively). So VTK would be rather unique in doing that for a scoped smart pointer I think..
>>
>> Elvis
>>
>>>
>>> Here is a pull request with the change:
>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
>>> b.kitware.com%2Fvtk%2Fvtk%2Fmerge_requests%2F2961&data=02%7C01%7Classo
>>> %40queensu.ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd61ecb3b38b142d582c
>>> 4efb2838b925c%7C1%7C0%7C636339189405346369&sdata=2Jv0QS1qlRUgSJvN%2BO8
>>> 0pXOt89kKC0NP7iXXRr2mg30%3D&reserved=0
>>>
>>> Andras
>>>
>>> -----Original Message-----
>>> From: David Lonie [mailto:david.lonie at kitware.com]
>>> Sent: Friday, June 23, 2017 1:51 PM
>>> To: Shawn Waldon <shawn.waldon at kitware.com>
>>> Cc: Andras Lasso <lasso at queensu.ca>; VTK Developers
>>> <vtk-developers at vtk.org>; Andrew Maclean <andrew.amaclean at gmail.com>
>>> Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21
>>>
>>> I'm so glad someone brought this up =)
>>>
>>> On Fri, Jun 23, 2017 at 10:52 AM, Shawn Waldon <shawn.waldon at kitware.com> wrote:
>>>>  But realistically what it does is
>>>> discourage use of vtkNew.  Most people I talk to say "I would use
>>>> vtkNew but then I have to clutter up my code with GetPointer calls
>>>> everywhere" and so they still use vtkSmartPointer.  Honestly I only
>>>> used it to clean up declarations (the problem discussed earlier in
>>>> this thread).  Now that we can use auto in VTK, I'll probably go back
>>>> to vtkSmartPointer since it is easier to use.
>>>
>>> I agree with Shawn. It's an unnecessary hassle to require the Get() to be used.
>>>
>>> I suppose I'm just a fan of the design philosophy for C++ about "don't prevent developers from shooting themselves in the foot." If a dev doesn't understand VTK's reference counting well enough to understand that you can't simply return a vtkNew'd pointer without increfing it first, they're going to have far greater problems using the toolkit than this.
>>>
>>> Besides, you could easily (and safely) return a vtkNew'd pointer from a function:
>>>
>>> vtkObject* function()
>>> {
>>>   vtkNew<vtkObject> obj;
>>>   obj->Register();
>>>   return obj; // If we had implicit conversions, anyway }
>>>
>>> Making the class unwieldly simply so that people don't need to learn how reference counts work is a poor justification IMO.
>>>
>>> Just my 2c,
>>> Dave
>>> _______________________________________________
>>> Powered by
>>> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&dat
>>> a=02%7C01%7Classo%40queensu.ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd6
>>> 1ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636339189405346369&sdata=EWly
>>> 8qDaOVuCYOrDhoQ7SyY3glwTdlm1J1nxY%2BBDO50%3D&reserved=0
>>>
>>> Visit other Kitware open-source projects at
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.ki
>>> tware.com%2Fopensource%2Fopensource.html&data=02%7C01%7Classo%40queens
>>> u.ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd61ecb3b38b142d582c4efb2838b
>>> 925c%7C1%7C0%7C636339189405346369&sdata=v7U9kMfdmgpWZ6WYn1nyPTuUHWzn4s
>>> J6XvYEJdpw50I%3D&reserved=0
>>>
>>> Search the list archives at:
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmarkma
>>> il.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40queensu
>>> .ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd61ecb3b38b142d582c4efb2838b9
>>> 25c%7C1%7C0%7C636339189405346369&sdata=otfBLrSAA5d7a9u6mrb56%2ByrIVM%2
>>> Bl3f%2BZNQpwOvqgO0%3D&reserved=0
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpublic
>>> .kitware.com%2Fmailman%2Flistinfo%2Fvtk-developers&data=02%7C01%7Classo%40queensu.ca%7C14c5314b70e443f5be4908d4bb1f0bf4%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636339189405346369&sdata=whCFetn1LnPSRi0CZ%2BD1aGlMHqdv5vSgzkRdcuLw8qI%3D&reserved=0
>>>


More information about the vtk-developers mailing list