HI Michael, I'm just reviewing your changes and thumbs up to the changes to osg::State, the only thing I've tweaked is moving the initialization of the cached matrix into the constructor so that we don't have to keep checking the existence of the matrix, we can just assume it's valid and set. Once I've done some testing I'll check this in.
In review the changes to Text3D.cpp the first for loop two block's you've modified I understand, but the last block I'm a bit perplexed by - the original code looks a bit out sync with the two other blocks as well. The part I'm looking at is why the local matrix is initialized to the getModeViewMatrix(), rather than the original_modelview matrix as is done with the other blocks. Did you spot this as well? Robert. On Tue, Jun 1, 2010 at 11:17 AM, Michael Platings <[email protected]> wrote: > Hi Robert, I prefer that solution too :) Here's the revised submission with > the suggested changes > > On 31 May 2010 17:06, Robert Osfield <[email protected]> wrote: >> >> Hi Michael, >> >> I've just reviewed your submission and feel that the implementation >> while a clever optimization makes the code less readable and long term >> more likely to introduce it's own quirky bugs as it's be easier for >> follow up submissions to break it. >> >> I understand the motivation behind the optimization though, so am >> wondering about what other solutions there might be. My first thought >> was alter osg::State::applyModelViewMatrix() to allow us, via a bool, >> to force the re-sending of the matrix to OpenGL. Or we could just >> have two methods - a lazy updating one like we have now and the non >> lazy updating one that Text3D requires. >> >> I am also wondering about whether we can just cache a RefMatrix in >> osg::State for just the cases such as this - so we could avoid the new >> completely. >> >> This all pushes complexity from Text3D into State, but I can see both >> changes above being useful in other codes so I'm not opposed to this. >> >> Thoughts? >> Robert. >> >> On Tue, May 25, 2010 at 11:53 AM, Michael Platings <[email protected]> >> wrote: >> > Hi Robert >> > I've modified Text3D so that it doesn't crash when RenderMode is >> > PER_FACE. >> > I've also reduced the number of times that new is called - gives me a 5% >> > frame rate increase. >> > >> > _______________________________________________ >> > 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
