Hi Robert,
SphericalManipulator can be dropped as it's only in the OSG-2.9.dev
series, not in OSG0-2.8, with the possibility of merging extra set
methods into OribitManipulator.
I agree. We can just merge the required methods.
TrackballManipulator is also now just a shell, the only thing it adds
is the default behaviour.  To me it doesn't feel right to just have a
class to set a default setting and having it just for backwards
compatibility.   However, TrackballManipulator has been available
since even before OSG-1.0, so it's not something we can just drop.  It
feels like perhaps we could just rename OrbitManipulator to
TrackballManipulator, and change the default setting to behavior like
the original TrackballManipulator.  OrbitManipulator might be a little
more generic name for this type of manipulator, but I'd personally
rather go with a slightly different name than have class proliferation
where we can otherwise avoid it.
Seems very logical. Yes, TrackballManipulator can not be dropped, even although its name is not so general as OrbitManipulator, as you said. I am also against class proliferation. At the end, there can be, maybe, just few base classes (Orbit, FirstPerson, ...) that will cover functionality of all the current manipulators.

I am still thinking whether to rename OrbitManipulator to Trackball. Sounds logical except that trackball is not expected to have fixed vertical axis. Please, postpone the resolution of this particular thing for a moment.

I also also wondering about the FirstPersonManipulator, might it and
UFOManipulator have an overlap that we could merge behaviors and just
have one.  The name of the UFOManipulator is pretty obscure,
FirstPersonManipulator will mean far more to most developers so it's
the one I'd be inclined to go for and drop UFOManipulator as a
seperate class and just make sure it's most of it's behaviour can be
ported into FirstPersonManipulator.
I agree.

BTW: Idea came to me to extend FirstPerson by ThirdPerson in the future by just adding a vector inside the class. Although I like the name FirstPersonManipulator, the name may be not general enough, maybe.
osgViewer/view.cpp - new manipulators are using CoordinateFrame [...]
These are just INFO messages, one could just change this to DEBUG_INFO
if you want to quieten them down further.
DEBUG_INFO would be a good alternative for me.

A couple of things I've noticed in testing.  The direction of zoom
associated with the mouse wheel movement is opposite. [...]
I'm not sure if this is constomizable in
your code yet, will need to do into the code.
I was not aware of it. Seems that both approaches should be implemented. I will take a look on it.
Another observation is the you've used const double/const float for
method parameters, in the early days of the OSG I experimented with
this as well with the optimistic belief that it'd provide compiler
with more information about being able to optimize, however I've never
come across a real benefit for it.  In theory one might also catch
bugs by forcing const, but there is a downside - the methods are just
more difficult to read and maintain as there is simply more keywords
in the way of what is important to the what the code is doing.   For
the last decade I've moved to only using const when it's actual
required, and to avoid making code more long winded that it need be,
and const built in types for parameters is a case for me where it's
just too long winded to justify.  You'll note that all cases of const
built in types have been purged from the rest of the OSG so I believe
it's appropriate to stick to this same coding style for the new osgGA
classes.
In that case, let's use what is standard in OSG. I am sometimes using const float&/const double& as well, but that is probably the same issue. And if it is not the most inner loop of the application... -> that should be inlined anyway! So, I am pretty ok with changing them to just float/double to keep OSG style.

w.r.t. performance: Just my personal opinion: I found very much inline code (long inline methods) inside some core OSG classes. My impression was that it makes the header very long thus difficult to quickly "look at" and "find something". Also, I think that too much inlining pollutes processor code caches, makes execution slower, and makes bigger executables. Thinking just about big inline methods. (I used to code in assembler.) Anyway, just a comment. No need to change anything now. Other people may have different opinion and I have not made any measurement. Also, intelligent compilers may decide to not inline something if it would slow down on target architecture. Hopefully, it is not a problem at all.


I will take a break for tomorrow (reason: other important work), but I will work on it at the weekend and will be back with the reverse wheel zoom.

Cheers,
John

_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org

Reply via email to