<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 12 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p
{mso-style-priority:99;
mso-margin-top-alt:auto;
margin-right:0cm;
mso-margin-bottom-alt:auto;
margin-left:0cm;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
span.EmailStyle18
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-GB" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">This patch is the correct one. My apologies.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">JB<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> vtk-developers-bounces@vtk.org [mailto:vtk-developers-bounces@vtk.org]
<b>On Behalf Of </b>Biddiscombe, John A.<br>
<b>Sent:</b> 10 August 2011 09:50<br>
<b>To:</b> Tharindu De Silva; Andy Bauer; VTK Developers<br>
<b>Subject:</b> Re: [vtk-developers] Cell Tree Locator Update<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The virtual call in the destructor was my fault. Sorry about that. I am aware of the badness of virtual calls in constructors and destructors. I think it’s
safe in this case, because it just frees some pointers and sets them to null, the base class does the same, but doesn’t know about one pointer, so worst case scenario is a memory leak (but it’s hard to see this happening with your change).<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The class does not honour the Max cells per node variable, it declares it’s own one and ignore the one inherited from abstract cell locator. the patch attached
fixes that and also cleans up a doc entry which was nonsense.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">JB<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Apologies I can’t use Gerrit due to inability to get an openID<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Tharindu De Silva [mailto:tsameera1@gmail.com]
<br>
<b>Sent:</b> 09 August 2011 23:44<br>
<b>To:</b> Andy Bauer; VTK Developers; Biddiscombe, John A.<br>
<b>Subject:</b> Re: [vtk-developers] Cell Tree Locator Update<o:p></o:p></span></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal" style="margin-bottom:12.0pt"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Tue, Aug 9, 2011 at 5:43 PM, Tharindu De Silva <<a href="mailto:tsameera1@gmail.com">tsameera1@gmail.com</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal">Hi Andy,<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"> Thank you for the comments. Sorry I didn't notice the trailing white spaces. <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> By 'example' i meant the test, I will go through the wiki page and try to make an example. <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="color:#888888">Tharindu<o:p></o:p></span></p>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal">On Tue, Aug 9, 2011 at 4:44 PM, Andy Bauer <<a href="mailto:andy.bauer@kitware.com" target="_blank">andy.bauer@kitware.com</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal">Hi Tharindu,<br>
<br>
I cleaned up the code a bit doing things like getting rid of trailing whitespace (gerrit will highlight this as red to warn about it), bracketed some if/else conditions, and added a std::vector to take care of some dynamic memory allocation. I also got rid
of the compile warnings in the test. These changes are at:<br>
<a href="http://review.source.kitware.com/#change,2418" target="_blank">http://review.source.kitware.com/#change,2418</a><br>
<br>
One thing to note is that I made it more apparent that a virtual function was getting called in the destructor. In general this is bad practice (see
<a href="http://www.artima.com/cppsource/nevercall.html" target="_blank">http://www.artima.com/cppsource/nevercall.html</a>).<br>
<br>
I was able to run the test successfully. In your previous email you mentioned an example. Did you mean test? I couldn't find anything called CellTreeLocator.cxx except for the test. If you wanted to add in a different example, please check out:
<a href="http://www.vtk.org/Wiki/VTK/Examples" target="_blank">http://www.vtk.org/Wiki/VTK/Examples</a>. This is not required but I'd encourage you to do this to help other people better understand and utilize your code.<br>
<span style="color:#888888"><br>
Andy</span><o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Tue, Aug 9, 2011 at 11:23 AM, Tharindu De Silva <<a href="mailto:tsameera1@gmail.com" target="_blank">tsameera1@gmail.com</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal">Hi,<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"> I revert the renaming of internal variables to their original names and made a note on the code that they were derived from avtCellLocatorBIH class in VisIT, so that the people familiar with that code can make improvements. The current
code can be found in <a href="http://review.source.kitware.com/#change,2413" target="_blank">http://review.source.kitware.com/#change,2413</a>.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"> There is an example named CellTreeLocator.cxx at Filtering/Examples/Cxx. Please let me know if you encounter any bugs in running this.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Thank you.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"> <o:p></o:p></p>
<div>
<p class="MsoNormal">On Tue, Aug 2, 2011 at 5:08 PM, Biddiscombe, John A. <<a href="mailto:biddisco@cscs.ch" target="_blank">biddisco@cscs.ch</a>> wrote:<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">I’m not sure that renaming all the internal variables from their original names was a good idea.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">The original code came from the visit avtXXX class and there’s a strong possibility that the visit developers will make changes/improvements
similar to the intersectBox type methods that I’m adding. </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">Renaming the internals serves no purpose other than to make it harder to sync our version with theirs.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">I am aware that I have radical opinions about the dreadful kitware code style but for me this is an unnecessary barrier to future collaboration.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">JB</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D"> </span><o:p></o:p></p>
<div style="border:none;border-top:solid windowtext 1.0pt;padding:3.0pt 0cm 0cm 0cm;border-color:-moz-use-text-color -moz-use-text-color">
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><b><span lang="EN-US" style="font-size:10.0pt">From:</span></b><span lang="EN-US" style="font-size:10.0pt"> Tharindu De Silva [mailto:<a href="mailto:tsameera1@gmail.com" target="_blank">tsameera1@gmail.com</a>]
<br>
<b>Sent:</b> 02 August 2011 18:18<br>
<b>To:</b> Biddiscombe, John A.; Andy Bauer; VTK Developers<br>
<b>Cc:</b> <a href="mailto:trshash@gmail.com" target="_blank">trshash@gmail.com</a></span><o:p></o:p></p>
<div>
<p class="MsoNormal"><br>
<b>Subject:</b> Re: [vtk-developers] Cell Tree Locator Update<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Hi,<o:p></o:p></p>
<div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> I did some formatting changes to the code according to vtk guidelines and added the patch given by John. The code can be found in <a href="http://review.source.kitware.com/#change,2362" target="_blank">http://review.source.kitware.com/#change,2362</a>.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Some formatting still needs to be done.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Helper classes, vtkCellTree and vtkCellTreeNode, are made public in the current version to make them accessible by sub-classing to test for additional functions implemented in vtkModifiedBSPTree.
This can be made protected after all the functions are available in vtkCellTreeLocator.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">The standard template library functions are used in this class that might need to be changed as well.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Please let me know if you come across any other bugs or if there are improvements that can be made to this code.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Thank you,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Tharindu <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">On Fri, Jul 29, 2011 at 4:02 PM, Biddiscombe, John A. <<a href="mailto:biddisco@cscs.ch" target="_blank">biddisco@cscs.ch</a>> wrote:<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">I’ve fixed the crash caused by a numeric overflow. some compilers might not mind and not cause the segfault – I believe the intent was
to allow the std:::numeric_limits max() to cause a large volume result which would exclude the bucket from consideration.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">I’d added a check to stop it. Might be worth adding a few other tests as this class is new and we have little experience with it. I’ll
add one myself I think.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">patch attached</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D">JB</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;color:#1F497D"> </span><o:p></o:p></p>
</div>
</div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;margin-bottom:12.0pt"><br>
_______________________________________________<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><o:p></o:p></p>
</div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>