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

Andras Lasso lasso at queensu.ca
Fri Jun 23 08:23:23 EDT 2017


> The reason for not allowing automatic conversion to pointer with vtkNew is to prevent people from returning a stale/dead pointer from a method.

It would be the same automatic conversion as in vtkSmartPointer. Nobody complains about that.

> It's safe to return a vtkSmartPointer from a method, and use the GetPointer from a vtkNew to do so, but not a raw pointer, unless you have some guarantee that some other reference to the pointer is keeping it alive.

The guarantee exists already. If the return type of the method is vtkSmartPointer then the vtkSmartPointer increments the reference count on the object before the local variable goes out of scope.

Andras

-----Original Message-----
From: David Cole [mailto:DLRdave at aol.com] 
Sent: Friday, June 23, 2017 7:24 AM
To: Andras Lasso <lasso at queensu.ca>
Cc: David Cole <DLRdave at aol.com>; Elvis Stansvik <elvis.stansvik at orexplore.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

The reason for not allowing automatic conversion to pointer with vtkNew is to prevent people from returning a stale/dead pointer from a method.

It would be disastrous to allow automatic conversion to a raw pointer.
The intent is to make it easy to create a local variable that goes away automatically when the scope closes. Allowing conversion to a raw pointer would make it very easy to write bad code.

It's safe to return a vtkSmartPointer from a method, and use the GetPointer from a vtkNew to do so, but not a raw pointer, unless you have some guarantee that some other reference to the pointer is keeping it alive.


HTH,
David C.




On Fri, Jun 23, 2017 at 7:04 AM, Andras Lasso <lasso at queensu.ca> wrote:
> I find vtkNew<…> to be the shortest and most readable way of creating 
> a new VTK object. However, it is rather inconvenient that GetPointer() 
> must be called to get the pointer. Is there a specific reason for 
> requiring using GetPointer()? Could we change vtkNew to have automatic 
> conversion to pointer type - the same way as in vtkSmartPointer?
>
>
>
> Andras
>
>
>
> From: David Cole via vtk-developers
> Sent: Friday, June 23, 2017 5:29 AM
> To: Elvis Stansvik
> Cc: VTK Developers; Andrew Maclean
> Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21
>
>
>
> One thing I would point out is some folks who might want to compile 
> the VTK examples may be using a slightly older version of VTK, and 
> perhaps one that is not even being compiled with a modern compiler 
> capable of compiling C++ 11...
>
> So I would refrain from using "auto" in the examples until such time 
> as all the people who want to build an example are pretty much 
> guaranteed to be using a VTK which requires C++ 11, and therefore a 
> compiler capable of dealing with C++ 11 constructs.
>
> I wouldn't do the "auto" thing just yet.
>
>
> David C.
>
>
>
> On Fri, Jun 23, 2017 at 4:47 AM, Elvis Stansvik 
> <elvis.stansvik at orexplore.com> wrote:
>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <elvis.stansvik at orexplore.com>:
>>> Sorry, accidently hit send. Fixes below.
>>>
>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <elvis.stansvik at orexplore.com>:
>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <andrew.amaclean at gmail.com>:
>>>>> Hi Bill, Elvis,
>>>>>    Elvis, personally I wouldn't like to see the homogenisation of 
>>>>> the examples by doing what you propose.
>>>>> The reasons are:
>>>>> 1) One of the advantages of the examples is seeing the different 
>>>>> approaches used by the contributors.
>>>>> 2) It may dissuade contributors by implicitly forcing them to use 
>>>>> a particular approach.
>>>>> 3) One of the really useful things in the example is the different 
>>>>> ways VTK is used.
>>>>
>>>> I absolutely agree with 1 and 3 (which I think are the same?), but 
>>>> I don't see how changing to auto would in affect anything in this 
>>>> regard.
>>>>
>>>> I also don't see how it would be a homogenization. The declarations 
>>>> I would change are already homogeneous in that they're all 
>>>> vtkSmartPointer<Foo> a = vtkSmartPointer<Foo>::New(). Changing to 
>>>> auto would not make it more or less homogeneous.
>>>>
>>>> It would be a
>>>
>>> ... It would be homogenisation if I'd change all 
>>> vtkNew/vtkSmartPointer to auto a = vtkSmartPointer..., but that's 
>>> not what this is about.
>>>
>>>> Note that this is not about changing vtkNew to vtkSmartPointer.
>>>>
>>>> And how would changing to auto in any way affect the approach taken 
>>>> by the example?
>>>>
>>>>
>>>>
>>>>> To me it matters little whether:
>>>>> auto actor = vtkSmartPointer<vtkActor>::New(); 
>>>>> vtkSmartPointer<vtkActor> actor =
>>>>>     vtkSmartPointer<vtkActor>::New();
>>>>>
>>>>> or whether "ren/renWin" is used instead of "renderer" or 
>>>>> "rendererWindow" in the examples.
>>>
>>> It matters little to me too, but it does matter. I think it's almost 
>>> indisputable that
>>>
>>>     auto someVar = vtkSmartPointer<SomeLongTypeName>::New()
>>>
>>> is more readable than
>>>
>>>     vtkSmartPointer<SomeLongTypeName> someVar =
>>>         vtkSmartPointer<SomeLongTypeName>::New();
>>>
>>> especially since the latter leads to many more lines to scan across 
>>> when looking for something in the examples.
>>
>> Another small plus I see with using auto is it's a keyword which 
>> would be highlighted, so the declarations would stand out more.
>>
>> E.g. looking at the code for this example
>>
>>
>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
>> nsen.github.io%2FVTKExamples%2Fsite%2FCxx%2FFiltering%2FConnectivityF
>> ilter%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4
>> ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146
>> 481&sdata=ha%2BCR8CriMPmQqz%2FxrAgI6lQ7RKn21tuZ6Z%2FitzKD%2Fg%3D&rese
>> rved=0
>>
>> I find it hard to quickly answer "what are the declarations?", since 
>> they have no highlighting compared to the surrounding statements. Had 
>> they been auto, it would have been easier since I think auto would 
>> have been highlighted.
>>
>> I think quickly identifying the variables involved helps the reading 
>> of the examples.
>>
>> Elvis
>>
>>>
>>> So, in short I agree with everything you say, but I can't see how 
>>> changing one way of doing declarations to another is a homogenization.
>>> And I do think spelling matters.
>>>
>>> I'm perfectly OK with leaving the examples exactly like they are 
>>> though, just wanted to explain how I see it.
>>>
>>> Elvis
>>>
>>>>>
>>>>> Of more importance are explanatory notes in the examples.
>>>>>
>>>>> Bill, I see you are using vtkNamedColors. This example shows what 
>>>>> other things you can do with this class:
>>>>>
>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fl
>>>>> orensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FVisualization%2FNam
>>>>> edColors%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f
>>>>> 2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338
>>>>> 069854146481&sdata=jnbJJe87OvOErNapbQL2HiAt6DnbOWifp67W4lNVqfo%3D&
>>>>> reserved=0
>>>>>
>>>>> For example, assign colors by name:
>>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetDa
>>>>> renderer->ta
>>>>> ());
>>>>> Create your own named color (in this case a red with an alpha of 0.5):
>>>>> namedColors->GetColor("Red", rgba);
>>>>> rgba[3] = 0.5; namedColors->SetColor("My Red", rgba);
>>>>>
>>>>> Regards
>>>>>    Andrew
>>>>>
>>>>>>
>>>>>> ---------- Forwarded message ----------
>>>>>> From: Bill Lorensen <bill.lorensen at gmail.com>
>>>>>> To: Elvis Stansvik <elvis.stansvik at orexplore.com>
>>>>>> Cc: vtkdev <vtk-developers at vtk.org>
>>>>>> Bcc:
>>>>>> Date: Thu, 22 Jun 2017 13:32:55 -0400
>>>>>> Subject: Re: [vtk-developers] vtkNew in examples (or auto?) Let's 
>>>>>> leave them as is for now. I want to make sure I understand this.
>>>>>>
>>>>>>
>>>>>> On Thu, Jun 22, 2017 at 1:13 PM, Elvis Stansvik 
>>>>>> <elvis.stansvik at orexplore.com> wrote:
>>>>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <bill.lorensen at gmail.com>:
>>>>>> >> I'm not sure what you mean by auto-fying
>>>>>> >
>>>>>> > Sorry, I should have been clearer. What I mean is changing 
>>>>>> > declarations such as
>>>>>> >
>>>>>> >
>>>>>> vtkSmartPointer<vtkActor> actor =
>>>>>> >     vtkSmartPointer<vtkActor>::New();
>>>>>> >
>>>>>> > into
>>>>>> >
>>>>>> >   auto actor = vtkSmartPointer<vtkActor>::New();
>>>>>> >
>>>>>> > I think it would cut down on the number of lines in many 
>>>>>> > examples, and make them more readable. (This would only be done 
>>>>>> > in places where the type of the variable is still clear from 
>>>>>> > the declaration.)
>>>>>> >
>>>>>> > Elvis
>>>>>> >
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >> On Thu, Jun 22, 2017 at 1:07 PM, Elvis Stansvik 
>>>>>> >> <elvis.stansvik at orexplore.com> wrote:
>>>>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen
>>>>>> >>> <bill.lorensen at gmail.com>:
>>>>>> >>>> I prefer vtkSmartPointer because it can be used like any 
>>>>>> >>>> other vtk pointer. No need for a GetPointer() is some cases. 
>>>>>> >>>> The example writer is free to use vtkSmartPointer or vtkNew. 
>>>>>> >>>> But I would leave them as there are.
>>>>>> >>>
>>>>>> >>> Right, that's a valid point. But how about auto-fying the 
>>>>>> >>> declarations? (but keep using vtkSmartPointer)
>>>>>> >>>
>>>>>> >>> My motivation is that when reading an example, I'm often 
>>>>>> >>> squinting to find the variable names in the declarations, 
>>>>>> >>> wedged in there somewhere between all those type names and 
>>>>>> >>> angle brackets. Especially as the lines are often broken due 
>>>>>> >>> to running long.
>>>>>> >>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> Other cleanup's sound great. I've also started using 
>>>>>> >>>> vtkNamedColors instead of setting float values.
>>>>>> >>>
>>>>>> >>> Great.
>>>>>> >>>
>>>>>> >>> Elvis
>>>>>> >>>
>>>>>> >>>>
>>>>>> >>>> On Thu, Jun 22, 2017 at 12:57 PM, Elvis Stansvik 
>>>>>> >>>> <elvis.stansvik at orexplore.com> wrote:
>>>>>> >>>>> Hi all,
>>>>>> >>>>>
>>>>>> >>>>> How about a refactor of the examples to use vtkNew instead 
>>>>>> >>>>> of vtkSmartPointer (where it makes sense)?
>>>>>> >>>>>
>>>>>> >>>>> E.g.
>>>>>> >>>>>
>>>>>> >>>>>   vtkNew<vtkActor> actor;
>>>>>> >>>>>   actor->SetMapper(mapper);
>>>>>> >>>>>
>>>>>> >>>>>   vtkNew<vtkRenderer> renderer;
>>>>>> >>>>>   renderer->AddActor(actor);
>>>>>> >>>>>
>>>>>> >>>>> instead of
>>>>>> >>>>>
>>>>>> >>>>>   vtkSmartPointer<vtkActor> actor =
>>>>>> >>>>>     vtkSmartPointer<vtkActor>::New();
>>>>>> >>>>>   actor->SetMapper(mapper);
>>>>>> >>>>>
>>>>>> >>>>>   vtkSmartPointer<vtkRenderer> renderer =
>>>>>> >>>>>     vtkSmartPointer<vtkRenderer>::New();
>>>>>> >>>>>   renderer->AddActor(actor);
>>>>>> >>>>>
>>>>>> >>>>> I think it would help with the readability of the examples. 
>>>>>> >>>>> Or are there other reasons for the prevalent use of 
>>>>>> >>>>> vtkSmartPointer?
>>>>>> >>>>>
>>>>>> >>>>> Another option would be to use auto, e.g.
>>>>>> >>>>>
>>>>>> >>>>>   auto actor = vtkSmartPointer<vtkActor>::New();
>>>>>> >>>>>
>>>>>> >>>>> Also, would anyone mind if I did a little naming cleanup, 
>>>>>> >>>>> mostly things like "renwin" -> "window" and "iren" -> "interactor"?
>>>>>> >>>>> Those
>>>>>> >>>>> abbreviations are not that bad, but I think it's better in 
>>>>>> >>>>> examples to spell out the variables in proper English.
>>>>>> >>>>>
>>>>>> >>>>> If there are no objections, I could try to prepare an MR 
>>>>>> >>>>> when time permits. If so, vtkNew, or auto?
>>>>>> >>>>>
>>>>>> >>>>> Elvis
>>>>>> >>>>> _______________________________________________
>>>>>> >>>>> Powered by
>>>>>> >>>>> https://na01.safelinks.protection.outlook.com/?url=www.kitw
>>>>>> >>>>> are.com&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7
>>>>>> >>>>> f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C
>>>>>> >>>>> 0%7C636338069854146481&sdata=MOVhItfPCXhoqRfpkkpfmBGyNrthSQ
>>>>>> >>>>> hA9SD%2Fxx0aWHI%3D&reserved=0
>>>>>> >>>>>
>>>>>> >>>>> Visit other Kitware open-source projects at
>>>>>> >>>>>
>>>>>> >>>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%
>>>>>> >>>>> 2F%2Fwww.kitware.com%2Fopensource%2Fopensource.html&data=02
>>>>>> >>>>> %7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a62
>>>>>> >>>>> 17%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C63633806985
>>>>>> >>>>> 4146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSOdAv5BC3HClNEGOV
>>>>>> >>>>> fY%3D&reserved=0
>>>>>> >>>>>
>>>>>> >>>>> Search the list archives at:
>>>>>> >>>>>
>>>>>> >>>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%
>>>>>> >>>>> 2F%2Fmarkmail.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%
>>>>>> >>>>> 7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a621
>>>>>> >>>>> 7%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854
>>>>>> >>>>> 146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUHrHWElowi%2B03r3
>>>>>> >>>>> OZk%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-develope
>>>>>> >>>>> rs&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f28
>>>>>> >>>>> 08d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C6
>>>>>> >>>>> 36338069854146481&sdata=Yhue4UpNDB3dlEf0wQYhK7KzsQQES0EEuJW
>>>>>> >>>>> VOoG27Zw%3D&reserved=0
>>>>>> >>>>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> --
>>>>>> >>>> Unpaid intern in BillsBasement at noware dot com
>>>>>> >>
>>>>>> >>
>>>>>> >>
>>>>>> >> --
>>>>>> >> Unpaid intern in BillsBasement at noware dot com
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Unpaid intern in BillsBasement at noware dot com
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> ___________________________________________
>>>>> Andrew J. P. Maclean
>>>>>
>>>>> ___________________________________________
>> _______________________________________________
>> Powered by
>> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&da
>> ta=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7C
>> d61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=M
>> OVhItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%2Fxx0aWHI%3D&reserved=0
>>
>> Visit other Kitware open-source projects at
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.k
>> itware.com%2Fopensource%2Fopensource.html&data=02%7C01%7Classo%40quee
>> nsu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb28
>> 38b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6
>> MnSOdAv5BC3HClNEGOVfY%3D&reserved=0
>>
>> Search the list archives at:
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmarkm
>> ail.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40queen
>> su.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb283
>> 8b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6S
>> OXUHrHWElowi%2B03r3OZk%3D&reserved=0
>>
>> Follow this link to subscribe/unsubscribe:
>>
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpubli
>> c.kitware.com%2Fmailman%2Flistinfo%2Fvtk-developers&data=02%7C01%7Cla
>> sso%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d
>> 582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf0
>> wQYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>
> _______________________________________________
> Powered by
> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&dat
> a=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd6
> 1ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOVh
> ItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%2Fxx0aWHI%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b
> 925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSO
> dAv5BC3HClNEGOVfY%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b9
> 25c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUH
> rHWElowi%2B03r3OZk%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf0wQYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>


More information about the vtk-developers mailing list