Hi John, Thanks for the changes. I've done a first pass review but will to go over it several more times to know what the best way forward will be.
Initial thoughts are that it's a step forward, but I am still surprised by RayIntersector being subclassed from LineSegmentIntersector, while I can see you've done this for convenience of implementing RayIntersector I still don't believe this is good design or even necessary that more efficient coding wise. Conceptually a Ray isn't a Line Segment, and in C++ if you are inheriting from a class you are explicitly stating that "is a" specialization of that class. Personally I would have implemented it with the RayIntersector "has a" LineSegmentIntersector thus hiding all the implementation details and interface that could cause problems with users, or I would have re factored the common elements out into a help class that both the RayIntersector utilize, the later approach being my strongest preference. I will going through the implementation a couple more time over the next week to see what common parts might be pulled out into a help class to see what can be done to clean up the design and implementation. Two little question I have right now are - I ask this as I'm trying to work out what might be the best interface for RayIntersector::Intersection.: What would be the appropriate way to specify the position of intersection along a ray that has it's start and end at infinity? Would an intersection points of a geometry at infinity like you've done in your modified osgpick.cpp require a Vec4d intersection point definition. To resolve these both of them suggest to me that we need a specialist RayIntersector::Intersector class, not quite sure how best to store the points yet though. Robert. Robert. On 24 May 2013 09:46, PCJohn <[email protected]> wrote: > Hi Robert, > > please, find attached submission of RayIntersector. It is implemented > according > to our discussion in February on osg-submissions (2013-02-18). Sorry for late > submission - it is caused by heavy load and my boss wishes that I must often > jump immediately to another topic, etc. > > RayIntersector is implemented to use standard LineSegmentIntersector routines > whenever possible (fast path), while using homogeneous path in other cases > (robust path). Such design suggests to derive RayIntersector from > LineSegmentIntersector and reuse its functionality for fast path. Moreover, it > enables CameraManipulators (and possibly other OSG routines) to be switched > easily to use RayIntersector as the basic class interface is the same. Using > RayIntersector in CameraManipulators would make them more robust to handle > infinite projection matrices correctly, and maybe handle other special setups, > etc. (up to now, this does not work). > > Feel free to make some code adjustments, or ask me to reformat some code that > is not according to OSG standard, etc. > > I am attaching osgpick example for demonstration purposes - If you do not > like osgpick example changes, just exclude them from the commit. "./osgpick -- > infinite-view-frustum cessna.osg" works only if there is RayIntersector used > (lines 112 and 113 of the example). If LineSegmentIntersector is used instead, > it can be seen that it is not able to handle picking correctly in --infinite- > view-frustum setup. > > If you accept the changes above, it was preliminary discussed in February that > we would use RayIntersector in CameraManipulators as these are not working > correctly in the similar scenarios as demonstrated in osgpick example. I would > made second submission about these afterwards. > > John (Jan Pečiva or Peciva) > _______________________________________________ > osg-submissions mailing list > [email protected] > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org > _______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
