Hi Robert, I am highly loaded this week, so I am responding late. Anyway, RayIntersector is on top of my programming priorities that are, unfortunately, pushed down by some other non-programming responsibilities this week. Anyway:
> First up I wouldn't suggest subclassing RayIntersector > from LineSegmentIntersector I was following different way of thinking. However, I think, your is more appropriate. I will try to rewrite it as you said and made both classes derived from osgUtil::Intersector. An alternative would be to make one more intermediate class. However, I have some design problems that I would like to ask you before I start changes: The biggest problem is with LineSegmentIntersector::Intersection struct. I would like to keep the structure the same between RayIntersector and LightSegmentIntersector to make the transition between using one and the second class as easy as possible. What is your suggestion here? Do you think that such thing is achievable? Or, should we just copy'n'paste the struct? If we manage to use the same Intersection struct for LineSegmentIntersector and RayIntersector, where we should place it? Should we create intermediate class between Intersector and Ray/LineSegmentIntersector for it? What name would be appropriate for the class? > the ratio now gets sorted distance which really isn't good I do not understand this sentence. Can you clarify it for me? Or, maybe I was not clear myself. Ratio still gets ratio and this is always true for LineSegmentIntersector. New variable "distance" gets distance and that is true for both - LineSegmentIntersector and RayIntersector. And as there is the same result by sorting Intersections by ratio and by distance, I changed operator < to sort by distance because it will work for both Intersectors. Some functions seems so standard for me, like enter(), leave(), reset(), and my getCoordinateTransformation(), and the most people would probably want to reuse them in their classes without any change. Thus, I am tempted to move them directly to osgUtil::Intersector class. They are mostly present as abstract methods there, so they would have standard implementation. Would you support this idea? Thanks for the answer. John On Thursday 20 of September 2012 15:20:33 Robert Osfield wrote: > Hi John, > > I haven't dived into to do a review yet but from your description I am > bit concerned. First up I wouldn't suggest subclassing RayIntersector > from LineSegmentIntersector, this might seem convinient in terms of > reusing code but from a conceptual and design standpoint it's > inapprpriate. Conceptuall for RayIntersector to be subclassed from > LineSegmentIntersector it would have to follow that RayIntersector "is > a" LineSegmentIntersector, or in more english terms Ray "is a" Line > Segment, which it's not, rather it's more like the other way round a > Line Segment is a special case of Ray. When you get mixing of > concepts like these it's almost always a inappropriate choice of OO > design. An example of how you've ended mixing the design is that the > ratio now gets sorted distance which really isn't good. > > If one wishes to reuse functionality then it would be appropriate to > refactor LineSegmentIntersector implementation so that the > functionality sits outwith LineSegmentIntersector and then both > RayIntersector and LineSegmentIntersector would reuse this common > functionality via a common class that provides that functionality. > > In terms of how one specifies the ray, I can see value in providing > set up via Vec4d pair for homogeneous coordinates for those who are > happy working with homogeneous coords, which should in theory include > all graphics programmers... but I suspect quite a few in the community > might struggle given how sometimes they struggle on lesser concepts. > So having the feature work so there is some means of working in Vec3d > position and Vec3d direction, such as as simply mapping these to a > Vec4d(position,1.0), Vec4d(direction,0.0) internally would be > appropriate. > > Thoughts? > Robert. > > On 19 September 2012 15:24, PCJohn <[email protected]> wrote: > > Hi Rui and Robert, > > > > I am attaching new RayIntersector class for infinite picking rays, derived > > from LineSegmentIntersector. I tried to share as much code as possible. > > Send me any comments if you have. > > > > The most important decision, I made: I am using two Vec4d for start and > > end > > point to specify them in homogeneous coordinates (w coordinate can be set > > to zero to indicate point at infinity). I considered it more general to > > alternative option to specify it by two Vec3d, one for start point and > > second for direction. But it can be changed. Or, some constructor getting > > point and direction can be easily added. Ideas? > > > > Second decision: LineSegmentIntersector::Intersection is now sorted by > > distance (not ratio). distance is now prefered as it contains always valid > > value while ratio is valid when using LineSegmentIntersector or when > > RayIntersector's ray is of finite length. > > > > Rui: I hope that RayIntersector will work for your cases. If not, let me > > know. > > > > Robert: In short, I overriden intersect(), intersects() and > > intersectAndClip() methods of LineSegmentIntersector by homogeneous > > versions for RayIntersector. Then, I was forced to replace some > > occurences of LineSegmentIntersectors by RayIntersector in OSG sources. > > Finally, I tested it on osgPick example and I improved it a little bit. > > > > One note: Picking for objects at infinity is not supported yet as this is > > not what people are usually doing. But it might be not so difficult to > > implement it with RayIntersector. > > > > Summary of changes: > > - RayIntersector class introduced for infinite ray support and infinite > > projection matrix > > - osgPick example extended to demonstrate infinite geometry and infinite > > projection matrix functionality > > > > John > > _______________________________________________ > > 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
