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.

Sign up for GitHub

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

Jump to bottom

Fix TransformControl's BlockOrbit behavior #2030

Merged
merged 2 commits into from
Jul 15, 2023
Merged

Fix TransformControl's BlockOrbit behavior #2030

merged 2 commits into from
Jul 15, 2023

Conversation

Henrique-BO
Copy link
Contributor

🦟 Bug fix

Summary

Currently, the TransformControl plugin sends a BlockOrbit event on every rendering call, even if it does not wish to block the camera orbit. Because of this, if another plugin sends a BlockOrbit event, it might get overwritten by TransformControl's and not have the desired effect.

This PR fixes this by making TransformControl send BlockOrbit false events only once (to unblock after a transform), allowing other plugins to block the camera while TransformControl is idle. true events are unchanged, and are still sent on every rendering call.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@Henrique-BO
Send BlockOrbit false events only once
ad3da9e 
Signed-off-by: Henrique-BO <henrique.barrosoliveira@usp.br>
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jul 11, 2023
@codecov
Copy link

codecov bot commented Jul 11, 2023 •
edited
Loading

Codecov Report

Merging #2030 (635fea7) into gz-sim7 (d93a83e) will increase coverage by 0.00%.
The diff coverage is 61.78%.

❗ Current head 635fea7 differs from pull request most recent head d0bbb20. Consider uploading reports for the commit d0bbb20 to get more accurate results

@@            Coverage Diff            @@
##           gz-sim7    #2030    +/-   ##
=========================================
  Coverage    65.00%   65.01%            
=========================================
  Files          352      354     +2     
  Lines        28439    28643   +204     
=========================================
+ Hits         18488    18621   +133     
- Misses        9951    10022    +71
Impacted Files Coverage Δ
include/gz/sim/SdfEntityCreator.hh 100.00% <ø> (ø)
include/gz/sim/rendering/MarkerManager.hh 100.00% <ø> (ø)
include/gz/sim/rendering/RenderUtil.hh 100.00% <ø> (ø)
include/gz/sim/rendering/SceneManager.hh 100.00% <ø> (ø)
.../plugins/component_inspector/ComponentInspector.cc 5.61% <0.00%> (ø)
.../gui/plugins/transform_control/TransformControl.cc 6.01% <0.00%> (-0.12%) ⬇️
...rc/systems/ackermann_steering/AckermannSteering.hh 100.00% <ø> (ø)
src/systems/buoyancy/Buoyancy.cc 82.25% <0.00%> (ø)
.../systems/environment_preload/EnvironmentPreload.cc 69.69% <0.00%> (ø)
src/systems/joint_controller/JointController.hh 100.00% <ø> (ø)
... and 28 more

... and 1 file with indirect coverage changes

@Henrique-BO Henrique-BO marked this pull request as ready for review July 12, 2023 14:46
@Henrique-BO Henrique-BO requested a review from mjcarroll as a code owner July 12, 2023 14:46
@azeey azeey self-requested a review July 15, 2023 05:37
azeey
azeey approved these changes Jul 15, 2023
View reviewed changes
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Great solution!

@azeey azeey merged commit e749020 into gazebosim:gz-sim7 Jul 15, 2023
@Henrique-BO Henrique-BO deleted the block_orbit branch July 17, 2023 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azeey azeey azeey approved these changes

@mjcarroll mjcarroll Awaiting requested review from mjcarroll mjcarroll is a code owner

Assignees
No one assigned
Labels
🌱 garden Ignition Garden
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Henrique-BO @azeey