engine icon indicating copy to clipboard operation
engine copied to clipboard

Multiple Mesh Colliders of Different Scales don't behave properly

Open CaseyHofland opened this issue 4 years ago • 8 comments

When using differently scaled mesh colliders of the same mesh, only 1 instance of the collider will be used on all entities.

As LeXXik put it (forum username): "Placing a mesh into the physics world is a computationally expensive operation - the engine generates an Ammo compound shape from a number of bvh triangle meshes, one for each mesh instance. Once generated the trimeshes then get cached for future re-use. As a result, when you use the same mesh for another collider, the trimeshes are taken from the cache, instead of generating new ones."

Provide as much information as possible. Include (where applicable): The forum discussion

Example:

Steps to Reproduce

  1. Add a mesh and ammo to your project.
  2. Create 2 or more entities of different scales.
  3. Give them both the same mesh collider.

Result: all colliders have the same scale. Desired: all colliders have their own scale.

CaseyHofland avatar Apr 03 '21 19:04 CaseyHofland

Martin had a quick look and it seems like it will need dedicated time to fix rather than be included in a bug blitz

yaustar avatar Jul 05 '21 08:07 yaustar

We could consider to upgrade the Ammo version, and use btScaledBvhTriangleMeshShape API which allows to use btBvhTriangleMeshShape with different scale without having to have separate btBvhTriangleMeshShape instance. This was added in Sept 2008 it seems .. not sure why its not in our Ammo version yet. https://github.com/kripken/ammo.js/pull/240

Otherwise we can implement the system using the duplication.

mvaligursky avatar Sep 30 '21 12:09 mvaligursky

Does it still need to be uniformly scaled?

yaustar avatar Sep 30 '21 12:09 yaustar

Random thought, I wonder if instead of using the mesh.id, we can construct a string of the mesh.id and scale here (eg '' + mesh.id + sx + sy + sz: https://github.com/playcanvas/engine/blob/a596c8b7881acbb318bca3f8b1b727aad62c3ecc/src/framework/components/collision/system.js#L318

That would allow different sized meshes to be built and cached using the same model data. It would mean an extra cost of calling _getNodeScaling to get the final scale on each look up though 🤔

yaustar avatar Aug 08 '23 15:08 yaustar

That's what we did in Solar Tools by patching the engine. And in addition we provided an option to round the scale to the nearest value set by the user. So for example he could group various rock objects to 0.75, 1.0, 1.25 scale variants and produce just 3 cached mesh shapes.

leonidaspir avatar Aug 08 '23 15:08 leonidaspir

@yaustar Yeah, something like that. Or create a hash from the scale.

willeastcott avatar Aug 09 '23 09:08 willeastcott

I've implemented something similar.

export const getScaleUniqueMeshId = (meshId: number, scale: Vec3): number => {
  // Removes scale values from meshId if present
  const rawMeshId = Math.floor(meshId);

  const { x, y, z } = scale.clone().abs().round();

  return Number(`${rawMeshId}.${x}${y}${z}`);
};

jbromberg avatar Aug 10 '23 16:08 jbromberg

Just in case anybody needs a quick patch for this issue, here it is (works with v1.75.x at least):

(function () {
    const app = (pc.AppBase || pc.Application).getApplication();
    app.once('start', () => {

        pc.getScaleUniqueMeshId = (meshId, scale) => {
            const rawMeshId = Math.floor(meshId);
            const precision = 2; //decimal places, 1 means rounding to 0.1, 2 to ~0.01, 3 to ~0.001 etc.
            const roundingPower = Math.pow(10, precision);
            let x = Math.abs(scale.x * roundingPower);
            let y = Math.abs(scale.y * roundingPower);
            let z = Math.abs(scale.z * roundingPower);
            return Number(`${rawMeshId}.${x}${y}${z}`);
        };

        if (typeof Ammo === 'undefined' || !app.systems.collision.implementations.mesh) return console.log('[MeshCollisionSystemImpl] could not detect Ammo');

        const originalCreateAmmoMeshFunction = app.systems.collision.implementations.mesh.createAmmoMesh.toString();
        if (originalCreateAmmoMeshFunction.includes('mesh.id')) {
            const patchedCreateAmmoMeshFunction = originalCreateAmmoMeshFunction.replaceAll('mesh.id', 'pc.getScaleUniqueMeshId(mesh.id, scale)').replaceAll('SEMANTIC_POSITION', 'pc.SEMANTIC_POSITION');
            console.log('MeshCollisionSystemImpl has been patched to support mesh collider scaling');
            app.systems.collision.implementations.mesh.createAmmoMesh = new Function('return ' + patchedCreateAmmoMeshFunction)();
        } else {
            console.warn('Could not patch MeshCollisionSystemImpl: mesh.id is not used in this version of the engine');
        }
    })
})();

Please note that this is not the safest solution (using Function constructor is the same piece of sh*t as as eval in terms of security, but I wouldn't like to rewrite the whole createAmmoMesh function just because of 3 tiny insertions... :) )

IgorIFGD avatar Jan 30 '25 18:01 IgorIFGD