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

Limit max camera node position values #824

Merged
merged 1 commit into from
Feb 27, 2023
Merged

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Feb 24, 2023

🦟 Bug fix

Fixes #710

Summary

Currently rendering crashes with ogre assertion error when the camera position is a really large value. This PR places a limit on the max length of of the node position vector that can be set to prevent the crash. Note that the max limit is different between ogre and ogre2 render engines

To test:

launch gazebo and use view angle plugin to set the pose of the camera to a large number:

  1. ign gazebo -v 4 shapes.sdf
  2. Go to top right menu, open View Angle plugin
  3. Scroll down to Camera Pose section and enter 100000000000 in the X field -> gazebo should no longer crash

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.

@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #824 (ebd366e) into ign-rendering6 (f80f9c3) will decrease coverage by 0.03%.
The diff coverage is 25.00%.

❗ Current head ebd366e differs from pull request most recent head 027c5a7. Consider uploading reports for the commit 027c5a7 to get more accurate results

@@                Coverage Diff                 @@
##           ign-rendering6     #824      +/-   ##
==================================================
- Coverage           79.65%   79.62%   -0.03%     
==================================================
  Files                 146      146              
  Lines               13437    13441       +4     
==================================================
  Hits                10703    10703              
- Misses               2734     2738       +4     
Impacted Files Coverage Δ
ogre2/src/Ogre2Node.cc 78.44% <25.00%> (-1.91%) ⬇️
include/gz/rendering/base/BaseGaussianNoisePass.hh 96.66% <0.00%> (-3.34%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ahcorde ahcorde merged commit 028879c into ign-rendering6 Feb 27, 2023
@ahcorde ahcorde deleted the node_position_limit branch February 27, 2023 08:42
@iche033 iche033 mentioned this pull request Mar 3, 2023
8 tasks
iche033 added a commit that referenced this pull request Mar 3, 2023
Tweak the max camera position value that's set in #824.

Signed-off-by: Ian Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants