<br><br><div class="gmail_quote">On Mon, Jan 31, 2011 at 11:04 AM, Sean McBride <span dir="ltr"><<a href="mailto:sean@rogue-research.com">sean@rogue-research.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im">On Sun, 30 Jan 2011 11:24:13 -0700, David Gobbi said:<br>
<br>
>The main issue with performance and IsNan() is more about how it is used<br>
>than about whether it is inlined. My guess is that Sean saw a performance<br>
>hit because IsNan() was used inside vtkLookupTable, which has some of the<br>
>hottest loops in all of VTK. The IsNan() check was applied to all<br>
>datatypes, even though it is not relevant for integer data.<br>
><br>
>I've patched vtkLookupTable so that it overloads vtkLinearLookup() and the<br>
>IsNan check is only done on float and double. The new code gives me a 40%<br>
>speedup in color mapping a 16-bit image to RGBA. The patch can be seen<br>
>here: <a href="http://vtk.org/gitweb?p=VTK.git;a=commitdiff;h=1c348fcb" target="_blank">http://vtk.org/gitweb?p=VTK.git;a=commitdiff;h=1c348fcb</a>. I merged it<br>
>directly into master.<br>
<br>
</div>David,<br>
<br>
Yes, exactly. Thanks for improving it. In fact I have a local branch<br>
with changes similar to what you just did, but I wanted to go slowly,<br>
and thought the leaf of the problem (isnan) was a good place to start.<br>
<br>
This part:<br>
<br>
+ // calling isnan() instead of vtkMath::IsNan() improves performance<br>
+#ifdef VTK_HAS_ISNAN<br>
+ if (isnan(v))<br>
+#else<br>
+ if (vtkMath::IsNan(v))<br>
+#endif<br>
<br>
I'm sure you agree is silly, and can be done away with once we fix<br>
vtkMath::IsNan().<br>
<br>
Try as I may, I can't figure out how to add a comment in Gerrit.</blockquote><div><br>You have to be signed in to make comments on Gerrit. Once you're signed in, if you double click on a "changed line of code" while reviewing, it will pop up an in-place text edit window in which you may type whatever you like.<br>
<br>Others can then see what "inline" comments you've made.<br><br>Or... from the top page where you are looking at the whole gerrit change, there should be a "Review" button, which allows you to make general comments on the overall change.<br>
<br><br>HTH,<br>David<br><br><br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> My<br>
patch did not include the installation of vtkMathConfigure.h because I<br>
don't know how to do that. Marcus' suggestion of using vtkConfigure.h<br>
is maybe a better idea anyway? What do we prefer? I can update my patch.<br>
<div class="im"><br>
Cheers,<br>
<br>
--<br>
____________________________________________________________<br>
Sean McBride, B. Eng <a href="mailto:sean@rogue-research.com">sean@rogue-research.com</a><br>
Rogue Research <a href="http://www.rogue-research.com" target="_blank">www.rogue-research.com</a><br>
Mac Software Developer Montréal, Québec, Canada<br>
<br>
<br>
</div><div><div></div><div class="h5">_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
</div></div></blockquote></div><br>