Hi All, I wasn't planning to revist the user data implementation this week, but the weaknesses in design has been lurking at the back of my mind, it just wasn't as clean as I'd have liked. With the feature freeze for 3.0 happening at the end of next week I couldn't help but want to get the implementation right for the stable release, so this morning I revisted the implementation.
The key awkwardness in the version checked in earlier this week was that osg::Object had a number of virtual access methods for user objects with this overriden by the UserDataContainer, but also the osg::Object agregated the UserDataContainer, so the releationship was one of both inheritance and composition. It worked and was easier to use, but it wasn't clean conceptually. Peter picked up on this design/implementation awkwardness and suggested that this could be resolved by moving the virtual access methods for UserDataContainer out of osg::Object and into UserDataContainer make this class a pure virtual class, with a DefaultUserDataContainer subclassing from this to provide the standard implementation. Moving the access methods out of osg::Object does mean that accessing the user objects will require an checks against and usage of object.getUserDataContainer(), and it introduces an implementation issue of osg::Object having a ref_ptr<UserDataContainer> when UserDataContainer is a subclass from osg::Object, which introduces a potential definition order problem for getting the code compiled. When mulling over the design I came to the conclusion that Peter's suggestion was a more elegant design, I had originall consider such an approach but the definition order issue is one that I wasn't original comfortable with so didn't pursue it. With thinking about the definition order issue I realised it could easily be walked around by using a C pointer to the UserDataContainer in osg::Object and have the osg::Object::setUserDataContainer(UserDataContainer*) implementation manage the ref()/unref() rather than rely upon osg::ref_ptr<> that requires the class definiation for it be able to be compiled. With the C pointer trick in the bag it became a simple refactor of the osg::Object and osg::UserDataContainer interfaces and implementations, with the s/getUserObject(..) access methods moving from osg::Object into pure virtual UserDataContainer base class. The setUserData/getUserData() stay put in osg::Object but are virtual to allow the UserDataContainer to override them. The Description list access methods were then moved back into osg::Node, keeping the osg::Object cleaner and avoiding the conceptual overlap between osg::Object and UserDataContainer. Finally a DefaultUserDataContainer implementation is provided, and is almost identical to my original UserDataContainer. Now osg::Object has one set of access method that I didn't move - the templated s/getUserValue(..) methods, I could have moved these to UserDataContainer but as the methods are implementated with forward declared templates there isn't an order definition issue so the implementation is free to provided separately and use the full UserDataContainer interface. In terms of convinience it's couldn't be simpler than having the g/setUserValue methods available in osg::Object, and the osguserdata example itself didn't have to be modified at all where s/getUserValue access was used so it works well. The only parts of the osguserdata example I have had to modify to account for these changes is the adding of the osg::Geometry object as a UserObject, and output of the UserObject list as both of these require the access methods only available in osg::UserDataContainer. I have been able to update the serialization support to handle the new implementation so everything functions as well as it did on the previous rev - only with a cleaner class relationship. These changes are now checked into svn/trunk so please test them out. One suggestion of Peter's that I have gone with is passing in an osg::Object* to the UserDataContainer methods that would have enabled him to share a single custom UserDataContainer implementation between objects in the scene and have the custom container work the correct data for each object. I rejected this suggestion in my previous email and still feel it's inappropriate. If one does want centralized access to user data associated with objects then there is no reason to place this in the scene graph - if you are going to have a data structure that is mapping Object pointers to internal data objects then it's global operation already, it's not something that needs or should be distributed throughut the scene graph. Even if one did create such a global mapping I'm doubtful that a Object pointer would even be the appropriate key is to use to access it as pointers to Object change when being serialized in and out - so re-run your app and all your pointers are different for the same scene graph objects. For this type of task I would lean towards have a user manage unique ObjectID that is used as the key, and the natural place to put this would be... in a custom UserDataContainer. Right, I'm hoping that this user object/meta object support is now good to go for 3.0, so I'm heading off to tackle other topics. Please test it - after lots of discussion on this thread it's going rather quite now that an implementation is live in svn/trunk. Cheers, Robert _______________________________________________ osg-users mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
