Improvements/fixes JME's Scene Locking

archive contribution-depot-jme2
1

And the patch for jme2

Index: ConnectionPoint.java
===================================================================
--- ConnectionPoint.java   (revision 4046)
+++ ConnectionPoint.java   (working copy)
@@ -34,10 +34,11 @@
         if(target == null) {
             return;
         }
-        if (((lockedMode & Spatial.LOCKED_TRANSFORMS) == 0)) {
+        if ((((lockedMode & LOCKED_TRANSFORMS) == 0)) || ((dirtyLocks & LOCKED_TRANSFORMS) != 0)) {
             worldScale.set(parent.getWorldScale()).multLocal(target.getWorldScale());
             parent.getWorldRotation().mult(target.getWorldRotation(), worldRotation);
             worldTranslation = parent.localToWorld( target.getWorldTranslation(), worldTranslation );
+            dirtyLocks &= (~LOCKED_TRANSFORMS);
         }
     }
    
Index: Geometry.java
===================================================================
--- Geometry.java   (revision 4046)
+++ Geometry.java   (working copy)
@@ -532,6 +532,16 @@
      */
     @Override
     public void draw(Renderer r) {
+          if ((this.lockedMode & LOCKED_MESH_DATA) != 0)
+       {
+          if ((this.dirtyLocks & LOCKED_MESH_DATA) != 0)
+          {
+             //refresh the display list
+             this.unlockMeshes(r);
+             this.lockMeshes(r);
+               dirtyLocks &= (~LOCKED_MESH_DATA);
+          }
+       }
     }
 
     /**
Index: Node.java
===================================================================
--- Node.java   (revision 4046)
+++ Node.java   (working copy)
@@ -399,7 +399,8 @@
 
     @Override
     public void updateWorldVectors(boolean recurse) {
-        if (((lockedMode & Spatial.LOCKED_TRANSFORMS) == 0)) {
+       if ((((lockedMode & LOCKED_TRANSFORMS) == 0)) || ((dirtyLocks & LOCKED_TRANSFORMS) != 0)) {
+         
             updateWorldScale();
             updateWorldRotation();
             updateWorldTranslation();
@@ -409,6 +410,7 @@
                     children.get(i).updateWorldVectors(true);
                 }
             }
+            dirtyLocks &= (~LOCKED_TRANSFORMS);
         }
     }
    
@@ -560,8 +562,10 @@
      */
     @Override
     public void updateWorldBound() {
-        if ((lockedMode & Spatial.LOCKED_BOUNDS) != 0) return;
+        if (((lockedMode & LOCKED_BOUNDS) != 0) && ((dirtyLocks & LOCKED_BOUNDS) == 0)) return;
+       
         if (children == null) {
+           dirtyLocks &= (~LOCKED_BOUNDS); 
             return;
         }
         BoundingVolume worldBound = null;
@@ -581,6 +585,7 @@
             }
         }
         this.worldBound = worldBound;
+        dirtyLocks &= (~LOCKED_BOUNDS);
     }
 
     @Override
Index: Spatial.java
===================================================================
--- Spatial.java   (revision 4046)
+++ Spatial.java   (working copy)
@@ -188,8 +188,42 @@
     public static final int LOCKED_TRANSFORMS = 4;
     public static final int LOCKED_SHADOWS = 8;
     public static final int LOCKED_BRANCH = 16;
+    public static final int LOCKED_ALL = LOCKED_BRANCH | LOCKED_SHADOWS | LOCKED_TRANSFORMS | LOCKED_MESH_DATA | LOCKED_BOUNDS;
 
+   
     /**
+     * This entire branch of the scene graph has its locks dirtied so that it can be
+     * updated/rendered (it will ignore locks).
+     * @param locksToDirty A bit flag containing each lock to dirty
+     */
+    public void refreshBranch(int locksToDirty)
+    {
+       refreshBranchToRoot(locksToDirty);
+       refreshBranchToLeaves(locksToDirty);
+    }
+   
+    public void refreshBranch()
+    {
+       refreshBranch(LOCKED_ALL);
+    }
+   
+    /**
+     * Refresh upward in this scene graph to the root node
+     */
+    protected void refreshBranchToRoot(int locksToDirty)
+    {
+       this.dirtyLocks |= locksToDirty;
+       
+    }
+   
+    /**
+     * Refresh downward in the scene graph to each leaf of element
+     */
+    protected void refreshBranchToLeaves(int locksToDirty)
+    {
+       this.dirtyLocks |= locksToDirty;
+    }
+    /**
      * A flag indicating how normals should be treated by the renderer.
      */
     protected NormalsMode normalsMode = NormalsMode.Inherit;
@@ -216,8 +250,15 @@
      * locked against certain changes.
      */
     protected int lockedMode = LOCKED_NONE;
-
+   
     /**
+     * The locks belonging to this scene element that should be temporarily ignored,
+     * because they need to be updated (just once). The locks indicated in this field will be
+     * removed (from dirtyLocks) once they have been updated.
+     */
+    protected int dirtyLocks = LOCKED_NONE;
+   
+    /**
      * Flag signaling how lights are combined for this node. By default set to
      * INHERIT.
      */
@@ -511,15 +552,18 @@
      *            true if this node started the update process.
      */
     public void updateGeometricState(float time, boolean initiator) {
-        if ((lockedMode & Spatial.LOCKED_BRANCH) != 0)
-            return;
+       if (((lockedMode & LOCKED_BRANCH) != 0) && ((dirtyLocks & LOCKED_BRANCH) == 0) )
+          return;
+
         updateWorldData(time);
-        if ((lockedMode & Spatial.LOCKED_BOUNDS) == 0) {
+        if (((lockedMode & LOCKED_BOUNDS) == 0)  || ((dirtyLocks & LOCKED_BOUNDS) != 0)) {
             updateWorldBound();
             if (initiator) {
                 propagateBoundToRoot();
             }
+            dirtyLocks &= (~LOCKED_BOUNDS);
         }
+        dirtyLocks &= (~LOCKED_BRANCH);
     }
 
     /**
@@ -567,10 +611,12 @@
      *            updating geometric state.
      */
     public void updateWorldVectors(boolean recurse) {
-        if (((lockedMode & Spatial.LOCKED_TRANSFORMS) == 0)) {
+       if ((((lockedMode & LOCKED_TRANSFORMS) == 0)) || ((dirtyLocks & LOCKED_TRANSFORMS) != 0)) {
+             
             updateWorldScale();
             updateWorldRotation();
             updateWorldTranslation();
+            dirtyLocks &= (~LOCKED_TRANSFORMS);
         }
     }
 
@@ -1414,14 +1460,44 @@
      *            Spatial.
      */
     public void setLocks(int locks) {
-        if ((lockedMode & Spatial.LOCKED_BOUNDS) != 0)
-            lockBounds();
-        if ((lockedMode & Spatial.LOCKED_MESH_DATA) != 0)
-            lockMeshes();
-        if ((lockedMode & Spatial.LOCKED_SHADOWS) != 0)
-            lockShadows();
-        if ((lockedMode & Spatial.LOCKED_TRANSFORMS) != 0)
-            lockTransforms();
+
+        if ((lockedMode & LOCKED_BOUNDS) != (locks & LOCKED_BOUNDS))
+        {
+           if ((locks & LOCKED_BOUNDS) != 0)
+              lockBounds();
+           else
+              unlockBounds();
+        }
+        if ((lockedMode & LOCKED_TRANSFORMS) != (locks & LOCKED_TRANSFORMS))
+        {
+           if ((locks & LOCKED_TRANSFORMS) != 0)
+              lockTransforms();
+           else
+              unlockTransforms();
+        }
+
+        if ((lockedMode & LOCKED_MESH_DATA) != (locks & LOCKED_MESH_DATA))
+        {
+           if ((locks & LOCKED_MESH_DATA) != 0)
+              lockMeshes();
+           else
+              unlockMeshes();
+        }
+
+        if ((lockedMode & LOCKED_SHADOWS) != (locks & LOCKED_SHADOWS))
+        {
+           if ((locks & LOCKED_SHADOWS) != 0)
+              lockShadows();
+           else
+              unlockShadows();
+        }
+        if ((lockedMode & LOCKED_BRANCH) != (locks & LOCKED_BRANCH))
+        {
+           if ((locks & LOCKED_BRANCH) != 0)
+              lockBranch();
+           else
+              unlockBranch();
+        }
     }
 
     /**
@@ -1433,14 +1509,44 @@
      *            is set.
      */
     public void setLocks(int locks, Renderer r) {
-        if ((lockedMode & Spatial.LOCKED_BOUNDS) != 0)
-            lockBounds();
-        if ((lockedMode & Spatial.LOCKED_MESH_DATA) != 0)
-            lockMeshes(r);
-        if ((lockedMode & Spatial.LOCKED_SHADOWS) != 0)
-            lockShadows();
-        if ((lockedMode & Spatial.LOCKED_TRANSFORMS) != 0)
-            lockTransforms();
+
+        if ((lockedMode & LOCKED_BOUNDS) != (locks & LOCKED_BOUNDS))
+        {
+           if ((locks & LOCKED_BOUNDS) != 0)
+              lockBounds();
+           else
+              unlockBounds();
+        }
+        if ((lockedMode & LOCKED_TRANSFORMS) != (locks & LOCKED_TRANSFORMS))
+        {
+           if ((locks & LOCKED_TRANSFORMS) != 0)
+              lockTransforms();
+           else
+              unlockTransforms();
+        }
+
+        if ((lockedMode & LOCKED_MESH_DATA) != (locks & LOCKED_MESH_DATA))
+        {
+           if ((locks & LOCKED_MESH_DATA) != 0)
+              lockMeshes(r);
+           else
+              unlockMeshes(r);
+        }
+
+        if ((lockedMode & LOCKED_SHADOWS) != (locks & LOCKED_SHADOWS))
+        {
+           if ((locks & LOCKED_SHADOWS) != 0)
+              lockShadows();
+           else
+              unlockShadows();
+        }
+        if ((lockedMode & LOCKED_BRANCH) != (locks & LOCKED_BRANCH))
+        {
+           if ((locks & LOCKED_BRANCH) != 0)
+              lockBranch();
+           else
+              unlockBranch();
+        }
     }
 
     /**



Please note that although I've done my best to translate the changes from jme1 to jme2, I dont currently use jme2, so I may have made some small mistakes.
2

This should be moved to the Contribution Depot in the Developers section for review.

3

Ok - by that do you mean I should repost it there, or is there a proper way to move posts?

4

The moderator (Core-Dump) should move it… Private message him if he doesn't get to it.

5

moved :slight_smile:

this improvement sounds interesting, could you also post a TestCase to show how to use it and to compare performance improvement?



is refreshBranch() not similar to a unlock()/lock()?

6

Before doing that, I've continued to make some modifications as I've been testing this a bit more.



There are two differences between refreshBranch() and unlock/lock.



Unlock and lock recursively work downwards in the scene graph from the starting point towards the leaves, whereas refreshBranch works in both directions - up to the root of the scene, and down to the leaves. This is important, because if you unlock a node, and one of its parents remains locked, then updating might not always proceed as expected.



Secondly, and more importantly, unlock and lock make permanent changes to the lockedMode of their Spatials, and the user is responsible for managing that. For example, you might unlock a spatial, or all the spatials in one branch of the tree, because one of them has moved and needs to be updated. You are responsible for, at some point in the future after that updating has occured, locking each of those spatials again. This gets especially tricky when a) you are using StandardGame and updating has to happen in the GL thread, at some unknown point in the future or b) the scene graph may change between the time you call unlock, and the time you call lock.



So, instead of all that, you just call refreshBranch(), once, on the scene element that has moved. Then, whenever the next update cycle happens, all the scene elements that need to be updated will be updated correctly, exactly one time, and then they will go back to being locked on their own (assuming they were locked to begin with).



 

7

wow, even i understood that… sounds good :slight_smile:

8

Right, well I'm at it there is another change that I've made to the way locks are currently implemented.



This isn't directly connected to the changes above, but is in a similar vein.



Right now, LockMeshes() is implemented differently from the other locks, in two ways. One, it wants to be passed a reference to the current Renderer (which is ignored everywhere anyways, replaced with a call to DisplaySystem.getRenderer() instead.) All the other lock methods take no arguments.



Secondly, unlike the other lock methods, LockMeshes() makes a major change at the point where it is called, rather than just altering the way updateing or rendering will proceed in the future. LockMeshes only effects Geometries, where when called it creates a new display list for that Geometry. Similarly, unlockMeshes releases a display list.



There are three reasons why this (creating a display list) shouldn't happen in the call to lockMeshes. One: it just doesn't fit with the expected behaviour of lock/unlock calls, which is only to adjust the lockedMode bitflags. Two: it means you can't ever call lock() from outside the openGL thread - again, important if you are using StandardGame. Three, it requires that you provide the renderer, which is just not something that should be needed to change the lock mode on a scene element.



So here is how I've changed lock meshes:


  1. lockMeshes() no longer requires a Renderer be passed to it as an argument.
  2. lockMeshes now only changes the lockedMode, even for Geometries/GeometryBatches (depending on whether you are using jme1 or 2).

    3)Display lists are now created, if needed, in the draw() and onDraw() calls in Geometry/GeometryBatch, instead of in the lock/unlock call.



    This last part requires explaining:

    During the render cycle (that is, in the draw() and onDraw() methods), if lockMeshes is set, then the geometry checks to see if a display list exists. If it doesn't, then it can create a new display list right there. On the other hand if unlockMeshes has been called, and the LOCKED_MESH_DATA flag is no longer set, then any existing display list can be released.



    So far as I know doing it this way won't incur any costs, but it does make life simpler, especially for multi-threaded apps.


9

The patches for these changes are too large to fit inside a post… I think I've attached them (as two text files, one for jme1, and one for jme2).



Here are the test apps, as requested. This is just a standard capsule, rotating around aimlessly. However, note that the rootNode is locked down in the init phase. The only reason that the capsule's rotations take effect is because after its localRotation is altered, I've called refreshBranch() on the capsule. You can remove that line, and you should see the capsule frozen in place.



Test app for JME2:



public class TestLocks extends SimpleGame {


    private Quaternion rotQuat = new Quaternion();
    private float angle = 0;
    private Vector3f axis = new Vector3f(1, 1, 0).normalizeLocal();
    private Capsule t;

    /**
     * Entry point for the test,
     *
     * @param args
     */
    public static void main(String[] args) {
       TestLocks app = new TestLocks();
         app.setConfigShowMode(ConfigShowMode.AlwaysShow);
        app.start();
    }

    protected void simpleUpdate() {
        if (timer.getTimePerFrame() < 1) {
            angle = angle + (timer.getTimePerFrame() * 1);
            if (angle > 360) {
                angle = 0;
            }
        }

        rotQuat.fromAngleNormalAxis(angle, axis);
        t.setLocalRotation(rotQuat);
       
        /*
         * Call this anytime a scene element has changed, so that it will
         * be updated on the next update cycle.
         */
        t.refreshBranch();
    }

    /**
     * builds the trimesh.
     *
     * @see com.jme.app.SimpleGame#initGame()
     */
    protected void simpleInitGame() {
        display.setTitle("Cylinder Test");

        t = new Capsule("Capsule", 40, 32, 16, 4, 10);
        t.setModelBound(new BoundingBox());
        t.updateModelBound();
       
        CullState cs = display.getRenderer().createCullState();
        cs.setCullFace(CullState.Face.Back);
        rootNode.setRenderState(cs);
       
        input = new FirstPersonHandler(cam, 10f, 1f);
       
        rootNode.attachChild(t);
       
        TextureState ts = display.getRenderer().createTextureState();
        ts.setEnabled(true);
        ts.setTexture(TextureManager.loadTexture(TestCapsule.class
                .getClassLoader().getResource("jmetest/data/images/Monkey.jpg"),
                Texture.MinificationFilter.BilinearNearestMipMap, Texture.MagnificationFilter.Bilinear));
        ts.getTexture().setWrap(Texture.WrapMode.Repeat);
        rootNode.setRenderState(ts);

        lightState.setTwoSidedLighting(false);
       
        rootNode.setLocks(Spatial.LOCKED_ALL);
    }
}



Test App for jme1:


public class TestLocks extends SimpleGame {


    private Quaternion rotQuat = new Quaternion();
    private float angle = 0;
    private Vector3f axis = new Vector3f(1, 1, 0).normalizeLocal();
    private Capsule t;

    /**
     * Entry point for the test,
     *
     * @param args
     */
    public static void main(String[] args) {
       TestLocks app = new TestLocks();
        app.setDialogBehaviour(ALWAYS_SHOW_PROPS_DIALOG);
        app.start();
    }

    protected void simpleUpdate() {
        if (timer.getTimePerFrame() < 1) {
            angle = angle + (timer.getTimePerFrame() * 1);
            if (angle > 360) {
                angle = 0;
            }
        }

        rotQuat.fromAngleNormalAxis(angle, axis);
        t.setLocalRotation(rotQuat);
       
        /*
         * Call this anytime a scene element has changed, so that it will
         * be updated on the next update cycle.
         */
        t.refreshBranch();
    }

    /**
     * builds the trimesh.
     *
     * @see com.jme.app.SimpleGame#initGame()
     */
    protected void simpleInitGame() {
        display.setTitle("Cylinder Test");

        t = new Capsule("Capsule", 40, 32, 16, 4, 10);
        t.setModelBound(new BoundingBox());
        t.updateModelBound();
       
        CullState cs = display.getRenderer().createCullState();
        cs.setCullMode(CullState.CS_BACK);
        rootNode.setRenderState(cs);
       
        input = new FirstPersonHandler(cam, 10f, 1f);
       
        rootNode.attachChild(t);
       
        TextureState ts = display.getRenderer().createTextureState();
        ts.setEnabled(true);
        ts.setTexture(TextureManager.loadTexture(getClass()
                .getClassLoader().getResource("jmetest/data/images/Monkey.jpg"),
                Texture.MM_LINEAR, Texture.FM_LINEAR));
        ts.getTexture().setWrap(Texture.WM_WRAP_S_WRAP_T);
        rootNode.setRenderState(ts);

        lightState.setTwoSidedLighting(false);
       
        /*
         * Lock down the whole scene.
         */
        rootNode.setLocks(SceneElement.LOCKED_ALL);
       
    }

}

10

Hello,



thank you for your work sbayless! I think this is a quite an interesting feature and exactly what i need right now.



I think there is something missing in the jme2 implementation. As seen in the jme1 one a refreshBranch() call recursivly checks both Parents and Children of the current node. Right now i think it only has effects on the current node; parents and children are not getting touched.



Here is a testcase with a node in front of your capsule; refresh Bounds get called on that node and nothing happens.



import com.jme.app.SimpleGame;
import com.jme.bounding.BoundingBox;
import com.jme.image.Texture;
import com.jme.input.FirstPersonHandler;
import com.jme.math.Quaternion;
import com.jme.math.Vector3f;
import com.jme.scene.Node;
import com.jme.scene.Spatial;
import com.jme.scene.shape.Capsule;
import com.jme.scene.state.CullState;
import com.jme.scene.state.TextureState;
import com.jme.util.TextureManager;

public class ttest extends SimpleGame {


    private Quaternion rotQuat = new Quaternion();
    private float angle = 0;
    private Vector3f axis = new Vector3f(1, 1, 0).normalizeLocal();
    private Capsule t;
    private Node node;

    /**
     * Entry point for the test,
     *
     * @param args
     */
    public static void main(String[] args) {
       ttest app = new ttest();
         app.setConfigShowMode(ConfigShowMode.AlwaysShow);
        app.start();
    }

    protected void simpleUpdate() {
        if (timer.getTimePerFrame() < 1) {
            angle = angle + (timer.getTimePerFrame() * 1);
            if (angle > 360) {
                angle = 0;
            }
        }

        rotQuat.fromAngleNormalAxis(angle, axis);
        node.setLocalRotation(rotQuat);
        
        /*
         * Call this anytime a scene element has changed, so that it will
         * be updated on the next update cycle.
         */
        node.refreshBranch();
    }

    /**
     * builds the trimesh.
     *
     * @see com.jme.app.SimpleGame#initGame()
     */
    protected void simpleInitGame() {
        display.setTitle("Cylinder Test");

        t = new Capsule("Capsule", 40, 32, 16, 4, 10);
        node = new Node("testt");
        t.setModelBound(new BoundingBox());
        t.updateModelBound();
        node.attachChild(t);
        
        CullState cs = display.getRenderer().createCullState();
        cs.setCullFace(CullState.Face.Back);
        rootNode.setRenderState(cs);
        
        input = new FirstPersonHandler(cam, 10f, 1f);
        
        rootNode.attachChild(node);
        
        TextureState ts = display.getRenderer().createTextureState();
        ts.setEnabled(true);
        ts.setTexture(TextureManager.loadTexture(ttest.class
                .getClassLoader().getResource("jmetest/data/images/Monkey.jpg"),
                Texture.MinificationFilter.BilinearNearestMipMap, Texture.MagnificationFilter.Bilinear));
        ts.getTexture().setWrap(Texture.WrapMode.Repeat);
        rootNode.setRenderState(ts);

        lightState.setTwoSidedLighting(false);
        
        rootNode.setLocks(Spatial.LOCKED_ALL);
    }
}



...or better readable: http://paste2.org/p/620705

I edited the following function (like you did in jme1) in Spatial.java:


    /**
     * Refresh upward in this scene graph to the root node
     */
    protected void refreshBranchToRoot(int locksToDirty)
    {
       this.dirtyLocks |= locksToDirty;
       
       if (this.getParent()!=null) {
          this.getParent().refreshBranchToRoot(locksToDirty);
       }       
    }



... and in Node.java:


    @Override
    protected void refreshBranchToLeaves(int locksToDirty) {
       super.refreshBranchToLeaves(locksToDirty);
           if(children == null) {
                return;
           }
         
           for (int i = 0, cSize = children.size(); i < cSize; i++) {
              Spatial pkChild = getChild(i);
              if (pkChild != null)
                 pkChild.refreshBranchToLeaves(locksToDirty);
           }
    }



It seems to work now. I created a patch suited for jme 2.0.1. I attached it for everybody who is interested.
11

Hey good catch vkb! I still use JME 1.0, and so the JME2 patch didn't get as much attention as it deserved.

12

This seems like a great improvement!

I'm glad work is still being done on JME1 and 2.

Has this been committed to the trunk?