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

Add some infinite and reversed perspectives. #574

Merged
merged 8 commits into from Apr 9, 2019

Conversation

Projects
None yet
3 participants
@sebcrozet
Copy link
Member

sebcrozet commented Apr 2, 2019

This adds the following to nalgebra-glm:

  • infinite_perspective_rh_no
  • infinite_perspective_rh_zo
  • reversed_perspective_rh_no
  • reversed_perspective_rh_zo
  • reversed_infinite_perspective_rh_no
  • reversed_infinite_perspective_rh_zo

Addresses #573

sebcrozet added some commits Apr 2, 2019

Add some infinite and reversed perspectives.
This adds:

infinite_perspective_rh_no
infinite_perspective_rh_zo
reversed_perspective_rh_no
reversed_perspective_rh_zo
reversed_infinite_perspective_rh_zo
reversed_infinite_perspective_rh_zo

Fix #573
@Ralith

This comment has been minimized.

Copy link
Contributor

Ralith commented Apr 3, 2019

Reviewing the article, it looks like I misremembered: reversed Z actually works fine on integer depth buffers, it's just that forward Z is terrible on floating-point depth buffers.

@ishitatsuyuki

This comment has been minimized.

Copy link

ishitatsuyuki commented Apr 3, 2019

You should not use reversed Z with OpenGL ([-1, 1]) range.

This doesn't make a difference for integer formats, but with floating-point, all the precision is stuck uselessly in the middle. (The value gets mapped into [0, 1] for storage in the depth buffer later, but that doesn't help, since the initial mapping to [-1, 1] has already destroyed all the precision in the far half of the range.) And by symmetry, the reversed-Z trick will not do anything here.
Depth Precision Visualized - NVIDIA Developer

Also, do you mind adding functions for the nalgebra types too? It doesn't have to be the same PR, but in that case I think you should leave the issue open.

@Ralith

Ralith approved these changes Apr 6, 2019

Copy link
Contributor

Ralith left a comment

I checked infinite_perspective_rh_no and reversed_infinite_perspective_rh_zo against an independent derivation and they look good to me.

@Ralith

This comment has been minimized.

Copy link
Contributor

Ralith commented Apr 6, 2019

Would it make sense for the documentation to specifically encourage people to use reversed_infinite_perspective_rh_zo? I think it's pretty clearly the sane default option, so long as you're able to set up your depth range and depth test appropriately.

@sebcrozet sebcrozet merged commit 049957f into dev Apr 9, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@sebcrozet sebcrozet deleted the glm_infinite_reversed_perspective branch Apr 9, 2019

@sebcrozet

This comment has been minimized.

Copy link
Member Author

sebcrozet commented Apr 9, 2019

Thank you @ishitatsuyuki and @Ralith for your reviews!
@Ralith I've not modified the documentation to encourage the use of reversed perspective as this has implications on the depth-range and depth test settings that may not be what you find on common tutorials. We could add this kind of advice to the user guide though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.