Add toggleable per-frame bounding-volume updates for animated models (SkinningControl)

jme3-core/src/main/java/com/jme3/anim/SkinningControl.java

@@ -31,6 +31,8 @@
  */
 package com.jme3.anim;
 
+import com.jme3.bounding.BoundingBox;
+import com.jme3.bounding.BoundingVolume;
 import com.jme3.export.InputCapsule;
 import com.jme3.export.JmeExporter;
 import com.jme3.export.JmeImporter;
@@ -131,6 +133,13 @@ public class SkinningControl extends AbstractControl implements JmeCloneable {
      */
     private transient Matrix4f[] boneOffsetMatrices;
 
+    /**
+     * When true, the bounding volumes of animated geometries are updated each
+     * frame to match the current pose, ensuring correct frustum culling.
+     * Disabled by default because it adds CPU cost every frame.
+     */
+    private boolean updateBounds = false;
+
     private MatParamOverride numberOfJointsParam = new MatParamOverride(VarType.Int, "NumberOfBones", null);
     private MatParamOverride jointMatricesParam = new MatParamOverride(VarType.Matrix4Array, "BoneMatrices", null);
 
@@ -246,6 +255,32 @@ public boolean isHardwareSkinningUsed() {
         return hwSkinningEnabled;
     }
 
+    /**
+     * Enables or disables per-frame bounding-volume updates for animated
+     * geometries. When enabled, the bounding volume of each deformed geometry
+     * is recomputed every render frame to match the current animation pose,
+     * ensuring correct frustum culling at the cost of additional CPU work.
+     * Disabled by default.
+     *
+     * @param updateBounds true to update bounds each frame, false to keep
+     *                     static bind-pose bounds (default=false)
+     * @see #isUpdateBounds()
+     */
+    public void setUpdateBounds(boolean updateBounds) {
+        this.updateBounds = updateBounds;
+    }
+
+    /**
+     * Returns whether per-frame bounding-volume updates are enabled for
+     * animated geometries.
+     *
+     * @return true if bounds are updated each frame, false otherwise
+     * @see #setUpdateBounds(boolean)
+     */
+    public boolean isUpdateBounds() {
+        return updateBounds;
+    }
+
     /**
      * Recursively finds and adds animated geometries to the targets list.
      *
@@ -297,6 +332,10 @@ private void controlRenderSoftware() {
             // NOTE: This assumes code higher up has already ensured this mesh is animated.
             // Otherwise, a crash will happen in skin update.
             applySoftwareSkinning(mesh, boneOffsetMatrices);
+            if (updateBounds) {
+                // Update the mesh bounding volume to reflect the animated vertex positions.
+                geometry.updateModelBound();
+            }
         }
     }
 
@@ -306,6 +345,18 @@ private void controlRenderSoftware() {
     private void controlRenderHardware() {
         boneOffsetMatrices = armature.computeSkinningMatrices();
         jointMatricesParam.setValue(boneOffsetMatrices);
+
+        if (updateBounds) {
+            // Hardware skinning transforms vertices on the GPU, so the CPU-side vertex
+            // buffer is not updated. Compute the animated bounding volume from the bind
+            // pose positions and the current skinning matrices so culling is correct.
+            for (Geometry geometry : targets) {
+                Mesh mesh = geometry.getMesh();
+                if (mesh != null && mesh.isAnimated()) {
+                    updateSkinnedMeshBound(geometry, mesh, boneOffsetMatrices);
+                }
+            }
+        }
     }
 
     @Override
@@ -751,6 +802,108 @@ private void applySkinningTangents(Mesh mesh, Matrix4f[] offsetMatrices, VertexB
         tb.updateData(ftb);
     }
 
+    /**
+     * Computes the bounding volume of an animated mesh from the bind pose
+     * positions and the current skinning matrices, then sets it on the geometry.
+     * This is used during hardware skinning to keep culling correct, since the
+     * GPU-transformed vertex positions are not reflected in the CPU-side vertex
+     * buffer.
+     *
+     * @param geometry       the geometry whose bound needs to be updated
+     * @param mesh           the animated mesh
+     * @param offsetMatrices the bone offset matrices for this frame
+     */
+    private static void updateSkinnedMeshBound(Geometry geometry, Mesh mesh,
+            Matrix4f[] offsetMatrices) {
+        VertexBuffer bindPosVB = mesh.getBuffer(Type.BindPosePosition);
+        if (bindPosVB == null) {
+            return;
+        }
+        VertexBuffer boneIndexVB = mesh.getBuffer(Type.BoneIndex);
+        VertexBuffer boneWeightVB = mesh.getBuffer(Type.BoneWeight);
+        if (boneIndexVB == null || boneWeightVB == null) {
+            return;
+        }
+        int maxWeightsPerVert = mesh.getMaxNumWeights();
+        if (maxWeightsPerVert <= 0) {
+            return;
+        }
+        int fourMinusMaxWeights = 4 - maxWeightsPerVert;
+
+        FloatBuffer bindPos = (FloatBuffer) bindPosVB.getData();
+        bindPos.rewind();
+        IndexBuffer boneIndex = IndexBuffer.wrapIndexBuffer(boneIndexVB.getData());
+        FloatBuffer boneWeightBuf = (FloatBuffer) boneWeightVB.getData();
+        boneWeightBuf.rewind();
+        // Use array() when available (heap buffer), otherwise copy to a local array.
+        float[] weights;
+        if (boneWeightBuf.hasArray()) {
+            weights = boneWeightBuf.array();
+        } else {
+            weights = new float[boneWeightBuf.limit()];
+            boneWeightBuf.get(weights);
+        }
+        int idxWeights = 0;
+
+        int numVerts = bindPos.limit() / 3;
+        float minX = Float.POSITIVE_INFINITY, minY = Float.POSITIVE_INFINITY,
+                minZ = Float.POSITIVE_INFINITY;
+        float maxX = Float.NEGATIVE_INFINITY, maxY = Float.NEGATIVE_INFINITY,
+                maxZ = Float.NEGATIVE_INFINITY;
+
+        for (int v = 0; v < numVerts; v++) {
+            float vtx = bindPos.get();
+            float vty = bindPos.get();
+            float vtz = bindPos.get();
+
+            float rx, ry, rz;
+            if (weights[idxWeights] == 0) {
+                idxWeights += 4;
+                rx = vtx;
+                ry = vty;
+                rz = vtz;
+            } else {
+                rx = 0;
+                ry = 0;
+                rz = 0;
+                for (int w = 0; w < maxWeightsPerVert; w++) {
+                    float weight = weights[idxWeights];
+                    Matrix4f mat = offsetMatrices[boneIndex.get(idxWeights++)];
+                    rx += (mat.m00 * vtx + mat.m01 * vty + mat.m02 * vtz + mat.m03) * weight;
+                    ry += (mat.m10 * vtx + mat.m11 * vty + mat.m12 * vtz + mat.m13) * weight;
+                    rz += (mat.m20 * vtx + mat.m21 * vty + mat.m22 * vtz + mat.m23) * weight;
+                }
+                idxWeights += fourMinusMaxWeights;
+            }
+
+            if (rx < minX) minX = rx;
+            if (rx > maxX) maxX = rx;
+            if (ry < minY) minY = ry;
+            if (ry > maxY) maxY = ry;
+            if (rz < minZ) minZ = rz;
+            if (rz > maxZ) maxZ = rz;
+        }
+
+        // Reuse the existing BoundingBox if possible to avoid allocation.
+        BoundingVolume bv = mesh.getBound();
+        BoundingBox bbox;
+        if (bv instanceof BoundingBox) {
+            bbox = (BoundingBox) bv;
+        } else {
+            bbox = new BoundingBox();
+        }
+        TempVars vars = TempVars.get();
+        try {
+            vars.vect1.set(minX, minY, minZ);
+            vars.vect2.set(maxX, maxY, maxZ);
+            bbox.setMinMax(vars.vect1, vars.vect2);
+        } finally {
+            vars.release();
+        }
+        // setModelBound() updates the mesh bound and triggers a world-bound refresh.
+        geometry.setModelBound(bbox);
+    }
+
     /**
      * Serialize this Control to the specified exporter, for example when saving
      * to a J3O file.
@@ -763,6 +916,7 @@ public void write(JmeExporter ex) throws IOException {
         super.write(ex);
         OutputCapsule oc = ex.getCapsule(this);
         oc.write(armature, "armature", null);
+        oc.write(updateBounds, "updateBounds", false);
     }
 
     /**
@@ -777,6 +931,7 @@ public void read(JmeImporter im) throws IOException {
         super.read(im);
         InputCapsule in = im.getCapsule(this);
         armature = (Armature) in.readSavable("armature", null);
+        updateBounds = in.readBoolean("updateBounds", false);
 
         for (MatParamOverride mpo : spatial.getLocalMatParamOverrides().getArray()) {
             if (mpo.getName().equals("NumberOfBones") || mpo.getName().equals("BoneMatrices")) {

jme3-core/src/test/java/com/jme3/test/PreventCoreIssueRegressions.java

@@ -40,6 +40,7 @@
 import com.jme3.app.state.ScreenshotAppState;
 import com.jme3.asset.AssetManager;
 import com.jme3.asset.DesktopAssetManager;
+import com.jme3.bounding.BoundingVolume;
 import com.jme3.input.InputManager;
 import com.jme3.input.dummy.DummyKeyInput;
 import com.jme3.input.dummy.DummyMouseInput;
@@ -130,4 +131,49 @@ public void testIssue1138() {
                     Vector3f.isValidVector(joint.getLocalTranslation()));
         }
     }
+
+    /**
+     * Test case for JME issue #343: Animated models should have proper model bound.
+     *
+     * <p>When software skinning is used, calling controlRender should update the
+     * bounding volumes of the animated geometries to reflect their current pose.
+     */
+    @Test
+    public void testIssue343() {
+        AssetManager am = JmeSystem.newAssetManager(
+                PreventCoreIssueRegressions.class.getResource("/com/jme3/asset/Desktop.cfg"));
+        Node cgModel = (Node) am.loadModel("Models/Elephant/Elephant.mesh.xml");
+        cgModel.scale(0.04f);
+
+        AnimComposer composer = cgModel.getControl(AnimComposer.class);
+        SkinningControl sControl = cgModel.getControl(SkinningControl.class);
+
+        // Force software skinning so bounds are computed from CPU vertex positions.
+        sControl.setHardwareSkinningPreferred(false);
+        // Enable per-frame bounds update (off by default).
+        sControl.setUpdateBounds(true);
+
+        // Record the world bound in the bind pose.
+        cgModel.updateGeometricState();
+        BoundingVolume bindPoseBound = cgModel.getWorldBound().clone();
+
+        // Advance the "legUp" animation, which raises a leg well beyond the bind pose.
+        composer.setCurrentAction("legUp");
+        cgModel.updateLogicalState(0.5f);
+
+        // Simulate the render pass: controlRender applies software skinning and
+        // calls geometry.updateModelBound() on each target.
+        RenderManager rm = new RenderManager(new NullRenderer());
+        ViewPort vp = rm.createMainView("test", new Camera(1, 1));
+        sControl.render(rm, vp);
+
+        // Propagate the refreshed bounds up the scene graph.
+        cgModel.updateGeometricState();
+        BoundingVolume animatedBound = cgModel.getWorldBound().clone();
+
+        // The bounding volume must differ from the bind pose bound.
+        Assert.assertFalse(
+                "Model bound should change after animation is applied via software skinning",
+                bindPoseBound.equals(animatedBound));
+    }
 }