[Merged by Bors] - bevy_pbr2: Fix clustering for orthographic projections #3316
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
superdump
force-pushed
the
clustered-orthographic-fix
branch
from
b82d790
to
c97269e
Compare
mockersf
added
A-Rendering
|
Works in Sponza and looks appropriately funky 🙂 : |
|
I forgot to pick in the debug visualisation improvements and comment fixes from one of my branches with bloom on top of clustered forward before making the PR for it. I used the third visualisation (cluster coherency) to check the clustering, and the z-slicing one as well to find and fix a bug. So I added those in here too. |
superdump
force-pushed
the
clustered-orthographic-fix
branch
from
021a030
to
09805f5
Compare
|
Updated on top of main. |
|
Looks good to me. Thanks for the quick fix! |
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Dec 14, 2021
# Objective PBR lighting was broken in the new renderer when using orthographic projections due to the way the depth slicing works for the clusters. Fix it. ## Solution - The default orthographic projection near plane is 0.0. The perspective projection depth slicing does a division by the near plane which gives a floating point NaN and the clustering all breaks down. - Orthographic projections have a linear depth mapping, so it made intuitive sense to me to do depth slicing with a linear mapping too. The alternative I saw was to try to handle the near plane being at 0.0 and using the exponential depth slicing, but that felt like a hack that didn't make sense. - As such, I have added code that detects whether the projection is orthographic based on `projection[3][3] == 1.0` and then implemented the orthographic mapping case throughout (when computing cluster AABBs, and when mapping a view space position (or light) to a cluster id in both the rust and shader code). ## Screenshots Before:  After: <img width="1392" alt="Screenshot 2021-12-13 at 16 36 53" src="https://user-images.githubusercontent.com/302146/145847314-6f3a2035-5d87-4896-8032-0c3e35e15b7d.png"> Old renderer (slightly lighter due to slight difference in configured intensity): <img width="1392" alt="Screenshot 2021-12-13 at 16 42 23" src="https://user-images.githubusercontent.com/302146/145847391-6a5e6fe0-22da-4fc1-a6c7-440543689a63.png">
bors
bot
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
PBR lighting was broken in the new renderer when using orthographic projections due to the way the depth slicing works for the clusters. Fix it.
Solution
projection[3][3] == 1.0and then implemented the orthographic mapping case throughout (when computing cluster AABBs, and when mapping a view space position (or light) to a cluster id in both the rust and shader code).Screenshots
Before:
After:
Old renderer (slightly lighter due to slight difference in configured intensity):