Re: vtk-developers Digest, Vol 158, Issue 21

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Andrew Maclean-3
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.

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.

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:

For example, assign colors by name:
renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
());
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 <[hidden email]>
To: Elvis Stansvik <[hidden email]>
Cc: vtkdev <[hidden email]>
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
<[hidden email]> wrote:
> 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>> 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
>> <[hidden email]> wrote:
>>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>> 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
>>>> <[hidden email]> 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 www.kitware.com
>>>>>
>>>>> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>>>>>
>>>>> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>>>>>
>>>>> Follow this link to subscribe/unsubscribe:
>>>>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> 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 www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Elvis Stansvik
2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:

> 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 aNote 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.
>
> 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://lorensen.github.io/VTKExamples/site/Cxx/Visualization/NamedColors/
>
> For example, assign colors by name:
> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
> ());
> 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 <[hidden email]>
>> To: Elvis Stansvik <[hidden email]>
>> Cc: vtkdev <[hidden email]>
>> 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
>> <[hidden email]> wrote:
>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>> >> 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
>> >> <[hidden email]> wrote:
>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen <[hidden email]>:
>> >>>> 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
>> >>>> <[hidden email]> 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 www.kitware.com
>> >>>>>
>> >>>>> Visit other Kitware open-source projects at
>> >>>>> http://www.kitware.com/opensource/opensource.html
>> >>>>>
>> >>>>> Search the list archives at:
>> >>>>> http://markmail.org/search/?q=vtk-developers
>> >>>>>
>> >>>>> Follow this link to subscribe/unsubscribe:
>> >>>>> http://public.kitware.com/mailman/listinfo/vtk-developers
>> >>>>>
>> >>>>
>> >>>>
>> >>>>
>> >>>> --
>> >>>> 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 www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Elvis Stansvik
Sorry, accidently hit send. Fixes below.

2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:

> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>> 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.

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://lorensen.github.io/VTKExamples/site/Cxx/Visualization/NamedColors/
>>
>> For example, assign colors by name:
>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
>> ());
>> 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 <[hidden email]>
>>> To: Elvis Stansvik <[hidden email]>
>>> Cc: vtkdev <[hidden email]>
>>> 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
>>> <[hidden email]> wrote:
>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>> >> 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
>>> >> <[hidden email]> wrote:
>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen <[hidden email]>:
>>> >>>> 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
>>> >>>> <[hidden email]> 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 www.kitware.com
>>> >>>>>
>>> >>>>> Visit other Kitware open-source projects at
>>> >>>>> http://www.kitware.com/opensource/opensource.html
>>> >>>>>
>>> >>>>> Search the list archives at:
>>> >>>>> http://markmail.org/search/?q=vtk-developers
>>> >>>>>
>>> >>>>> Follow this link to subscribe/unsubscribe:
>>> >>>>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>> >>>>>
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>> --
>>> >>>> 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 www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Elvis Stansvik
2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:

> Sorry, accidently hit send. Fixes below.
>
> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>> 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://lorensen.github.io/VTKExamples/site/Cxx/Filtering/ConnectivityFilter/

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://lorensen.github.io/VTKExamples/site/Cxx/Visualization/NamedColors/
>>>
>>> For example, assign colors by name:
>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
>>> ());
>>> 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 <[hidden email]>
>>>> To: Elvis Stansvik <[hidden email]>
>>>> Cc: vtkdev <[hidden email]>
>>>> 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
>>>> <[hidden email]> wrote:
>>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>> >> 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
>>>> >> <[hidden email]> wrote:
>>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>> >>>> 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
>>>> >>>> <[hidden email]> 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 www.kitware.com
>>>> >>>>>
>>>> >>>>> Visit other Kitware open-source projects at
>>>> >>>>> http://www.kitware.com/opensource/opensource.html
>>>> >>>>>
>>>> >>>>> Search the list archives at:
>>>> >>>>> http://markmail.org/search/?q=vtk-developers
>>>> >>>>>
>>>> >>>>> Follow this link to subscribe/unsubscribe:
>>>> >>>>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>>> >>>>>
>>>> >>>>
>>>> >>>>
>>>> >>>>
>>>> >>>> --
>>>> >>>> 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 www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

VTK - Dev mailing list
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
<[hidden email]> wrote:

> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>> Sorry, accidently hit send. Fixes below.
>>
>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>> 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://lorensen.github.io/VTKExamples/site/Cxx/Filtering/ConnectivityFilter/
>
> 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://lorensen.github.io/VTKExamples/site/Cxx/Visualization/NamedColors/
>>>>
>>>> For example, assign colors by name:
>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
>>>> ());
>>>> 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 <[hidden email]>
>>>>> To: Elvis Stansvik <[hidden email]>
>>>>> Cc: vtkdev <[hidden email]>
>>>>> 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
>>>>> <[hidden email]> wrote:
>>>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>> >> 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
>>>>> >> <[hidden email]> wrote:
>>>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>> >>>> 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
>>>>> >>>> <[hidden email]> 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 www.kitware.com
>>>>> >>>>>
>>>>> >>>>> Visit other Kitware open-source projects at
>>>>> >>>>> http://www.kitware.com/opensource/opensource.html
>>>>> >>>>>
>>>>> >>>>> Search the list archives at:
>>>>> >>>>> http://markmail.org/search/?q=vtk-developers
>>>>> >>>>>
>>>>> >>>>> Follow this link to subscribe/unsubscribe:
>>>>> >>>>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>>>> >>>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> --
>>>>> >>>> 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 www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/mailman/listinfo/vtk-developers
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Andras Lasso

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: [hidden email]
Sent: Friday, June 23, 2017 5:29 AM
To: [hidden email]
Cc: [hidden email]; [hidden email]
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
<[hidden email]> wrote:
> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>> Sorry, accidently hit send. Fixes below.
>>
>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>> 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%2Florensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FFiltering%2FConnectivityFilter%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=ha%2BCR8CriMPmQqz%2FxrAgI6lQ7RKn21tuZ6Z%2FitzKD%2Fg%3D&reserved=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%2Florensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FVisualization%2FNamedColors%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=jnbJJe87OvOErNapbQL2HiAt6DnbOWifp67W4lNVqfo%3D&reserved=0
>>>>
>>>> For example, assign colors by name:
>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
>>>> ());
>>>> 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 <[hidden email]>
>>>>> To: Elvis Stansvik <[hidden email]>
>>>>> Cc: vtkdev <[hidden email]>
>>>>> 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
>>>>> <[hidden email]> wrote:
>>>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>> >> 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
>>>>> >> <[hidden email]> wrote:
>>>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>> >>>> 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
>>>>> >>>> <[hidden email]> 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.kitware.com&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOVhItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSOdAv5BC3HClNEGOVfY%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUHrHWElowi%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
>>>>> >>>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> --
>>>>> >>>> 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&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOVhItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSOdAv5BC3HClNEGOVfY%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUHrHWElowi%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
>
_______________________________________________
Powered by https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOVhItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSOdAv5BC3HClNEGOVfY%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUHrHWElowi%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


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

VTK - Dev mailing list
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 <[hidden email]> 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
> <[hidden email]> wrote:
>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>> Sorry, accidently hit send. Fixes below.
>>>
>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>>> 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%2Florensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FFiltering%2FConnectivityFilter%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=ha%2BCR8CriMPmQqz%2FxrAgI6lQ7RKn21tuZ6Z%2FitzKD%2Fg%3D&reserved=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%2Florensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FVisualization%2FNamedColors%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=jnbJJe87OvOErNapbQL2HiAt6DnbOWifp67W4lNVqfo%3D&reserved=0
>>>>>
>>>>> For example, assign colors by name:
>>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
>>>>> ());
>>>>> 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 <[hidden email]>
>>>>>> To: Elvis Stansvik <[hidden email]>
>>>>>> Cc: vtkdev <[hidden email]>
>>>>>> 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
>>>>>> <[hidden email]> wrote:
>>>>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>> >> 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
>>>>>> >> <[hidden email]> wrote:
>>>>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen
>>>>>> >>> <[hidden email]>:
>>>>>> >>>> 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
>>>>>> >>>> <[hidden email]> 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.kitware.com&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOVhItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSOdAv5BC3HClNEGOVfY%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUHrHWElowi%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
>>>>>> >>>>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> --
>>>>>> >>>> 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&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOVhItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSOdAv5BC3HClNEGOVfY%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUHrHWElowi%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
>>
> _______________________________________________
> Powered by
> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOVhItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSOdAv5BC3HClNEGOVfY%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%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUHrHWElowi%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
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Elvis Stansvik
In reply to this post by VTK - Dev mailing list
2017-06-23 11:29 GMT+02:00 David Cole <[hidden email]>:

> 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.

Alright, that makes sense.

Elvis

>
>
> David C.
>
>
>
> On Fri, Jun 23, 2017 at 4:47 AM, Elvis Stansvik
> <[hidden email]> wrote:
>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>> Sorry, accidently hit send. Fixes below.
>>>
>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>>> 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://lorensen.github.io/VTKExamples/site/Cxx/Filtering/ConnectivityFilter/
>>
>> 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://lorensen.github.io/VTKExamples/site/Cxx/Visualization/NamedColors/
>>>>>
>>>>> For example, assign colors by name:
>>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
>>>>> ());
>>>>> 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 <[hidden email]>
>>>>>> To: Elvis Stansvik <[hidden email]>
>>>>>> Cc: vtkdev <[hidden email]>
>>>>>> 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
>>>>>> <[hidden email]> wrote:
>>>>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>> >> 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
>>>>>> >> <[hidden email]> wrote:
>>>>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>> >>>> 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
>>>>>> >>>> <[hidden email]> 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 www.kitware.com
>>>>>> >>>>>
>>>>>> >>>>> Visit other Kitware open-source projects at
>>>>>> >>>>> http://www.kitware.com/opensource/opensource.html
>>>>>> >>>>>
>>>>>> >>>>> Search the list archives at:
>>>>>> >>>>> http://markmail.org/search/?q=vtk-developers
>>>>>> >>>>>
>>>>>> >>>>> Follow this link to subscribe/unsubscribe:
>>>>>> >>>>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>>>>> >>>>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>> --
>>>>>> >>>> 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 www.kitware.com
>>
>> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>>
>> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>>
>> Follow this link to subscribe/unsubscribe:
>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Andras Lasso
In reply to this post by VTK - Dev mailing list
> 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:[hidden email]]
Sent: Friday, June 23, 2017 7:24 AM
To: Andras Lasso <[hidden email]>
Cc: David Cole <[hidden email]>; Elvis Stansvik <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
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 <[hidden email]> 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
> <[hidden email]> wrote:
>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>> Sorry, accidently hit send. Fixes below.
>>>
>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>>> 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 <[hidden email]>
>>>>>> To: Elvis Stansvik <[hidden email]>
>>>>>> Cc: vtkdev <[hidden email]>
>>>>>> 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
>>>>>> <[hidden email]> wrote:
>>>>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>> >> 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
>>>>>> >> <[hidden email]> wrote:
>>>>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen
>>>>>> >>> <[hidden email]>:
>>>>>> >>>> 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
>>>>>> >>>> <[hidden email]> 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
>>>>>> >>>>>
>>>>>> >>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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:
>>>>>> >>>>>
>>>>>> >>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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:
>>>>>> >>>>>
>>>>>> >>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

VTK - Dev mailing list
Yes, and this was done intentionally to avoid the kinds of bugs people create when they don't think it all the way through. Forcing you to use Get or GetPointer forces you to think about it when that is what you're doing.

With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.

There's nothing illegal about it as long as you know there's a reference kept somewhere... but it makes it easier to get a pointer to a dead object and the designers of vtkNew explicitly wanted to avoid that problem.


David

On Jun 23, 2017, at 8:23 AM, Andras Lasso <[hidden email]> wrote:

>> 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:[hidden email]]
> Sent: Friday, June 23, 2017 7:24 AM
> To: Andras Lasso <[hidden email]>
> Cc: David Cole <[hidden email]>; Elvis Stansvik <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
> 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 <[hidden email]> 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
>> <[hidden email]> wrote:
>>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>> Sorry, accidently hit send. Fixes below.
>>>>
>>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>>>> 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 <[hidden email]>
>>>>>>> To: Elvis Stansvik <[hidden email]>
>>>>>>> Cc: vtkdev <[hidden email]>
>>>>>>> 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
>>>>>>> <[hidden email]> wrote:
>>>>>>>> 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>>>>> 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
>>>>>>>>> <[hidden email]> wrote:
>>>>>>>>>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen
>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>> 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
>>>>>>>>>>> <[hidden email]> 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
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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:
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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:
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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
>>

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Andras Lasso
> With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.

Well, this may be the main source of misunderstanding. Why do you think vtkSmartPointer would work any better in this case?

Have a look at the code below. Do you think it is safe to create an image and return it like that?

vtkImageData* CreateImage()
{
  vtkSmartPointer<vtkImageData> newImage = vtkSmartPointer<vtkImageData>::New();
  return newImage;
}

vtkImageData* myImage = CreateImage(); // is this safe?
vtkSmartPointer<vtkImageData>* myImage = CreateImage(); // is this safe?

Andras

-----Original Message-----
From: David Cole [mailto:[hidden email]]
Sent: Friday, June 23, 2017 8:49 AM
To: Andras Lasso <[hidden email]>
Cc: David Cole <[hidden email]>; Elvis Stansvik <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21

Yes, and this was done intentionally to avoid the kinds of bugs people create when they don't think it all the way through. Forcing you to use Get or GetPointer forces you to think about it when that is what you're doing.

With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.

There's nothing illegal about it as long as you know there's a reference kept somewhere... but it makes it easier to get a pointer to a dead object and the designers of vtkNew explicitly wanted to avoid that problem.


David

On Jun 23, 2017, at 8:23 AM, Andras Lasso <[hidden email]> wrote:

>> 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:[hidden email]]
> Sent: Friday, June 23, 2017 7:24 AM
> To: Andras Lasso <[hidden email]>
> Cc: David Cole <[hidden email]>; Elvis Stansvik
> <[hidden email]>; VTK Developers
> <[hidden email]>; Andrew Maclean <[hidden email]>
> 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 <[hidden email]> 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
>> <[hidden email]> wrote:
>>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>> Sorry, accidently hit send. Fixes below.
>>>>
>>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>>>> 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%2Flor
>>> e
>>> nsen.github.io%2FVTKExamples%2Fsite%2FCxx%2FFiltering%2FConnectivity
>>> F
>>> ilter%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d
>>> 4
>>> ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C63633806985414
>>> 6
>>> 481&sdata=ha%2BCR8CriMPmQqz%2FxrAgI6lQ7RKn21tuZ6Z%2FitzKD%2Fg%3D&res
>>> e
>>> 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%2F
>>>>>> l
>>>>>> orensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FVisualization%2FNa
>>>>>> m
>>>>>> edColors%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0
>>>>>> f
>>>>>> 2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C63633
>>>>>> 8
>>>>>> 069854146481&sdata=jnbJJe87OvOErNapbQL2HiAt6DnbOWifp67W4lNVqfo%3D
>>>>>> &
>>>>>> reserved=0
>>>>>>
>>>>>> For example, assign colors by name:
>>>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetD
>>>>>> renderer->a
>>>>>> 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 <[hidden email]>
>>>>>>> To: Elvis Stansvik <[hidden email]>
>>>>>>> Cc: vtkdev <[hidden email]>
>>>>>>> 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
>>>>>>> <[hidden email]> wrote:
>>>>>>>> 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>>>>> 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
>>>>>>>>> <[hidden email]> wrote:
>>>>>>>>>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen
>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>> 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
>>>>>>>>>>> <[hidden email]> 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
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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:
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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:
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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&d
>>> a
>>> ta=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7
>>> C
>>> 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%40que
>>> e
>>> nsu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2
>>> 8
>>> 38b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B
>>> 6
>>> MnSOdAv5BC3HClNEGOVfY%3D&reserved=0
>>>
>>> Search the list archives at:
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmark
>>> m
>>> ail.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40quee
>>> n
>>> su.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb28
>>> 3
>>> 8b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6
>>> S
>>> OXUHrHWElowi%2B03r3OZk%3D&reserved=0
>>>
>>> Follow this link to subscribe/unsubscribe:
>>>
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpubl
>>> i
>>> c.kitware.com%2Fmailman%2Flistinfo%2Fvtk-developers&data=02%7C01%7Cl
>>> a
>>> sso%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142
>>> d
>>> 582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf
>>> 0
>>> wQYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>>
>> _______________________________________________
>> Powered by
>> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&da
>> t
>> a=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd
>> 6
>> 1ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOV
>> h
>> ItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%2Fxx0aWHI%3D&reserved=0
>>
>> Visit other Kitware open-source projects at
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.k
>> i
>> tware.com%2Fopensource%2Fopensource.html&data=02%7C01%7Classo%40queen
>> s
>> u.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838
>> b
>> 925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnS
>> O
>> dAv5BC3HClNEGOVfY%3D&reserved=0
>>
>> Search the list archives at:
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmarkm
>> a
>> il.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40queens
>> u
>> .ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b
>> 9
>> 25c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXU
>> H
>> rHWElowi%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%7Clas
>> so%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d5
>> 82c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf0w
>> QYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

VTK - Dev mailing list
> vtkImageData* myImage = CreateImage(); // is this safe?

No, it's not.

> vtkSmartPointer<vtkImageData> myImage = CreateImage(); // is this safe?

Maybe, but I don't think it's guaranteed.

And the designers of vtkNew recognized that as a flaw in
vtkSmartPointer and insisted on creating the GetPointer method as a
safeguard against accidentally doing those things.

I'm not saying vtkSmartPointer is perfect: I'm saying vtkNew is an
IMPROVEMENT over it BECAUSE it doesn't have a cast to raw pointer
operator.

I feel like I've said the same thing 4 different ways now. I'll pause
and let others chime in if there is more discussion to be had...


D

On Fri, Jun 23, 2017 at 9:11 AM, Andras Lasso <[hidden email]> wrote:

>> With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.
>
> Well, this may be the main source of misunderstanding. Why do you think vtkSmartPointer would work any better in this case?
>
> Have a look at the code below. Do you think it is safe to create an image and return it like that?
>
> vtkImageData* CreateImage()
> {
>   vtkSmartPointer<vtkImageData> newImage = vtkSmartPointer<vtkImageData>::New();
>   return newImage;
> }
>
> vtkImageData* myImage = CreateImage(); // is this safe?
> vtkSmartPointer<vtkImageData>* myImage = CreateImage(); // is this safe?
>
> Andras
>
> -----Original Message-----
> From: David Cole [mailto:[hidden email]]
> Sent: Friday, June 23, 2017 8:49 AM
> To: Andras Lasso <[hidden email]>
> Cc: David Cole <[hidden email]>; Elvis Stansvik <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
> Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21
>
> Yes, and this was done intentionally to avoid the kinds of bugs people create when they don't think it all the way through. Forcing you to use Get or GetPointer forces you to think about it when that is what you're doing.
>
> With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.
>
> There's nothing illegal about it as long as you know there's a reference kept somewhere... but it makes it easier to get a pointer to a dead object and the designers of vtkNew explicitly wanted to avoid that problem.
>
>
> David
>
> On Jun 23, 2017, at 8:23 AM, Andras Lasso <[hidden email]> wrote:
>
>>> 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:[hidden email]]
>> Sent: Friday, June 23, 2017 7:24 AM
>> To: Andras Lasso <[hidden email]>
>> Cc: David Cole <[hidden email]>; Elvis Stansvik
>> <[hidden email]>; VTK Developers
>> <[hidden email]>; Andrew Maclean <[hidden email]>
>> 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 <[hidden email]> 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
>>> <[hidden email]> wrote:
>>>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>>> Sorry, accidently hit send. Fixes below.
>>>>>
>>>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>>>>> 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%2Flor
>>>> e
>>>> nsen.github.io%2FVTKExamples%2Fsite%2FCxx%2FFiltering%2FConnectivity
>>>> F
>>>> ilter%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d
>>>> 4
>>>> ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C63633806985414
>>>> 6
>>>> 481&sdata=ha%2BCR8CriMPmQqz%2FxrAgI6lQ7RKn21tuZ6Z%2FitzKD%2Fg%3D&res
>>>> e
>>>> 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%2F
>>>>>>> l
>>>>>>> orensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FVisualization%2FNa
>>>>>>> m
>>>>>>> edColors%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0
>>>>>>> f
>>>>>>> 2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C63633
>>>>>>> 8
>>>>>>> 069854146481&sdata=jnbJJe87OvOErNapbQL2HiAt6DnbOWifp67W4lNVqfo%3D
>>>>>>> &
>>>>>>> reserved=0
>>>>>>>
>>>>>>> For example, assign colors by name:
>>>>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetD
>>>>>>> renderer->a
>>>>>>> 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 <[hidden email]>
>>>>>>>> To: Elvis Stansvik <[hidden email]>
>>>>>>>> Cc: vtkdev <[hidden email]>
>>>>>>>> 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
>>>>>>>> <[hidden email]> wrote:
>>>>>>>>> 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>>>>>> 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
>>>>>>>>>> <[hidden email]> wrote:
>>>>>>>>>>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen
>>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>>> 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
>>>>>>>>>>>> <[hidden email]> 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
>>>>>>>>>>>>>
>>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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:
>>>>>>>>>>>>>
>>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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:
>>>>>>>>>>>>>
>>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%">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&d
>>>> a
>>>> ta=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7
>>>> C
>>>> 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%40que
>>>> e
>>>> nsu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2
>>>> 8
>>>> 38b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B
>>>> 6
>>>> MnSOdAv5BC3HClNEGOVfY%3D&reserved=0
>>>>
>>>> Search the list archives at:
>>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmark
>>>> m
>>>> ail.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40quee
>>>> n
>>>> su.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb28
>>>> 3
>>>> 8b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6
>>>> S
>>>> OXUHrHWElowi%2B03r3OZk%3D&reserved=0
>>>>
>>>> Follow this link to subscribe/unsubscribe:
>>>>
>>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpubl
>>>> i
>>>> c.kitware.com%2Fmailman%2Flistinfo%2Fvtk-developers&data=02%7C01%7Cl
>>>> a
>>>> sso%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142
>>>> d
>>>> 582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf
>>>> 0
>>>> wQYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>>>
>>> _______________________________________________
>>> Powered by
>>> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&da
>>> t
>>> a=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd
>>> 6
>>> 1ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOV
>>> h
>>> ItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%2Fxx0aWHI%3D&reserved=0
>>>
>>> Visit other Kitware open-source projects at
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.k
>>> i
>>> tware.com%2Fopensource%2Fopensource.html&data=02%7C01%7Classo%40queen
>>> s
>>> u.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838
>>> b
>>> 925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnS
>>> O
>>> dAv5BC3HClNEGOVfY%3D&reserved=0
>>>
>>> Search the list archives at:
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmarkm
>>> a
>>> il.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40queens
>>> u
>>> .ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b
>>> 9
>>> 25c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXU
>>> H
>>> rHWElowi%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%7Clas
>>> so%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d5
>>> 82c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf0w
>>> QYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>>
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Shawn Waldon-2
In reply to this post by Andras Lasso


On Fri, Jun 23, 2017 at 9:11 AM, Andras Lasso <[hidden email]> wrote:
> With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.

Well, this may be the main source of misunderstanding. Why do you think vtkSmartPointer would work any better in this case?

Have a look at the code below. Do you think it is safe to create an image and return it like that?

vtkImageData* CreateImage()
{
  vtkSmartPointer<vtkImageData> newImage = vtkSmartPointer<vtkImageData>::New();
  return newImage;
}

vtkImageData* myImage = CreateImage(); // is this safe?
vtkSmartPointer<vtkImageData>* myImage = CreateImage(); // is this safe?

Neither one of these is safe.  The image will be deleted when the smart pointer goes out of scope but before the returned value is put into the smart pointer in the calling function.  I understand what Elvis is saying about the design of vtkNew wanting to make people think about when to call Get/GetPointer.  I see their point.  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.

Shawn

Andras

-----Original Message-----
From: David Cole [mailto:[hidden email]]
Sent: Friday, June 23, 2017 8:49 AM
To: Andras Lasso <[hidden email]>
Cc: David Cole <[hidden email]>; Elvis Stansvik <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21

Yes, and this was done intentionally to avoid the kinds of bugs people create when they don't think it all the way through. Forcing you to use Get or GetPointer forces you to think about it when that is what you're doing.

With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.

There's nothing illegal about it as long as you know there's a reference kept somewhere... but it makes it easier to get a pointer to a dead object and the designers of vtkNew explicitly wanted to avoid that problem.


David

On Jun 23, 2017, at 8:23 AM, Andras Lasso <[hidden email]> wrote:

>> 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:[hidden email]]
> Sent: Friday, June 23, 2017 7:24 AM
> To: Andras Lasso <[hidden email]>
> Cc: David Cole <[hidden email]>; Elvis Stansvik
> <[hidden email]>; VTK Developers
> <[hidden email]>; Andrew Maclean <[hidden email]>
> 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 <[hidden email]> 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
>> <[hidden email]> wrote:
>>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>> Sorry, accidently hit send. Fixes below.
>>>>
>>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>>>> 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%2Flor
>>> e
>>> nsen.github.io%2FVTKExamples%2Fsite%2FCxx%2FFiltering%2FConnectivity
>>> F
>>> ilter%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d
>>> 4
>>> ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C63633806985414
>>> 6
>>> 481&sdata=ha%2BCR8CriMPmQqz%2FxrAgI6lQ7RKn21tuZ6Z%2FitzKD%2Fg%3D&res
>>> e
>>> 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%2F
>>>>>> l
>>>>>> orensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FVisualization%2FNa
>>>>>> m
>>>>>> edColors%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0
>>>>>> f
>>>>>> 2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C63633
>>>>>> 8
>>>>>> 069854146481&sdata=jnbJJe87OvOErNapbQL2HiAt6DnbOWifp67W4lNVqfo%3D
>>>>>> &
>>>>>> reserved=0
>>>>>>
>>>>>> For example, assign colors by name:
>>>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetD
>>>>>> renderer->a
>>>>>> 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 <[hidden email]>
>>>>>>> To: Elvis Stansvik <[hidden email]>
>>>>>>> Cc: vtkdev <[hidden email]>
>>>>>>> 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
>>>>>>> <[hidden email]> wrote:
>>>>>>>> 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>>>>> 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
>>>>>>>>> <[hidden email]> wrote:
>>>>>>>>>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen
>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>> 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
>>>>>>>>>>> <[hidden email]> 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
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%" rel="noreferrer" target="_blank">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:
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%" rel="noreferrer" target="_blank">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:
>>>>>>>>>>>>
>>>>>>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=http%3A%" rel="noreferrer" target="_blank">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&d
>>> a
>>> ta=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7
>>> C
>>> 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%40que
>>> e
>>> nsu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2
>>> 8
>>> 38b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B
>>> 6
>>> MnSOdAv5BC3HClNEGOVfY%3D&reserved=0
>>>
>>> Search the list archives at:
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmark
>>> m
>>> ail.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40quee
>>> n
>>> su.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb28
>>> 3
>>> 8b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6
>>> S
>>> OXUHrHWElowi%2B03r3OZk%3D&reserved=0
>>>
>>> Follow this link to subscribe/unsubscribe:
>>>
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpubl
>>> i
>>> c.kitware.com%2Fmailman%2Flistinfo%2Fvtk-developers&data=02%7C01%7Cl
>>> a
>>> sso%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142
>>> d
>>> 582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf
>>> 0
>>> wQYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>>
>> _______________________________________________
>> Powered by
>> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&da
>> t
>> a=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd
>> 6
>> 1ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MOV
>> h
>> ItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%2Fxx0aWHI%3D&reserved=0
>>
>> Visit other Kitware open-source projects at
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.k
>> i
>> tware.com%2Fopensource%2Fopensource.html&data=02%7C01%7Classo%40queen
>> s
>> u.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838
>> b
>> 925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnS
>> O
>> dAv5BC3HClNEGOVfY%3D&reserved=0
>>
>> Search the list archives at:
>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmarkm
>> a
>> il.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40queens
>> u
>> .ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b
>> 9
>> 25c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXU
>> H
>> rHWElowi%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%7Clas
>> so%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d5
>> 82c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf0w
>> QYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers



_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Andras Lasso
In reply to this post by VTK - Dev mailing list
OK, great, so we agree that there is no difference in safety in vtkSmartPointer with implicit pointer cast and vtkNew with implicit pointer cast. It's all about personal preference now: is implicit cast a feature or a flaw.

To not burden the mailing list with votes, I've created this poll - please mark your preference - either for keeping or changing current behavior:
https://goo.gl/forms/MEkBKvyBXfyPQ1Aj1

Andras

-----Original Message-----
From: David Cole [mailto:[hidden email]]
Sent: Friday, June 23, 2017 10:38 AM
To: Andras Lasso <[hidden email]>
Cc: David Cole <[hidden email]>; Elvis Stansvik <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21

> vtkImageData* myImage = CreateImage(); // is this safe?

No, it's not.

> vtkSmartPointer<vtkImageData> myImage = CreateImage(); // is this safe?

Maybe, but I don't think it's guaranteed.

And the designers of vtkNew recognized that as a flaw in vtkSmartPointer and insisted on creating the GetPointer method as a safeguard against accidentally doing those things.

I'm not saying vtkSmartPointer is perfect: I'm saying vtkNew is an IMPROVEMENT over it BECAUSE it doesn't have a cast to raw pointer operator.

I feel like I've said the same thing 4 different ways now. I'll pause and let others chime in if there is more discussion to be had...


D

On Fri, Jun 23, 2017 at 9:11 AM, Andras Lasso <[hidden email]> wrote:

>> With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.
>
> Well, this may be the main source of misunderstanding. Why do you think vtkSmartPointer would work any better in this case?
>
> Have a look at the code below. Do you think it is safe to create an image and return it like that?
>
> vtkImageData* CreateImage()
> {
>   vtkSmartPointer<vtkImageData> newImage = vtkSmartPointer<vtkImageData>::New();
>   return newImage;
> }
>
> vtkImageData* myImage = CreateImage(); // is this safe?
> vtkSmartPointer<vtkImageData>* myImage = CreateImage(); // is this safe?
>
> Andras
>
> -----Original Message-----
> From: David Cole [mailto:[hidden email]]
> Sent: Friday, June 23, 2017 8:49 AM
> To: Andras Lasso <[hidden email]>
> Cc: David Cole <[hidden email]>; Elvis Stansvik
> <[hidden email]>; VTK Developers
> <[hidden email]>; Andrew Maclean <[hidden email]>
> Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21
>
> Yes, and this was done intentionally to avoid the kinds of bugs people create when they don't think it all the way through. Forcing you to use Get or GetPointer forces you to think about it when that is what you're doing.
>
> With vtkSmartPointer, it's possible to "return smartPointer;" from a method EVEN IF the return type of the method is a RAW pointer.
>
> There's nothing illegal about it as long as you know there's a reference kept somewhere... but it makes it easier to get a pointer to a dead object and the designers of vtkNew explicitly wanted to avoid that problem.
>
>
> David
>
> On Jun 23, 2017, at 8:23 AM, Andras Lasso <[hidden email]> wrote:
>
>>> 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:[hidden email]]
>> Sent: Friday, June 23, 2017 7:24 AM
>> To: Andras Lasso <[hidden email]>
>> Cc: David Cole <[hidden email]>; Elvis Stansvik
>> <[hidden email]>; VTK Developers
>> <[hidden email]>; Andrew Maclean <[hidden email]>
>> 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 <[hidden email]> 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
>>> <[hidden email]> wrote:
>>>> 2017-06-23 10:33 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>>> Sorry, accidently hit send. Fixes below.
>>>>>
>>>>> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <[hidden email]>:
>>>>>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <[hidden email]>:
>>>>>>> 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%2Flo
>>>> r
>>>> e
>>>> nsen.github.io%2FVTKExamples%2Fsite%2FCxx%2FFiltering%2FConnectivit
>>>> y
>>>> F
>>>> ilter%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808
>>>> d
>>>> 4
>>>> ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C6363380698541
>>>> 4
>>>> 6
>>>> 481&sdata=ha%2BCR8CriMPmQqz%2FxrAgI6lQ7RKn21tuZ6Z%2FitzKD%2Fg%3D&re
>>>> s
>>>> e
>>>> 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:
>>>>>>>
>>>>>>> <a href="https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2">https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2
>>>>>>> F
>>>>>>> l
>>>>>>> orensen.github.io%2FVTKExamples%2Fsite%2FCxx%2FVisualization%2FN
>>>>>>> a
>>>>>>> m
>>>>>>> edColors%2F&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f
>>>>>>> 0
>>>>>>> f
>>>>>>> 2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C6363
>>>>>>> 3
>>>>>>> 8
>>>>>>> 069854146481&sdata=jnbJJe87OvOErNapbQL2HiAt6DnbOWifp67W4lNVqfo%3
>>>>>>> D
>>>>>>> &
>>>>>>> reserved=0
>>>>>>>
>>>>>>> For example, assign colors by name:
>>>>>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").Get
>>>>>>> renderer->D
>>>>>>> renderer->a
>>>>>>> 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 <[hidden email]>
>>>>>>>> To: Elvis Stansvik <[hidden email]>
>>>>>>>> Cc: vtkdev <[hidden email]>
>>>>>>>> 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
>>>>>>>> <[hidden email]> wrote:
>>>>>>>>> 2017-06-22 19:09 GMT+02:00 Bill Lorensen <[hidden email]>:
>>>>>>>>>> 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
>>>>>>>>>> <[hidden email]> wrote:
>>>>>>>>>>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen
>>>>>>>>>>> <[hidden email]>:
>>>>>>>>>>>> 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
>>>>>>>>>>>> <[hidden email]> 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.kit
>>>>>>>>>>>>> w
>>>>>>>>>>>>> are.com&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d
>>>>>>>>>>>>> 7
>>>>>>>>>>>>> f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7
>>>>>>>>>>>>> C
>>>>>>>>>>>>> 0%7C636338069854146481&sdata=MOVhItfPCXhoqRfpkkpfmBGyNrthS
>>>>>>>>>>>>> Q
>>>>>>>>>>>>> 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=0
>>>>>>>>>>>>> 2
>>>>>>>>>>>>> %7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6
>>>>>>>>>>>>> 2
>>>>>>>>>>>>> 17%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C6363380698
>>>>>>>>>>>>> 5
>>>>>>>>>>>>> 4146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6MnSOdAv5BC3HClNEGO
>>>>>>>>>>>>> V
>>>>>>>>>>>>> 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%7C0ced79d0c7c54d7f0f2808d4ba1a62
>>>>>>>>>>>>> 1
>>>>>>>>>>>>> 7%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C63633806985
>>>>>>>>>>>>> 4
>>>>>>>>>>>>> 146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOXUHrHWElowi%2B03r
>>>>>>>>>>>>> 3
>>>>>>>>>>>>> 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-develop
>>>>>>>>>>>>> e
>>>>>>>>>>>>> rs&data=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2
>>>>>>>>>>>>> 8
>>>>>>>>>>>>> 08d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C
>>>>>>>>>>>>> 6
>>>>>>>>>>>>> 36338069854146481&sdata=Yhue4UpNDB3dlEf0wQYhK7KzsQQES0EEuJ
>>>>>>>>>>>>> W
>>>>>>>>>>>>> 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&
>>>> d
>>>> a
>>>> ta=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%
>>>> 7
>>>> C
>>>> 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%40qu
>>>> e
>>>> e
>>>> nsu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb
>>>> 2
>>>> 8
>>>> 38b925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2
>>>> B
>>>> 6
>>>> MnSOdAv5BC3HClNEGOVfY%3D&reserved=0
>>>>
>>>> Search the list archives at:
>>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmar
>>>> k
>>>> m
>>>> ail.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40que
>>>> e
>>>> n
>>>> su.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2
>>>> 8
>>>> 3
>>>> 8b925c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg
>>>> 6
>>>> S
>>>> OXUHrHWElowi%2B03r3OZk%3D&reserved=0
>>>>
>>>> Follow this link to subscribe/unsubscribe:
>>>>
>>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpub
>>>> l
>>>> i
>>>> c.kitware.com%2Fmailman%2Flistinfo%2Fvtk-developers&data=02%7C01%7C
>>>> l
>>>> a
>>>> sso%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b14
>>>> 2
>>>> d
>>>> 582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlE
>>>> f
>>>> 0
>>>> wQYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>>>
>>> _______________________________________________
>>> Powered by
>>> https://na01.safelinks.protection.outlook.com/?url=www.kitware.com&d
>>> a
>>> t
>>> a=02%7C01%7Classo%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7C
>>> d
>>> 6
>>> 1ecb3b38b142d582c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=MO
>>> V
>>> h
>>> ItfPCXhoqRfpkkpfmBGyNrthSQhA9SD%2Fxx0aWHI%3D&reserved=0
>>>
>>> Visit other Kitware open-source projects at
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
>>> k
>>> i
>>> tware.com%2Fopensource%2Fopensource.html&data=02%7C01%7Classo%40quee
>>> n
>>> s
>>> u.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb283
>>> 8
>>> b
>>> 925c%7C1%7C0%7C636338069854146481&sdata=019a%2BwqJEgmPJZDHThfa%2B6Mn
>>> S
>>> O
>>> dAv5BC3HClNEGOVfY%3D&reserved=0
>>>
>>> Search the list archives at:
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmark
>>> m
>>> a
>>> il.org%2Fsearch%2F%3Fq%3Dvtk-developers&data=02%7C01%7Classo%40queen
>>> s
>>> u
>>> .ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d582c4efb2838
>>> b
>>> 9
>>> 25c%7C1%7C0%7C636338069854146481&sdata=4QoQc%2FSmBX0Wscw5TR%2BYg6SOX
>>> U
>>> H
>>> rHWElowi%2B03r3OZk%3D&reserved=0
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpubl
>>> i
>>> c
>>> .kitware.com%2Fmailman%2Flistinfo%2Fvtk-developers&data=02%7C01%7Cla
>>> s
>>> so%40queensu.ca%7C0ced79d0c7c54d7f0f2808d4ba1a6217%7Cd61ecb3b38b142d
>>> 5
>>> 82c4efb2838b925c%7C1%7C0%7C636338069854146481&sdata=Yhue4UpNDB3dlEf0
>>> w
>>> QYhK7KzsQQES0EEuJWVOoG27Zw%3D&reserved=0
>>>
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

David Lonie-2
In reply to this post by Shawn Waldon-2
I'm so glad someone brought this up =)

On Fri, Jun 23, 2017 at 10:52 AM, Shawn Waldon <[hidden email]> 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 www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Andras Lasso
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.

Here is a pull request with the change:
https://gitlab.kitware.com/vtk/vtk/merge_requests/2961

Andras

-----Original Message-----
From: David Lonie [mailto:[hidden email]]
Sent: Friday, June 23, 2017 1:51 PM
To: Shawn Waldon <[hidden email]>
Cc: Andras Lasso <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
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 <[hidden email]> 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 www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Elvis Stansvik
2017-06-24 15:53 GMT+02:00 Andras Lasso <[hidden email]>:
> 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://gitlab.kitware.com/vtk/vtk/merge_requests/2961
>
> Andras
>
> -----Original Message-----
> From: David Lonie [mailto:[hidden email]]
> Sent: Friday, June 23, 2017 1:51 PM
> To: Shawn Waldon <[hidden email]>
> Cc: Andras Lasso <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
> 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 <[hidden email]> 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 www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Search the list archives at: http://markmail.org/search/?q=vtk-developers
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/mailman/listinfo/vtk-developers
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Andras Lasso
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.

Andras

-----Original Message-----
From: Elvis Stansvik [mailto:[hidden email]]
Sent: Saturday, June 24, 2017 12:36 PM
To: Andras Lasso <[hidden email]>
Cc: David Lonie <[hidden email]>; Shawn Waldon <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21

2017-06-24 15:53 GMT+02:00 Andras Lasso <[hidden email]>:
> 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:[hidden email]]
> Sent: Friday, June 23, 2017 1:51 PM
> To: Shawn Waldon <[hidden email]>
> Cc: Andras Lasso <[hidden email]>; VTK Developers
> <[hidden email]>; Andrew Maclean <[hidden email]>
> 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 <[hidden email]> 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
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Andrew Maclean-3
Hi Andras, 

One thing that may be relevant is this link: http://marc.info/?l=vtk-developers&m=126471789304821
Note: I think
vtkSmartPointer<vtkPolyData> output;
should be:
vtkSmartPointer<vtkPolyData> readerOutput;

I think the example there nicely demonstrates the difference between vtkNew and vtkSmartPointer.

Maybe something like this could be incorporated into the testing in https://gitlab.kitware.com/vtk/vtk/merge_requests/2961

Regards
   Andrew

On Sun, Jun 25, 2017 at 6:07 AM, Andras Lasso <[hidden email]> wrote:
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.

Andras

-----Original Message-----
From: Elvis Stansvik [mailto:[hidden email]]
Sent: Saturday, June 24, 2017 12:36 PM
To: Andras Lasso <[hidden email]>
Cc: David Lonie <[hidden email]>; Shawn Waldon <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21

2017-06-24 15:53 GMT+02:00 Andras Lasso <[hidden email]>:
> 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:[hidden email]]
> Sent: Friday, June 23, 2017 1:51 PM
> To: Shawn Waldon <[hidden email]>
> Cc: Andras Lasso <[hidden email]>; VTK Developers
> <[hidden email]>; Andrew Maclean <[hidden email]>
> 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 <[hidden email]> 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
>



--
___________________________________________
Andrew J. P. Maclean

___________________________________________

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: vtk-developers Digest, Vol 158, Issue 21

Elvis Stansvik
In reply to this post by Andras Lasso
2017-06-24 22:07 GMT+02:00 Andras Lasso <[hidden email]>:

> 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:[hidden email]]
> Sent: Saturday, June 24, 2017 12:36 PM
> To: Andras Lasso <[hidden email]>
> Cc: David Lonie <[hidden email]>; Shawn Waldon <[hidden email]>; VTK Developers <[hidden email]>; Andrew Maclean <[hidden email]>
> Subject: Re: [vtk-developers] vtk-developers Digest, Vol 158, Issue 21
>
> 2017-06-24 15:53 GMT+02:00 Andras Lasso <[hidden email]>:
>> 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:[hidden email]]
>> Sent: Friday, June 23, 2017 1:51 PM
>> To: Shawn Waldon <[hidden email]>
>> Cc: Andras Lasso <[hidden email]>; VTK Developers
>> <[hidden email]>; Andrew Maclean <[hidden email]>
>> 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 <[hidden email]> 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
>>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers

12
Loading...