From e23553a33bf590abf30617f597858370f85a414a Mon Sep 17 00:00:00 2001 From: Mr Robot Date: Fri, 2 Oct 2026 17:43:10 +0200 Subject: [PATCH 1/3] Refresh instanced bounds after source geometry updates --- .../jme3/scene/instancing/InstancedNode.java | 11 ++ .../scene/instancing/InstancedBoundsTest.java | 175 ++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsTest.java 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..fca28e4553 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 @@ -199,6 +199,17 @@ public InstancedNode(String name) { addControl(control); } + @Override + protected void updateWorldBound() { + // Source geometries can change their bounds without changing the + // transform of the InstancedGeometry that renders them. Refresh the + // aggregate bounds after all source children have been updated. + for (InstancedGeometry ig : instancesMap.values()) { + ig.updateWorldBound(); + } + super.updateWorldBound(); + } + private void renderFromControl(Camera cam) { for (InstancedGeometry ig : instancesMap.values()) { ig.updateInstances(cam); 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..75ffea1115 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsTest.java @@ -0,0 +1,175 @@ +/* + * 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 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()); + } + } +} From f16b477b0b26cb13d80dd73751488fb58284da2b Mon Sep 17 00:00:00 2001 From: Mr Robot Date: Fri, 2 Oct 2026 18:31:48 +0200 Subject: [PATCH 2/3] Cover parent-driven updates of instanced bounds --- .../scene/instancing/InstancedBoundsTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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 index 75ffea1115..fc23d6734b 100644 --- a/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsTest.java +++ b/jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsTest.java @@ -86,6 +86,21 @@ public void movingNestedParentUpdatesBounds() { 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); From afd0075a9fd47f7aa7a674f2529b6dca151069f9 Mon Sep 17 00:00:00 2001 From: Mr Robot Date: Tue, 27 Oct 2026 14:42:57 +0100 Subject: [PATCH 3/3] Invalidate only affected instanced bounds Notify geometry groups when source bounds are invalidated and let the normal lazy update path refresh only the affected instanced group. Remove the unconditional rebuild of every group. Add regression coverage for selective refresh, coalesced changes, whole-node movement, membership changes, bound-only updates, and lazy queries with either child order. --- .../main/java/com/jme3/scene/Geometry.java | 8 + .../com/jme3/scene/GeometryGroupNode.java | 12 + .../jme3/scene/instancing/InstancedNode.java | 20 +- .../InstancedBoundsRefreshTest.java | 284 ++++++++++++++++++ 4 files changed, 313 insertions(+), 11 deletions(-) create mode 100644 jme3-core/src/test/java/com/jme3/scene/instancing/InstancedBoundsRefreshTest.java 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 fca28e4553..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 @@ -199,17 +199,6 @@ public InstancedNode(String name) { addControl(control); } - @Override - protected void updateWorldBound() { - // Source geometries can change their bounds without changing the - // transform of the InstancedGeometry that renders them. Refresh the - // aggregate bounds after all source children have been updated. - for (InstancedGeometry ig : instancesMap.values()) { - ig.updateWorldBound(); - } - super.updateWorldBound(); - } - private void renderFromControl(Camera cam) { for (InstancedGeometry ig : instancesMap.values()) { ig.updateInstances(cam); @@ -404,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); + } + } +}