Hi Lukasz,

I have now done a code review and as things stand don't feel they are
appropriate to merge.

The problems are that it introduces a DeltaTime property to osg::FrameStamp
but there are two distinct concenpts of time stored by FrameStamp -
SimulationTime and FrameTime and it's not clear what this new property
referrs to.  From your implementation I can see it a DeltaFrameTime,
however, normally for simulation work you'd want to use DeltaSimulationTime
so perhaps you haven't spotted this subtle difference.  For consistency it
would be required to add a DeltaSimulationTime and DeltaFrameTime, or
perhaps SimulationTimeDelta, FrameTimeDelta would flow better.

Another missing element is that the FrameStamp values are passed into
shaders via the osg_SimulationTime and osg_FrameTime unifiorms, the code
for this is in osgUtil::SceneView. If we do add the delta's in then it
would make sense to expose them in shaders like we do from SimulationTIme
and FrameTime.

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

Reply via email to