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

Port Camera Distortion effect from gazebo11 #502

Merged
merged 18 commits into from
Dec 21, 2021

Conversation

WilliamLewww
Copy link
Contributor

🎉 Distortion pass

Summary

Ported the distortion effect from Gazebo Classic to Ignition.

Checklist

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

@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Dec 3, 2021
@osrf-triage osrf-triage added this to Inbox in Core development Dec 3, 2021
ogre/include/ignition/rendering/ogre/OgreDistortionPass.hh Outdated Show resolved Hide resolved
ogre/include/ignition/rendering/ogre/OgreDistortionPass.hh Outdated Show resolved Hide resolved
ogre/src/OgreDistortionPass.cc Outdated Show resolved Hide resolved
ogre/src/OgreDistortionPass.cc Outdated Show resolved Hide resolved
ogre/src/OgreDistortionPass.cc Outdated Show resolved Hide resolved
ogre/src/OgreDistortionPass.cc Outdated Show resolved Hide resolved
ogre/src/OgreDistortionPass.cc Outdated Show resolved Hide resolved
ogre/src/media/materials/scripts/distortion.compositor Outdated Show resolved Hide resolved
ogre/src/media/materials/scripts/distortion.material Outdated Show resolved Hide resolved
src/DistortionPass.cc Show resolved Hide resolved
Core development automation moved this from Inbox to In review Dec 4, 2021
@WilliamLewww WilliamLewww marked this pull request as ready for review December 6, 2021 18:33
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

@scpeters scpeters changed the title Distortion pass Port Camera Distortion effect from gazebo11 Dec 6, 2021
@WilliamLewww WilliamLewww force-pushed the wlew/distortion branch 3 times, most recently from 708a7ba to cc4c301 Compare December 6, 2021 19:37
@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #502 (a20d58f) into ign-rendering6 (504d8ce) will decrease coverage by 0.72%.
The diff coverage is 0.37%.

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

@@                Coverage Diff                 @@
##           ign-rendering6     #502      +/-   ##
==================================================
- Coverage           55.42%   54.70%   -0.73%     
==================================================
  Files                 195      198       +3     
  Lines               19763    20029     +266     
==================================================
+ Hits                10954    10956       +2     
- Misses               8809     9073     +264     
Impacted Files Coverage Δ
...lude/ignition/rendering/base/BaseDistortionPass.hh 0.00% <0.00%> (ø)
src/DistortionPass.cc 0.00% <0.00%> (ø)
ogre/src/OgreDistortionPass.cc 0.43% <0.43%> (ø)
...e/ignition/rendering/base/BaseGaussianNoisePass.hh 100.00% <0.00%> (+3.33%) ⬆️

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 504d8ce...423f819. Read the comment docs.

Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

nice, did a first pass review.

Can you port this fix over? gazebosim/gazebo-classic#3136

@WilliamLewww WilliamLewww force-pushed the wlew/distortion branch 5 times, most recently from 86425b3 to 155a404 Compare December 7, 2021 22:04
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

can you add an integration test to render_pass.cc? You can take a look at the test in gazebo:
https://github.com/osrf/gazebo/blob/gazebo11/test/integration/camera_sensor.cc#L697

@iche033
Copy link
Contributor

iche033 commented Dec 8, 2021

to run ogre 1.x tests, you can set the RENDER_ENGINE_VALUES to ogre, e.g.

RENDER_ENGINE_VALUES=ogre ./bin/INTEGRATION_render_pass

@iche033
Copy link
Contributor

iche033 commented Dec 9, 2021

There are a few windows warnings:
https://build.osrfoundation.org/job/ign_rendering-pr-win/2317/msbuild/new/

The newly added integration test is failing on homebrew which runs ogre 1.x tests:
https://build.osrfoundation.org/job/ignition_rendering-ci-pr_any-homebrew-amd64/2135/testReport/(root)/RenderPass_RenderPassTest/Distortion_ogre/

can you take a look?

@WilliamLewww WilliamLewww force-pushed the wlew/distortion branch 4 times, most recently from 8b999b2 to 71aad9c Compare December 16, 2021 19:21
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
include/ignition/rendering/base/BaseDistortionPass.hh Outdated Show resolved Hide resolved
test/integration/render_pass.cc Outdated Show resolved Hide resolved
test/integration/render_pass.cc Show resolved Hide resolved
Signed-off-by: William Lew <WilliamMilesLew@gmail.com>
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

looks good to me.

@ahcorde ahcorde merged commit 5e19061 into gazebosim:ign-rendering6 Dec 21, 2021
Core development automation moved this from In review to Done Dec 21, 2021
@chapulina chapulina moved this from Done to Highlights in Core development Dec 22, 2021
@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants