Funky child translation issue

Ok, so that’s three+ hours of my life I will never get back but I’ve found an interesting bug in jme Node children translation.



If you have a node with a certain number of children, modify the translation of one of those children, and then modify the translation of the parent, and child after the moved child will not move at all. If you were to then move the parent again then they all snap back to where they should rightfully be.



Here’s a test case:

[java]package mygame;



import com.jme3.app.SimpleApplication;

import com.jme3.material.Material;

import com.jme3.math.ColorRGBA;

import com.jme3.math.FastMath;

import com.jme3.math.Vector3f;

import com.jme3.renderer.RenderManager;

import com.jme3.scene.Geometry;

import com.jme3.scene.Node;

import com.jme3.scene.shape.Box;



/**

  • test
  • @author normenhansen

    */

    public class MoveTest extends SimpleApplication {



    Node main;

    Node middleChild;



    float angle = 0;



    public static void main(String[] args) {

    MoveTest app = new MoveTest();

    app.start();

    }



    @Override

    public void simpleInitApp() {



    // Create a parent node containing a big white box.

    Box b = new Box(Vector3f.ZERO, 1, 1, 1);

    Geometry geom = new Geometry(“Box”, b);

    geom.updateModelBound();



    Material mat = new Material(assetManager, “Common/MatDefs/Misc/Unshaded.j3md”);

    mat.setColor(“Color”, ColorRGBA.White);

    geom.setMaterial(mat);



    main = new Node( “main” );

    main.attachChild( geom );



    rootNode.attachChild(main);



    // Add three children to the big white box,

    // a red one then a green one and then a blue one.

    // The green one we hold onto a reference so we can move it later.

    for( int i = 0; i < 3; i++ ) {



    Box b2 = new Box(Vector3f.ZERO, 0.5f, 0.5f, 0.5f);

    Geometry geom2 = new Geometry(“Box-small”, b2);

    geom2.updateModelBound();



    Material mat2 = new Material(assetManager, “Common/MatDefs/Misc/Unshaded.j3md”);

    switch( i ) {

    case 0:

    mat2.setColor(“Color”, ColorRGBA.Red);

    break;

    case 1:

    mat2.setColor(“Color”, ColorRGBA.Green);

    break;

    case 2:

    mat2.setColor(“Color”, ColorRGBA.Blue);

    break;

    }

    geom2.setMaterial(mat2);



    Node child = new Node( “child” + i );

    child.attachChild(geom2);

    child.setLocalTranslation( (i-1) * 2, 1.2f, 0 );



    main.attachChild(child);



    // If it’s the middle child then remember it

    if( i == 1 ) {

    middleChild = child;

    }

    }



    }



    @Override

    public void simpleUpdate(float tpf) {



    angle += tpf * FastMath.QUARTER_PI;

    if( angle > FastMath.TWO_PI )

    angle -= FastMath.TWO_PI;



    float x = FastMath.cos( angle ) * 3;

    float z = FastMath.sin( angle ) * 3;



    // Now, make the white box (and all of its children) orbit slightly

    // around the origin.

    // Make the green box orbit around the white box.



    boolean showWorking = true;

    if( showWorking ) {

    // If main’s location is set first then everything works

    // as one would expect.

    main.setLocalTranslation( -x / 2f, 0, z / 2f );

    middleChild.setLocalTranslation( x, 1.2f, z );

    } else {

    // If the child’s location is set first then the third

    // child does not move at all.

    // That’s bad.

    middleChild.setLocalTranslation( x, 1.2f, z );

    main.setLocalTranslation( -x / 2f, 0, z / 2f );

    }



    }



    @Override

    public void simpleRender(RenderManager rm) {

    //TODO: add render code

    }

    }[/java]



    Run it once to see how one might expect it to work… then edit and switch showWorking to false… note that it no longer works as one would expect as the blue box just stays put.



    You’ll have to trust me that if main.setLocalTranslation() is called on its own that everything self-corrects because I didn’t code that into the test. My guess is that it has something to do with the update state stuff and I’m through looking for it tonight. I think I can use the above as a work-around with some effort and I’m trying to get a release out. :slight_smile:
1 Like

Of course both versions should do exactly the same and they are not, The blue node only moves for me when i set it to false.

@Momoko_Fan ok I fixed this issue, but since it’s relative to the pretty tricky local updates i’d like you to review the change please.



in Node.setTransformRefresh we iterate over the node’s children to properly set the transform flags on them.

But in the code it seems that if one child has already the flags set on it (for example if setLocalTranslation has been call directly on it) the iteration is stopped preventing other children to have their flags updated



@Override

protected void setTransformRefresh(){

super.setTransformRefresh();

for (Spatial child : children){

if ((child.refreshFlags & RF_TRANSFORM) != 0)

return;



child.setTransformRefresh();

}

}



I guess it’s a typo and that it was meant to be a “continue” instead of a “return”



@Override

protected void setTransformRefresh(){

super.setTransformRefresh();

for (Spatial child : children){

if ((child.refreshFlags & RF_TRANSFORM) != 0)

continue;



child.setTransformRefresh();

}

}





So I changed that, and it fixed the issue.

I also changed it in the setLightListRefresh because it was th same logic.

1 Like

Yes, my “logic” was that I should stop updating the bottom of the tree if the top already has refresh flag set, however by using a “return” statement, that also skips the children further in the loop …



Thank you Remy for fixing this and Paul for finding it!