From 9bfa6ac1a117a265543d7090f8e9a77ebfed4095 Mon Sep 17 00:00:00 2001 From: Thomas Geymayer Date: Sat, 8 Jun 2013 11:28:49 +0200 Subject: [PATCH] Restructure Canvas/PropertyBasedElement Nodes inside the osg scenegraph now hold a strong reference to the according canvas element. Canvas elements in turn now only hold a weak reference to the according node. This simplifies for example the canvas::Group as it does not need to keep a list of children on its own anymore. This is now stored inside the scenegraph (as it was already before). The restructuring will also allow to use a canvas::Group for the canvas gui inside FlightGear and share for example the handling of stacking based on z-index. --- simgear/canvas/Canvas.cxx | 27 ++-- simgear/canvas/Canvas.hxx | 3 +- simgear/canvas/elements/CanvasElement.cxx | 38 ++++-- simgear/canvas/elements/CanvasElement.hxx | 32 +++-- simgear/canvas/elements/CanvasGroup.cxx | 146 +++++++++++----------- simgear/canvas/elements/CanvasGroup.hxx | 7 +- simgear/canvas/elements/CanvasImage.cxx | 23 ++-- simgear/props/PropertyBasedElement.cxx | 19 +++ simgear/props/PropertyBasedElement.hxx | 15 +++ simgear/props/PropertyBasedMgr.cxx | 8 ++ 10 files changed, 196 insertions(+), 122 deletions(-) diff --git a/simgear/canvas/Canvas.cxx b/simgear/canvas/Canvas.cxx index 37877d5f..25345e76 100644 --- a/simgear/canvas/Canvas.cxx +++ b/simgear/canvas/Canvas.cxx @@ -75,9 +75,13 @@ namespace canvas } //---------------------------------------------------------------------------- - Canvas::~Canvas() + void Canvas::onDestroy() { - + if( _root_group ) + { + _root_group->clearEventListener(); + _root_group->onDestroy(); + } } //---------------------------------------------------------------------------- @@ -111,19 +115,6 @@ namespace canvas return _texture.serviceable(); } - //---------------------------------------------------------------------------- - void Canvas::destroy() - { - if( _root_group ) - _root_group->clearEventListener(); - - // TODO check if really not in use anymore - getProps()->getParent() - ->removeChild( getProps()->getName(), - getProps()->getIndex(), - false ); - } - //---------------------------------------------------------------------------- void Canvas::addParentCanvas(const CanvasWeakPtr& canvas) { @@ -133,7 +124,8 @@ namespace canvas ( SG_GENERAL, SG_WARN, - "Canvas::addParentCanvas: got an expired parent: " << _node->getPath() + "Canvas::addParentCanvas(" << _node->getPath(true) << "): " + "got an expired parent!" ); return; } @@ -150,7 +142,8 @@ namespace canvas ( SG_GENERAL, SG_WARN, - "Canvas::addChildCanvas: got an expired child: " << _node->getPath() + "Canvas::addChildCanvas(" << _node->getPath(true) << "): " + " got an expired child!" ); return; } diff --git a/simgear/canvas/Canvas.hxx b/simgear/canvas/Canvas.hxx index 3504b46c..f132a7cf 100644 --- a/simgear/canvas/Canvas.hxx +++ b/simgear/canvas/Canvas.hxx @@ -71,7 +71,7 @@ namespace canvas typedef osg::ref_ptr CullCallbackPtr; Canvas(SGPropertyNode* node); - virtual ~Canvas(); + virtual void onDestroy(); void setSystemAdapter(const SystemAdapterPtr& system_adapter); SystemAdapterPtr getSystemAdapter() const; @@ -80,7 +80,6 @@ namespace canvas CanvasMgr* getCanvasMgr() const; bool isInit() const; - void destroy(); /** * Add a canvas which should be marked as dirty upon any change to this diff --git a/simgear/canvas/elements/CanvasElement.cxx b/simgear/canvas/elements/CanvasElement.cxx index af725d57..522a2ef7 100644 --- a/simgear/canvas/elements/CanvasElement.cxx +++ b/simgear/canvas/elements/CanvasElement.cxx @@ -43,19 +43,40 @@ namespace canvas const std::string NAME_TRANSFORM = "tf"; //---------------------------------------------------------------------------- - void Element::removeListener() + Element::OSGUserData::OSGUserData(ElementPtr element): + element(element) { - _node->removeChangeListener(this); + } //---------------------------------------------------------------------------- Element::~Element() { - removeListener(); + } + + //---------------------------------------------------------------------------- + void Element::setSelf(const PropertyBasedElementPtr& self) + { + PropertyBasedElement::setSelf(self); + + _transform->setUserData + ( + new OSGUserData(boost::static_pointer_cast(self)) + ); + } + + //---------------------------------------------------------------------------- + void Element::onDestroy() + { + if( !_transform.valid() ) + return; + + // The transform node keeps a reference on this element, so ensure it is + // deleted. BOOST_FOREACH(osg::Group* parent, _transform->getParents()) { - parent->removeChild(_transform); + parent->removeChild(_transform.get()); } } @@ -239,14 +260,15 @@ namespace canvas } //---------------------------------------------------------------------------- - osg::ref_ptr Element::getMatrixTransform() + osg::MatrixTransform* Element::getMatrixTransform() { - return _transform; + return _transform.get(); } - osg::ref_ptr Element::getMatrixTransform() const + //---------------------------------------------------------------------------- + osg::MatrixTransform const* Element::getMatrixTransform() const { - return _transform; + return _transform.get(); } //---------------------------------------------------------------------------- diff --git a/simgear/canvas/elements/CanvasElement.hxx b/simgear/canvas/elements/CanvasElement.hxx index b7d06f75..693aec88 100644 --- a/simgear/canvas/elements/CanvasElement.hxx +++ b/simgear/canvas/elements/CanvasElement.hxx @@ -45,6 +45,18 @@ namespace canvas public PropertyBasedElement { public: + + /** + * Store pointer to window as user data + */ + class OSGUserData: + public osg::Referenced + { + public: + ElementPtr element; + OSGUserData(ElementPtr element); + }; + typedef boost::function StyleSetterFunc; typedef boost::function @@ -61,20 +73,14 @@ namespace canvas std::string type; ///!< Interpolation type }; - /** - * Remove the property listener of the element. - * - * You will need to call the appropriate methods (#childAdded, - * #childRemoved, #valueChanged) yourself to ensure the element still - * receives the needed events. - */ - void removeListener(); - /** * */ virtual ~Element() = 0; + virtual void setSelf(const PropertyBasedElementPtr& self); + virtual void onDestroy(); + ElementWeakPtr getWeakPtr() const; /** @@ -102,8 +108,8 @@ namespace canvas */ bool isVisible() const; - osg::ref_ptr getMatrixTransform(); - osg::ref_ptr getMatrixTransform() const; + osg::MatrixTransform* getMatrixTransform(); + osg::MatrixTransform const* getMatrixTransform() const; virtual void childAdded( SGPropertyNode * parent, SGPropertyNode * child ); @@ -154,8 +160,8 @@ namespace canvas uint32_t _attributes_dirty; bool _transform_dirty; - osg::ref_ptr _transform; - std::vector _transform_types; + osg::observer_ptr _transform; + std::vector _transform_types; Style _style; std::vector _bounding_box; diff --git a/simgear/canvas/elements/CanvasGroup.cxx b/simgear/canvas/elements/CanvasGroup.cxx index 4b6591f9..e0ad87e0 100644 --- a/simgear/canvas/elements/CanvasGroup.cxx +++ b/simgear/canvas/elements/CanvasGroup.cxx @@ -96,25 +96,13 @@ namespace canvas //---------------------------------------------------------------------------- ElementPtr Group::getChild(const SGPropertyNode* node) { - ChildList::iterator child = findChild(node); - if( child == _children.end() ) - return ElementPtr(); - - return child->second; + return findChild(node, ""); } //---------------------------------------------------------------------------- ElementPtr Group::getChild(const std::string& id) { - for( ChildList::iterator child = _children.begin(); - child != _children.end(); - ++child ) - { - if( child->second->get("id") == id ) - return child->second; - } - - return ElementPtr(); + return findChild(0, id); } //---------------------------------------------------------------------------- @@ -147,10 +135,10 @@ namespace canvas { std::vector groups; - BOOST_FOREACH( ChildList::value_type child, _children ) + for(size_t i = 0; i < _transform->getNumChildren(); ++i) { - const ElementPtr& el = child.second; - if( el->getProps()->getStringValue("id") == id ) + const ElementPtr& el = getChildByIndex(i); + if( el->get("id") == id ) return el; GroupPtr group = boost::dynamic_pointer_cast(el); @@ -171,8 +159,9 @@ namespace canvas //---------------------------------------------------------------------------- void Group::clearEventListener() { - BOOST_FOREACH( ChildList::value_type child, _children ) - child.second->clearEventListener(); + + for(size_t i = 0; i < _transform->getNumChildren(); ++i) + getChildByIndex(i)->clearEventListener(); Element::clearEventListener(); } @@ -180,8 +169,8 @@ namespace canvas //---------------------------------------------------------------------------- void Group::update(double dt) { - BOOST_FOREACH( ChildList::value_type child, _children ) - child.second->update(dt); + for(size_t i = 0; i < _transform->getNumChildren(); ++i) + getChildByIndex(i)->update(dt); Element::update(dt); } @@ -190,9 +179,9 @@ namespace canvas bool Group::traverse(EventVisitor& visitor) { // Iterate in reverse order as last child is displayed on top - BOOST_REVERSE_FOREACH( ChildList::value_type child, _children ) + for(size_t i = _transform->getNumChildren(); i --> 0;) { - if( child.second->accept(visitor) ) + if( getChildByIndex(i)->accept(visitor) ) return true; } return false; @@ -210,9 +199,9 @@ namespace canvas return false; bool handled = false; - BOOST_FOREACH( ChildList::value_type child, _children ) + for(size_t i = 0; i < _transform->getNumChildren(); ++i) { - if( child.second->setStyle(style) ) + if( getChildByIndex(i)->setStyle(style) ) handled = true; } @@ -224,16 +213,17 @@ namespace canvas { osg::BoundingBox bb; - BOOST_FOREACH( ChildList::value_type child, _children ) + for(size_t i = 0; i < _transform->getNumChildren(); ++i) { - if( !child.second->getMatrixTransform()->getNodeMask() ) + const ElementPtr& child = getChildByIndex(i); + if( !child->getMatrixTransform()->getNodeMask() ) continue; bb.expandBy ( - child.second->getTransformedBounds + child->getTransformedBounds ( - child.second->getMatrixTransform()->getMatrix() * m + child->getMatrixTransform()->getMatrix() * m ) ); } @@ -255,10 +245,9 @@ namespace canvas // Add to osg scene graph... _transform->addChild( element->getMatrixTransform() ); - _children.push_back( ChildList::value_type(child, element) ); // ...and ensure correct ordering - handleZIndexChanged( --_children.end() ); + handleZIndexChanged(element); return; } @@ -279,8 +268,8 @@ namespace canvas if( _child_factories.find(node->getNameString()) != _child_factories.end() ) { - ChildList::iterator child = findChild(node); - if( child == _children.end() ) + ElementPtr child = getChild(node); + if( !child ) SG_LOG ( SG_GL, @@ -289,8 +278,9 @@ namespace canvas ); else { - _transform->removeChild( child->second->getMatrixTransform() ); - _children.erase(child); + // Remove child from the scenegraph (this automatically invalidates the + // reference on the element hold by the MatrixTransform) + child->onDestroy(); } } else @@ -306,56 +296,52 @@ namespace canvas { if( node->getParent()->getParent() == _node && node->getNameString() == "z-index" ) - return handleZIndexChanged( findChild(node->getParent()), + return handleZIndexChanged( getChild(node->getParent()), node->getIntValue() ); } //---------------------------------------------------------------------------- - void Group::handleZIndexChanged(ChildList::iterator child, int z_index) + void Group::handleZIndexChanged(ElementPtr child, int z_index) { - if( child == _children.end() ) + if( !child ) return; - osg::Node* tf = child->second->getMatrixTransform(); - int index = _transform->getChildIndex(tf), - index_new = index; - - ChildList::iterator next = child; - ++next; + osg::ref_ptr tf = child->getMatrixTransform(); + size_t index = _transform->getChildIndex(tf), + index_new = index; - while( next != _children.end() - && next->first->getIntValue("z-index", 0) <= z_index ) + for(;; ++index_new) { - ++index_new; - ++next; - } + if( index_new + 1 == _transform->getNumChildren() ) + break; - if( index_new != index ) - { - _children.insert(next, *child); + // Move to end of block with same index (= move upwards until the next + // element has a higher index) + if( getChildByIndex(index_new + 1)->get("z-index", 0) > z_index ) + break; } - else + + if( index_new == index ) { - ChildList::iterator prev = child, - check = child; - while( check != _children.begin() - && (--check)->first->getIntValue("z-index", 0) > z_index ) + // We were not able to move upwards so now try downwards + for(;; --index_new) { - --index_new; - --prev; + if( index_new == 0 ) + break; + + // Move to end of block with same index (= move downwards until the + // previous element has the same or a lower index) + if( getChildByIndex(index_new - 1)->get("z-index", 0) <= z_index ) + break; } if( index == index_new ) return; - - _children.insert(prev, *child); } _transform->removeChild(index); _transform->insertChild(index_new, tf); - _children.erase(child); - SG_LOG ( SG_GENERAL, @@ -365,14 +351,34 @@ namespace canvas } //---------------------------------------------------------------------------- - Group::ChildList::iterator Group::findChild(const SGPropertyNode* node) + ElementPtr Group::getChildByIndex(size_t index) const { - return std::find_if - ( - _children.begin(), - _children.end(), - boost::bind(&ChildList::value_type::first, _1) == node - ); + OSGUserData* ud = + static_cast(_transform->getChild(index)->getUserData()); + return ud->element; + } + + //---------------------------------------------------------------------------- + ElementPtr Group::findChild( const SGPropertyNode* node, + const std::string& id ) const + { + for(size_t i = 0; i < _transform->getNumChildren(); ++i) + { + ElementPtr el = getChildByIndex(i); + + if( node ) + { + if( el->getProps() == node ) + return el; + } + else + { + if( el->get("id") == id ) + return el; + } + } + + return ElementPtr(); } } // namespace canvas diff --git a/simgear/canvas/elements/CanvasGroup.hxx b/simgear/canvas/elements/CanvasGroup.hxx index 43fa3681..f3ba781e 100644 --- a/simgear/canvas/elements/CanvasGroup.hxx +++ b/simgear/canvas/elements/CanvasGroup.hxx @@ -99,15 +99,16 @@ namespace canvas protected: static ElementFactories _child_factories; - ChildList _children; virtual void childAdded(SGPropertyNode * child); virtual void childRemoved(SGPropertyNode * child); virtual void childChanged(SGPropertyNode * child); - void handleZIndexChanged(ChildList::iterator child, int z_index = 0); + void handleZIndexChanged(ElementPtr child, int z_index = 0); - ChildList::iterator findChild(const SGPropertyNode* node); + ElementPtr getChildByIndex(size_t index) const; + ElementPtr findChild( const SGPropertyNode* node, + const std::string& id ) const; }; } // namespace canvas diff --git a/simgear/canvas/elements/CanvasImage.cxx b/simgear/canvas/elements/CanvasImage.cxx index 077d0042..5500f32d 100644 --- a/simgear/canvas/elements/CanvasImage.cxx +++ b/simgear/canvas/elements/CanvasImage.cxx @@ -350,22 +350,27 @@ namespace canvas //---------------------------------------------------------------------------- void Image::setSrcCanvas(CanvasPtr canvas) { - if( !_src_canvas.expired() ) - _src_canvas.lock()->removeParentCanvas(_canvas); - if( !_canvas.expired() ) - _canvas.lock()->removeChildCanvas(_src_canvas); + CanvasPtr src_canvas = _src_canvas.lock(), + self_canvas = _canvas.lock(); + + if( src_canvas ) + src_canvas->removeParentCanvas(self_canvas); + if( self_canvas ) + self_canvas->removeChildCanvas(src_canvas); - _src_canvas = canvas; + _src_canvas = src_canvas = canvas; _attributes_dirty |= SRC_CANVAS; _geom->setCullCallback(canvas ? new CullCallback(canvas) : 0); - if( !_src_canvas.expired() ) + if( src_canvas ) { setupDefaultDimensions(); - _src_canvas.lock()->addParentCanvas(_canvas); - if( !_canvas.expired() ) - _canvas.lock()->addChildCanvas(_src_canvas); + if( self_canvas ) + { + self_canvas->addChildCanvas(src_canvas); + src_canvas->addParentCanvas(self_canvas); + } } } diff --git a/simgear/props/PropertyBasedElement.cxx b/simgear/props/PropertyBasedElement.cxx index b1667e8c..d0e4f461 100644 --- a/simgear/props/PropertyBasedElement.cxx +++ b/simgear/props/PropertyBasedElement.cxx @@ -30,10 +30,29 @@ namespace simgear //------------------------------------------------------------------------------ PropertyBasedElement::~PropertyBasedElement() + { + onDestroy(); + removeListener(); + } + + //---------------------------------------------------------------------------- + void PropertyBasedElement::removeListener() { _node->removeChangeListener(this); } + //---------------------------------------------------------------------------- + void PropertyBasedElement::destroy() + { + if( !_node ) + return; + + // TODO check if really not in use anymore + if( _node->getParent() ) + _node->getParent() + ->removeChild(_node->getName(), _node->getIndex(), false); + } + //------------------------------------------------------------------------------ SGConstPropertyNode_ptr PropertyBasedElement::getProps() const { diff --git a/simgear/props/PropertyBasedElement.hxx b/simgear/props/PropertyBasedElement.hxx index bdae0537..b2ccd30b 100644 --- a/simgear/props/PropertyBasedElement.hxx +++ b/simgear/props/PropertyBasedElement.hxx @@ -43,6 +43,20 @@ namespace simgear PropertyBasedElement(SGPropertyNode* node); virtual ~PropertyBasedElement(); + /** + * Remove the property listener of the element. + * + * You will need to call the appropriate methods (#childAdded, + * #childRemoved, #valueChanged) yourself to ensure the element still + * receives the needed events. + */ + void removeListener(); + + /** + * Destroys this element (removes node from property tree) + */ + void destroy(); + virtual void update(double delta_time_sec) = 0; SGConstPropertyNode_ptr getProps() const; @@ -67,6 +81,7 @@ namespace simgear } virtual void setSelf(const PropertyBasedElementPtr& self); + virtual void onDestroy() {}; protected: diff --git a/simgear/props/PropertyBasedMgr.cxx b/simgear/props/PropertyBasedMgr.cxx index 46f3c736..621d11db 100644 --- a/simgear/props/PropertyBasedMgr.cxx +++ b/simgear/props/PropertyBasedMgr.cxx @@ -126,6 +126,7 @@ namespace simgear _elements.resize(index + 1); } else if( _elements[index] ) + { SG_LOG ( SG_GENERAL, @@ -133,6 +134,10 @@ namespace simgear _name_elements << "[" << index << "] already exists!" ); + // Give anything holding a reference to this element to release it + _elements[index]->onDestroy(); + } + PropertyBasedElementPtr el = _element_factory(child); el->setSelf( el ); _elements[index] = el; @@ -158,8 +163,11 @@ namespace simgear "can't removed unknown " << _name_elements << "[" << index << "]!" ); else + { // remove the element... + _elements[index]->onDestroy(); _elements[index].reset(); + } } } // namespace simgear -- 2.39.5