Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi Chris, I'm not seeing a particularly easy way to deep-copy the rest of the stuff > in osg::DefaultUserDataContainer, either. The description list is a vector > of strings, which are deep-copied, so that's fine, but the object list is > harder as I'd need to cast the const off the CopyOp if I were to deep-copy > those objects (which is undesirable) or copy the CopyOp to get a non-const > version, which I can't do, as there's no way to determine if it's actually > a user-provided derived class. > I have had a chance to look at the DefaultUserDataContainer::DefaultUserDataContainer(const DefaultUserDataContainer& udc, const osg::CopyOp& copyop) implementation and it looks fine for me. Could it be that you are just interepreting things a bit different than the implementation provides. The CopyOp::Options::DEEP_COPY_ALL is what you should use if you want to copy all the contents of a subgraph, including a UserDataContainer. DEEP_COPY_ALL enables deep copy of all options. At this point I think the implementation is correct and there is nothing to fix. Robert. -- You received this message because you are subscribed to the Google Groups "OpenSceneGraph Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to osg-users+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/osg-users/510d2ba3-e4b2-4251-942b-7bd59443f5d4%40googlegroups.com. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi, It's been pointed out to me that the DefaultUserDataContainer object list can be a little bit more deeply copied with this change: Code: --- a/src/osg/UserDataContainer.cpp +++ b/src/osg/UserDataContainer.cpp @@ -56,7 +56,10 @@ DefaultUserDataContainer::DefaultUserDataContainer(const DefaultUserDataContaine itr != udc._objectList.end(); ++itr) { -_objectList.push_back(copyop(itr->get())); +if (copyop.getCopyFlags()::CopyOp::DEEP_COPY_USERDATA) +_objectList.push_back(osg::clone(itr->get(), copyop)); +else +_objectList.push_back(copyop(itr->get())); } } This will make _objectList at least point to copies of the objects instead of literally the same objects, which is a step in the right direction, but those copies will still be shallow copies. Cheers, Chris -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=76837#76837 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi, I'm not seeing a particularly easy way to deep-copy the rest of the stuff in osg::DefaultUserDataContainer, either. The description list is a vector of strings, which are deep-copied, so that's fine, but the object list is harder as I'd need to cast the const off the CopyOp if I were to deep-copy those objects (which is undesirable) or copy the CopyOp to get a non-const version, which I can't do, as there's no way to determine if it's actually a user-provided derived class. Cheers, Chris -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=76755#76755 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi, Is there a way to copy something if all I have is a ref_ptr to Referenced? I see that Object has a clone method, but Referenced doesn't. This poses a potential problem with deep-copying user data, as osg::DefaultUserDataContainer::_userData is a ref_ptr. Cheers, Chris -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=76753#76753 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi Chris, On Mon, 23 Sep 2019 at 17:33, Chris Djali wrote: > Good, but would the new enum entry be for the current behaviour or the new > behaviour? If the current behaviour is a bug, then it makes sense to only > keep the old behaviour as the new enum value so everyone gets the fix, but > conceivably there could be applications that rely on the current behaviour > not changing, so would want the same name and value to keep the same > behaviour. > Relying upon buggy behavior is always a dangerous thing, especially if you don't know about it. Unfortunately there is no easy way to know whether there are any applications that are relying upon buggy behavior. Given UserData is a relatively niche feature I think we are probably not taking too much of risk with fixing it's behavior. Cheers Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi, Good, but would the new enum entry be for the current behaviour or the new behaviour? If the current behaviour is a bug, then it makes sense to only keep the old behaviour as the new enum value so everyone gets the fix, but conceivably there could be applications that rely on the current behaviour not changing, so would want the same name and value to keep the same behaviour. Cheers, Chris -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=76733#76733 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi Chris, On Sun, 22 Sep 2019 at 18:54, Chris Djali wrote: > How do you want backwards compatibility to be handled? The current > behaviour could just be dropped, or it could be available under a new flag > name like SHALLOW_COPY_USERDATA (potentially with the same value as the > current deep copy one), or the new behaviour could be under a new flag name > like DEEP_COPY_USERDATA_AND_CONTENTS. > It's been a while since I worked on this part of the OSG, but a new enum entry sounds sensible. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi, How do you want backwards compatibility to be handled? The current behaviour could just be dropped, or it could be available under a new flag name like SHALLOW_COPY_USERDATA (potentially with the same value as the current deep copy one), or the new behaviour could be under a new flag name like DEEP_COPY_USERDATA_AND_CONTENTS. Cheers, Chris -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=76727#76727 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] DEEP_COPY_USERDATA isn't that deep
Hi Chris, My guess is that the deep copy support wasn't built in when I wrote it. Different parts of the OSG developed at different points so sometimes you get little inconsistencies. Feel free to modify the code to do a deep copy when requested and generate a PR for it. Robert. On Fri, 20 Sep 2019 at 19:54, Chris Djali wrote: > Hi, > > Is it intentional that using osg::CopyOp::DEEP_COPY_USERDATA copies the > UserDataContainer, but doesn't deep-copy the actual things in it unless > osg::CopyOp::DEEP_COPY_OBJECTS is also enabled? This means that things can > be added or removed, but changes to existing things are shared. > > We have a situation where we only want to deep-copy the userdata and not > anything else, so can't set the deep-copy objects flag. This could be > supported either by ORing objects into the copy op in the userdata > container copy constructor, or by adding another copy op for copying > userdata even more deeply, otherwise we're going to have to manually > implement a less pretty workaround. > > Cheers, > Chris > > -- > Read this topic online here: > http://forum.openscenegraph.org/viewtopic.php?p=76725#76725 > > > > > > ___ > 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] DEEP_COPY_USERDATA isn't that deep
Hi, Is it intentional that using osg::CopyOp::DEEP_COPY_USERDATA copies the UserDataContainer, but doesn't deep-copy the actual things in it unless osg::CopyOp::DEEP_COPY_OBJECTS is also enabled? This means that things can be added or removed, but changes to existing things are shared. We have a situation where we only want to deep-copy the userdata and not anything else, so can't set the deep-copy objects flag. This could be supported either by ORing objects into the copy op in the userdata container copy constructor, or by adding another copy op for copying userdata even more deeply, otherwise we're going to have to manually implement a less pretty workaround. Cheers, Chris -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=76725#76725 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org