Hi Rui, thanks for you suggestions. It made me to give it number of further thoughts. You are right that making two different classes would be simple, straightforward and easy to understand. When thinking more deeply about it, it has just one major drawback - we would need to use RayIntersectors throughout OSG as LineSegmentIntersector is simply not robust. It is the scene (loaded model), not the programmer, who tells which intersector (line/ray) to use. osg::Transform or osg::Camera may turn Line intersection to ray intersection. Opposite happens on any node when cutting the ray to its bounding sphere. Thus, using LineSegmentIntersector alone is not robust solution, because it is scene graph who tells which intersector should be used. I was surprised by finding this yesterday evening when meditating over ideas of your email. From that moment, my idea would be to have just LineSegmentIntersector (can be renamed to RayIntersector, but that may break backward compatibility). This class would use fast Vec3 paths whenever possible and homogeneous paths in all remaining special cases.
This is a simple design that would just load LineSegmentIntersector with few Vec4 constructors, setOrigin, setDirection, etc. and some more code in implementation. Using two classes would force OSG to use only RayIntersector for the sake of robustness, and it would remove the nice feature of switching to Vec3 computations whenever there are just simple transformations. Other ideas? (I might miss something.) John On Thursday 14 of February 2013 22:36:00 Wang Rui wrote: Hi John, To be honest, I'm not good at thinking of structures using different design patterns with enough agility, so I always try to find the simplest way to finish the work with easy-to-read interface. If let me write the infinite line intersection functionality, I will simply make them two individual classes, both derived from Intersector only. The code they share can be placed in the osg/Math header or somewhere else so the KdTree implementation could also benefit from it instead of having own ones (the latter is what we have at present). The intersection result class don't have to be shared in my opinion, because it will help high-level developers differentiate both and design their framework without potential confusions. For the homogeneous representation, I'm sure it is cool and professional, but will also suggest some more direct overrides for beginners, like setLineEquation(A, B, C, D), or setOrigin() and setDirection(). These could help users quickly find how to make use of the new RayIntersector in their applications. Hope it will help. I'm looking forward to your implementation and your futher projects (I've looked into some of them in your another thread and was really interested in some of them). Regards, Wang Rui 2013/2/14 PCJohn <[email protected]> Hi Robert and Rui, I have finished one big topic in my work and before I dive into another, I might take a look once again on the design pattern issue in my previous RayIntersector submission (unless someone else wants to take care about it). My idea would be to have one class as an adapter (staying in the place of the current LineSegmentIntersector) while this adapter will forward all the calls to a real intersector class. There will be two intersector classes - one for line intersections, the second for ray intersections. The adapter will create the first or the second intersector based on the need of an user. This may be recognized from, for instance, method parameters or used methods. My idea would be: If user uses Vec3 constructors and Vec3 methods, let's use line intersections, and if he uses homogeneous coordiates (Vec4), let's use ray intersections. Another option would be to let the user specify which intersector he wants (with default set to line intersector). Thoughts? Other ideas? Giving it more thinking: Anyway, what will we get by these design patterns? We would need to change code throughout OSG to not use LineSegmentIntersector, but our adapter. Another problem: Line intersector may change to RayIntersector during the scene graph traversal because a transformation matrix in Camera or Projection, or even Transform may transform the points to infinity. One option is to have only one intersector class that would use faster 3D-code when possible, and slower homogeneous code otherwise. Other option would be to have both intersectors and to update LineSegmentIntersector::clone() to create RayIntersector whenever _start or _end were transformed to infinity while LineSegmentIntersector will be used in regular cases. The same for RayIntersector::clone(). The last design problem is that LineSegmentIntersector and RayIntersector can not be derived from each other as one is carrying _start and _end as Vec3 and the other as Vec4. If we do not mind to waste few bytes, or if we do not mind to store Vec3 + separate float for w component, we could do that. Otherwise, we will need an abstract class for code sharing between the two intersection classes. The code sharing involves: five methods (~200 lines of code), Intersection struct containing the result of the intersection (the struct should be the same for both intersectors, IMHO, to avoid easy switching between the classes) and TriangleIntersection struct (~200 lines of code). Thoughts? John On Wednesday 13 of February 2013 16:33:21 Wang Rui wrote: Hi Robert, I'd like to submit my changes to fix the "near/far values for intersection" problem, which can be obviously found in osgdepthpartition and osgpick (with the option --relative-camera-scene) examples. In the osgdepthpartition example, we can't actually perform correct intersection test (in WINDOW/VIEW/PROJ coordinate frames) with earth/sun nodes because the necessary near/far values are stored in slaves instead of the main camera. I provide a new computeGlobalNearFar() method in ViewerBase to obtain a global near/far pair and apply it to the main camera, and update the osgdepthpartition example to show how it works. In the osgpick example with --relative-camera-scene, you will see that when picking the cube object, only one result is returned (but of course there should be two). That's because the cube is drawn under a post camera in the scene graph. The CullVisitor will reset old near/far values every time when it finishes traversing a child camera, so the depth data of the cube is already lost when we apply getCalculatedNearPlane() and getCalculatedFarPlane() to the main camera in SceneView::cull(). I changed this behavior by providing the getGlobalNearPlane()/getGlobalFarPlane() methods, and now osgpick can work with correct results. The computed result may not satisfy everyone as it collect all perspective projection matrices to get a global near/far pair. But in some cases we may want to ignore some of them (e.g., RTT camera matrices). This can be done by providing an extra option to the Camera/Projection classes. But at least my changes should not affect the rendering work and should be compatible with most current applications. So I'm glad to submit them now. :-) Some months ago, you, PCJohn and me discussed about the idea of infinite line intersection (seems that for design pattern reason the RayIntersector is not merged to the trunk yet). It could also fix the intersection problem in large environments. But the correct near/far values are still not returned to users, which may be very useful for debugging and implementing frustum-related algorithms. I hope this patch could help. Cheers, Wang Rui _______________________________________________ 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
