-
-
Notifications
You must be signed in to change notification settings - Fork 208
Add joint motors for revolute and prismatic joints #913
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
base: main
Are you sure you want to change the base?
Conversation
9e42c78 to
e51413d
Compare
|
Thanks for the PR! This looks very nice and impressive already, based on a quick look :) Just as a heads-up: we have some plans to potentially replace the joint XPBD solver with something else, most likely a more traditional impulse-based solver like the one used by most other engines and our contacts, or mayybe AVBD if we can get it working well enough. That's one of the main reasons why I haven't personally prioritized implementing joint motors yet. Still, that may take a while, and this work is very useful regardless; motors are something that a lot of people have been asking for, and the API should remain mostly the same even if the solver implementation were to change in the future. Now, some general comments based on a first glance at the implementation. I like having
Ultimately, it seems to me like we need some joint-specific motor APIs, both for correctness and flexibility. There's roughly two approaches I can think of:
// Example: revolute joint motor
#[derive(Component)]
pub struct RevoluteJoint {
pub body1: Entity,
pub body2: Entity,
pub frame1: JointFrame,
pub frame2: JointFrame,
#[cfg(feature = "3d")]
pub hinge_axis: Vector,
pub angle_limit: Option<AngleLimit>,
// `MotorSettings` is like this PR's `LinearJointMotor`
// or `AngularJointMotor`, but as just a single type;
// in this case, it's used as an angular motor
pub motor: MotorSettings,
pub point_compliance: Scalar,
pub align_compliance: Scalar,
pub limit_compliance: Scalar,
}
// Example: revolute joint motor
#[derive(Component, Deref, DerefMut)]
pub struct RevoluteMotor(MotorSettings);
// Example: spherical joint motor
// Alternatively, we could have a separate `SwingMotor` and `TwistMotor`
#[cfg(feature = "3d")]
#[derive(Component)]
pub struct SphericalMotor {
pub swing: MotorSettings,
pub twist: MotorSettings,
}With either approach, each joint can store only the motors that are relevant for it, and can even support multiple axes. For example, Jolt's Option 2 looks interesting and does maybe feel more natural from a composition and ECS standpoint. I've been thinking that joint limits are optional in a similar way as motors, so we could even land on a design where the base joint type, its limit(s), and its motor(s) are all separate components: commands.spawn((
// The base joint type
RevoluteJoint::new(entity1, entity2),
// Optional limit
RevoluteLimit::new(-FRAC_PI_2, FRAC_PI_2),
// Optional motor
RevoluteMotor::from_target_velocity(1.0),
));This would lead to a rather nice conceptual split:
However I think the gains are still somewhat dubious, and it does add some complexity, since you need to query for multiple different components, and users could technically create invalid or unsupported configurations, like a spherical joint motor on a prismatic joint. So for now, I think I would just go with Option 1, since it's more common and probably the most straightforward to implement. I also do kinda like that e.g. all revolute joint configuration and features are on the actual Curious if you have ideas or thoughts here though :) |
|
@Jondolf I greatly appreciate your words, thoughts, and the attention you already put into this. This PR's API is an aspect I'm particularly not satisfied with yet. There is too much freedom to shoot oneself in the foot in the constructor variants, and then there's the syntax point you mentioned. I am fine with both options. I agree with the nice conceptual separation from option 2, but joint limits and motors make no sense without a joint to attach to, so it's more natural to me to have these as attributes rather than components. We agree on which is the best option then. Worst case scenario: we're both wrong on this and adjust. No big deal, the adjustment effort looks similar either way. I should be able to bring this ready for a proper review in the next few days. |
4933db4 to
007131e
Compare
Supports velocity and position control with optional timestep-independent spring-damper parameters (frequency, damping_ratio). Includes warm starting and motor force feedback via JointForces.
007131e to
2555b89
Compare
Merge both XPBD traits.
|
I've tinkered with this for a long time and I do believe option 1 is the way to go. I'm wondering if I updated the PR description and will mark it as ready for review. |
|
Thanks, sounds good! It's fine to leave
I think the interface would ideally let you set a target relative orientation and angular velocity. In PhysX, the D6 (aka 6DOF) joint supports this with two different angular drive models, twist & swing, and SLERP (link). In other engines like Jolt and Bepu, it's a bit less clear how they determine the rotation "path" for angular motors/servos, but they still also allow setting a target orientation as a quaternion. I haven't yet looked into how exactly it should be implemented though |
This prevents the motors from overpowering the limits at low substep counts
|
Hi! I finally did a pass on this PR, and made some changes:
I also updated the PR description to match the changes. I'll probably do a bit more polish work over the next day or two, and add a brief migration guide, after which I'll merge, unless you have something to add 🙂 |
Forces cargo to use exactly the dependency versions from Cargo.lock Avoids potential CI issues
|
Hello, Thanks for these changes, the API is indeed a lot nicer now and I'm much more satisfied with the state of this PR. Good point on the motors vs limits solving order too. I don't see your changes in the PR description, maybe you kept these as a draft? CI failed. It looks like an infra issue. I added another commit in an attempt to fix this. I'll check this out again later today. |
Objective
Supports velocity and position control with optional timestep-independent spring-damper parameters (frequency, damping_ratio). Includes warm starting and motor force feedback via JointForces.
Partially fixes #465
This implementation only works for joints with a single axis (
PrismaticJoint,RevoluteJoint).Additional work is needed to support multiple axes (
SphericalJoint).Solution
Add three
MotorModel:ForceBasedandAccelerationBasedare directly inspired from the Rapier eponymous models, theSpringDampermodel doesn't exists in Rapier but seems much easier to use to me. The latter is the default.The motors are solved through XPBD like any other joint constraint.
Relevant Rapier sources:
Breaking change
(first commit only, outdated by the second one)
The current implementation minimizes breaking changes, however, users who directly add the XPBD solver systems instead of using the plugin will have to make changes for motors to work:Similarly, users who manually implemented
XpbdConstraintfor custom joints may need to implementXpbdMotorConstraintinstead if they want motors to work.Users who manually implemented
XpbdConstraintfor custom joints may need to write a custom implementation for the newXpbdConstraint::warm_start_motorsmethod as its default implementation does nothing.Users who manually add the
solve_xpbd_jointsystems instead of using the plugins may need to also addwarm_start_xpbd_motors. (why would anyone do that though?)Testing
joint_motors_2dandjoint_motors_3dexamples provide an interactive way to test these,Showcase
Concise showcase
See the examples included in this PR for more detailed examples.
Screencast.from.2026-01-11.14-55-37.webm