[vtk-developers] DoublePi proposal

[David Gobbi]

Well, I looked through VTK to see how vtkMath::Pi() was being used,
and now I don't think that there is any reason for a single-precision
pi at all.  In VTK 6, vtkMath::Pi() should return a double-precision
value, period.

I found 62 classes that call vtkMath::Pi().  Only one of them,
vtkExodusReader, seemed to have a reason for using a single-precision
value.  The others should have switched to vtkMath::DoublePi() a long
time ago.  My only guess as to why they weren't is that they were
missed during the big float-to-double conversion that occurred when
VTK 4.4 was released.

So my new proposal is to do nothing for VTK 5.10 (because
vtkMath::FloatPi() isn't really needed for anything), but to still
change vtkMath::Pi() to "double" for VTK 6.  Then eventually
vtkMath::DoublePi() can be deprecated.

 - David



On Wed, Feb 29, 2012 at 5:17 PM, David Gobbi <david.gobbi at gmail.com> wrote:
> Hi J-C,
>
> All of those are good examples of why vtkMath:Pi() should be
> returning double instead of float... in each of those four locations,
> a double-precision Pi should be used instead of a single-precision Pi.
> So don't changed them to FloatPi().
>
> I've also noticed that many of the vtkParametricFunction subclasses in
> VTK are using vtkMath::Pi() when they should be using  double-precision pi.
>
>  - David
>
>
> On Wed, Feb 29, 2012 at 4:44 PM, Jean-Christophe Fillion-Robin
> <jchris.fillionr at kitware.com> wrote:
>> Within Slicer vtkMath::Pi() is called at four location.  See details below.
>>
>> We will make sure to use vtkMath::FloatPi() when we will integrate VTK 5.10.
>>
>> Thanks
>> Jc
>>
>>
>>
>> Modules/Loadable/Annotations/VTKWidgets/vtkAnnotationGlyphSource2D.cxx
>> 352:  theta = 2.0 * vtkMath::Pi() / 8.0;
>>
>> Libs/MRML/Logic/vtkMRMLSliceLogic.cxx
>> 2128:    double axisMisalignmentDegrees=acos(dotProd)*180.0/vtkMath::Pi();
>>
>> Libs/vtkTeem/vtkPreciseHyperStreamline.cxx
>> 591:  vtkFloatingPointType theta=2.0*vtkMath::Pi()/this->NumberOfSides;
>>
>> Base/Logic/vtkSlicerGlyphSource2D.cxx
>> 346:  theta = 2.0 * vtkMath::Pi() / 8.0
>>
>>
>> On Wed, Feb 29, 2012 at 6:41 PM, Jean-Christophe Fillion-Robin
>> <jchris.fillionr at kitware.com> wrote:
>>>
>>> +1 :)
>>>
>>>
>>> On Wed, Feb 29, 2012 at 4:57 PM, David Gobbi <david.gobbi at gmail.com>
>>> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> I'm wondering if, for VTK 6, we can make the vtkMath::Pi() return a
>>>> double instead of a float.  Currently, most VTK code is calling
>>>> vtkMath::DoublePi() and, in my opinion, vtkMath::Pi() is more
>>>> readable.
>>>>
>>>> Here's my proposal:
>>>>
>>>> 1) In VTK 5.10, a vtkMath::FloatPi() method would be added.  Any VTK
>>>> 5.10 code that calls vtkMath::Pi() would be changed to call
>>>> vtkMath::FloatPi() instead.
>>>>
>>>> 2) In VTK 6, vtkMath::Pi() would be changed so that it returns double,
>>>> and then all instances of vtkMath::DoublePi() can be changed to
>>>> vtkMath::Pi() to improve readability.
>>>>
>>>> If anyone thinks this is a good idea, I can submit a patch for the VTK
>>>> 5.10 changes to gerrit.
>>>>
>>>>
>>>> To reiterate what I'm proposing, in VTK 5.10, the following methods would
>>>> exist:
>>>>
>>>> double vtkMath::DoublePi();
>>>> float vtkMath::Pi();
>>>> float vtkMath::FloatPi();  - new method for single precision
>>>>
>>>> In VTK 6.0, the following methods would exist:
>>>>
>>>> double vtkMath::DoublePi();
>>>> double vtkMath::Pi();
>>>> float vtkMath::FloatPi();
>>>>
>>>>  - David
>>>> _______________________________________________
>>>> Powered by www.kitware.com
>>>>
>>>> Visit other Kitware open-source projects at
>>>> http://www.kitware.com/opensource/opensource.html
>>>>
>>>> Follow this link to subscribe/unsubscribe:
>>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>>
>>>
>>>
>>>
>>> --
>>> +1 919 869 8849
>>>
>>
>>
>>
>> --
>> +1 919 869 8849
>>