[vtk-developers] Cell Tree Locator Update

Tue Aug 9 17:43:41 EDT 2011

On Tue, Aug 9, 2011 at 5:43 PM, Tharindu De Silva <tsameera1 at gmail.com>wrote:

> Hi Andy,
>
>           Thank you for the comments.  Sorry I didn't notice the trailing
> white spaces.
>
>           By 'example' i meant the test,  I will go through the wiki page
> and try to make an example.
>
> Thanks,
>
> Tharindu
> On Tue, Aug 9, 2011 at 4:44 PM, Andy Bauer <andy.bauer at kitware.com> wrote:
>
>> 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/a862c3ae/attachment.html>