Hi Robert, hi all, Sorry for being silent these days... I just got back on the OSG development.
Well I've read your posts and I must admit I'm a bit disapointed because proposed changes seem rather radical to me, and implies some features will be dropped/reduced... > The backwards compatibility support for UserData looks like it'll > introduce a memory leak Ok. You're right. Sorry! > For the existing UserData and DescriptionList I'm currently inclined > to hardwire this into ValueContainer as getUserData() and > getDescitionList(). This is a bit clunky but it'll be less > complicated trying to adapt both of these into osg::Object. I did not understand why you hardwired userData and descriptionList instead of letting them be metadata as any other. I don't think this is less complicated. On the contrary, I feel we must "factorize" code as much as possible. And this seems akward to me because this forces to have more lines of code, and thus have more things to debug. I feel more confortable with less methods and less lines of code, don't you? BTW, moving descriptionList availability from Node to Object seems not a good idea as we're trying to make them disapear. However this may centralize metadata to one place, so why not, as long as deprecation is clear and explicit...? This is subject to discussion! > The idea of having a ValueBase class which only provides the > Referenced class and a local clone() seems rather too minimal. Object > has clone functionality built into it and hooks into the OSG's > serializers I'm quite in favor of using osg::Object as a base for metadata, especially for serialization. But the fact it already stores a name is problemetic when you want your data to be indexed by name, or if you wish your data be put in a database backend. For instance, using a map would require names duplicaion (one in the map, one in osg::Object), or a strange map<string*, Object, string pointer compare> thing. Any opinion/idea on this? Mine is that an osg::Object without name would be ideal, but this would bring another class! Unless we move the name as a metadata too? Well this could be a nice idea: some meta names would be reserved, and especially one for the name, one for userData, and one for descriptionList. Thoughts? > I also wonder if perhaps ValueContainer and ValueContainerMap might > just become oen class ValueMap I'm not in favor of this. Grégoire and I explicitely made two classes to let the user choose the container. Especially we wanted to let database backend (a requirement of some OSG users) be easy to plug. As Peter told, I guess the "ability to subclass a custom UserDataContainer implementation" is important. However I uderstand your point of view on sharing containers, especially regarding to serialization. So what about something not sharable, but which user can subclass? > Another route for adding extra flexiblity would be to just subclass > your own user data scheme from osg::Object and assign it as UserData > or as UserObject The proposal of putting a user-defined container as a metadata may solve the problem but seems also a bit a burden for the user. > The container used within ValueContainer could be an > std::vector<osg::ref_ptr<osg::Object>> to allow indexing via a > conventional array style access The proposal of a vector<ref_ptr<Object> > seems okay for a default implementation (instead of a map). You're right about trying "to keep the class count down" (and my implementation is certainely not optimal), but I keep thinking the container should be customizable and I would keep the ValueContainer class. As a conclusion, I can say this subject is not closed yet! ;-) Cheers, Sukender PVLE - Lightweight cross-platform game engine - http://pvle.sourceforge.net/ ----- "Robert Osfield" <robert.osfi...@gmail.com> a écrit : > Hi Peter, > > I'm open to suggestions on making the implementation more flexible, > however, any extra complexity has to be strongly justified - i.e. > with > concrete usage case, so could you flesh out what you'd want to do > with > the subclassing from UserDataContainer. > > Another route for adding extra flexiblity would be to just subclass > your own user data scheme from osg::Object and assign it as UserData > or as UserObject, then do a dynamic cast to get access to your custom > implementation. Currenly you can share UserData and UserObjects > between their owner osg::Object. > > As for sharing whole UserDataContainer objects between their owner > osg::Objects, this is something I considered and implemented > initially > but then decided to make things simpler and just have an internal > data > container subclassed from osg::Referenced. Adding the capability of > sharing UserDataContainer won't be difficult, but will require us to > push the declaration of the UserDataContainer into the public scope > something that will constain how flexibile we can be with the > implementation and it's interface, and would require us to implement > UserDataContainer as an osg::Object rather than Referenced so we > could > get the cloning support that osg::Object provides. > > A subclassable UserDataContainer would also push us towards > subclassing from osg::Object, which isn't difficult, but it does open > the doors users nesting UserDataContainer within a UserDataContainer, > now this could be seen as an advantage or just extra complexity. > Initially I implemented UserDataContainer as a subclass from > osg::Object but then pulled back as the public interface for access > to > the > internal UserDataContainer conflicted with the UserDataContainer's > own > access functions - I had to make the osg::Object methods virtual and > override then in UserDataContainer and it ended up being rather > messy. > > The other issue that a subclassable UserDataContainer brings is that > what happens during serialization - when we writing out we could use > the standard UserDataContainer access methods, but when reading back > in you'd want to construct explitly the custom UserDataContainer for > each node than needs it. You can probably solve this by serializng > the UserDataContainer itself, and have a custom wrapper to implement > the CustomUserDataContainer so that the serializer creates everything > appropriately. This is fine but it leaves the question of how to map > the old UserData and Description list in a backwards compatible way. > > For now I am going to get the serializers working for > .osgb/.osgt,/osgx as they are and if that all works I'll check my > work > in so that others can test the build and runtime side. Once we have > the simplicist implementation working fine we'll be able to a base > for > concrete proposals for adding even greater flexibility, but be > prepared to fight your corner if the extra flexibility comes with > extra complexity ;-) > > Robert. > > On Tue, May 31, 2011 at 5:36 PM, Peter Amstutz > <peter.amst...@tseboston.com> wrote: > > Hi Robert, > > > > This looks pretty straightforward and I agree that less complexity > is > > good. However a would modify it slightly to support two related > > features I have been lobbying for, which are the ability to share > > UserDataContainers between osg::Objects, and the ability to subclass > a > > custom UserDataContainer implementation. > > > > The idea is that there is generally not a 1:1 correlation between > the > > application entity model and the scene graph. Because of this, one > > often wants to be able to share or inherit metadata information, or > look > > up metadata on the fly to avoid copying large amounts of > application > > data into the scene graph. > > > > I think it feasible to add one level of indirection without adding > too > > much complexity. For example: > > > > - Make osg::UserDataContainer a pure abstract class. > > > > class OSG_EXPORT UserDataContainer : public osg::Referenced > > { > > public: > > void addUserObject(Object* owner, Object* obj) = 0; > > void setUserObject(Object* owner, unsigned int i, Object* > obj) = 0; > > void removeUserObject(Object* owner, unsigned int i); > > > > unsigned int getUserObjectIndex(Object* owner, const > osg::Object* obj) const = 0; > > unsigned int getUserObjectIndex(Object* owner, const > std::string& name) const = 0; > > > > Object* getUserObject(Object* owner, unsigned int i) = 0; > > const Object* getUserObject(Object* owner, unsigned int i) > const = 0; > > > > Object* getUserObject(Object* owner, const std::string& name) > = 0; > > const Object* getUserObject(Object* owner, const std::string& > name) const = 0; > > > > unsigned int getNumUserObjects(Object* owner) const = 0; > > > > }; > > > > - Provide a default implementation which doesn't do anything with > the "owner" field > > class OSG_EXPORT DefaultUserDataContainer : public > UserDataContainer > > { > > ref_ptr<Referenced> _userData; > > b DescriptionList _descriptionList; > > ObjectList _objectList; > > public: > > void addUserObject(Object* owner, Object* obj) { > > _objectList.push_back(obj); > > } > > /* etc */ > > }; > > > > > > - The methods implementated in osg::Object just forward to the > container along with the owner: > > > > void osg::Object::addUserObject(Object* obj) { > > if (!_container) { > > _container = new osg::DefaultUserDataContainer(); > > } > > _container->addUserObject(this, obj); > > } > > > > void osg::Object::setUserObject(Object* owner, unsigned int > i, Object* obj) { > > if (!_container) { > > _container = new osg::DefaultUserDataContainer(); > > } > > _container->setUserObject(this, i, obj); > > } > > > > /* etc */ > > > > - osg::Object also gains a couple of new methods: > > osg::UserDataContainer* osg::Object::getUserDataContainer(); > > osg::Object::setUserDataContainer(osg::UserDataContainer*); > > > > > > Does this seem reasonable? > > > > - Peter > > > > -- > > Peter Amstutz > > Senior Software Engineer > > Technology Solutions Experts > > Natick, MA > > 02131 > > > > _______________________________________________ > > osg-users mailing list > > osg-users@lists.openscenegraph.org > > > http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org > > > _______________________________________________ > osg-users mailing list > osg-users@lists.openscenegraph.org > http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org _______________________________________________ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
