Thanks Robert for your detailed reply and your great work with all submissions!
On Dec 11, 2007 4:55 PM, Robert Osfield <[EMAIL PROTECTED]> wrote: > Hi Karl, > > Thanks for the meaty submission. I have done a review, but at this > point have decided to hold back from merging as the changes while > desirable in terms of end user functionality, are quite intrusive and > more wide spread than I feel comfortable with right now. What I look > to do is break the improvements to stats down into several stages, > with discussion between each to clarify what the best way to implement > particular features would be. My intension was not to be intrusive. I realized after my submission that it was a lot of code especially for my first submisson and that it is very important that everything fits in well with the thinking behind. I will try to answer your concerns. > > > Areas of concern for me right are focused on the way that stats > gathering has been pushed into GraphicsContext and elsewhere w.r.t > before swap buffer and after swap buffer timings. FrameStamp also has > a new FrameTime variable, which makes for three separate time values > in the FrameStamp object, which really could introduce confusion. Why > all these additions? > About the FrameStamp class: The frameTime is the efficient time that the frame elapsed (except from swapBuffers). I have also review my code in the GraphicsContext and you should just need one Timer there. It could be called _elapsedFrameTime and be started/restarted right after swapBuffers. Instead of doing the delta_s() calculation between two timers it is much more appropriate to the get the elapsed time from the _elapsedFrameTime->time_s() method. The same change applies of course also to the GraphicsThread class. Sorry that I didn't saw that earlier. The frameTime is used in the PointDrawCallback in the StatsHandler class to update the vertices positions of the frameRateLineStrip (fetched by the viewerStats->getAttribute(...), line 615 in StatsHandler.cpp). I also noticed one row in the PointDrawCallback that has no effect was by mistake not removed before the post of the submission (line 599: double totFrameTime = ...) > > One area that isn't controversial is moving the > setFrameStamp/getFramStamp from osgViewer::View to osg::View, this is > reasonable and non intrusive so I've gone ahead and merged this > change. > Cool, will I enter the list of contributors for that tiny little change!? :-) Regards, Karl > > Thanks for your patience, > Robert. > > On Nov 16, 2007 3:00 PM, Karl Heijdenberg > <[EMAIL PROTECTED]> wrote: > > Hi all, > > > > I have done some work in our OSG project to be able to display more > > and better statistics. The changes that I have done is compatible with > > the latest svn version (2007-11-16) and version 2.2.0. I hope this > > work can be of interest for the community and if it is considered good > > enough it would be great to have it added permanently to the > > OpenSceneGraph. > > > > To use the statistics, replace the original source files with the > > files in the attached osg_stats.tgz archive: > > > > I have taken some example screen shots of the statistics. > > > > In example1.jpg I use osgviewer to display two osg example models. > > > > The upper part of the statistics shows a continuously updated graph > > which displays the load over the last 240 frames. As you can see a > > couple of frames ago there was some extra load to the graphics. This > > was due to the wireframe mode that was active to during that time. > > > > Below the load graph you can see the event and update stats for the > > eight latest frames (hard to see since its almost no work done there). > > Last frame is drawn furthest to the left side and the eighth last > > frame furthest to the right. > > > > The outlined box below the event and update stats shows statistics for > > the one and only camera used for the osgviewer. The name of the camera > > would have been written next to 'Camera:' if it had been set to the > > object (see example2.jpg). The light blue statistics displays the > > number of vertices, drawables, lights and bins after the cull process > > for the camera. It also displays the number of GL primitives of each > > present type. Since there only are TRIANLGE_STRIPS in the view of > > example1.jpg thats the only information displayed. See example2.jpg > > and example3.jpg for statistics with more GL primitives. > > > > Below the camera statistics there are statistics presented for the > > whole scene (in the purple outlined box) within a view. If there had > > been more than one view the scene stats for the next view would have > > been presented to the right of the first one. The same thing applies > > to the name of the view (as for the camera) which would have been > > displayed after the 'Scene stats for view ' text if it had been set to > > the view object. > > > > In example2.jpg there is one master camera for the right half of the > > image and a slave camera for the left half of the image. Each camera > > has its own statistics box. (example2.jpg uses a composite viewer to > > set up the cameras). > > > > No total scene stats are shown in the example2.jpg since 's' only has > > been pressed two times not three. > > > > The viewer will loop these display modes when you press 's' in your > viewer: > > > > 1) Frame rate stats is displayed. > > 2) 1 + Viewer stats is displayed. > > 3) 1 + 2 + Scene stats is displayed. > > 4 All stats will be removed. > > > > example3.jpg is an example of a scene from our graphics engine > > developed at Saab Aerosystems. > > > > One thing that I think have to be rethought is the allocation of > > attributeMapList elements in osg/Stats class where I changed the > > allocation from 25 to 240 attributeMapList elements which is not good > > from a memory perspective. I had to do this though to be able to store > > frameTime for enough frames in the past for the load graph. There is > > probably a more efficient way to index frames back in time than with > > this memory expensive structure. It is also slow to look up values > > from. A ring buffer could eventually be used instead. > > > > Regards Karl > > > > _______________________________________________ > > 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
