From 579bcac8be49df9bcee893939aadc016bc6c6bfc Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Tue, 7 Jul 2026 15:39:03 +0100 Subject: [PATCH] Rebuild mesh collision shapes when the entity world scale changes The rebuild-on-scale-change check in updateMeshTransform has been dead code since the Designer livelink was removed in 2015 - onTransformChanged had no callers, so scaling an entity with a mesh collider at runtime never rebuilt the shape. The collision system now watches mesh components with a built shape and rebuilds them from the rigid body update loop when their world scale drifts from the scale the shape was built with. The Ammo backend also no longer caches triangle data that has a bake scale: the cache is keyed by mesh id alone, so baked-scale data leaked the first builder's scale into every other user of the mesh and made rebuilds return stale geometry (the reason the manual doRecreatePhysicalShape workaround stopped working after #6087). Scaled builds are now owned by their shape and destroyed with it, so continuous scale tweens don't grow the cache. Fixes #6212 Fixes #3062 Co-Authored-By: Claude Fable 5 --- src/framework/components/collision/system.js | 86 ++++++++++---- src/framework/components/rigid-body/system.js | 3 + .../physics/ammo/ammo-physics-shape.js | 63 ++++++++-- src/framework/physics/physics-world.js | 3 +- .../components/collision/component.test.mjs | 111 ++++++++++++++++++ 5 files changed, 233 insertions(+), 33 deletions(-) diff --git a/src/framework/components/collision/system.js b/src/framework/components/collision/system.js index 90b89eb3453..8c3eac117a2 100644 --- a/src/framework/components/collision/system.js +++ b/src/framework/components/collision/system.js @@ -20,6 +20,9 @@ const p2 = new Vec3(); const p3 = new Vec3(); const quat = new Quat(); const quat2 = new Quat(); +const worldScale = new Vec3(); + +const SCALE_CHANGE_TOLERANCE = 1e-5; // Note that `shape` is deliberately absent from this list - it is runtime // state created and owned by the type implementation, not component data @@ -87,8 +90,7 @@ const collisionImpls = { // the model comes from assets - no placeholder is created beforeInitialize() {}, createPhysicalShape: createMeshShape, - recreatePhysicalShapes: recreateMeshShapes, - updateTransform: updateMeshTransform + recreatePhysicalShapes: recreateMeshShapes }, compound: { @@ -150,6 +152,11 @@ function destroyShape(system, component) { } function beforeRemove(system, entity, component) { + const watchIndex = system._meshComponents.indexOf(component); + if (watchIndex !== -1) { + system._meshComponents.splice(watchIndex, 1); + } + if (component._shape) { if (component._compoundParent && !component._compoundParent.entity._destroying) { system._removeCompoundChild(component._compoundParent, component._shape); @@ -234,12 +241,6 @@ function recreateShapes(system, component) { } } -function updateShapeTransform(system, component, position, rotation, scale) { - if (component.entity.trigger) { - component.entity.trigger.updateTransform(); - } -} - // Builds a PhysicsMeshSource for one mesh. Vertex and index data are exposed through lazy // accessors so they are only extracted when the backend actually builds triangle data - // sources whose geometry is already cached (by mesh id) never touch the vertex buffer. @@ -332,8 +333,12 @@ function createMeshShape(system, entity, component) { shapeScale = scale; } - // record the scale the shape was built with, for the rebuild-on-scale-change check + // record the scale the shape was built with and watch it for changes - a runtime + // rescale of the entity rebuilds the shape (see _updateMeshScales) component._builtWorldScale = scale; + if (!system._meshComponents.includes(component)) { + system._meshComponents.push(component); + } return world.createShape({ type: 'mesh', @@ -428,18 +433,14 @@ function recreateMeshShapes(system, component) { doRecreateMeshShape(system, component); } -function updateMeshTransform(system, component, position, rotation, scale) { - if (component.shape && component._builtWorldScale) { - const entityTransform = component.entity.getWorldTransform(); - const worldScale = entityTransform.getScale(); - - // if the scale changed then recreate the shape - if (!worldScale.equals(component._builtWorldScale)) { - doRecreateMeshShape(system, component); - } - } - - updateShapeTransform(system, component, position, rotation, scale); +// Returns whether a freshly extracted world scale differs from the scale a mesh shape was +// built with. The scale is derived from the world matrix, so merely rotating an entity +// perturbs the extracted values by float noise - compare with a relative tolerance so only +// real scale changes trigger a rebuild +function scaleChanged(scale, builtScale) { + return Math.abs(scale.x - builtScale.x) > SCALE_CHANGE_TOLERANCE * Math.max(1, Math.abs(builtScale.x)) || + Math.abs(scale.y - builtScale.y) > SCALE_CHANGE_TOLERANCE * Math.max(1, Math.abs(builtScale.y)) || + Math.abs(scale.z - builtScale.z) > SCALE_CHANGE_TOLERANCE * Math.max(1, Math.abs(builtScale.z)); } /** @@ -448,6 +449,15 @@ function updateMeshTransform(system, component, position, rotation, scale) { * @category Physics */ class CollisionComponentSystem extends ComponentSystem { + /** + * Mesh components with a built shape, watched for entity world scale changes. Maintained + * by createMeshShape and beforeRemove. + * + * @type {CollisionComponent[]} + * @private + */ + _meshComponents = []; + /** * Creates a new CollisionComponentSystem instance. * @@ -564,9 +574,37 @@ class CollisionComponentSystem extends ComponentSystem { this.physicsWorld.removeCompoundChild(collision.shape, shape); } - onTransformChanged(component, position, rotation, scale) { - const impl = getImpl(component.type); - (impl.updateTransform ?? updateShapeTransform)(this, component, position, rotation, scale); + /** + * Rebuilds mesh shapes whose entity world scale no longer matches the scale they were + * built with. Driven by the rigid body system at the start of each physics update. + * + * @ignore + */ + _updateMeshScales() { + if (!this.physicsWorld) { + return; + } + + const components = this._meshComponents; + // walk backwards - a rebuild that has lost its mesh sources removes the component + for (let i = components.length - 1; i >= 0; i--) { + const component = components[i]; + if (!component._shape || !component.enabled || !component.entity.enabled) { + continue; + } + + // skip compound children - the mesh rebuild path does not detach the old shape + // from the parent compound, so rebuilding one here would leave the parent + // referencing a destroyed shape + if (component._compoundParent) { + continue; + } + + const scale = component.entity.getWorldTransform().getScale(worldScale); + if (scaleChanged(scale, component._builtWorldScale)) { + doRecreateMeshShape(this, component); + } + } } // Destroys the previous collision type and creates a new one based on the new type provided diff --git a/src/framework/components/rigid-body/system.js b/src/framework/components/rigid-body/system.js index 63ef3d2f298..4c33332dc94 100644 --- a/src/framework/components/rigid-body/system.js +++ b/src/framework/components/rigid-body/system.js @@ -759,6 +759,9 @@ class RigidBodyComponentSystem extends ComponentSystem { // Check to see whether we need to update gravity on the physics world this._world.setGravity(this.gravity); + // rebuild mesh collision shapes whose entity world scale changed since they were built + this.app.systems.collision?._updateMeshScales(); + const triggers = this._triggers; for (i = 0, len = triggers.length; i < len; i++) { triggers[i].updateTransform(); diff --git a/src/framework/physics/ammo/ammo-physics-shape.js b/src/framework/physics/ammo/ammo-physics-shape.js index 8edd99154d0..7a97035c1ff 100644 --- a/src/framework/physics/ammo/ammo-physics-shape.js +++ b/src/framework/physics/ammo/ammo-physics-shape.js @@ -7,6 +7,25 @@ import { Debug } from '../../../core/debug.js'; * @import { Vec3 } from '../../../core/math/vec3.js' */ +// Bake scales within this tolerance of unity are treated as unscaled. The world scale of a +// rotated but unscaled entity is extracted from its world matrix, so it carries float noise - +// without the tolerance such entities would needlessly bypass the triangle data cache +const UNIT_SCALE_TOLERANCE = 1e-5; + +/** + * Returns whether a bake scale is close enough to unity to be ignored. + * + * @param {Vec3|null} scale - The bake scale, or null. + * @returns {boolean} True if the scale is null or within tolerance of (1, 1, 1). + */ +function isUnitScale(scale) { + return !scale || ( + Math.abs(scale.x - 1) <= UNIT_SCALE_TOLERANCE && + Math.abs(scale.y - 1) <= UNIT_SCALE_TOLERANCE && + Math.abs(scale.z - 1) <= UNIT_SCALE_TOLERANCE + ); +} + /** * Writes a position/rotation pair into the world's cached btTransform and returns it. * @@ -33,12 +52,20 @@ function getTransform(world, position, rotation) { * cache is keyed by the source id so sources sharing geometry share triangle data - source * data accessors are only read on a cache miss. * + * Only unscaled triangle data enters the cache: an id can be shared by sources with different + * bake scales, so baked-scale data would leak one component's scale into another's shape and + * make a rebuild after a scale change return stale geometry. Scaled builds are appended to + * ownedTriMeshes instead, and are destroyed with the shape that owns them. + * * @param {AmmoPhysicsWorld} world - The owning world. * @param {PhysicsMeshSource} source - The geometry source. + * @param {object[]} ownedTriMeshes - Receives the built trimesh when it cannot be cached. * @returns {object} The btTriangleMesh. */ -function getTriMesh(world, source) { - let triMesh = world._triMeshCache.get(source.id); +function getTriMesh(world, source, ownedTriMeshes) { + const bakeScale = isUnitScale(source.bakeScale) ? null : source.bakeScale; + + let triMesh = bakeScale ? null : world._triMeshCache.get(source.id); if (!triMesh) { const positions = source.positions; const stride = source.stride; @@ -51,14 +78,17 @@ function getTriMesh(world, source) { let i1, i2, i3; triMesh = new Ammo.btTriangleMesh(); - world._triMeshCache.set(source.id, triMesh); + if (bakeScale) { + ownedTriMeshes.push(triMesh); + } else { + world._triMeshCache.set(source.id, triMesh); + } const vertexCache = new Map(); Debug.assert(typeof triMesh.getIndexedMeshArray === 'function', 'Ammo.js version is too old, please update to a newer Ammo.'); const indexedArray = triMesh.getIndexedMeshArray(); indexedArray.at(0).m_numTriangles = numTriangles; - const bakeScale = source.bakeScale; const sx = bakeScale ? bakeScale.x : 1; const sy = bakeScale ? bakeScale.y : 1; const sz = bakeScale ? bakeScale.z : 1; @@ -144,9 +174,10 @@ function createHullChild(world, compound, source) { * @param {AmmoPhysicsWorld} world - The owning world. * @param {object} compound - The btCompoundShape to add to. * @param {PhysicsMeshSource} source - The geometry source. + * @param {object[]} ownedTriMeshes - Receives trimeshes owned by the compound. */ -function createTriMeshChild(world, compound, source) { - const triMesh = getTriMesh(world, source); +function createTriMeshChild(world, compound, source, ownedTriMeshes) { + const triMesh = getTriMesh(world, source, ownedTriMeshes); const triMeshShape = new Ammo.btBvhTriangleMeshShape(triMesh, true /* useQuantizedAabbCompression */); @@ -224,16 +255,24 @@ const shapeFactories = { mesh: (world, desc) => { const shape = new Ammo.btCompoundShape(); + // triangle data built with a baked scale is private to this shape - it stays out of + // the shared cache and is destroyed with the shape + const ownedTriMeshes = []; + const sources = desc.sources; for (let i = 0; i < sources.length; i++) { const source = sources[i]; if (source.convexHull) { createHullChild(world, shape, source); } else { - createTriMeshChild(world, shape, source); + createTriMeshChild(world, shape, source, ownedTriMeshes); } } + if (ownedTriMeshes.length > 0) { + shape._ownedTriMeshes = ownedTriMeshes; + } + if (desc.scale) { const vec = new Ammo.btVector3(desc.scale.x, desc.scale.y, desc.scale.z); shape.setLocalScaling(vec); @@ -267,12 +306,20 @@ function createShape(world, desc) { */ function destroyShape(shape) { // mesh shapes own their sub-shapes (compound children are owned by other components, - // and the cached triangle data outlives the shape) + // and cached triangle data outlives the shape) if (shape._shapeType === 'mesh') { const numShapes = shape.getNumChildShapes(); for (let i = 0; i < numShapes; i++) { Ammo.destroy(shape.getChildShape(i)); } + + // triangle data built with a baked scale is owned by the shape, not the cache + const ownedTriMeshes = shape._ownedTriMeshes; + if (ownedTriMeshes) { + for (let i = 0; i < ownedTriMeshes.length; i++) { + Ammo.destroy(ownedTriMeshes[i]); + } + } } Ammo.destroy(shape); diff --git a/src/framework/physics/physics-world.js b/src/framework/physics/physics-world.js index fddc5916332..373c8e70207 100644 --- a/src/framework/physics/physics-world.js +++ b/src/framework/physics/physics-world.js @@ -31,7 +31,8 @@ import { PhysicsJoint } from './physics-joint.js'; /** * @typedef {object} PhysicsMeshSource * @property {number} id - A stable cache key for the source geometry (mesh id). Backends may - * cache built triangle data per id for the lifetime of the world. + * cache triangle data built without a bake scale per id for the lifetime of the world. An id + * can be shared by sources carrying different bake scales, so scaled data is not cacheable. * @property {Float32Array|number[]} positions - Vertex positions, possibly interleaved. * @property {number} stride - The number of floats between consecutive positions (3 when * tightly packed). diff --git a/test/framework/components/collision/component.test.mjs b/test/framework/components/collision/component.test.mjs index 49615339327..d367d546ec1 100644 --- a/test/framework/components/collision/component.test.mjs +++ b/test/framework/components/collision/component.test.mjs @@ -4,6 +4,7 @@ import { Quat } from '../../../../src/core/math/quat.js'; import { Vec3 } from '../../../../src/core/math/vec3.js'; import { Asset } from '../../../../src/framework/asset/asset.js'; import { Entity } from '../../../../src/framework/entity.js'; +import { NullPhysicsWorld } from '../../../../src/framework/physics/null/null-physics-world.js'; import { Model } from '../../../../src/scene/model.js'; import { createApp } from '../../../app.mjs'; import { jsdomSetup, jsdomTeardown } from '../../../jsdom.mjs'; @@ -473,4 +474,114 @@ describe('CollisionComponent', function () { }); + describe('mesh world scale watch', function () { + + beforeEach(function () { + app.systems.rigidbody.setPhysicsWorld(new NullPhysicsWorld()); + }); + + // creates an initialized mesh collision entity - the null backend never reads the + // geometry, so a stub render source is enough + function createMeshEntity(parent = app.root) { + const e = new Entity(); + e.addComponent('collision', { type: 'mesh' }); + parent.addChild(e); + e.collision.render = { meshes: [{ id: 1, primitive: [{ base: 0, count: 3 }] }] }; + return e; + } + + it('rebuilds the shape when the entity world scale changes', function () { + const e = createMeshEntity(); + const shape = e.collision.shape; + expect(shape).to.exist; + + e.setLocalScale(2, 2, 2); + app.systems.rigidbody.onUpdate(1 / 60); + + expect(e.collision.shape).to.exist; + expect(e.collision.shape).to.not.equal(shape); + expect(e.collision._builtWorldScale.equals(new Vec3(2, 2, 2))).to.equal(true); + }); + + it('rebuilds the shape when an ancestor scale changes', function () { + const parent = new Entity(); + app.root.addChild(parent); + const e = createMeshEntity(parent); + const shape = e.collision.shape; + + parent.setLocalScale(3, 3, 3); + app.systems.rigidbody.onUpdate(1 / 60); + + expect(e.collision.shape).to.not.equal(shape); + }); + + it('does not rebuild without a scale change', function () { + const e = createMeshEntity(); + const shape = e.collision.shape; + + app.systems.rigidbody.onUpdate(1 / 60); + app.systems.rigidbody.onUpdate(1 / 60); + + expect(e.collision.shape).to.equal(shape); + }); + + it('does not rebuild when the entity merely moves or rotates', function () { + const e = createMeshEntity(); + const shape = e.collision.shape; + + e.setLocalPosition(1, 2, 3); + e.setLocalEulerAngles(10, 20, 30); + app.systems.rigidbody.onUpdate(1 / 60); + + expect(e.collision.shape).to.equal(shape); + }); + + it('defers the rebuild while the component is disabled', function () { + const e = createMeshEntity(); + const shape = e.collision.shape; + + e.collision.enabled = false; + e.setLocalScale(2, 2, 2); + app.systems.rigidbody.onUpdate(1 / 60); + expect(e.collision.shape).to.equal(shape); + + e.collision.enabled = true; + app.systems.rigidbody.onUpdate(1 / 60); + expect(e.collision.shape).to.not.equal(shape); + }); + + it('leaves compound children alone when an ancestor is scaled', function () { + const parent = new Entity(); + parent.addComponent('collision', { type: 'compound' }); + app.root.addChild(parent); + + const e = createMeshEntity(parent); + const shape = e.collision.shape; + expect(e.collision._compoundParent).to.equal(parent.collision); + + parent.setLocalScale(2, 2, 2); + app.systems.rigidbody.onUpdate(1 / 60); + + expect(e.collision.shape).to.equal(shape); + }); + + it('stops watching a component that changes type or is removed', function () { + const system = app.systems.collision; + + const e = createMeshEntity(); + expect(system._meshComponents).to.include(e.collision); + + e.collision.type = 'box'; + expect(system._meshComponents).to.have.lengthOf(0); + + const e2 = createMeshEntity(); + const component = e2.collision; + expect(system._meshComponents).to.include(component); + + e2.destroy(); + expect(system._meshComponents).to.have.lengthOf(0); + }); + + }); + });