Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mesh collision optimization #6087

Merged
merged 2 commits into from Feb 27, 2024
Merged

Mesh collision optimization #6087

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4ef47b
mesh scale
LeXXik Feb 25, 2024
ac017f9
lint
LeXXik Feb 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 20 additions & 15 deletions src/framework/components/collision/system.js
View file
Open in desktop
Expand Up @@ -332,7 +332,7 @@ class CollisionMeshSystemImpl extends CollisionSystemImpl {
// special handling
beforeInitialize(component, data) {}

createAmmoMesh(mesh, node, shape, checkDupes = true) {
createAmmoMesh(mesh, node, shape, scale, checkDupes = true) {
const system = this.system;
let triMesh;

Expand Down Expand Up @@ -367,10 +367,14 @@ class CollisionMeshSystemImpl extends CollisionSystemImpl {
const indexedArray = triMesh.getIndexedMeshArray();
indexedArray.at(0).m_numTriangles = numTriangles;

const sx = scale ? scale.x : 1;
const sy = scale ? scale.y : 1;
const sz = scale ? scale.z : 1;

const addVertex = (index) => {
const x = positions[index * stride];
const y = positions[index * stride + 1];
const z = positions[index * stride + 2];
const x = positions[index * stride] * sx;
const y = positions[index * stride + 1] * sy;
const z = positions[index * stride + 2] * sz;

let idx;
if (checkDupes) {
Expand Down Expand Up @@ -407,9 +411,11 @@ class CollisionMeshSystemImpl extends CollisionSystemImpl {

const triMeshShape = new Ammo.btBvhTriangleMeshShape(triMesh, true /* useQuantizedAabbCompression */);

const scaling = system._getNodeScaling(node);
triMeshShape.setLocalScaling(scaling);
Ammo.destroy(scaling);
if (!scale) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if instead of passing the scale parameter to the function, and then either using it, or extracting it from the node, we should not pass it in, and always extract it from the node? And apply directly to vertices that you introduced here. I'm just not sure what the advantage is in having to pass it in, considering we can extract it from the node?

Copy link
Contributor Author

@LeXXik LeXXik Feb 26, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, in case of Render Asset that node is a temp node, which will always have a scale 1, so not needed. In case of Model Asset it will be a node from the mesh instance. I wasn't sure if nodes on model mesh instances could have a different scale, so left it there as is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see .. there were two places we apply scale to ammo mesh. One is based on the node of the mesh-instance, and the other is based on the node of the collision component. Your change only handles the first of those.

I somehow assumed we have just one scale applied.

const scaling = system._getNodeScaling(node);
triMeshShape.setLocalScaling(scaling);
Ammo.destroy(scaling);
}

const transform = system._getNodeTransform(node);
shape.addChildShape(transform, triMeshShape);
Expand All @@ -422,25 +428,24 @@ class CollisionMeshSystemImpl extends CollisionSystemImpl {
if (data.model || data.render) {

const shape = new Ammo.btCompoundShape();
const entityTransform = entity.getWorldTransform();
const scale = entityTransform.getScale();

if (data.model) {
const meshInstances = data.model.meshInstances;
for (let i = 0; i < meshInstances.length; i++) {
this.createAmmoMesh(meshInstances[i].mesh, meshInstances[i].node, shape, data.checkVertexDuplicates);
this.createAmmoMesh(meshInstances[i].mesh, meshInstances[i].node, shape, null, data.checkVertexDuplicates);
}
const vec = new Ammo.btVector3(scale.x, scale.y, scale.z);
shape.setLocalScaling(vec);
Ammo.destroy(vec);
} else if (data.render) {
const meshes = data.render.meshes;
for (let i = 0; i < meshes.length; i++) {
this.createAmmoMesh(meshes[i], tempGraphNode, shape, data.checkVertexDuplicates);
this.createAmmoMesh(meshes[i], tempGraphNode, shape, scale, data.checkVertexDuplicates);
}
}

const entityTransform = entity.getWorldTransform();
const scale = entityTransform.getScale();
const vec = new Ammo.btVector3(scale.x, scale.y, scale.z);
shape.setLocalScaling(vec);
Ammo.destroy(vec);

return shape;
}

Expand Down