-
Notifications
You must be signed in to change notification settings - Fork 312
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
[GUI.Component] ConstraintAttachBodyPerformer: Add RigidType support #4801
[GUI.Component] ConstraintAttachBodyPerformer: Add RigidType support #4801
Conversation
static const typename DataTypes::Coord point1 {}; | ||
static const typename DataTypes::Coord point2 {}; | ||
static const typename DataTypes::Deriv normal {}; | ||
|
||
bconstraint->addContact(normal, point1, point2, normal.norm(), 0, index, point2, point1); | ||
bconstraint->addContact(normal, point1, point2, 0.0, 0, index, point2, point1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that a contact is added between two null vectors is very confusing. It appears that it was not the case initially: b664374. It worth having a discussion on this during the next meeting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the code, the only parameters of this function used are the two coords (point1
and point2
) and the two indices (0
and index
) the rest are just unused and thus not necessary (for the templated method and the rigid spec).
b4e0ef4
to
8c76365
Compare
8c76365
to
5db46e1
Compare
static const typename DataTypes::Coord point1 {}; | ||
static const typename DataTypes::Coord point2 {}; | ||
static const typename DataTypes::Deriv normal {}; | ||
|
||
bconstraint->addContact(normal, point1, point2, normal.norm(), 0, index, point2, point1); | ||
bconstraint->addContact(normal, point1, point2, 0.0, 0, index, point2, point1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the code, the only parameters of this function used are the two coords (point1
and point2
) and the two indices (0
and index
) the rest are just unused and thus not necessary (for the templated method and the rigid spec).
Sofa/GUI/Component/src/sofa/gui/component/performer/ConstraintAttachBodyPerformer.inl
Show resolved
Hide resolved
Dear @bakpaul, So I let it to false for the moment 😅 Here it the backtrace (on mac) if the option is to "true":
|
I think this is because you need to call the bwdInit after the init and addContact calls when this parameter is used (by checking quickly the code). It is harmless when the template is Vec3D but it prepares the computation for when you want to keep the orientation -> again this is really a bad API and might require some refactoring. If you still have the diff locally, could you try this ? I guess the usability of your feature would really gain from this if it was working. But if this doesn't make any change, I am ok with the state of the PR. Tell me how it goes ! |
80c6ef7
to
4de2af2
Compare
4de2af2
to
2255330
Compare
[ci-build][with-all-tests] |
adding instantiation with RigidType + removal of "hard" usage of VecTypes.
In my case, it was to support grabbing a beam (BeamAdapter) with the mouse and constraints.
Screen.Recording.2024-07-01.at.17.08.16.mov
Reminder: BilateralConstraint needs a GenericConstraintSolver (+FreeMotionAL), LCPConstraintSolver is not usable
By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).
Reviewers will merge this pull-request only if