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

[SofaSimulationGraph] Fix CollisionGroupManager wrong search of deformable object node #1060

Conversation

fspadoni
Copy link
Contributor

@fspadoni fspadoni commented May 22, 2019

Fix in DefaultCollisionGroupManager::createGroups the wrong search of deformable object nodes in collision group nodes already created.

The search was always failing and a new collision group was always created and added in the scene graph.

This PR is supposed to explain the wrong behavior issue #994


This PR:

  • builds with SUCCESS for all platforms on the CI.
  • does not generate new warnings.
  • does not generate new unit test failures.
  • does not generate new scene test failures.
  • does not break API compatibility.
  • is more than 1 week old (or has fast-merge label).

Reviewers will merge only if all these checks are true.

… collision group nodes already created. The search was always failing and a new collision group was always created and added in the scene graph.
@hugtalbot hugtalbot self-assigned this May 23, 2019
@hugtalbot hugtalbot added pr: fix Fix a bug pr: status to review To notify reviewers to review this pull-request labels May 23, 2019
@hugtalbot
Copy link
Contributor

Nice Federico, will you PR the removal of the MultiGroup (maybe another PR)? if not, I can do it
Cheers

removedGroup.push_back(group2);
//delete group2;
removedGroup.push_back(collGroup2);
//delete group2;
Copy link
Contributor

Choose a reason for hiding this comment

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

could be removed

@@ -16,6 +17,6 @@
<include name="FEM" href="Objects/TorusFEM.xml" dx="2.5" />
<include name="Spring" href="Objects/TorusSpring.xml" dx="5" rx="90" />
<include name="FFD" href="Objects/TorusFFD.xml" dx="7.5" />
<include name="TorusRigid" href="Objects/TorusRigid.xml" dx="10" rx="90" />
<!-- <include name="TorusRigid" href="Objects/TorusRigid.xml" dx="10" rx="90" /> -->
Copy link
Contributor

Choose a reason for hiding this comment

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

You deleted one of the tori on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I deleted one for debugging. I will add it back.

@fspadoni
Copy link
Contributor Author

fspadoni commented May 23, 2019 via email

@hugtalbot
Copy link
Contributor

[ci-build][with-all-tests]

@guparan guparan added the STC#7 label May 27, 2019
@guparan
Copy link
Contributor

guparan commented May 29, 2019

[ci-build][with-all-tests]

@guparan guparan added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels May 29, 2019
@fspadoni
Copy link
Contributor Author

fspadoni commented Jun 4, 2019

[ci-build][with-regression-tests]

…on. It was previously returning always the root node. The function is not tested in the case of multiple parent nodes.
@fspadoni
Copy link
Contributor Author

[ci-build][with-regression-tests][with-scene-tests]

@epernod epernod added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Jun 12, 2019
@guparan guparan added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Jun 12, 2019
@fspadoni fspadoni added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Jul 3, 2019
@guparan guparan added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels Jul 3, 2019
@damienmarchal damienmarchal merged commit 2914eef into sofa-framework:master Jul 5, 2019
@guparan guparan added this to the v19.12 milestone Jan 14, 2020
@guparan guparan changed the title [CollisionGroupManager] Fix wrong search of deformable object node [SofaSimulationGraph] Fix wrong search of deformable object node Jan 14, 2020
@guparan guparan changed the title [SofaSimulationGraph] Fix wrong search of deformable object node [SofaSimulationGraph] Fix CollisionGroupManager wrong search of deformable object node Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix Fix a bug pr: status ready Approved a pull-request, ready to be squashed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants