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

Do not use assume every RenderPanel has a ViewController. #613

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Oct 19, 2020

This pull request gets the current Ogre::Viewport from the panel's RenderWindow instead of assuming it'll have an rviz_common::ViewController (which it doesn't if the panel is that of a rviz_default_plugins::Camera display.

With this patch, interactive markers work (and do not make rviz crash) when a user attempts to interact with marker controls via camera feed (which, to my surprise, is supported on ROS 1).

CI up to rviz2, rviz_default_plugins, and rviz_rendering_tests:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Get the Viewport from its RenderWindow instead.

Ensures interactive markers work (and do not make rviz crash)
when interaction goes through a camera feed and not the main
window.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Contributor Author

hidmic commented Oct 19, 2020

@mjcarroll any chance we can backport this to Eloquent?

Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM

@hidmic I'm happy to review backports to Foxy and Eloquent if you want to open up PRs.

@jacobperron jacobperron added the bug Something isn't working label Oct 19, 2020
@hidmic
Copy link
Contributor Author

hidmic commented Oct 20, 2020

Alright, going in!

@hidmic hidmic merged commit 885ba70 into ros2 Oct 20, 2020
@hidmic hidmic deleted the hidmic/drop-view-controller-assumption branch October 20, 2020 13:42
hidmic added a commit that referenced this pull request Oct 20, 2020
Get the Viewport from its RenderWindow instead.

Ensures interactive markers work (and do not make rviz crash)
when interaction goes through a camera feed and not the main
window.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
hidmic added a commit that referenced this pull request Oct 20, 2020
Get the Viewport from its RenderWindow instead.

Ensures interactive markers work (and do not make rviz crash)
when interaction goes through a camera feed and not the main
window.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
hidmic added a commit that referenced this pull request Oct 20, 2020
Get the Viewport from its RenderWindow instead.

Ensures interactive markers work (and do not make rviz crash)
when interaction goes through a camera feed and not the main
window.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
hidmic added a commit that referenced this pull request Oct 20, 2020
Get the Viewport from its RenderWindow instead.

Ensures interactive markers work (and do not make rviz crash)
when interaction goes through a camera feed and not the main
window.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants