LinkedAssets aren't sharing meshes when imported

Troubleshooting import-assets
1

I’m using linked assets with the expectation to have their meshes shared. However, if I check the meshes in-code, they aren’t shared at all (just when they are linked). So, I’m wondering if I’m checking them wrong, there is a bug, or it is intended to not share the meshes between linked assets.

They are shared, however, if I link them in-code instead of in the sceneComposer (and so, using the importer)

The code I use to check them is:

public static boolean meshShareBuffers(Mesh mesh1, Mesh mesh2) {
        if(mesh1 == mesh2) {
            return true;
        }

        if(mesh1 == null || mesh2 == null) {
            return false;
        }

        for (VertexBuffer vb : mesh1.getBufferList().getArray()) {
            if(mesh2.getBuffer(vb.getBufferType()) != vb) {
                return false;
            }
        }

        return true;
    }

And the test-case I used is:

public class TestMeshSharing extends SimpleApplication {
    
    public static void main(String[] args) {
        TestMeshSharing app = new TestMeshSharing();
        app.start();
    }

    @Override
    public void simpleInitApp() {
        Geometry original = (Geometry) ((Node)assetManager.loadModel("Models/original.j3o")).getChild("Box");
        
        // If we load the same model twice from the assetManager, meshes ARE SHARED
        Geometry geom = original;
        Geometry geom2 = (Geometry) ((Node)assetManager.loadModel("Models/original.j3o")).getChild("Box");
        System.out.println("Shared (original x2): " + meshShareBuffers(geom.getMesh(), geom2.getMesh())); // Returns true

        
        // If we load the same model (with the box linked) twice from the assetManager, meshes ARE SHARED
        geom = (Geometry) ((Node)assetManager.loadModel("Models/linked.j3o")).getChild("Box");
        geom2 = (Geometry) ((Node)assetManager.loadModel("Models/linked.j3o")).getChild("Box");
        System.out.println("Shared (linked x2): " + meshShareBuffers(geom.getMesh(), geom2.getMesh())); // Returns true
        
        
        // If we load the a model with a box linked, and we compare it with the original box (not linked), meshes ARE _NOT_ SHARED
        geom = original;
        geom2 = (Geometry) ((Node)assetManager.loadModel("Models/linked.j3o")).getChild("Box");
        System.out.println("Shared (original == linked): " + meshShareBuffers(geom.getMesh(), geom2.getMesh())); // Returns false
        
        
        // If we load a scene with the same box linked multiple times, their meshes ARE _NOT_ SHARED
        Node scene = (Node) assetManager.loadModel("Models/linkedTwice.j3o");
        
        geom = (Geometry) ((Node)scene.getChild("linked1")).getChild("Box");
        geom2 = (Geometry) ((Node)scene.getChild("linked2")).getChild("Box");
        
        System.out.println("Shared (linked == linked): " + meshShareBuffers(geom.getMesh(), geom2.getMesh())); // Returns false
        
        
        // If we add a new linked box by code, the result is the expected, their meshes ARE SHARED
        AssetLinkNode linked3 = new AssetLinkNode("linked3", new ModelKey("Models/original.j3o"));
        scene.attachChild(linked3);
        
        linked3.attachLinkedChild(assetManager, new ModelKey("Models/original.j3o"));
        geom2 = (Geometry) linked3.getChild("Box");
        
        System.out.println("Shared (original == manual-linked): " + meshShareBuffers(original.getMesh(), geom2.getMesh())); // Returns false
    }
}

EDIT: The difference seems to be the “load” method. It works fine if using the assetManager (manager.loadAsset(AssetKey), used in AssetLinkNode.attachLinkedChild) but it doesn’t if using the importer (BinaryImporter.load(AssetInfo), used in AssetLinkNode.read)

1 Like
2

I’ve tried to fix it without much success. If I try to replace the BinaryImporter.load by an AssetManager.loadAsset, just like how it is done for AudioNode, it messes up the importer and I can’t get further parameters from it.

The complete test-case (it should work with a copy-paste and execute) is:

public class TestAssetLinkNode extends SimpleApplication {
    
    public static void main(String[] args) {
        TestAssetLinkNode app = new TestAssetLinkNode();
        app.start();
    }
    
    /**
     * If using the assetManager (true), it messes up the binary importer, if not (false), it doesn't share meshes
     */
    public static boolean READ_FROM_ASSETMANAGER = true;
    
    /**
     * If the linked model is preloaded (true), it works fine then when reading the model containing the linked (it is on cache)
     */
    public static boolean PRELOAD_LINKED_MODEL = false;
    
    public static Material SAVE_MATERIAL;
    
    @Override
    public void simpleInitApp() {
        
        SAVE_MATERIAL = new Material(assetManager, "Common/MatDefs/Misc/Unshaded.j3md");
        SAVE_MATERIAL.setColor("Color", ColorRGBA.Brown);
        
        
        String homePath = System.getProperty("user.home");
        String sceneFileName = "toDelete.j3o";
        String linkedFileName = "toDelete2.j3o";
        
        File sceneFile = new File(homePath + File.separator + sceneFileName);
        File linkedFile = new File(homePath + File.separator + linkedFileName);
        
        if(sceneFile.exists() || linkedFile.exists()) {
            throw new RuntimeException();
        }
        
        BinaryExporter exporter = BinaryExporter.getInstance();
        
        try {
            
            // Creating the linked model and saving it to a file.
            Geometry linked = new Geometry("linked", new Box(1, 1, 1));
            
            exporter.save(linked, linkedFile);
            
            // Creating the scene, linked the model, and saving it to a file.
            AssetLinkNodePatch scene = new AssetLinkNodePatch("scene", new ModelKey(linkedFileName));
            exporter.save(scene, sceneFile);
            
            // Loading and attaching it to the scene.
            assetManager.registerLocator(homePath, FileLocator.class);
            
            if(PRELOAD_LINKED_MODEL) {
                assetManager.loadModel(linkedFileName);
            }
        
            rootNode.attachChild(assetManager.loadModel(sceneFileName));
            
        } catch (IOException ex) {
            Logger.getLogger(TestAssetLinkNode.class.getName()).log(Level.SEVERE, null, ex);
        } finally {
            try {
                // Deleting the files.
                Files.delete(Paths.get(sceneFile.toURI()));
                Files.delete(Paths.get(linkedFile.toURI()));
            } catch (IOException ex) {
                Logger.getLogger(TestAssetLinkNode.class.getName()).log(Level.SEVERE, null, ex);
            }
        }
    }
    
    
    
    
    
    
    public static class AssetLinkNodePatch extends Node {

        protected ArrayList<ModelKey> assetLoaderKeys = new ArrayList<ModelKey>();
        protected Map<ModelKey, Spatial> assetChildren = new HashMap<ModelKey, Spatial>();

        public AssetLinkNodePatch() {
        }

        public AssetLinkNodePatch(ModelKey key) {
            this(key.getName(), key);
        }

        public AssetLinkNodePatch(String name, ModelKey key) {
            super(name);
            assetLoaderKeys.add(key);
        }

        /**
         *  Called internally by com.jme3.util.clone.Cloner.  Do not call directly.
         */
        @Override
        public void cloneFields( Cloner cloner, Object original ) {
            super.cloneFields(cloner, original);

            // This is a change in behavior because the old version did not clone
            // this list... changes to one clone would be reflected in all.
            // I think that's probably undesirable. -pspeed
            this.assetLoaderKeys = cloner.clone(assetLoaderKeys);
            this.assetChildren = new HashMap<ModelKey, Spatial>();
        }

        /**
         * Add a "linked" child. These are loaded from the assetManager when the
         * AssetLinkNode is loaded from a binary file.
         * @param key
         */
        public void addLinkedChild(ModelKey key) {
            if (assetLoaderKeys.contains(key)) {
                return;
            }
            assetLoaderKeys.add(key);
        }

        public void removeLinkedChild(ModelKey key) {
            assetLoaderKeys.remove(key);
        }

        public ArrayList<ModelKey> getAssetLoaderKeys() {
            return assetLoaderKeys;
        }

        public void attachLinkedChild(AssetManager manager, ModelKey key) {
            addLinkedChild(key);
            Spatial child = manager.loadAsset(key);
            assetChildren.put(key, child);
            attachChild(child);
        }

        public void attachLinkedChild(Spatial spat, ModelKey key) {
            addLinkedChild(key);
            assetChildren.put(key, spat);
            attachChild(spat);
        }

        public void detachLinkedChild(ModelKey key) {
            Spatial spatial = assetChildren.get(key);
            if (spatial != null) {
                detachChild(spatial);
            }
            removeLinkedChild(key);
            assetChildren.remove(key);
        }

        public void detachLinkedChild(Spatial child, ModelKey key) {
            removeLinkedChild(key);
            assetChildren.remove(key);
            detachChild(child);
        }

        /**
         * Loads the linked children AssetKeys from the AssetManager and attaches them to the Node<br>
         * If they are already attached, they will be reloaded.
         * @param manager
         */
        public void attachLinkedChildren(AssetManager manager) {
            detachLinkedChildren();
            for (Iterator<ModelKey> it = assetLoaderKeys.iterator(); it.hasNext();) {
                ModelKey assetKey = it.next();
                Spatial curChild = assetChildren.get(assetKey);
                if (curChild != null) {
                    curChild.removeFromParent();
                }
                Spatial child = manager.loadAsset(assetKey);
                attachChild(child);
                assetChildren.put(assetKey, child);
            }
        }

        public void detachLinkedChildren() {
            Set<Map.Entry<ModelKey, Spatial>> set = assetChildren.entrySet();
            for (Iterator<Map.Entry<ModelKey, Spatial>> it = set.iterator(); it.hasNext();) {
                Map.Entry<ModelKey, Spatial> entry = it.next();
                entry.getValue().removeFromParent();
                it.remove();
            }
        }

        @Override
        public void read(JmeImporter e) throws IOException {
            super.read(e);
            InputCapsule capsule = e.getCapsule(this);
            BinaryImporter importer = BinaryImporter.getInstance();
            AssetManager loaderManager = e.getAssetManager();

            assetLoaderKeys = (ArrayList<ModelKey>) capsule.readSavableArrayList("assetLoaderKeyList", new ArrayList<ModelKey>());
            for (Iterator<ModelKey> it = assetLoaderKeys.iterator(); it.hasNext();) {
                ModelKey modelKey = it.next();
                AssetInfo info = loaderManager.locateAsset(modelKey);
                Spatial child = null;
                if (info != null) {
                    
                    // --- PROBLEM - CRASH ---
                    
                    if(!READ_FROM_ASSETMANAGER) {
                        // With this line, it loads the spatial, but meshes aren't shared
                        child = (Spatial) importer.load(info);
                    } else {
                        // With this line, however, it messes up the importer
                        child = loaderManager.loadAsset(modelKey);
                    }
                    
                    // --- --------------- ---
                    
                    
                }
                if (child != null) {
                    // I can't access to the child parent directly from here.
//                    child.parent = this;
//                    children.add(child);
                    attachChild(child);
                    assetChildren.put(modelKey, child);
                } else {
                    Logger.getLogger(this.getClass().getName()).log(Level.WARNING, "Cannot locate {0} for asset link node {1}",
                                                                        new Object[]{ modelKey, key });
                }
            }
            
            // Reading extra parameter -- Crashes if: READ_FROM_ASSETMANAGER == true && PRELOAD_LINKED_MODEL == false
            Material material = (Material) capsule.readSavable("material", null);
            for(Spatial child : assetChildren.values()) {
                child.setMaterial(material);
            }
            ///////////////////
            
            
        }

        @Override
        public void write(JmeExporter e) throws IOException {
            SafeArrayList<Spatial> childs = children;
            children = new SafeArrayList<Spatial>(Spatial.class);
            super.write(e);
            OutputCapsule capsule = e.getCapsule(this);
            capsule.writeSavableArrayList(assetLoaderKeys, "assetLoaderKeyList", null);
            children = childs;
            
            // Writing extra parameter
            capsule.write(SAVE_MATERIAL, "material", null);
            ////////////////////////
        }
        
    }

There I change the AssetLinkNode’s read method to load the linked spatial with the assetManager. After it loads the spatial, an extra parameter is tried to be read, crashing the application with a cast exception:

java.lang.ClassCastException: com.jme3.bounding.BoundingBox cannot be cast to com.jme3.material.Material
	at mygame.TestAssetLinkNode$AssetLinkNodePatch.read(TestAssetLinkNode.java:264)
	at com.jme3.export.binary.BinaryImporter.readObject(BinaryImporter.java:342)
	at com.jme3.export.binary.BinaryImporter.load(BinaryImporter.java:242)
	at com.jme3.export.binary.BinaryImporter.load(BinaryImporter.java:125)
	at com.jme3.export.binary.BinaryImporter.load(BinaryImporter.java:109)
	at com.jme3.asset.DesktopAssetManager.loadLocatedAsset(DesktopAssetManager.java:259)
	at com.jme3.asset.DesktopAssetManager.loadAsset(DesktopAssetManager.java:373)
	at com.jme3.asset.DesktopAssetManager.loadModel(DesktopAssetManager.java:416)
	at com.jme3.asset.DesktopAssetManager.loadModel(DesktopAssetManager.java:420)
	at mygame.TestAssetLinkNode.simpleInitApp(TestAssetLinkNode.java:101)
	at com.jme3.app.SimpleApplication.initialize(SimpleApplication.java:220)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.initInThread(LwjglAbstractDisplay.java:130)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:211)
	at java.lang.Thread.run(Thread.java:745)

Why is this happening and how can it be solved?

3

Finally, I found a solution, but I’m not sure is the best way to fix it. The problem isn’t because of using the importer or the assetManager, it is because of when ussing the assetManager the same importer as the read method caller is being used. So, if instead of using importer.load(info); we use e.load(info) the same issue will appear.

The workaround I’m using is to fetch the model from the assetManager cache first and if it is null, call the load from the new importer instance. So, the only needed code to be changed in AssetLinkNode is the iterator one, leaving it like:

for (Iterator<ModelKey> it = assetLoaderKeys.iterator(); it.hasNext();) {
    ModelKey modelKey = it.next();

    Spatial child = loaderManager.getFromCache(modelKey);
    if(child == null) {
        AssetInfo info = loaderManager.locateAsset(modelKey);
        child = (Spatial) importer.load(info);
        
        if(child != null) {
            loaderManager.addToCache(modelKey, child);
        }
    } else {
        // This miss, however, the DesktopAssetManager.registerAndCloneSmartAsset.
        child = child.clone();

        // This miss can be solved using the commented line instead, what would retrieve again the model from the cache
//        child = loaderManager.loadAsset(modelKey);
    }

    if (child != null) {
        child.parent = this;
        children.add(child);
        assetChildren.put(modelKey, child);
    } else {
        Logger.getLogger(this.getClass().getName()).log(Level.WARNING, "Cannot locate {0} for asset link node {1}",
                                                            new Object[]{ modelKey, key });
    }
}
4

I think it’s expected behavior.

5

It doesn’t make much sense to me. If you link it is to have it sharing the mesh. It’s the main and only difference between “link” and “attach”. Currently, the AssetLinkNode makes no sense for a j3o.

6

Do you mean that we don’t share mesh in the case when we use assetManager, right?

7

That’s incorrect. Think of it like files: Copy & Link. If you link a mesh in, it will be updated in your scene when it’s changed. It also saves disk-space in the j3o.

But yeah, technically it could/should really share meshes and the user should call .clone if that’s not desired. An inconsistency between two import methods is bad as well

8

Yes, it should be sharing but it isn’t, so the AssetLinkNode loses it benefits once on a j3o.

.deepClone() (.clone is like a “linked” should be)

9

hm, can you create a ticket in my bitbucket about this? I will look at this after shader node editor.

10

Well… it is more a jme3-core problem than an editor one.

11

yes, but from time to time I spend time to fix core problems when I see it in my editor using PRs :wink:

12

xD ok, what about my fix?. I expected some discussion about it before doing the PR but this thread was kind of ignored.

13

I think we shouldn’t use binary importer there, but I will recheck it later…

14

The issue is that BinaryImporter has local state which gets corrupted when you ask it to load another model, so why not fix that instead? For example, it could detect when its already loading something else, and then instead delegate the loading of the new thing into a new instance of BinaryImporter. Something like this:

Object load(AssetInfo asset) {
     if (currentlyLoading) {
         return new BinaryImporter().load(asset);
     }

     currentlyLoading = true;
         try {
             // load the model
             // ...
         } finally {
             currentlyLoading = false;
             // ...
         }
}
15

My though on this are more likely than javasabr’s:

It bothers me the binaryImporter usage in there at first. What if you are loading (calling the read(JmeImporter)) with an importer different that the binaryImporter?

16

I think it should be like this:

17

This seems to be the “logical” fix, but I’ve already tried it and it has its problems as already commented.

18

Ok, I will look at this more deeply.

19

@NemesisMate fixed asset linked node to reuse shared data between loaded models. by JavaSaBr · Pull Request #739 · jMonkeyEngine/jmonkeyengine · GitHub

20

I’m not fully familiarized with the load system but this seems to be a a much more elegant and real solution rather than a simple patch.