New animation system: shared stateful static objects cause unexpected threading issues

[pspeed42]

In general, JME operates in a "threading not supported" mode but in a "you are fine to do whatever until you attach it to the scene graph" sort of way. Meaning that you can load and manipulate Spatials from separate threads but once you attach them to the render-thread managed scene, then hands off.

The animation system internally uses several global objects that keep state. By default, these are shared across every loaded animation, no matter where it was loaded.

Examples at the lowest level are AnimInterpolators.NLerp and AnimInterpolators.LinearVec3f:
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/anim/interpolator/AnimInterpolators.java#L46

...which are then used by one shared FrameInterpolator instance:
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/anim/interpolator/FrameInterpolator.java#L41
...that is the default for MorphTrack and TransformTrack:
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/anim/MorphTrack.java#L55
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/anim/TransformTrack.java#L53

And while you could meticulously set your own:
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/anim/TransformTrack.java#L302
...it won't be serialized:
https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/java/com/jme3/anim/TransformTrack.java#L332

At a high level, the right solution would be to share a single FrameInterpolator instance (and its own instances of NLerp, LinearVector3f, etc.) across the AnimComposer/SkinningControl and not across the whole Java virtual machine. I'm still trying to figure out the simplest/least-impactful way to make that happen. (Locally I've modified all of these cases to create their own instances and it solves the problems.)

As it stands, if you load Jaime.j3o into a scene and let him run around while meanwhile loading a totally separate model in another thread... posing or modifying that animation from that other thread will cause Jaime to glitch or crash. Which breaks the mental threading model under which JME normally operates.

Or in my case, when having a game load thread animate physics collision shapes while the rendering thread is trying to play animations:
https://www.youtube.com/watch?v=lWAEevlytPE

[pavly-gerges]

At a high level, the right solution would be to share a single FrameInterpolator instance (and its own instances of NLerp, LinearVector3f, etc.) across the AnimComposer/SkinningControl and not across the whole Java virtual machine. I'm still trying to figure out the simplest/least-impactful way to make that happen. (Locally I've modified all of these cases to create their own instances and it solves the problems.)

Does using MorphTrack#setFrameInterpolator(FrameInterpolator interpolator) to set different interpolators across different tracks solves the problem ?

If so, i suppose we could simply, refractor AnimInterpolators to a factory class that creates and returns different types of interpolators that could be used within a FrameInterpolator and applied and serialized within a MorphTrack ?

EDIT :
For example : instead of :

    public static final AnimInterpolator<Quaternion> NLerp = new AnimInterpolator<Quaternion>() {
        private Quaternion next = new Quaternion();

        @Override
        public Quaternion interpolate(float t, int currentIndex, TrackDataReader<Quaternion> data, TrackTimeReader times, Quaternion store) {
            data.getEntryClamp(currentIndex, store);
            data.getEntryClamp(currentIndex + 1, next);
            store.nlerp(next, t);
            return store;
        }
    };

it will be :

public class DefaultQuaternionInterpolator extends AnimInterpolator<Quaternion> {
        private Quaternion next = new Quaternion();

        @Override
        public Quaternion interpolate(float t, int currentIndex, TrackDataReader<Quaternion> data, TrackTimeReader times, Quaternion store) {
            data.getEntryClamp(currentIndex, store);
            data.getEntryClamp(currentIndex + 1, next);
            store.nlerp(next, t);
            return store;
        }

}

public static AnimInterpolator<Quaternion> createNLerp() {
     return new DefaultQuaternionInterpolator();
}

and on FrameInterpolator may be :

public class FrameInterpolator {
....
public FrameInterpolator(final AnimInterpolator<Vector3f> translators, final AnimInterpolator<Vector3f>  scalors, final AnimInterpolator<Quaternion> rotators) {
     .......
} 

public static FrameInterpolator createDefaultInterpolator() {
     return new FrameInterpolator(AnimInterpolator.createLinearVec3f(), AnimInterpolator.createLinearVec3f(), AnimInterpolators.createNLerp());
}

And on TransformTrack :

public class TransformTrack implements AnimTrack<Transform> {

    private double length;
    private FrameInterpolator interpolator = FrameInterpolator.createDefaultInterpolator();
    private HasLocalTransform target;
    ....
    }

[pspeed42]

The above is what I've done locally to fix the issue. Which means it creates around 2400 FrameInterpolator instances for the two models I have loaded. It's not really the right solution in the end but it does work.

[pavly-gerges]

The above is what I've done locally to fix the issue. Which means it creates around 2400 FrameInterpolator instances for the two models I have loaded. It's not really the right solution in the end but it does work.

But, this seems to be the only solution tho.

EDIT :
So, Could it be for each single model there would be one FrameInterpolator (i think this is a user side code and the engine shouldn't restrict the number of objects ?).

[pspeed42]

I quote my own post: "At a high level, the right solution would be to share a single FrameInterpolator instance (and its own instances of NLerp, LinearVector3f, etc.) across the AnimComposer/SkinningControl and not across the whole Java virtual machine. "

So yes, one per model is already what I said.

[stephengold]

Is anyone working on this issue?

[pspeed42]

I'm not working on it as I've hacked my local JME in an ugly way to work around this issue. It's way down on my very long to-do list to fix this in JME properly.

...but if you're looking for something to do, this is a nice meaty/tricky one.

[stephengold]

What if, instead of a global default FrameInterpolator there were one default instance per thread? Would that resolve this issue?

[pavly-gerges]

I think it will resolve the issue, the original issue comes from using the static storage specifier, I suggested above to create a factory class that can create different instances of frame interpolators to feed in each thread.