diff --git a/jme3-core/src/main/java/com/jme3/scene/Geometry.java b/jme3-core/src/main/java/com/jme3/scene/Geometry.java index 4d30c61e0d..60d7e30980 100644 --- a/jme3-core/src/main/java/com/jme3/scene/Geometry.java +++ b/jme3-core/src/main/java/com/jme3/scene/Geometry.java @@ -308,6 +308,14 @@ public void updateModelBound() { setBoundRefresh(); } + @Override + protected void setBoundRefresh() { + super.setBoundRefresh(); + if (isGrouped()) { + groupNode.onBoundChange(this); + } + } + /** * updateWorldBound updates the bounding volume that contains * this geometry. The location of the geometry is based on the location of diff --git a/jme3-core/src/main/java/com/jme3/scene/GeometryGroupNode.java b/jme3-core/src/main/java/com/jme3/scene/GeometryGroupNode.java index db245392ca..fea6be727e 100644 --- a/jme3-core/src/main/java/com/jme3/scene/GeometryGroupNode.java +++ b/jme3-core/src/main/java/com/jme3/scene/GeometryGroupNode.java @@ -42,6 +42,18 @@ public GeometryGroupNode(String name) { * @param geom The Geometry whose transform changed. */ public abstract void onTransformChange(Geometry geom); + + /** + * Called when a grouped geometry's world bound needs to be refreshed. + * The bound has not necessarily been recomputed yet. This notification + * includes changes to the model bound as well as transform changes. + * + *

The default implementation does nothing.

+ * + * @param geom the geometry whose world bound was invalidated + */ + public void onBoundChange(Geometry geom) { + } /** * Called by {@link Geometry geom} to specify that its diff --git a/jme3-core/src/main/java/com/jme3/scene/instancing/InstancedNode.java b/jme3-core/src/main/java/com/jme3/scene/instancing/InstancedNode.java index 1d61893a5f..37fffa6623 100644 --- a/jme3-core/src/main/java/com/jme3/scene/instancing/InstancedNode.java +++ b/jme3-core/src/main/java/com/jme3/scene/instancing/InstancedNode.java @@ -393,6 +393,15 @@ public void onTransformChange(Geometry geom) { // Handled automatically } + @Override + public void onBoundChange(Geometry geom) { + InstancedGeometry ig = igByGeom.get(geom); + if (ig != null) { + // Keep the group's bound lazy and coalesce changes to its sources. + ig.forceRefresh(false, true, false); + } + } + @Override public void onMaterialChange(Geometry geom) { relocateInInstancedGeometry(geom); diff --git a/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsRefreshTest.java b/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsRefreshTest.java new file mode 100644 index 0000000000..67672fe822 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsRefreshTest.java @@ -0,0 +1,284 @@ +/* + * Copyright (c) 2026 jMonkeyEngine + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * * Neither the name of 'jMonkeyEngine' nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.jme3.scene.instancing; + +import com.jme3.asset.DesktopAssetManager; +import com.jme3.bounding.BoundingBox; +import com.jme3.bounding.BoundingVolume; +import com.jme3.material.Material; +import com.jme3.material.MaterialDef; +import com.jme3.math.Transform; +import com.jme3.math.Vector3f; +import com.jme3.scene.Geometry; +import com.jme3.scene.Spatial; +import com.jme3.scene.shape.Box; +import com.jme3.shader.VarType; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * Verifies that changed instance bounds invalidate only the affected groups, + * and that each group is refreshed at most once during a scene update. + */ +public class InstancedBoundsRefreshTest { + + @Test + public void unchangedGroupsRetainCachedBounds() { + Fixture fixture = new Fixture(); + fixture.update(); + + fixture.assertRefreshCounts(0, 0); + fixture.assertBounds(); + } + + @Test + public void severalSourceChangesRefreshOnlyTheirGroupOnce() { + Fixture fixture = new Fixture(); + fixture.geometries[0][0].move(20, 0, 0); + fixture.geometries[0][1].move(30, 0, 0); + fixture.geometries[0][0].move(10, 0, 0); + fixture.update(); + + fixture.assertRefreshCounts(1, 0); + fixture.assertBounds(); + } + + @Test + public void movingNodeRefreshesEachGroupOnce() { + Fixture fixture = new Fixture(); + fixture.node.move(20, 0, 0); + fixture.update(); + + fixture.assertRefreshCounts(1, 1); + fixture.assertBounds(); + } + + @Test + public void movingUninstancedChildLeavesGroupsCached() { + Fixture fixture = new Fixture(); + fixture.uninstanced.move(20, 0, 0); + fixture.update(); + + fixture.assertRefreshCounts(0, 0); + fixture.assertBounds(); + } + + @Test + public void membershipChangesRefreshOnlyTheirGroup() { + Fixture fixture = new Fixture(); + Geometry added = fixture.createGeometry(0, 50); + fixture.node.attachChild(added); + fixture.node.instance(); + fixture.update(); + + fixture.assertRefreshCounts(1, 0); + fixture.assertBounds(); + + fixture.resetCounts(); + fixture.node.detachChild(added); + fixture.update(); + + fixture.assertRefreshCounts(1, 0); + fixture.assertBounds(); + } + + @Test + public void boundOnlyRefreshInvalidatesOnlyItsGroup() { + Fixture fixture = new Fixture(); + fixture.meshes[0].setBound(new CountingBox(fixture.counters[0], 4, 5, 6)); + fixture.geometries[0][0].forceRefresh(false, true, false); + fixture.update(); + + fixture.assertRefreshCounts(1, 0); + fixture.assertBounds(); + } + + @Test + public void changingModelBoundRefreshesOnlyItsGroup() { + Fixture fixture = new Fixture(); + fixture.geometries[0][0].setModelBound( + new CountingBox(fixture.counters[0], 4, 5, 6)); + fixture.update(); + + fixture.assertRefreshCounts(1, 0); + fixture.assertBounds(); + } + + @Test + public void queryingBatchBoundRefreshesBeforeNodeTraversal() { + Fixture fixture = new Fixture(); + fixture.geometries[0][0].move(20, 0, 0); + fixture.batches[0].getWorldBound(); + + fixture.assertRefreshCounts(1, 0); + fixture.update(); + fixture.assertRefreshCounts(1, 0); + fixture.assertBounds(); + } + + @Test + public void batchesBeforeSourcesAreRefreshedOnlyOnce() { + Fixture fixture = new Fixture(); + fixture.node.swapChildren(fixture.node.getChildIndex(fixture.batches[0]), 0); + fixture.node.swapChildren(fixture.node.getChildIndex(fixture.batches[1]), 1); + fixture.geometries[0][0].move(20, 0, 0); + fixture.geometries[0][1].move(30, 0, 0); + fixture.update(); + + fixture.assertRefreshCounts(1, 0); + fixture.assertBounds(); + } + + private static class Fixture { + final InstancedNode node = new InstancedNode("instances"); + final Geometry[][] geometries = new Geometry[2][2]; + final Box[] meshes = new Box[2]; + final Material[] materials = new Material[2]; + final InstancedGeometry[] batches = new InstancedGeometry[2]; + final RefreshCounter[] counters = {new RefreshCounter(), new RefreshCounter()}; + final Geometry uninstanced = new Geometry("uninstanced", new Box(1, 1, 1)); + + Fixture() { + MaterialDef def = new MaterialDef(new DesktopAssetManager(), "test"); + def.addMaterialParam(VarType.Boolean, "UseInstancing", false); + for (int group = 0; group < geometries.length; group++) { + meshes[group] = new Box(1, 1, 1); + meshes[group].setBound(new CountingBox(counters[group], 1, 1, 1)); + materials[group] = new Material(def); + materials[group].setBoolean("UseInstancing", true); + for (int i = 0; i < geometries[group].length; i++) { + Geometry geometry = createGeometry(group, group * 20 + i * 4); + geometries[group][i] = geometry; + node.attachChild(geometry); + } + } + uninstanced.setMaterial(materials[0]); + uninstanced.setBatchHint(Spatial.BatchHint.Never); + node.attachChild(uninstanced); + node.instance(); + for (Spatial child : node.getChildren()) { + if (child instanceof InstancedGeometry) { + InstancedGeometry batch = (InstancedGeometry) child; + for (int group = 0; group < materials.length; group++) { + if (batch.getMaterial() == materials[group]) { + batches[group] = batch; + } + } + } + } + update(); + for (int group = 0; group < batches.length; group++) { + assertNotNull(batches[group]); + counters[group].batchBound = batches[group].getWorldBound(); + } + assertBounds(); + resetCounts(); + } + + Geometry createGeometry(int group, float x) { + Geometry geometry = new Geometry("source", meshes[group]); + geometry.setMaterial(materials[group]); + geometry.setLocalTranslation(x, 0, 10); + return geometry; + } + + void update() { + node.updateLogicalState(0); + node.updateGeometricState(); + } + + void resetCounts() { + for (RefreshCounter counter : counters) { + counter.refreshes = 0; + } + } + + void assertRefreshCounts(int first, int second) { + assertEquals(first, counters[0].refreshes, "First group refreshes"); + assertEquals(second, counters[1].refreshes, "Second group refreshes"); + } + + void assertBounds() { + for (InstancedGeometry batch : batches) { + BoundingVolume expected = null; + for (Geometry geometry : batch.getGeometries()) { + if (geometry == null) { + continue; + } + if (expected == null) { + expected = geometry.getWorldBound().clone(); + } else { + expected.mergeLocal(geometry.getWorldBound()); + } + } + assertEquals(expected, batch.getWorldBound()); + } + } + } + + private static class RefreshCounter { + BoundingVolume batchBound; + int refreshes; + } + + /** + * A group rebuild starts by copying its first source bound into the cached + * group bound. Count only copies into that exact object, so ordinary node + * updates and bound queries do not affect the measurement. + */ + private static class CountingBox extends BoundingBox { + private final RefreshCounter counter; + + CountingBox(RefreshCounter counter, float xExtent, float yExtent, float zExtent) { + super(Vector3f.ZERO, xExtent, yExtent, zExtent); + this.counter = counter; + } + + @Override + public BoundingVolume transform(Transform transform, BoundingVolume store) { + if (!(store instanceof CountingBox)) { + store = new CountingBox(counter, 0, 0, 0); + } + return super.transform(transform, store); + } + + @Override + public BoundingVolume clone(BoundingVolume store) { + if (store != null && store == counter.batchBound) { + counter.refreshes++; + } + return super.clone(store); + } + } +} diff --git a/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsTest.java b/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsTest.java new file mode 100644 index 0000000000..fc23d6734b --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsTest.java @@ -0,0 +1,190 @@ +/* + * Copyright (c) 2026 jMonkeyEngine + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * * Neither the name of 'jMonkeyEngine' nor the names of its contributors + * may be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.jme3.scene.instancing; + +import com.jme3.asset.DesktopAssetManager; +import com.jme3.bounding.BoundingBox; +import com.jme3.light.DefaultLightFilter; +import com.jme3.light.LightList; +import com.jme3.light.PointLight; +import com.jme3.material.Material; +import com.jme3.material.MaterialDef; +import com.jme3.math.Vector3f; +import com.jme3.renderer.Camera; +import com.jme3.scene.Geometry; +import com.jme3.scene.Node; +import com.jme3.scene.Spatial; +import com.jme3.scene.shape.Box; +import com.jme3.shader.VarType; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * Verifies that instance bounds follow changes to the source geometries. + */ +public class InstancedBoundsTest { + + @Test + public void movingInstanceUpdatesBounds() { + checkMovedInstanceBounds(false); + } + + @Test + public void movingInstanceAfterItsBatchUpdatesBounds() { + checkMovedInstanceBounds(true); + } + + private void checkMovedInstanceBounds(boolean batchFirst) { + Fixture fixture = new Fixture(false); + if (batchFirst) { + fixture.node.swapChildren(0, 1); + } + fixture.geometry.move(20, 0, 0); + fixture.update(); + + assertEquals(new Vector3f(20, 0, 10), fixture.geometry.getWorldBound().getCenter()); + fixture.assertBounds(); + } + + @Test + public void movingNestedParentUpdatesBounds() { + Fixture fixture = new Fixture(true); + fixture.geometry.getParent().move(20, 0, 0); + fixture.update(); + fixture.assertBounds(); + } + + @Test + public void parentTraversalUpdatesInstancedChildBounds() { + Fixture fixture = new Fixture(false); + Node root = new Node("root"); + root.attachChild(fixture.node); + root.updateGeometricState(); + + fixture.geometry.move(20, 0, 0); + root.updateLogicalState(0); + root.updateGeometricState(); + + fixture.assertBounds(); + assertEquals(fixture.geometry.getWorldBound(), root.getWorldBound()); + } + + @Test + public void changingModelBoundUpdatesBounds() { + Fixture fixture = new Fixture(false); + fixture.geometry.setModelBound(new BoundingBox(Vector3f.ZERO, 4, 5, 6)); + fixture.update(); + fixture.assertBounds(); + } + + @Test + public void queryingNodeBoundUpdatesInstanceBounds() { + Fixture fixture = new Fixture(false); + fixture.geometry.move(20, 0, 0); + // A direct bounds query also runs the normal lazy bound update path. + assertEquals(fixture.geometry.getWorldBound(), fixture.node.getWorldBound()); + fixture.assertBounds(); + } + + @Test + public void movingInstanceKeepsNearbyPointLight() { + Fixture fixture = new Fixture(false); + PointLight light = new PointLight(new Vector3f(20, 0, 10)); + light.setRadius(3); + fixture.node.addLight(light); + fixture.update(); + + Camera camera = new Camera(100, 100); + camera.setFrustum(1, 100, -50, 50, 50, -50); + camera.lookAtDirection(Vector3f.UNIT_Z, Vector3f.UNIT_Y); + DefaultLightFilter filter = new DefaultLightFilter(); + LightList filtered = new LightList(fixture.batch); + filter.setCamera(camera); + filter.filterLights(fixture.batch, filtered); + assertEquals(0, filtered.size(), "The light initially does not reach the instance"); + + fixture.geometry.move(20, 0, 0); + fixture.update(); + fixture.batch.updateInstances(camera); + assertEquals(1, fixture.batch.getNumVisibleInstances()); + filter.setCamera(camera); + filtered.clear(); + filter.filterLights(fixture.batch, filtered); + assertEquals(1, filtered.size(), "The light now reaches the moved instance"); + assertSame(light, filtered.get(0)); + } + + private static class Fixture { + final InstancedNode node = new InstancedNode("instances"); + final Geometry geometry = new Geometry("instance", new Box(1, 1, 1)); + final InstancedGeometry batch; + + Fixture(boolean nested) { + MaterialDef def = new MaterialDef(new DesktopAssetManager(), "test"); + def.addMaterialParam(VarType.Boolean, "UseInstancing", false); + Material material = new Material(def); + material.setBoolean("UseInstancing", true); + geometry.setMaterial(material); + geometry.setLocalTranslation(0, 0, 10); + Node parent = node; + if (nested) { + parent = new Node("nested"); + node.attachChild(parent); + } + parent.attachChild(geometry); + node.instance(); + InstancedGeometry found = null; + for (Spatial child : node.getChildren()) { + if (child instanceof InstancedGeometry) { + found = (InstancedGeometry) child; + } + } + assertNotNull(found); + batch = found; + update(); + assertBounds(); + } + + void update() { + node.updateLogicalState(0); + node.updateGeometricState(); + } + + void assertBounds() { + assertEquals(geometry.getWorldBound(), batch.getWorldBound()); + assertEquals(geometry.getWorldBound(), node.getWorldBound()); + } + } +}