Hi Robert, thanks for looking on the submission. I am in a big time pressure because I am leaving to US at the end of next week. Anyway, I gave this very high priority so I will try to respond as fast as I can to any further discussion, if any. Unfortunately, I will be online until next Friday morning only :-(.
Comments in the text bellow. On Thursday 30 of May 2013 12:16:11 Robert Osfield wrote: > 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. I see and I agree what you said. Subclassing was very convenient, even for replacing LineSegmentIntersector by RayIntersector in any code (CameraManipulators, etc.) without any further changes. Anyway, RayIntersector is not "is a" specialization as you said. So, you are right. > 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. Feel free to make any changes. > 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? For position of intersection, we can use Vec4 or ratio. Vec4 approach would use xyz of the vector for the direction and w set to zero for infinity. Concerning ratio approach, my friend (mathematical expert) told me that, in general, we should not use single float, but two (x as factor and w as homogeneous part). I argued that if we think about start and end vectors as directions only (xyz), we can use single float to make linear interpolation between the two to create direction to the intersection point, thus single float should be enough. I might ask him after the weekend, if needed, for clarification. His two-float solution would probably be robust enough to handle even one point at infinity correctly or both finite coordinates. My one-float hack is probably for both points at infinity only. Do we need to investigate more deeply on this point? > Would an intersection points of a geometry at infinity like you've > done in your modified osgpick.cpp require a Vec4d intersection point > definition. We can use Vec4 that would cover reguar and infinity intersections, or Vec3 while we need a flag to indicate whether Vec3 contains 3D intersection point or direction to infinity where the intersection happened. Still a note about start and end point at infinity - this create difficulties for intersection routines. In theory, it should not intersect regular 3D geometry. Only objects at infinity. The objects at inifinity are flat. They have no thickness and they are 2D object somehow. In some local coordinates, these objects are intersected not by line but by plane. Thus, I did not attempt even to think about algorithms for this and my algorithms work only if ray starting point is not at infinity. If I did not answered your questions or more clarification is needed, just write. After the weekend, I might get even more math background from my friend, but only if necessary. John > 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-openscenegrap > > h.org > _______________________________________________ > 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
