Bug in CloneImportExport: Identical objects in hierarchy

Hi everyone,



here’s more news from the bug tracking department. Well, actually not so new, as this issue has been brought up before, but as the last post related to this problem dates back to October '06, I thought I’d post this again:



There is (still) a bug in CloneImportExport that causes nodes in cloned scene graph branches to share the same objects for their transformations if the transformations have identical values. That is, if you would clone a scene graph hierarchy that looks like this:


- Node1 (Translation1 [0, 0, 0])
  - Node2 (Translation2 [0, 0, 0])
    + Geometry


...then the vectors used for Translation1 and Translation2 would point to the same objects in memory in the cloned hierarchy. So, if you would change the values of Transformation1 (e.g. shift it by (1, 0, 0)), they would also change in Transformation2 (e.g. resulting in a shift of (2, 0, 0) of the Geometry).

This bug obviously results from the use of a HashMap in CloneImportExport that is used to get the clone objects. When loading a clone of the stored object (or scene graph branch), the cloned Savables are stored in a HashMap when they are created, and re-used when an equal object is needed. As long as this equality is checked by using the equals(Object) and hashCode() methods of the objects, objects that are equal according to these methods will in fact be identical in the cloned scene graph hierarchy.

I'm not sure, but maybe this issue can be fixed by using IdentityHashMaps instead of the regular HashMaps for the fields oldToNew and newToOld in CloneImportExport. I'll check on that...

Update:

Yep, using java.util.IdentityHashMaps works perfectly. The transformations are created separately, and changing one of them results in the expected (and desired) behavior.

Thanks BattleWorm!



It seems as though this bug was also the culprit behind a keyframe animation problem I was experiencing in cloned 3ds models. The cloned model animations were going twice the speed and distance of the original model. After changing the HashMap to IdentityHashMap the problem was gone.  :smiley:

This nasty bug has plagued me too I think - kudos for finding it (again)! I'm sure if any of you can post a patch it will be applied to cvs… I'd make one myself but am not quite sure if all or just one HashMap should be replaced and don't have the time to find that out just now.

I just changed the following two Maps:



    /**
     * The mapping from new savable copy to the old savable - used to look up
     * the old savable's capsule
     */
    private IdentityHashMap<Savable, Savable> newToOld = new IdentityHashMap<Savable, Savable>();
    /**
     * The mapping from the old savable to the new copy - used when looking up
     * references
     */
    private IdentityHashMap<Savable, Savable> oldToNew = new IdentityHashMap<Savable, Savable>();



As BattleWorm suggested and it seems to work fine now. But I agree that there may be other hidden issues with the other Maps.

@See Thread http://www.jmonkeyengine.com/jmeforum/index.php?topic=3844.msg32011#msg32011 from Oktober 2006…



I think this is the same :stuck_out_tongue:



hf

snare

snareoj said:

@See Thread http://www.jmonkeyengine.com/jmeforum/index.php?topic=3844.msg32011#msg32011 from Oktober 2006...

I think this is the same :P

hf
snare


Yes, I think you're right. Too bad it wasn't fixed back then. Hopefully someone with commit right will put this fix into CVS this time  ://

Hope so too :wink:

Sorry folks, haven't checked back for a while…



@snareoj: Oh, didn't notice that the idea of using IdentityHashMaps had already been brought up in that thread. Anyway, thanks for the hint! At least it shows me that I'm not completely wrong :wink:

Integrated.  Nice catch.

Thanks!  :slight_smile:

Thanks,too ! :slight_smile: