Improved SSAO2 for sample count <16 #13652
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
@Popov72, could you please take a look at this one? It is fairly important IMHO. |
|
Related issue: #12630 |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13652/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/13652/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/13652/merge#BCU1XR#0 |
|
I agree with this change, but it would be easier to add it to the other PR, which is about to be merged. Also, please make it a parameter: if people don't understand what it does / don't need to change it, they'll leave it unchanged, but people who do understand it will have the option to do so (I'm not a big fan of magical constant values that can't be changed - once we can change them, they are not magical anymore!). |
Sure. The thing is that I don't know when I will have time to figure out how the serialization thing you asked me to fix in that PR works. But I will try to find some time this evening.
I can see where you are coming from, but at the same time, I agree with the person in the forum that he felt that he needed to understand and mess with too many internal parameters to get SSAO2 that looked right. But perhaps I can solve that by commenting that you normally wouldn't need to change it. I hope I can make it default to 0.02 so people don't need to change it to get decent default 8-sample SSAO2? |
|
|
||
| // We need to filter away diffs very close to 0 due to precision errors, | ||
| // otherwise we will get a ton of incorrect occlusions with low sample counts. | ||
| occlusion += (difference >= 0.02 ? 1.0 : 0.0) * rangeCheck; |
There was a problem hiding this comment.
0.02 looks like a magic number to me. Could this be configurable ?
|
Just noticed @Popov72 comments and yup totally agree :-) making it configurable with default 0.02 would be great and lets merge it in the other PR ??? By the way, thanks a ton for all the improvments, it starts to look amazing !!! |
|
The fix has now been moved to PR #13621 , closing this PR. |
Summary
This fix massively improves SSAO2 when using low sample counts, like the default 8.
Background
It was discovered as part of PR #13621 but is submitted as a new PR since 13621 is now all about adding control over the SSAO2 denoiser without changing anything by default.
This small change however is so massively positive in all the tests I have done that it shouldn't be left out by default but be considered a bugfix.
How it works
The fix is rather simple. When calculating if sample X is occluded by sample Y in the SSAO2 calculations we considered everything with a depth difference of 0 (zero) to mean Y occludes X. However, this does not take any calculation or precision errors into account, which causes samples along a "flat" surface to occlude each other. Basically, the flat surface was occluding itself...
This fix adds an epsilon of sorts to the calculation. The value 0.02 was selected from testing and gave the best improvements on all tests I did and at the same time practically left the 16-sample case untouched.
That said, on hardware that doesn't have the same precision in the shaders. might benefit from a higher threshold, as would some scenes like the kitchen example. But going over 0.02 starts to affect samples >=16 which isn't great from a backward compatibility standpoint.
I did not make this into a tunable setting, since I don't expect that a user of Babylon.js would understand how it affects the shader calculations. This should just work.
Test results
In the screenshots below, the "8x" part denotes how many SSAO2 samples were used, old version is on the left and the new one is on the right.
Notice how the fixed version is pretty similar to the 16x reference when the sample count goes down. But the old version goes bananas with incorrect occlusions everywhere. Also note that the two versions at sample count 16 are basically identical.
The images were saved without the blur-stage of the SSAO2 filter active, to better show the SSAO2 calculations.
Terrain (#N96NXC#127)
16x
12x
8x
4x
Cat (#T3K2SA#1)
16x
12x
8x
4x
Room (#LCUPCU#14)
16x
12x
8x
4x