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

RenderOrder is in different scale in Reverse Z #514

Merged

Conversation

darksylinc
Copy link
Contributor

I could not find info online about this problem, so i had to write a
test myself.

The obvious solution is to multiply the depth bias by 100 or so; but I
was wondering what would be the side effects on other geometry.

I.e. what would happen to geometry that is meant to be in front of the
plane? We know a depth bias of 300 works at close range at reverse Z
(where at normal Z a depth bias of 3 was enough), but would the plane
get on top of everything as you move the camera away?

So I made a test: https://www.youtube.com/watch?v=s2XdH3fYUac

We can safely multiply by something like 100 or so, and we will get
better results with reverse Z than we could with normal Z + the usual
depth bias constant scale.

With Normal Z, wven with a depth bias of 3 will eventually be on top of
everything, but with Reverse Z + depth bias of 300, that never happens.

Apparently multiplying the constant bias by 100.0f is the right thing
to do.

Fixes #427

Signed-off-by: Matias N. Goldberg [email protected]

🦟 Bug fix

Fixes #427

Checklist

  • Signed all commits for DCO
  • Added tests
  • 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

I could not find info online about this problem, so i had to write a
test myself.

The obvious solution is to multiply the depth bias by 100 or so; but I
was wondering what would be the side effects on other geometry.

I.e. what would happen to geometry that is meant to be in front of the
plane? We know a depth bias of 300 works at close range at reverse Z
(where at normal Z a depth bias of 3 was enough), but would the plane
get on top of everything as you move the camera away?

So I made a test: https://www.youtube.com/watch?v=s2XdH3fYUac

We can safely multiply by something like 100 or so, and we will get
better results with reverse Z than we could with normal Z + the usual
depth bias constant scale.

With Normal Z, wven with a depth bias of 3 will eventually be on top of
everything, but with Reverse Z + depth bias of 300, that never happens.

Apparently multiplying the constant bias by 100.0f *is* the right thing
to do.

Fixes gazebosim#427

Signed-off-by: Matias N. Goldberg <[email protected]>
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Dec 11, 2021
@codecov
Copy link

codecov bot commented Dec 11, 2021

Codecov Report

Merging #514 (5bc9e4b) into ign-rendering6 (93a5307) will increase coverage by 0.00%.
The diff coverage is 80.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-rendering6     #514   +/-   ##
===============================================
  Coverage           55.42%   55.42%           
===============================================
  Files                 195      195           
  Lines               19758    19762    +4     
===============================================
+ Hits                10951    10954    +3     
- Misses               8807     8808    +1     
Impacted Files Coverage Δ
ogre2/src/Ogre2Material.cc 84.21% <80.00%> (-0.10%) ⬇️

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 93a5307...5bc9e4b. 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.

oh interesting, I didn't expect the fix to be a straightforward multiplication factor. The results look good, thanks for testing!

@iche033 iche033 merged commit d06d2d4 into gazebosim:ign-rendering6 Dec 13, 2021
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Jan 26, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 2, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 3, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 4, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 4, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 5, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 7, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 7, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 8, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Mar 1, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
@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

srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Mar 28, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Mar 28, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Mar 28, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Mar 28, 2022
…im#514

Signed-off-by: Rhys Mainwaring <[email protected]>

gazebosim#514 causes render order issues for Metal - reverting until resolved.
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Jun 27, 2022
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Jul 7, 2022
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Aug 11, 2022
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Sep 8, 2022
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Sep 14, 2022
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Nov 24, 2022
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Dec 14, 2022
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Jan 29, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Jan 30, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 8, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 16, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Mar 4, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Mar 18, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request May 8, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request May 18, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request May 26, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request May 30, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Jul 8, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Sep 3, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Oct 31, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Dec 15, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Dec 15, 2023
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 25, 2024
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Feb 29, 2024
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Apr 23, 2024
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request May 13, 2024
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Jul 1, 2024
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Aug 1, 2024
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Sep 19, 2024
srmainwaring added a commit to srmainwaring/gz-rendering that referenced this pull request Oct 26, 2024
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.

RenderOrder in Simple Demo is not working (ogre2)
3 participants