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

Allow to change camera user hfov in camera_view plugin #1807

Merged
merged 4 commits into from
Mar 3, 2023

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Nov 22, 2022

Signed-off-by: Alejandro Hernández Cordero [email protected]

🎉 New feature

Set the user camera's horizontal fov through the View Angle plugin.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • 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.

@ahcorde ahcorde self-assigned this Nov 22, 2022
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Nov 22, 2022
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Merging #1807 (e82b48f) into ign-gazebo6 (89df3b7) will decrease coverage by 0.03%.
The diff coverage is n/a.

❗ Current head e82b48f differs from pull request most recent head 948d28f. Consider uploading reports for the commit 948d28f to get more accurate results

@@               Coverage Diff               @@
##           ign-gazebo6    #1807      +/-   ##
===============================================
- Coverage        64.99%   64.97%   -0.03%     
===============================================
  Files              324      324              
  Lines            26562    26562              
===============================================
- Hits             17264    17258       -6     
- Misses            9298     9304       +6     
Impacted Files Coverage Δ
src/SimulationRunner.cc 90.82% <0.00%> (-0.95%) ⬇️

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

@iche033
Copy link
Contributor

iche033 commented Nov 23, 2022

I think the previous PR was closed due to crashes on the gz-rendering side. It don't know if it's still be an issue. Maybe @jennuine has more info.

  • Update: just remembered I was able to reproduce the crash by manually changing the <horizontal_fov> tag

@jennuine
Copy link
Contributor

I think the previous PR was closed due to crashes on the gz-rendering side. It don't know if it's still be an issue. Maybe @jennuine has more info.

  • Update: just remembered I was able to reproduce the crash by manually changing the <horizontal_fov> tag

Correct, we closed the original PR because it was exposing this bug gazebosim/gz-rendering#710
The crash occurs when you set the HFOV to high (e.g., 150) then move the camera away. For more details: #1499 (review)

@azeey azeey requested a review from iche033 November 28, 2022 19:52
@iche033
Copy link
Contributor

iche033 commented Nov 29, 2022

here's more info about the crash from debugging: gazebosim/gz-rendering#710 (comment). I think the short term workaround is to set some bounds on the camera pos.

@azeey
Copy link
Contributor

azeey commented Feb 13, 2023

We discussed adding a limit in the ViewAngle plugin on the hfov users can set so as to reduce the chances of running into the gazebosim/gz-rendering#710 bug. But it seems to me a more reliable solution would be to limit the maximum distance of the camera from the origin per this comment. @iche033, is that fairly trivial to do?

@iche033
Copy link
Contributor

iche033 commented Feb 13, 2023

yeah in gz-rendering, we can just limit pose of nodes in Ogre2Node: https:/gazebosim/gz-rendering/blob/gz-rendering7/ogre2/src/Ogre2Node.cc#L107-L113. We just need to figure out a reasonable max value to limit the pos to.

Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
@iche033
Copy link
Contributor

iche033 commented Mar 3, 2023

limit max FOV to PI in 948d28f. Together with gazebosim/gz-rendering#827, gz-sim should no longer crash

@ahcorde
Copy link
Contributor Author

ahcorde commented Mar 3, 2023

Thank you for the fix @iche033

@ahcorde ahcorde merged commit 21f064f into ign-gazebo6 Mar 3, 2023
@ahcorde ahcorde deleted the ahcorde/6/view_angle/hfov branch March 3, 2023 19:11
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.

4 participants