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

Fix banding and discoloration issue in 3D scene #629

Closed
wants to merge 5 commits into from
Closed

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Jul 24, 2024

🦟 Bug fix

Summary

The MinimalScene uses the GammaAdjust effect for gamma correction otherwise the scene will appear darker than it should when using ogre2 + OpenGL. However, it was found that caused caused banding and discoloration issues in the 3D scene, see screenshots here. This PR removes the GammaAdjust QML effect and instead applies a workaround in the OpenGL RHI to convert the input ogre2 render texture from the sRGB format to linear RGB format before passing it to Qt. The scene now display the correct colors but without banding issues.

Note this may not be necessary in Qt 6 as I see that there is a new class and API to set the flags of its RHI texture to sRGB color space

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.

@iche033 iche033 requested a review from jennuine as a code owner July 24, 2024 01:06
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Jul 24, 2024
Signed-off-by: Ian Chen <[email protected]>
@iche033 iche033 marked this pull request as ready for review July 25, 2024 00:02
Copy link
Contributor

@jennuine jennuine 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, whenever we migrate to Qt6 we'll need to remember to try w/o the workaround before we forward port the code. Thanks Ian.

@jennuine jennuine mentioned this pull request Jul 26, 2024
@iche033
Copy link
Contributor Author

iche033 commented Jul 29, 2024

replaced by #630

@iche033 iche033 closed this Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants