[vtk-developers] vtkMath inline choices...

[David Gobbi]

I think it is best if all code that uses isnan() is explicit about the
data type, rather than using an overloaded isnan().

The reason is as follows: when I see any float/double function
being applied to an integer, including functions like sin(), cos(),
rint(), etcetera, my immediate thought is "something is wrong
here".  And I don't want to have to search for a header file and
look at the isnan() definition to convince myself that the code is,
in fact, correct.  I'd rather have the code in vtkLookupTable.cxx
(or in any other class that uses isnan) be sufficiently explicit
that I don't have to look elsewhere.

That's just my 2 cents.

 - David


On Mon, Jan 31, 2011 at 11:17 AM, Moreland, Kenneth <kmorel at sandia.gov> wrote:
> Since we are talking about inlining vtkMath::IsNaN anyway, why not just
> provide overloaded inlined functions for integer types that always return
> false?  That way the compiler removes the check for you without having to
> write any type-dependent case code.
>
> -Ken
>
>
> On 1/30/11 11:24 AM, "David Gobbi" <david.gobbi at gmail.com> wrote:
>
> Hi Ken,
>
> The main issue with performance and IsNan() is more about how it is used
> than about whether it is inlined.  My guess is that Sean saw a performance
> hit because IsNan() was used inside vtkLookupTable, which has some of the
> hottest loops in all of VTK.  The IsNan() check was applied to all
> datatypes, even though it is not relevant for integer data.
>
> I've patched vtkLookupTable so that it overloads vtkLinearLookup() and the
> IsNan check is only done on float and double.  The new code gives me a 40%
> speedup in color mapping a 16-bit image to RGBA.  The patch can be seen
> here: http://vtk.org/gitweb?p=VTK.git;a=commitdiff;h=1c348fcb.  I merged it
> directly into master.
>
> There is also a function in vtkScalarsToColorsPainter that applies IsNan()
> regardless of scalar type.  It could benefit from a similar patch.
>
>  - David
>
>
>
>    ****      Kenneth Moreland
>     ***      Sandia National Laboratories
> ***********
> *** *** ***  email: kmorel at sandia.gov
> **  ***  **  phone: (505) 844-8919
>     ***      web:   http://www.cs.unm.edu/~kmorel
>
>