* Introduce GeometryGroupNode as abstract class for implementations that group / optimize geometries

* Reimplement BatchNode on top of GeometryBatchNode

jme3-core/src/main/java/com/jme3/scene/BatchNode.java

@@ -34,7 +34,6 @@ package com.jme3.scene;
 import com.jme3.export.*;
 import com.jme3.material.Material;
 import com.jme3.math.Matrix4f;
-import com.jme3.math.Transform;
 import com.jme3.math.Vector3f;
 import com.jme3.scene.mesh.IndexBuffer;
 import com.jme3.util.SafeArrayList;
@@ -65,7 +64,7 @@ import java.util.logging.Logger;
  * TODO more automagic (batch when needed in the updateLogicalState)
  * @author Nehon
  */
-public class BatchNode extends Node implements Savable {
+public class BatchNode extends GeometryGroupNode implements Savable {
 
     private static final Logger logger = Logger.getLogger(BatchNode.class.getName());
     /**
@@ -96,6 +95,29 @@ public class BatchNode extends Node implements Savable {
     public BatchNode(String name) {
         super(name);
     }
+    
+    @Override
+    public void onTransformChange(Geometry geom) {
+        updateSubBatch(geom);
+    }
+
+    @Override
+    public void onMaterialChange(Geometry geom) {
+        throw new UnsupportedOperationException(
+                "Cannot set the material of a batched geometry, "
+                + "change the material of the parent BatchNode.");
+    }
+
+    @Override
+    public void onMeshChange(Geometry geom) {
+        throw new UnsupportedOperationException(
+                "Cannot set the mesh of a batched geometry");
+    }
+
+    @Override
+    public void onGeoemtryUnassociated(Geometry geom) {
+        setNeedsFullRebatch(true);
+    }
 
     @Override
     public void updateGeometricState() {
@@ -284,8 +306,8 @@ public class BatchNode extends Node implements Savable {
             }
         } else if (s instanceof Geometry) {
             Geometry g = (Geometry) s;
-            if (g.isBatched()) {
+            if (g.isGrouped()) {
-                g.unBatch();
+                g.unassociateFromGroupNode();
             }
         }
     }
@@ -297,7 +319,7 @@ public class BatchNode extends Node implements Savable {
 
             if (!isBatch(n) && n.getBatchHint() != BatchHint.Never) {
                 Geometry g = (Geometry) n;
-                if (!g.isBatched() || rebatch) {
+                if (!g.isGrouped() || rebatch) {
                     if (g.getMaterial() == null) {
                         throw new IllegalStateException("No material is set for Geometry: " + g.getName() + " please set a material before batching");
                     }
@@ -542,7 +564,7 @@ public class BatchNode extends Node implements Savable {
         for (Geometry geom : geometries) {
             Mesh inMesh = geom.getMesh();
             if (!isBatch(geom)) {
-                geom.batch(this, globalVertIndex);
+                geom.associateWithGroupNode(this, globalVertIndex);
             }
 
             int geomVertCount = inMesh.getVertexCount();

jme3-core/src/main/java/com/jme3/scene/Geometry.java

@@ -41,7 +41,7 @@ import com.jme3.export.JmeImporter;
 import com.jme3.export.OutputCapsule;
 import com.jme3.material.Material;
 import com.jme3.math.Matrix4f;
-import com.jme3.math.Transform;
+import com.jme3.renderer.Camera;
 import com.jme3.scene.VertexBuffer.Type;
 import com.jme3.util.TempVars;
 import java.io.IOException;
@@ -71,12 +71,15 @@ public class Geometry extends Spatial {
      */
     protected boolean ignoreTransform = false;
     protected transient Matrix4f cachedWorldMat = new Matrix4f();
+    
     /**
-     * used when geometry is batched
+     * Specifies which {@link GeometryGroupNode} this <code>Geometry</code>
+     * is managed by.
      */
-    protected BatchNode batchNode = null;
+    protected GeometryGroupNode groupNode;
     /**
-     * the start index of this geometry's mesh in the batchNode mesh
+     * The start index of this <code>Geometry's</code> inside 
+     * the {@link GeometryGroupNode}.
      */
     protected int startIndex;    
     /**
@@ -106,12 +109,22 @@ public class Geometry extends Spatial {
      */
     public Geometry(String name, Mesh mesh) {
         this(name);
+        
         if (mesh == null) {
-            throw new NullPointerException();
+            throw new IllegalArgumentException("mesh cannot be null");
         }
 
         this.mesh = mesh;
     }
+    
+    @Override
+    public boolean checkCulling(Camera cam) {
+        if (isGrouped()) {
+            setLastFrustumIntersection(Camera.FrustumIntersect.Outside);
+            return false;
+        }
+        return super.checkCulling(cam);
+    }
 
     /**
      * @return If ignoreTransform mode is set.
@@ -148,6 +161,10 @@ public class Geometry extends Spatial {
         }
 
         lodLevel = lod;
+        
+        if (isGrouped()) {
+            groupNode.onMeshChange(this);
+        }
     }
 
     /**
@@ -192,12 +209,13 @@ public class Geometry extends Spatial {
         if (mesh == null) {
             throw new IllegalArgumentException();
         }
-        if (isBatched()) {
-            throw new UnsupportedOperationException("Cannot set the mesh of a batched geometry");
-        }
 
         this.mesh = mesh;
         setBoundRefresh();
+        
+        if (isGrouped()) {
+            groupNode.onMeshChange(this);
+        }
     }
 
     /**
@@ -218,10 +236,11 @@ public class Geometry extends Spatial {
      */
     @Override
     public void setMaterial(Material material) {
-        if (isBatched()) {
-            throw new UnsupportedOperationException("Cannot set the material of a batched geometry, change the material of the parent BatchNode.");
-        }
         this.material = material;
+        
+        if (isGrouped()) {
+            groupNode.onMaterialChange(this);
+        }
     }
 
     /**
@@ -278,39 +297,48 @@ public class Geometry extends Spatial {
 
     @Override
     protected void updateWorldTransforms() {
-
         super.updateWorldTransforms();
         computeWorldMatrix();
 
-        if (isBatched()) {        
+        if (isGrouped()) {
-            batchNode.updateSubBatch(this);     
+            groupNode.onTransformChange(this);   
         }
+        
         // geometry requires lights to be sorted
         worldLights.sort(true);
     }
 
     /**
-     * Batch this geometry, should only be called by the BatchNode.
+     * Associate this <code>Geometry</code> with a {@link GeometryGroupNode}.
-     * @param node the batchNode
+     * 
-     * @param startIndex the starting index of this geometry in the batched mesh
+     * Should only be called by the parent {@link GeometryGroupNode}.
+     * 
+     * @param node Which {@link GeometryGroupNode} to associate with.
+     * @param startIndex The starting index of this geometry in the group.
      */
-    protected void batch(BatchNode node, int startIndex) {
+    protected void associateWithGroupNode(GeometryGroupNode node, int startIndex) {
-        this.batchNode = node;
+        if (isGrouped()) {
-        this.startIndex = startIndex;       
+            unassociateFromGroupNode();
-        setCullHint(CullHint.Always);
+        }
+        
+        this.groupNode = node;
+        this.startIndex = startIndex;
     }
 
     /**
-     * unBatch this geometry. 
+     * Removes the {@link GeometryGroupNode} association from this 
+     * <code>Geometry</code>.
+     * 
+     * Should only be called by the parent {@link GeometryGroupNode}.
      */
-    protected void unBatch() {
+    protected void unassociateFromGroupNode() {
-        this.startIndex = 0;
+        if (groupNode != null) {
-        //once the geometry is removed from the screnegraph the batchNode needs to be rebatched.
+            // Once the geometry is removed 
-        if (batchNode != null) {
+            // from the parent, the group node needs to be updated.
-            this.batchNode.setNeedsFullRebatch(true);
+            groupNode.onGeoemtryUnassociated(this);
-            this.batchNode = null;
+            groupNode = null;
+            startIndex = 0;
         }
-        setCullHint(CullHint.Dynamic);
     }
 
     @Override
@@ -321,9 +349,10 @@ public class Geometry extends Spatial {
     @Override
     protected void setParent(Node parent) {
         super.setParent(parent);
-        //if the geometry is batched we also have to unbatch it
+        
-        if (parent == null && isBatched()) {
+        // If the geometry is managed by group node we need to unassociate.
-            unBatch();
+        if (parent == null && isGrouped()) {
+            unassociateFromGroupNode();
         }
     }
 
@@ -424,8 +453,22 @@ public class Geometry extends Spatial {
     protected void breadthFirstTraversal(SceneGraphVisitor visitor, Queue<Spatial> queue) {
     }
 
+    /**
+     * Determine whether this <code>Geometry</code> is managed by a 
+     * {@link GeometryGroupNode} or not.
+     * 
+     * @return True if managed by a {@link GeometryGroupNode}.
+     */
+    public boolean isGrouped() {
+        return groupNode != null;
+    }
+    
+    /**
+     * @deprecated Use {@link #isGrouped()} instead.
+     */
+    @Deprecated
     public boolean isBatched() {
-        return batchNode != null;
+        return isGrouped();
     }
 
     /**
@@ -438,11 +481,14 @@ public class Geometry extends Spatial {
     @Override
     public Geometry clone(boolean cloneMaterial) {
         Geometry geomClone = (Geometry) super.clone(cloneMaterial);
-        //this geometry is batched but the clonned one should not be
+        
-        if (isBatched()) {
+        // This geometry is managed,
-            geomClone.batchNode = null;
+        // but the cloned one is not attached to anything, hence not managed.
-            geomClone.unBatch();
+        if (isGrouped()) {
+            groupNode = null;
+            startIndex = 0;
         }
+        
         geomClone.cachedWorldMat = cachedWorldMat.clone();
         if (material != null) {
             if (cloneMaterial) {

jme3-core/src/main/java/com/jme3/scene/GeometryGroupNode.java

@@ -0,0 +1,73 @@
+package com.jme3.scene;
+
+/**
+ * An abstract class for implementations that perform grouping of geometries
+ * via instancing or batching.
+ * 
+ * @author Kirill Vainer
+ */
+public abstract class GeometryGroupNode extends Node {
+    
+    /**
+     * Construct a <code>GeometryGroupNode</code>
+     */
+    public GeometryGroupNode() {
+        super();
+    }
+
+    /**
+     * Construct a <code>GeometryGroupNode</code>
+     * 
+     * @param name The name of the GeometryGroupNode.
+     */
+    public GeometryGroupNode(String name) {
+        super(name);
+    }
+    
+    /**
+     * Called by {@link Geometry geom} to specify that its world transform
+     * has been changed.
+     * 
+     * @param geom The Geometry whose transform changed.
+     */
+    public abstract void onTransformChange(Geometry geom);
+    
+    /**
+     * Called by {@link Geometry geom} to specify that its 
+     * {@link Geometry#setMaterial(com.jme3.material.Material) material}
+     * has been changed.
+     * 
+     * @param geom The Geometry whose material changed.
+     * 
+     * @throws UnsupportedOperationException If this implementation does
+     * not support dynamic material changes.
+     */
+    public abstract void onMaterialChange(Geometry geom);
+    
+    /**
+     * Called by {@link Geometry geom} to specify that its 
+     * {@link Geometry#setMesh(com.jme3.scene.Mesh) mesh}
+     * has been changed.
+     * 
+     * This is also called when the geometry's 
+     * {@link Geometry#setLodLevel(int) lod level} changes.
+     * 
+     * @param geom The Geometry whose mesh changed.
+     * 
+     * @throws UnsupportedOperationException If this implementation does
+     * not support dynamic mesh changes.
+     */
+    public abstract void onMeshChange(Geometry geom);
+    
+    /**
+     * Called by {@link Geometry geom} to specify that it
+     * has been unassociated from its <code>GeoemtryGroupNode</code>.
+     * 
+     * Unassociation occurs when the {@link Geometry} is 
+     * {@link Spatial#removeFromParent() detached} from its parent
+     * {@link Node}.
+     * 
+     * @param geom The Geometry which is being unassociated.
+     */
+    public abstract void onGeoemtryUnassociated(Geometry geom);
+}