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

Fixed collision visual bounding boxes #702

Merged
merged 2 commits into from
Mar 24, 2021
Merged

Conversation

jennuine
Copy link
Contributor

@jennuine jennuine commented Mar 24, 2021

Signed-off-by: Jenn Nguyen [email protected]

🦟 Bug fix

Summary

Encountered a new bug with collision visualizations. When collisions are turned on for a model, then any collision entity that was selected from the entity tree (which "highlights" the collision entity by putting a bounding box around it), will have bounding boxes around them once you turn off, then turn back on collisions. The only way to make the bounding boxes disappear is to select the collision entity again then deselect but once you turn collisions off then back on again for the model the bounding boxes reappear.

Steps to reproduce bug before PR:

  1. ign gazebo -v 4 "https://fuel.ignitionrobotics.org/1.0/OpenRobotics/worlds/Edifice demo"
  2. Turn on collisions for Mecanum lift
  3. In entity tree, open Mecanum lift > chassis
  4. Click on collision entities for bar_1 and bar_2
  5. Right-click Mecanum lift and toggle off collisions
  6. Right-click Mecanum lift and toggle back on collisions

bar_1 and bar_2 should have bounding boxes around them.

If you run the above steps above with this PR, then the bounding boxes will not be shown around the collisions unless it is the most recent highlighted one from the entity tree. For example, if collisions are turned off and you select bar_2 from the entity tree then when you turn collisions back on (with bar_2 highlighted), bar_2 will have a bounding box around it.

This bug fix will need to be backported to ign-gazebo4 for Dome.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

@jennuine jennuine requested a review from chapulina March 24, 2021 01:25
@jennuine jennuine requested a review from iche033 as a code owner March 24, 2021 01:25
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Mar 24, 2021
@codecov
Copy link

codecov bot commented Mar 24, 2021

Codecov Report

Merging #702 (fd03ec9) into main (84ae007) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

❗ Current head fd03ec9 differs from pull request most recent head 4dfe6d3. Consider uploading reports for the commit 4dfe6d3 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #702      +/-   ##
==========================================
- Coverage   65.36%   65.33%   -0.03%     
==========================================
  Files         240      240              
  Lines       17514    17522       +8     
==========================================
  Hits        11448    11448              
- Misses       6066     6074       +8     
Impacted Files Coverage Δ
src/rendering/RenderUtil.cc 42.10% <0.00%> (-0.36%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 563e2c2...4dfe6d3. Read the comment docs.

@chapulina chapulina added beta Targeting beta release of upcoming collection bug Something isn't working labels Mar 24, 2021
@chapulina chapulina added GUI Gazebo's graphical interface (not pure Ignition GUI) rendering Involves Ignition Rendering labels Mar 24, 2021
@chapulina
Copy link
Contributor

Also cancelled the Windows build for this PR due to the long queue. I think it's ready to go in! Merging!

@chapulina chapulina merged commit e93bdaa into main Mar 24, 2021
@chapulina chapulina deleted the jennuine/collision_bugfix branch March 24, 2021 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection bug Something isn't working 🏢 edifice Ignition Edifice GUI Gazebo's graphical interface (not pure Ignition GUI) rendering Involves Ignition Rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants