Hi John, Your submissions didn't include the include/osgGA/OrbitManipulator that you must have modified as I'm getting the compile error:
/home/robert/OpenSceneGraph/src/osgGA/OrbitManipulator.cpp:154: error: no ‘void osgGA::OrbitManipulator::setHeading(double)’ member function declared in class ‘osgGA::OrbitManipulator’ /home/robert/OpenSceneGraph/src/osgGA/OrbitManipulator.cpp:167: error: no ‘double osgGA::OrbitManipulator::getHeading() const’ member function declared in class ‘osgGA::OrbitManipulator’ /home/robert/OpenSceneGraph/src/osgGA/OrbitManipulator.cpp:188: error: no ‘void osgGA::OrbitManipulator::setElevation(double)’ member function declared in class ‘osgGA::OrbitManipulator’ /home/robert/OpenSceneGraph/src/osgGA/OrbitManipulator.cpp:201: error: no ‘double osgGA::OrbitManipulator::getElevation() const’ member function declared in class ‘osgGA::OrbitManipulator’ /home/robert/OpenSceneGraph/src/osgGA/OrbitManipulator.cpp: In member function ‘virtual bool osgGA::OrbitManipulator::handleMouseWheel(const osgGA::GUIEventAdapter&, osgGA::GUIActionAdapter&)’: /home/robert/OpenSceneGraph/src/osgGA/OrbitManipulator.cpp:227: warning: suggest brackets around && within || Thanks, Robert. On Mon, May 31, 2010 at 9:27 AM, PCJohn <[email protected]> wrote: > Hi Robert, > > found one more bug this morning. Sending update. I was worrying that you > could work on it already so I am sending just updated files of my yesterday > evening submission: > - StandardManipulator (header only), FirstPersonManipulator.cpp, > OrbitManipulator.cpp -> the bug is related to setting center when wheel > movement is reversed (negative). Fixed in this three files. > - Program.cpp -> if validation fails, the shaders are not working properly > (some uniforms were not linked successfully, etc.). This sometimes appear on > my machine. In my opinion, it should be warning (not INFO) as it is a > serious rendering problem. > > Feel free to comment my yesterday ideas. > Cheers, > John > > > PCJohn wrote: > > Hi Robert, > > I am back, looking on manipulators once again. Because you decided to break > backward compatibility, maybe, there can be some more adjustments to clean > up the design. Giving them for discussion: > > - computeHomePosition() - update, handle, set, perform are clear actions on > object for me, but compute or calculate are rather obscure for me. Update > simply updates object. But compute or calculate computes something and is it > returning the computed value to the user, is it printing it to the screen or > the result is forgotten? It is somehow mysterious about the meaning, reason, > or destination. > > I am not rigorous about it, just thinking whether > updateHomePosition() would be a better name for it. > Pros: cleaning of api, cons: further breaking of api compatibility. > Your opinion? > > - w.r.t FirstPersonManipulator name, it came to me idea of renaming it to > something like LookAroundManipulator > that is more general (includes First/Third Person view). Just a tought. I > like FirstPerson by name and LookAround by idea. > > - feel free to suggest different naming for anything in my code as well > > - #define NEW_HOME_POSITION in CameraManipulator - I do not understand it as > it is not used anywhere in CameraManipulator header or .cpp. Can it be > removed, or it has a reason for existence? > > - getSide/Front/UpVector can be made either static, or parameter can be > purged and cb used instead (second option preferred myself). What is your > opinion? > > - reverse mouse wheel direction fixed. Default is "standard" OSG wheel > direction used in trunk before while opposite direction can be set by > negative values in OrbitManipulator::setWheelZoomFactor() > > - some doc added > > > SphericalManipulator: > - handling of movement seems to be similar -> nothing ported to OrbitManip. > - shift and alt keys for controlling only elevation or only heading was > omitted for now as I consider it a "special" functionality that is more > appropriate for user-derived class > - MAP rotation mode - it seems to me that it is looking on the sphere from > the top and rotate it by changing heading. Seems to me quite a special > functionality. I am not sure about it and whether it should be included in > OrbitManipulator or some specialized class. -> not porting it now > - zoomOn(BoundingSphere) omitted as it seems to do the same thing as setting > distance to the 3.5 * BoundingSphere._radius > > Things implemented according to SphericalManipulator: > - allow throw functionality (set/get AllowThrow) > - setHeading and setElevation implemented (+tested) > > > Although I am pressed by some deadlines, I can still make some small > adjustments if needed. > Unfortunately, moving the rest of manipulators to the new architecture is > time consuming and not possible for me now. Anyway, we have the architecture > and we started at least. > > Using the opportunity and sending some files with just updated doc. I > believe it can be helpful. > Cheers, > John > > > Robert Osfield wrote: > > Hi John et. al, > > (I composed the majority of this post before your reply, so you've > already contributed to some of the topics I'll cover, but I'll keep > these listed here for general discussion purposes) > > I've been thinking further about the various osgGA camera manipulator > classes, there are now too many for sure, so I want to rationalize > them. > > 1) Renaming MatrixMapulator. Originally it had been my thought to > make it possible to have a MatrixManipulator control matrices in the > scene as well as the camera, but it really wouldn't fulfill that role > well with the way it's evolved. So in essense MarixManipulator is a > Camera Manipulator - this is how it's always been used, and not > likely to change from this, so renaming this base class > CameraManipuator seems sensible. > > 2) MatrixManipulator and StandardManipulator are really one of the > same thing - a base class for camera manipulators, the only reason why > it looks like StandardManipulator has been implemented was to avoid > conflicts between the subclasses from MatrixManipulator like > SphericalManipulator and UFOManipulator that haven't been ported > across to the new scheme, if we can resolve this then it looks like > the need for a separate MatrixManipulator/StandardManipulator goes > away and we can just merge the functionality and rename it > CameraManipulator. > > 3) SphericalManipulator now looks redundant thanks to the new > OrbitManipulator so it can be dropped. FirstPersonManipulator also > looks to be a useful base to make a replacement of UFOManipulator, so > we could potentially just merge the functionality and do away with > UFOManipulator. > > Now these proposed changes do break the API and will mean that some > client apps might not compile, and will require a few minor tweaks > such as renaming includes and classes and perhaps a few methods. > However, we are about to go for OSG-3.0 and I'd rather have a clean > set of API's where possible, and break API compatibility when the > balances of improvement of streamlined API outweigh the negative > impact. > > Robert. > _______________________________________________ > 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 > > > _______________________________________________ > 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
