[vtk-developers] Cell Tree Locator Update

[Andy Bauer]

Hi Tharindu,

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:
http://review.source.kitware.com/#change,2418

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
http://www.artima.com/cppsource/nevercall.html).

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: http://www.vtk.org/Wiki/VTK/Examples.
This is not required but I'd encourage you to do this to help other people
better understand and utilize your code.

Andy

On Tue, Aug 9, 2011 at 11:23 AM, Tharindu De Silva <tsameera1 at gmail.com>wrote:

> Hi,
>
>   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
> http://review.source.kitware.com/#change,2413.
>
>  There is an example named CellTreeLocator.cxx at Filtering/Examples/Cxx.
>  Please let me know if you encounter any bugs in running this.
>
> Thank you.
>
>
> On Tue, Aug 2, 2011 at 5:08 PM, Biddiscombe, John A. <biddisco at cscs.ch>wrote:
>
>>  I’m not sure that renaming all the internal variables from their
>> original names was a good idea.****
>>
>> 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. ****
>>
>> ** **
>>
>> Renaming the internals serves no purpose other than to make it harder to
>> sync our version with theirs.****
>>
>> ** **
>>
>> 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.*
>> ***
>>
>> ** **
>>
>> JB****
>>
>> ** **
>>
>> *From:* Tharindu De Silva [mailto:tsameera1 at gmail.com]
>> *Sent:* 02 August 2011 18:18
>> *To:* Biddiscombe, John A.; Andy Bauer; VTK Developers
>> *Cc:* trshash at gmail.com
>>
>> *Subject:* Re: [vtk-developers] Cell Tree Locator Update****
>>
>>  ** **
>>
>> Hi,****
>>
>> ** **
>>
>>          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
>> http://review.source.kitware.com/#change,2362.****
>>
>> ** **
>>
>> Some formatting still needs to be done.****
>>
>> ** **
>>
>> 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.***
>> *
>>
>> ** **
>>
>> The standard template library functions are used in this class that might
>> need to be changed as well.****
>>
>> ** **
>>
>> Please let me know if you come across any other bugs or if there are
>>  improvements that can be made to this code.****
>>
>> ** **
>>
>> Thank you,****
>>
>> ** **
>>
>> Tharindu ****
>>
>> ** **
>>
>> On Fri, Jul 29, 2011 at 4:02 PM, Biddiscombe, John A. <biddisco at cscs.ch>
>> wrote:****
>>
>> 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.****
>>
>>  ****
>>
>> 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.****
>>
>>  ****
>>
>> patch attached****
>>
>>  ****
>>
>> JB****
>>
>>  ****
>>
>>
>> _______________________________________________
>> 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
>>
>> ****
>>
>> ** **
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20110809/149d59f3/attachment.html>