[committed]BoneAnimatoin RT_CYCLE bug & Preventing unnecessary updateSkin() call

Even when I don’t use animation interpolation or blending,

bone.propogateBoneChange() is called by BoneTransform.setCurrentFrame()

and it makes skin update per every update() call.

It’s because BoneAnimation.updateCurrentFrame() returns true almost all of the time

I suggest the patch for this problem below.



And one more,

when we use the repeat type RT_CYCLE,

cycleMode should be 1 (forward direction)

cycleMode should be -1 (backward direction)



I think we don’t have to use multiplication

(It occurs exception for my example case)




Modified version of the path is below…

  1. I want less skinnode update for the performance reason. (in no interpolation mode)

        I think it is told enough above.


  2. If you test it by Debug mode, you will know what the problem is.

    Because Spatial eats up all the exception from Controllers in Spatial.updateWorldData(),

    no error is reported.



    I speeded up the animation, because it simulates what I want to show.

    In fact, what I want to show is “When framerate is low, BoneAnimation throws ArrayIndexOutOfBoundsException frequently and it degrades performance more and affects other controller to run”



    I’ll describe it by code



    private boolean updateCurrentTime(float time, int repeat, float speed) {   <<<=== 1

    // if framerate varies, time may be large enough to rotate animation for a loop (from start frame to end frame for just one update call),

    //  let’s assume time == 0.11f

    //                      keyframeTimes = {0f, 0.033f, 0.066f, 0.1f }






    case Controller.RT_CYCLE: {

                    if (currentFrame >= endFrame) {

                        cycleMode *= -1;

                        currentFrame = endFrame;

                        prevFrame = endFrame - 1;

                        currentTime = keyframeTime[currentFrame];

                    } else if (prevFrame <= startFrame) {

                        cycleMode *= -1;

                        currentFrame = startFrame + 1;

                        prevFrame = startFrame;

                        currentTime = keyframeTime[currentFrame];

                    }



                    currentTime += (time) * speed * cycleMode;

    // time == 0.11f, cycleMode ==1, speed == 1

    // currentTime = 0.11f (which is larger than the keyframe time of end frame)




                    if (cycleMode == 1) {

                        if (currentTime > keyframeTime[endFrame]) {

                            currentTime = keyframeTime[endFrame];

                        }

    // as current time is larger than the keyframe time of end frame, it is set to 0.1f for this example



                        while (currentTime >= keyframeTime[currentFrame]) {

    // now loop is repeated until currentFrame == endFrame + 1 because of the equation. Throws ArrayIndexOutOfBoundsException

                            currentFrame += cycleMode;

                            prevFrame += cycleMode;

                        }



                    } else {

                        if (currentTime < keyframeTime[startFrame]) {

                            currentTime = keyframeTime[startFrame];

                        }



                        while (currentTime <= keyframeTime[currentFrame]) {

    // Here is the same. Throws ArrayIndexOutOfBoundsException

                            currentFrame += cycleMode;

                            prevFrame += cycleMode;

                        }

                    }

                    break;

                }



    Because I’m not good at English, I can’t explain it in more detail.  :oops:



    And I found some other problems while writing this post.
  3. As I said above, Spatial silently eats up the exception.

        It prevents other controller running.

        I think fatal log message or other error handing is needed (Spatial.updateWorldData:544)


  4. BoneAnimation always set the interpolation to true when it is read from resource.

        watch the code BoneAnimation.read:1016

        I guess it is inserted for the test and not removed.

If there is no objection to this patch, I'll apply this according to the policy  :slight_smile:



I'm cautious because this is my first commit to this project.



Can I assume that no reply means agreement on this?

I have not delved into this area of the code much. But what you posted makes sense.

I'm heavily using Animation System and it occurs some problems to me.



This patch I suggested gave some progress on performance and got rid of some errors in case of mine.



I want to get some advise from you guys, if it's right or wrong. :frowning:

I found easy test for error reproduction and refined the patch code



The test is a modified version of TestSimpleBoneAnimation.java

The repeat type is set to RT_CYCLE,

and animation speed is set to 10000 100.



package jmetest.renderer;

import com.jme.animation.AnimationController;
import com.jme.animation.Bone;
import com.jme.animation.BoneAnimation;
import com.jme.animation.BoneTransform;
import com.jme.animation.SkinNode;
import com.jme.app.SimpleGame;
import com.jme.bounding.BoundingBox;
import com.jme.image.Texture;
import com.jme.input.NodeHandler;
import com.jme.math.Quaternion;
import com.jme.math.Vector3f;
import com.jme.renderer.ColorRGBA;
import com.jme.scene.Controller;
import com.jme.scene.Node;
import com.jme.scene.shape.Box;
import com.jme.scene.state.MaterialState;
import com.jme.scene.state.TextureState;
import com.jme.util.BoneDebugger;
import com.jme.util.TextureManager;

public class TestSimpleBoneAnimation extends SimpleGame {

    SkinNode mySkin;
    Bone theBone, theBone2;

    public static void main(String[] args) {
        TestSimpleBoneAnimation game = new TestSimpleBoneAnimation();
        game.setConfigShowMode(ConfigShowMode.AlwaysShow);
        game.start();
    }

    protected void simpleRender() {
        BoneDebugger.drawBones(rootNode, display.getRenderer(), true);
    }

    protected void simpleUpdate() {
    }

    protected void simpleInitGame() {
        Node modelNode = new Node("model");
        Box b = new Box("test", new Vector3f(0, 0, 0), 2f, .5f, .5f);
        b.setModelBound(new BoundingBox());
        b.updateModelBound();
        mySkin = new SkinNode("test skin");
        mySkin.addSkin(b);
        modelNode.attachChild(mySkin);

        TextureState ts = display.getRenderer().createTextureState();
        ts.setTexture(TextureManager.loadTexture(TestSimpleBoneAnimation.class
                .getClassLoader().getResource(
                        "test/data/model/Player/trex-eye.tga"),
                Texture.MinificationFilter.Trilinear, Texture.MagnificationFilter.Bilinear, 0.0f, true));
        b.setRenderState(ts);

        MaterialState ms = display.getRenderer().createMaterialState();
        ms.setSpecular(new ColorRGBA(0.9f, 0.9f, 0.9f, 1));
        ms.setShininess(10);
        b.setRenderState(ms);

        theBone = new Bone("Bone01");
        int[] verts = new int[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
                14, 15, 16, 17, 18, 19, 20, 21, 22, 23 };
        float[] weights = new float[] { 0.463532f, 1, 1, 0.399379f, 1, 1, 1, 1,
                1, 0.039898f, 0.038145f, 1, 0.039898f, 0.463532f, 0.399379f,
                0.038145f, 1, 1, 0.038145f, 0.399379f, 0.463532f, 0.039898f, 1,
                1 };

        for (int x = 0; x < verts.length; x++) {
            mySkin.addBoneInfluence(0, verts[x], theBone, weights[x]);
        }

        modelNode.attachChild(theBone);

        theBone2 = new Bone("Bone02");
        int[] verts2 = new int[] { 0, 13, 20, 3, 14, 19, 9, 12, 21, 10, 15, 18 };
        float[] weights2 = new float[] { 0.536468f, 0.536468f, 0.536468f,
                0.600621f, 0.600621f, 0.600621f, 0.960102f, 0.960102f,
                0.960102f, 0.961855f, 0.961855f, 0.961855f };

        for (int x = 0; x < verts2.length; x++) {
            mySkin.addBoneInfluence(0, verts2[x], theBone2, weights2[x]);
        }

        theBone.attachChild(theBone2);
        theBone.updateGeometricState(0, true);
        mySkin.setSkeleton(theBone);

        Quaternion b1Q1 = new Quaternion().fromAngleAxis(-0.00123886f,
                new Vector3f(0, 0, 1));
        Quaternion b1Q2 = new Quaternion().fromAngleAxis(-86.3343f,
                new Vector3f(0, 1, 0));
        Quaternion b1Q3 = new Quaternion().fromAngleAxis(90.0012f,
                new Vector3f(1, 0, 0));
        theBone.setLocalRotation(b1Q1.mult(b1Q2).mult(b1Q3));
        theBone.setLocalTranslation(new Vector3f(0.0168828f, -2.66517e-009f,
                0.060972f));

        Quaternion b2Q1 = new Quaternion().fromAngleAxis(-13.8716f,
                new Vector3f(0, 0, 1));
        Quaternion b2Q2 = new Quaternion().fromAngleAxis(2.66225e-012f,
                new Vector3f(0, 1, 0));
        Quaternion b2Q3 = new Quaternion().fromAngleAxis(5.51267e-013f,
                new Vector3f(1, 0, 0));
        theBone2.setLocalRotation(b2Q1.mult(b2Q2).mult(b2Q3));
        theBone2.setLocalTranslation(new Vector3f(0.0639104f, -1.04178f,
                4.47038e-008f));

        theBone.updateGeometricState(0, true);
        mySkin.normalizeWeights();
        mySkin.regenInfluenceOffsets();

        rootNode.attachChild(modelNode);

        this.input = new NodeHandler(theBone, 10, 10);
       
        Quaternion[] rotations = new Quaternion[5];
        Vector3f[] translations = new Vector3f[5];
       
        Vector3f axis = new Vector3f(1,0,0);
        rotations[0] = new Quaternion();
        rotations[0].fromAngleAxis(10, axis);
        rotations[1] = new Quaternion();
        rotations[1].fromAngleAxis(30, axis);
        rotations[2] = new Quaternion();
        rotations[2].fromAngleAxis(60, axis);
        rotations[3] = new Quaternion();
        rotations[3].fromAngleAxis(90, axis);
        rotations[4] = new Quaternion();
        rotations[4].fromAngleAxis(120, axis);
       
        translations[0] = new Vector3f(0,0,0);
        translations[1] = new Vector3f(0,1,0);
        translations[2] = new Vector3f(0,2,0);
        translations[3] = new Vector3f(0,1,0);
        translations[4] = new Vector3f(0,2,0);
       
        BoneTransform bt = new BoneTransform();
        bt.setBone(theBone);
        bt.setRotations(rotations);
        bt.setTranslations(translations);
       
        float[] times = new float[5];
        times[0] = 0.3333f;
        times[1] = 0.6333f;
        times[2] = 0.9333f;
        times[3] = 1.3333f;
        times[4] = 1.6333f;
       
        BoneAnimation bac = new BoneAnimation();
        bac.addBoneTransforms(bt);
        bac.setTimes(times);
        bac.setEndFrame(4);
       
        AnimationController ac = new AnimationController();
        ac.setSpeed(100);
        ac.addAnimation(bac);
        ac.setActiveAnimation(bac);
        ac.setRepeatType(Controller.RT_CYCLE);
        theBone.addController(ac);
    }

}




Index: src/com/jme/animation/BoneAnimation.java
===================================================================
--- src/com/jme/animation/BoneAnimation.java   (revision 4043)
+++ src/com/jme/animation/BoneAnimation.java   (working copy)
@@ -375,6 +375,7 @@
             if (endFrame >= keyframeTime.length) {
                 endFrame = keyframeTime.length - 1;
             }
+            int oldFrame = currentFrame;
             if (updateCurrentTime(time, repeat, speed)) {
                 if (interpolate) {
                     lastTime += time;
@@ -392,7 +393,7 @@
                                     result);
                         }
                     }
-                } else {
+                } else if (oldFrame != currentFrame) {
                     if(props == null || !props.isAllowTranslation()) {
                         for (int i = 0; i < boneTransforms.size(); i++) {
                             boneTransforms.get(i).setCurrentFrame(currentFrame);
@@ -517,7 +518,7 @@
                         currentTime = keyframeTime[endFrame];
                     }
 
-                    while (currentTime >= keyframeTime[currentFrame]) {
+                    while (currentTime > keyframeTime[currentFrame]) {
                         currentFrame += cycleMode;
                         prevFrame += cycleMode;
                     }
@@ -526,7 +527,7 @@
                         currentTime = keyframeTime[startFrame];
                     }
 
-                    while (currentTime <= keyframeTime[currentFrame]) {
+                    while (currentTime < keyframeTime[currentFrame]) {
                         currentFrame += cycleMode;
                         prevFrame += cycleMode;
                     }

Can you explain a bit more what the test shows ?

What is expected, what is wrong without the patch ?



When i try your test there is no animation at all.

Is nobody interested in this?   :?



No problem?