From e202d4e4a516b802b96dc75b1172b74f75fcce1a Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Thu, 2 Aug 2012 17:26:31 +0200 Subject: [PATCH 1/2] bug: 823 change rotation animation to use a subclass of SGRotateTransform The animation is implemented in the computeWorldToLocalMatrix() and computeLocalToWorldMatrix() virtual functions. Doing the animation in a cull callback breaks picking. Fix for http://code.google.com/p/flightgear-bugs/issues/detail?id=823 --- simgear/scene/model/SGRotateTransform.cxx | 6 +- simgear/scene/model/SGRotateTransform.hxx | 3 + simgear/scene/model/animation.cxx | 181 +++++++++++----------- 3 files changed, 93 insertions(+), 97 deletions(-) diff --git a/simgear/scene/model/SGRotateTransform.cxx b/simgear/scene/model/SGRotateTransform.cxx index 4f2a4227..d1792f19 100644 --- a/simgear/scene/model/SGRotateTransform.cxx +++ b/simgear/scene/model/SGRotateTransform.cxx @@ -31,9 +31,9 @@ #include "SGRotateTransform.hxx" -static void -set_rotation (osg::Matrix &matrix, double position_rad, - const SGVec3d ¢er, const SGVec3d &axis) +void SGRotateTransform::set_rotation (osg::Matrix &matrix, double position_rad, + const SGVec3d ¢er, + const SGVec3d &axis) { double temp_angle = -position_rad; diff --git a/simgear/scene/model/SGRotateTransform.hxx b/simgear/scene/model/SGRotateTransform.hxx index 2cf1d375..dc6f95de 100644 --- a/simgear/scene/model/SGRotateTransform.hxx +++ b/simgear/scene/model/SGRotateTransform.hxx @@ -64,6 +64,9 @@ public: virtual osg::BoundingSphere computeBound() const; + // Useful for other classes too. + static void set_rotation (osg::Matrix &matrix, double position_rad, + const SGVec3d ¢er, const SGVec3d &axis); private: SGVec3d _center; SGVec3d _axis; diff --git a/simgear/scene/model/animation.cxx b/simgear/scene/model/animation.cxx index 78b0d414..25dcb20a 100644 --- a/simgear/scene/model/animation.cxx +++ b/simgear/scene/model/animation.cxx @@ -71,51 +71,6 @@ using namespace simgear; // Static utility functions. //////////////////////////////////////////////////////////////////////// -/** - * Set up the transform matrix for a spin or rotation. - */ -static void -set_rotation (osg::Matrix &matrix, double position_deg, - const SGVec3d ¢er, const SGVec3d &axis) -{ - double temp_angle = -SGMiscd::deg2rad(position_deg); - - double s = sin(temp_angle); - double c = cos(temp_angle); - double t = 1 - c; - - // axis was normalized at load time - // hint to the compiler to put these into FP registers - double x = axis[0]; - double y = axis[1]; - double z = axis[2]; - - matrix(0, 0) = t * x * x + c ; - matrix(0, 1) = t * y * x - s * z ; - matrix(0, 2) = t * z * x + s * y ; - matrix(0, 3) = 0; - - matrix(1, 0) = t * x * y + s * z ; - matrix(1, 1) = t * y * y + c ; - matrix(1, 2) = t * z * y - s * x ; - matrix(1, 3) = 0; - - matrix(2, 0) = t * x * z - s * y ; - matrix(2, 1) = t * y * z + s * x ; - matrix(2, 2) = t * z * z + c ; - matrix(2, 3) = 0; - - // hint to the compiler to put these into FP registers - x = center[0]; - y = center[1]; - z = center[2]; - - matrix(3, 0) = x - x*matrix(0, 0) - y*matrix(1, 0) - z*matrix(2, 0); - matrix(3, 1) = y - x*matrix(0, 1) - y*matrix(1, 1) - z*matrix(2, 1); - matrix(3, 2) = z - x*matrix(0, 2) - y*matrix(1, 2) - z*matrix(2, 2); - matrix(3, 3) = 1; -} - /** * Set up the transform matrix for a translation. */ @@ -726,51 +681,81 @@ SGTranslateAnimation::createAnimationGroup(osg::Group& parent) // Implementation of rotate/spin animation //////////////////////////////////////////////////////////////////////// -//Cull callback -class RotAnimCallback : public osg::NodeCallback +class SGRotAnimTransform : public SGRotateTransform { public: - RotAnimCallback(SGCondition const* condition, - SGExpressiond const* animationValue, - double initialValue = 0.0) : - _condition(condition), - _animationValue(animationValue), - _initialValue(initialValue), - _lastValue(0.0) - { } - virtual void operator()(osg::Node* node, osg::NodeVisitor* nv); -public: + SGRotAnimTransform(); + SGRotAnimTransform(const SGRotAnimTransform&, + const osg::CopyOp& copyop = osg::CopyOp::SHALLOW_COPY); + META_Node(simgear, SGRotAnimTransform); + virtual bool computeLocalToWorldMatrix(osg::Matrix& matrix, + osg::NodeVisitor* nv) const; + virtual bool computeWorldToLocalMatrix(osg::Matrix& matrix, + osg::NodeVisitor* nv) const; SGSharedPtr _condition; SGSharedPtr _animationValue; - double _initialValue; - double _lastValue; + // used when condition is false + mutable double _lastAngle; }; -void RotAnimCallback::operator()(osg::Node* node, osg::NodeVisitor* nv) +SGRotAnimTransform::SGRotAnimTransform() + : _lastAngle(0.0) { - using namespace osg; - SGRotateTransform* transform = static_cast(node); - EffectCullVisitor* cv = dynamic_cast(nv); +} - if (!cv) - return; +SGRotAnimTransform::SGRotAnimTransform(const SGRotAnimTransform& rhs, + const osg::CopyOp& copyop) + : SGRotateTransform(rhs, copyop), _condition(rhs._condition), + _animationValue(rhs._animationValue), _lastAngle(rhs._lastAngle) +{ +} + +bool SGRotAnimTransform::computeLocalToWorldMatrix(osg::Matrix& matrix, + osg::NodeVisitor* nv) const +{ double angle = 0.0; if (!_condition || _condition->test()) { - angle = _animationValue->getValue() - _initialValue; - _lastValue = angle; + angle = _animationValue->getValue(); + _lastAngle = angle; } else { - angle = _lastValue; + angle = _lastAngle; } - const SGVec3d& sgcenter = transform->getCenter(); - const SGVec3d& sgaxis = transform->getAxis(); - Matrixd mat = Matrixd::translate(-sgcenter[0], -sgcenter[1], -sgcenter[2]) - * Matrixd::rotate(SGMiscd::deg2rad(angle), sgaxis[0], sgaxis[1], sgaxis[2]) - * Matrixd::translate(sgcenter[0], sgcenter[1], sgcenter[2]) - * *cv->getModelViewMatrix(); - ref_ptr refmat = new RefMatrix(mat); - cv->pushModelViewMatrix(refmat.get(), transform->getReferenceFrame()); - traverse(transform, nv); - cv->popModelViewMatrix(); + double angleRad = SGMiscd::deg2rad(angle); + if (_referenceFrame == RELATIVE_RF) { + // FIXME optimize + osg::Matrix tmp; + set_rotation(tmp, angleRad, getCenter(), getAxis()); + matrix.preMult(tmp); + } else { + osg::Matrix tmp; + SGRotateTransform::set_rotation(tmp, angleRad, getCenter(), getAxis()); + matrix = tmp; + } + return true; +} + +bool SGRotAnimTransform::computeWorldToLocalMatrix(osg::Matrix& matrix, + osg::NodeVisitor* nv) const +{ + double angle = 0.0; + if (!_condition || _condition->test()) { + angle = _animationValue->getValue(); + _lastAngle = angle; + } else { + angle = _lastAngle; + } + double angleRad = SGMiscd::deg2rad(angle); + if (_referenceFrame == RELATIVE_RF) { + // FIXME optimize + osg::Matrix tmp; + set_rotation(tmp, -angleRad, getCenter(), getAxis()); + matrix.postMult(tmp); + } else { + osg::Matrix tmp; + set_rotation(tmp, -angleRad, getCenter(), getAxis()); + matrix = tmp; + } + return true; } // Cull callback @@ -892,21 +877,28 @@ SGRotateAnimation::SGRotateAnimation(const SGPropertyNode* configNode, osg::Group* SGRotateAnimation::createAnimationGroup(osg::Group& parent) { - SGRotateTransform* transform = new SGRotateTransform; - transform->setName("rotate animation"); - if (_isSpin) { - SpinAnimCallback* cc; - cc = new SpinAnimCallback(_condition, _animationValue, _initialValue); - transform->setCullCallback(cc); - } else if (_animationValue || !_animationValue->isConst()) { - RotAnimCallback* cc = new RotAnimCallback(_condition, _animationValue, _initialValue); - transform->setCullCallback(cc); - } - transform->setCenter(_center); - transform->setAxis(_axis); - transform->setAngleDeg(_initialValue); - parent.addChild(transform); - return transform; + if (_isSpin) { + SGRotateTransform* transform = new SGRotateTransform; + transform->setName("spin rotate animation"); + SpinAnimCallback* cc; + cc = new SpinAnimCallback(_condition, _animationValue, _initialValue); + transform->setCullCallback(cc); + transform->setCenter(_center); + transform->setAxis(_axis); + transform->setAngleDeg(_initialValue); + parent.addChild(transform); + return transform; + } else { + SGRotAnimTransform* transform = new SGRotAnimTransform; + transform->setName("rotate animation"); + transform->_condition = _condition; + transform->_animationValue = _animationValue; + transform->_lastAngle = _initialValue; + transform->setCenter(_center); + transform->setAxis(_axis); + parent.addChild(transform); + return transform; + } } @@ -1956,7 +1948,8 @@ public: virtual void transform(osg::Matrix& matrix) { osg::Matrix tmp; - set_rotation(tmp, _value, _center, _axis); + SGRotateTransform::set_rotation(tmp, SGMiscd::deg2rad(_value), _center, + _axis); matrix.preMult(tmp); } private: From 78a78a17ccfecf58fd9651b0e1de001d86d6d19c Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Fri, 3 Aug 2012 17:15:15 +0200 Subject: [PATCH 2/2] DeletionManager Class for safely deleting objects that may be active in different threads. This is now used in the implementation of spin animations. --- simgear/scene/model/animation.cxx | 26 +++++++++--- simgear/scene/util/CMakeLists.txt | 2 + simgear/scene/util/DeletionManager.cxx | 58 ++++++++++++++++++++++++++ simgear/scene/util/DeletionManager.hxx | 48 +++++++++++++++++++++ 4 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 simgear/scene/util/DeletionManager.cxx create mode 100644 simgear/scene/util/DeletionManager.hxx diff --git a/simgear/scene/model/animation.cxx b/simgear/scene/model/animation.cxx index 25dcb20a..7e94e5e6 100644 --- a/simgear/scene/model/animation.cxx +++ b/simgear/scene/model/animation.cxx @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -778,7 +779,8 @@ protected: // more than one camera. It is probably safe to overwrite the // reference values in multiple threads, but we'll provide a // threadsafe way to manage those values just to be safe. - struct ReferenceValues { + struct ReferenceValues : public osg::Referenced + { ReferenceValues(double t, double rot, double vel) : _time(t), _rotation(rot), _rotVelocity(vel) { @@ -800,19 +802,31 @@ void SpinAnimCallback::operator()(osg::Node* node, osg::NodeVisitor* nv) if (!_condition || _condition->test()) { double t = nv->getFrameStamp()->getReferenceTime(); double rps = _animationValue->getValue() / 60.0; - ReferenceValues* refval = static_cast(_referenceValues.get()); - if (!refval || refval->_rotVelocity != rps) { - ReferenceValues* newref = 0; - if (!refval) { + ref_ptr + refval(static_cast(_referenceValues.get())); + if (!refval || refval->_rotVelocity != rps) { + ref_ptr newref; + if (!refval.valid()) { // initialization newref = new ReferenceValues(t, 0.0, rps); } else { double newRot = refval->_rotation + (t - refval->_time) * refval->_rotVelocity; newref = new ReferenceValues(t, newRot, rps); } + // increment reference pointer, because it will be stored + // naked in _referenceValues. + newref->ref(); if (_referenceValues.assign(newref, refval)) { - delete refval; + if (refval.valid()) { + DeletionManager::instance()->addStaleObject(refval.get()); + refval->unref(); + } + } else { + // Another thread installed new values before us + newref->unref(); } + // Whatever happened, we can use the reference values just + // calculated. refval = newref; } double rotation = refval->_rotation + (t - refval->_time) * rps; diff --git a/simgear/scene/util/CMakeLists.txt b/simgear/scene/util/CMakeLists.txt index 670d21af..4c523c9d 100644 --- a/simgear/scene/util/CMakeLists.txt +++ b/simgear/scene/util/CMakeLists.txt @@ -2,6 +2,7 @@ include (SimGearComponent) set(HEADERS CopyOp.hxx + DeletionManager.hxx NodeAndDrawableVisitor.hxx Noise.hxx OsgMath.hxx @@ -28,6 +29,7 @@ set(HEADERS set(SOURCES CopyOp.cxx + DeletionManager.cxx NodeAndDrawableVisitor.cxx Noise.cxx PrimitiveUtils.cxx diff --git a/simgear/scene/util/DeletionManager.cxx b/simgear/scene/util/DeletionManager.cxx new file mode 100644 index 00000000..dbc24dad --- /dev/null +++ b/simgear/scene/util/DeletionManager.cxx @@ -0,0 +1,58 @@ +// DeletionManager.hxx -- defer deletions to a safe time +// +// Copyright (C) 2012 Tim Moore timoore33@gmail.com +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Library General Public License for more details. +// +// You should have received a copy of the GNU Library General Public +// License along with this library; if not, write to the +// Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, +// Boston, MA 02110-1301, USA. + +#include "DeletionManager.hxx" + +#include +#include +#include "OsgSingleton.hxx" + +namespace simgear +{ +using namespace osg; + +bool DeletionManager::handle(const osgGA::GUIEventAdapter& ea, + osgGA::GUIActionAdapter&, + osg::Object*, osg::NodeVisitor*) +{ + if (ea.getEventType() == osgGA::GUIEventAdapter::FRAME) + { + OpenThreads::ScopedLock lock(_mutex); + _staleObjects.resize(0); + } + return false; +} + +void DeletionManager::addStaleObject(Referenced* obj) +{ + OpenThreads::ScopedLock lock(_mutex); + _staleObjects.push_back(obj); +} + +void DeletionManager::install(Node* node) +{ + node->addEventCallback(instance()); +} + +DeletionManager* DeletionManager::instance() +{ + return SingletonRefPtr::instance(); +} +} + diff --git a/simgear/scene/util/DeletionManager.hxx b/simgear/scene/util/DeletionManager.hxx new file mode 100644 index 00000000..82d7410d --- /dev/null +++ b/simgear/scene/util/DeletionManager.hxx @@ -0,0 +1,48 @@ +// DeletionManager.hxx -- defer deletions to a safe time +// +// Copyright (C) 2012 Tim Moore timoore33@gmail.com +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Library General Public License for more details. +// +// You should have received a copy of the GNU Library General Public +// License along with this library; if not, write to the +// Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, +// Boston, MA 02110-1301, USA. + +#ifndef SIMGEAR_DELETIONMANAGER_HXX +#define SIMGEAR_DELETIONMANAGER_HXX 1 + +#include + +#include +#include +#include +#include +#include + +namespace simgear +{ +class DeletionManager : public osgGA::GUIEventHandler +{ +public: + virtual bool handle(const osgGA::GUIEventAdapter& ea, + osgGA::GUIActionAdapter& aa, + osg::Object* object, osg::NodeVisitor* nv); + void addStaleObject(osg::Referenced* obj); + static void install(osg::Node* node); + static DeletionManager* instance(); +protected: + OpenThreads::Mutex _mutex; + std::vector > _staleObjects; +}; +} + +#endif