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.

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

Adding Shapes to Colliders should recompute mass #490

Closed
bjornbytes opened this issue Oct 4, 2021 · 4 comments
Closed

Adding Shapes to Colliders should recompute mass #490

bjornbytes opened this issue Oct 4, 2021 · 4 comments

Comments

@bjornbytes
Copy link
Owner

In #249, colliders with pre-attached shapes initialized their mass data properly. However, adding new shapes to a collider doesn't adjust its mass, which can lead to weird center-of-mass/inertia tensor issues.

Mass should probably get recomputed when a shape is added/removed from a collider, or there should be a way to manually recompute mass from all attached shapes (the advantage of this would be you could add N shapes and then compute mass once instead of N times, but I don't think this is a particularly expensive operation so it might not matter).

LÖVE has Body:resetMassData (previously called Body:setMassFromShapes).

dMassAdd might be helpful.

@jmiskovic
Copy link
Contributor

jmiskovic commented Oct 6, 2021

I'm iterating through shapes and using dMassAdd to arrive at composite mass and inertia tensor. I've ran into this ODE limitation:

When assigning a mass to a rigid body, the center of mass must be (0,0,0) relative to the body's position.

This means we need to also add offsets to each shape until the center of mass is at (0,0,0).

Let's say the user has placed identical box shapes at (0,0,0) and (2,0,0), and the composite collider is at (0,0,0). When computing the mass parameters, those shape positions need to be shifted to (-1,0,0) and (1,0,0). They will not end up at exact place where user has put them!

We could compensate for this error: after moving shapes we could also shift the collider by (+1,0,0) to make the shapes end where they are supposed to be. But this just introduces new issues as the collider is now at (1,0,0) even though it was placed at (0,0,0).

Should we just document this limitation by saying that Shape:setPosition() will only enforce the relative positions between shapes in collider?

void lovrColliderRecomputeInertia(Collider* collider) {
  dMass totalMassData;
  dMassSetZero(&totalMassData);

  const float density = 1.0f;   // compute inertia matrix for default density
  float cx, cy, cz, mass, inertia[6];

  size_t count;
  Shape** shapes = lovrColliderGetShapes(collider, &count);
  for (size_t i = 0; i < count; i++) {
    lovrShapeGetMass(shapes[i], density, &cx, &cy, &cz, &mass, inertia);
    dMass shapeMassData;
    dMassSetParameters(&shapeMassData, mass, cx, cy, cz,
      inertia[0], inertia[1], inertia[2], inertia[3], inertia[4], inertia[5]);
    dMassAdd(&totalMassData, &shapeMassData);
  }
  cx = totalMassData.c[0];
  cy = totalMassData.c[1];
  cz = totalMassData.c[2];

  // shapes get shifted so that center of mass is at (0,0,0)
  for (size_t i = 0; i < count; i++) {
    const dReal* p = dGeomGetOffsetPosition(shapes[i]->id);
    dGeomSetOffsetPosition(shapes[i]->id, p[0] - cx, p[1] - cy, p[2] - cz);
  }

  dMassTranslate(&totalMassData, -cx, -cy, -cz);
  dBodySetMass(collider->body, &totalMassData);
}

@ottworks
Copy link

ottworks commented Oct 7, 2021

I think it would be misleading if shapes didn't end up where you put them. Store the offset and return it in Collider:getLocalCenter()?

@jmiskovic
Copy link
Contributor

In my experience you don't really want your colliders to be offset from point of mass. It makes applying forces and torques harder. For instance, if you apply force with 3 axis components, the collider will start spinning because you applied it off center. To actually apply the force in mass center you'd have to fetch the local center and transform it into world coordinates. A bother, and not something we want to inflict on users.

On the other hand the getLocalCenter() will always return 0,0,0 because of this ODE's limitation so it's not useful at the moment.

@bjornbytes
Copy link
Owner Author

Adding/Removing/Moving/Changing shapes now recomputes mass by default. Collider has Collider:setAutomaticMass which enables/disables this behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants