Thanks for the clarification, one day though I would like to review that function again, but currently with Mathias last optimization it no longer has popped up as a bottleneck. So I must defer to the rule of "if it aint broken, don't break it."
----- Original Message ----- From: "Chris Denham" <[EMAIL PROTECTED]> To: "James Killian" <[EMAIL PROTECTED]>; "OpenSceneGraph Submissions" <[email protected]> Sent: Thursday, October 16, 2008 8:28 AM Subject: Re: [osg-submissions] Typo in UFOManipulator.cpp? > Hi James, > I think you misunderstood my message. > I am not saying there is anything wrong with matrix inversion, only that a > matrix inversion seems to be missing from the UFOManipulator code. ie. there > is an inconsistency between the implementation of > UFOManipiulator::getMatrix() and UFOManipulator::getInverseMatrix() when the > _offset matrix is not set to identity. > I have no evidence of any problems with matrix inversion, so don't try and > pin that on me! ;-) LOL. > ChrisD. > > ----- Original Message ----- > From: "James Killian" <[EMAIL PROTECTED]> > To: "Chris Denham" <[EMAIL PROTECTED]>; "OpenSceneGraph Submissions" > <[email protected]> > Sent: Thursday, October 16, 2008 1:47 PM > Subject: Re: [osg-submissions] Typo in UFOManipulator.cpp? > > > > > >> This inconsistency can cause problems when switching from UFO to other > >> manipulators, because the UFO:Manipulator::getMatrix function is not > >> necessarily returning a correct inverse of the currently set > >> ModelViewTransform. > > > > If I am reading this correctly are you saying the inverse() is not giving > > the correct result? When I was pursuing to optimize invert_4x4() I was > > going to suggest to return the determinant rather than a bool to model how > > others compute the inverse. I don't understand why the current invert_4x4 > > had to have all the branches in there now. I did actually submit the > > optimized code to this function back on July 31st. > > > > What would be cool is if in fact the current inverse() is incorrect to > > provide some unit test that submits the values which cause it to fail. > > The testing code I wrote did not cause it to fail. > > > > > > > > James Killian > > ----- Original Message ----- > > From: "Chris Denham" <[EMAIL PROTECTED]> > > To: "Robert Osfield" <[EMAIL PROTECTED]>; "OpenSceneGraph > > Submissions" <[email protected]> > > Sent: Wednesday, October 15, 2008 6:28 AM > > Subject: Re: [osg-submissions] Typo in UFOManipulator.cpp? > > > > > >> Wow, fast work Robert, tnx. > >> However, just spotted another inconsistency between > >> UFOManipulator::getMatrix() and UFOManipulator::getInverseMatrix() > >> It assumes that inverse(_inverseMatrix * _offset) = _offset * _matrix > >> This is only true when _offset=identity, so I think it should be > >> inverse(_offset) * _matrix > >> This inconsistency can cause problems when switching from UFO to other > >> manipulators, because the UFO:Manipulator::getMatrix function is not > >> necessarily returning a correct inverse of the currently set > >> ModelViewTransform. > >> It was tempting to change the name of the _offset member to > >> _inverseOffset, > >> or to maintain both variables, but in the end went for the minimal > >> change. > >> Modified OSG2.6 file attached. > >> Chris D. > >> > >> ////OSG 2.6/////// > >> osg::Matrixd UFOManipulator::getMatrix() const > >> { > >> return (_offset * _matrix); > >> } > >> > >> osg::Matrixd UFOManipulator::getInverseMatrix() const > >> { > >> return (_inverseMatrix * _offset); > >> } > >> /////////// with correction ///////////////////////////////// > >> osg::Matrixd UFOManipulator::getMatrix() const > >> { > >> return (osg::Matrix::inverse(_offset) * _matrix); > >> } > >> > >> osg::Matrixd UFOManipulator::getInverseMatrix() const > >> { > >> return (_inverseMatrix * _offset); > >> } > >> //////////////////////////////////////////// > >> > >> > >> ----- Original Message ----- > >> From: "Robert Osfield" <[EMAIL PROTECTED]> > >> To: "Chris Denham" <[EMAIL PROTECTED]>; "OpenSceneGraph Submissions" > >> <[email protected]> > >> Sent: Wednesday, October 15, 2008 10:39 AM > >> Subject: Re: [osg-submissions] Typo in UFOManipulator.cpp? > >> > >> > >>> Thanks for fix the Chris, now merged and submitted to SVN. > >>> > >>> On Wed, Oct 15, 2008 at 10:30 AM, Chris Denham <[EMAIL PROTECTED]> > >>> wrote: > >>>> I noticed that UFOManipulator _matrix and _inverseMatrix may be > >>>> inconsistently set due to typo in > >>>> UFOManipulator::home(). > >>>> I assume the intention is that _matrix and _inverseMatrix are kept > >>>> consistent, so corrected file attached. > >>>> > >>>> ///////////// OSG 2.6 ////////////////// > >>>> _inverseMatrix.makeLookAt( _homeEye, _homeCenter, _homeUp ); > >>>> _matrix.invert( _matrix ); > >>>> ///////////// after typo correction ///////////////// > >>>> _inverseMatrix.makeLookAt( _homeEye, _homeCenter, _homeUp ); > >>>> _matrix.invert( _inverseMatrix ); > >>>> /////////////////////////////////////// > >>>> > >>>> Chris. > >>>> > >>>> _______________________________________________ > >>>> 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
