Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loading multiple GLB files with animations fails #1003

Closed
boonto opened this Issue Jan 8, 2019 · 9 comments

Comments

Projects
None yet
3 participants
@boonto
Copy link
Contributor

commented Jan 8, 2019

I ran into this issue when loading a simple GLB file first and then loading another one with an animation.
When loading the animation GLB file second, all of the TransformTrack data was scrambled.

After looking into it, I found that the bufferIndex when reading the data is wrong, which causes the GlbLoader to use the buffer of the previously loaded GLB file.

@stephengold

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2019

Could you provide any more details about your findings?

@boonto

This comment has been minimized.

Copy link
Contributor Author

commented Jan 9, 2019

Sure, it was late yesterday. Here is a minimal example:

import com.jme3.anim.AnimComposer;
import com.jme3.app.SimpleApplication;
import com.jme3.math.ColorRGBA;
import com.jme3.scene.Spatial;

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

    @Override
    public void simpleInitApp() {
        this.viewPort.setBackgroundColor(ColorRGBA.DarkGray);

//        Spatial cube = this.assetManager.loadModel("cube.glb");
//        Spatial monkey = this.assetManager.loadModel("monkey.glb");
        Spatial anim = this.assetManager.loadModel("sway.glb");

        anim.breadthFirstTraversal(spatial -> {
            AnimComposer control = spatial.getControl(AnimComposer.class);
            if (control != null) {
                control.getAnimClipsNames()
                       .stream()
                       .findFirst()
                       .ifPresent(control::setCurrentAction);
            }
        });

        this.rootNode.attachChild(anim);
    }
}

This snippet works.
But when the cube/monkey is loaded first the program crashes, due to looking for the animations in the previously loaded model's data.
When I encountered it yesterday it even successfully loaded the data and produced wrong animations.

With debugging it can be observed starting here.

Here are the models: resources.zip
The cube is just a cube.
The monkey is Suzanne with 2 times subdivision applied.
And the sway animation is a 2 bone armature affecting a cube.
The models/animations were created in the latest Blender 2.80 beta version.

Also the error is on the current master branch. I don't know about v3.2.x.

@boonto boonto changed the title Loading multiple GLB files scrambles animations Loading multiple GLB files with animations fails Jan 9, 2019

@stephengold

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2019

At the start of each loadModel(), the GltfLoader's dataCache is cleared and a new animations array is instantiated. I don't see what data are being reused. Could you clarify for me?

@boonto

This comment has been minimized.

Copy link
Contributor Author

commented Jan 10, 2019

I don't think thats the case for the GlbLoaders's data. This is what gets accessed by the readData method in the end.

@pspeed42

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2019

Yeah, it does seem like data is never cleared.

...actually, the fact that the asset loader system was written to keep state like this is dangerous and leaky. I mean, if the loader keeps all of this data around imagine loading a giant ogre or gltf or whatever file and never loading another one of that particular type... all of the data hangs out in RAM for no reason.

Future todo item would be to rewrite loaders to have a clear/reset method or just instantiate them every time instead of keeping thread-locals around. I'm not sure if there is any advantage to keeping them around (and many many disadvantages).

@stephengold

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2019

Now I get it. That explains why the issue only occurs with .glb, not .gltf. I'll add a data.clear() to address the immediate issue.

I don't think thats the case for the GlbLoaders's data. This is what gets accessed by the readData method in the end.

@stephengold stephengold self-assigned this Jan 10, 2019

stephengold added a commit that referenced this issue Jan 11, 2019

@stephengold

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

The fix is integrated into the master branch at GitHub. @boonto please test.

@stephengold stephengold added the bug label Jan 11, 2019

@boonto

This comment has been minimized.

Copy link
Contributor Author

commented Jan 12, 2019

Works!

@stephengold

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2019

Fixed in master at commit hash 34b32bf .

Lets make this a high priority for the next JME release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.