RenderPassSsao: improve SSAO blur performance #6684

querielo · 2024-06-10T21:03:16Z

This PR suggests splitting one large blur pass from "RenderPass based SSAO" into four smaller blur passes without compromising quality.

Main	PR

Tested on MacBook 14" Pro (2021), used device.maxPixelRatio = window.devicePixelRatio; to increase resolution of framebuffers.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

… big pass (2 gaussian for high frequent signal, 2 interleaved for low frequency)

mvaligursky · 2024-06-11T15:25:11Z

Hi @querielo - that's definitely a good way to speed it up. But I wonder why doing 4 passes instead of just typical two separable passes?

querielo · 2024-06-11T19:46:27Z

Hi. @mvaligursky

The main idea is that the first two passes weaken the high-frequency signal. The next two passes are used to eliminate low-frequency signal (there are strided/interleaved blurs with a large step, applied diagonally).

Experimentally, it seems that increasing the filter kernel size is necessary to remove low-frequency SSAO patterns. For example, using a 17x17 kernel on my computer results in a noticeable slowdown, but it appears to achieve the effect of four passes.

The suggested four-pass approach is just one way to implement blurring. An advanced developer could add RenderPassDepthAwareBlur to afterPasses themselves, depending on the specific SSAO pattern generated.

4 passes	2 passes, 11x11	2 passes, 17x17

By the low-frequency signal, I mean the pattern you can see in the next image (kernelSize=11)

mvaligursky · 2024-06-12T10:47:51Z

I had a bit of a play with your branch. My findings:

two pass BOX blur with 9 samples seems to be equivalent to the current 9x9 filter, as expected. I assume this is faster.
two pass GAUSSIAN blur with 9 samples is a lot lower quality compared to that - see especially around the yellow torches
four pass you had set up seems to match in quality with the two pass BOX with 9 samples - so I'm not sure we need to use 4 passes.

I think the main reason you need 4 passes is the use of the GAUSSIAN weights instead of BOX.

mvaligursky · 2024-06-12T10:49:29Z

And so my recommendation is: switch it to 2 pass BOX filtering. We can expose the number of taps to the user as that controls the quality vs the cost.

querielo · 2024-06-12T11:19:51Z

@mvaligursky I switched the blur to 2 pass BOX filtering.

src/core/math/math.js

querielo · 2024-06-12T11:26:48Z

Offtop: It looks like 6f29e1b breaks the background.

mvaligursky · 2024-06-12T11:28:53Z

src/extras/render-passes/render-pass-ssao.js

@@ -308,6 +351,8 @@ class RenderPassSsao extends RenderPassShaderQuad {
    }

    createRenderTarget(name) {
+        // TODO: consider using a pool of 2 texture buffers


considering we're down to 2 blurs, remove the comment

mvaligursky · 2024-06-12T11:31:19Z

src/extras/render-passes/render-pass-depth-aware-blur.js

+     * @param {Vec2} [options.direction] - The direction of the blur. Defaults to (1, 0).
+     * @param {string} [options.channels] - The color channels to apply the blur to ('r'|'g'|'b'|'a'|'rg'|..|'ba'|'rgb'|'gba'|'rgba'). Defaults to 'rgba'.
+     */
+    init(renderTarget = null, options = {}) {


I would not override the init function, but instead add a setup function which does all this.

mvaligursky · 2024-06-12T11:34:37Z

src/extras/render-passes/render-pass-depth-aware-blur.js

+                totalWeight += ${weightCoefs[middle].toFixed(4)};`;
+
+        // TODO: move calculating UV coordinates to the vertex shader and pass them as varying
+        for (let i = 1; i <= kernelWidth; i++) {


we're trying to move away from generating shader from javascript as much as possible, as it's a lot harder to understand and modify. We try to create a single shader string as much as possible, and then in code generate a list of defines to pre-pend. See RenderPassCompose as an example. It'd be great to modify this in a similar fashion.

even though the shader here is trying to be pretty generic, which makes it harder (with the types and channels)

mvaligursky · 2024-06-12T11:39:52Z

Offtop: It looks like 6f29e1b breaks the background.

It just got brighter as a result of this #6687 I suspect?

mvaligursky · 2024-06-17T09:48:01Z

Looking much better, thanks!

I'd suggest to remove the option / API to change the blur type. Box looks better than Gaussian, and so the SSAO should just use Box. Only expose values that the user would benefit from adjusting.

mvaligursky · 2024-06-17T09:50:08Z

src/extras/render-passes/render-pass-depth-aware-blur.js

+uniform vec2 sourceInvResolution;
+uniform int filterSize;
+uniform vec2 direction;
+uniform float kernel[KERNEL_SIZE];


hide the kernel behind #ifdef KERNEL to avoid the cost when the box filter is used

mvaligursky · 2024-06-17T09:51:06Z

src/extras/render-passes/render-pass-depth-aware-blur.js

-                float diff = (sampleDepth - depth);
-                return max(0.0, 1.0 - diff * diff);
-            }
+        this.sourceInvResolutionId?.setValue(sourceInvResolutionValueTmp);


no need for those ? there as those are transpiled to if .. we always set those up, so they're never undefined.

VS Code TS checker highlighted this error, suggesting that sourceInvResolutionId could be undefined as it is not defined during creation. I'll check if Playcanvas linter highlights it.

yep, linter highlights it, we have lots of these warnings in the engine, but they're not correct, but we don't see a way to remove them.

mvaligursky · 2024-06-17T09:52:10Z

src/extras/render-passes/render-pass-depth-aware-blur.js

-        super.execute();
+        const defines = `#define KERNEL_SIZE ${this.kernelSize}\n`;
+
+        // CHECK: should we destroy the shader?


shaders are expensive to compile, so we don't destroy them, to make it faster when the shader is needed again

Then I will remove the comment.

mvaligursky · 2024-06-17T09:53:52Z

src/extras/render-passes/render-pass-depth-aware-blur.js

-                // simple dithering helps a lot (assumes 8 bits target)
-                // this is most useful with high quality/large blurs
-                // ao += ((random(gl_FragCoord.xy) - 0.5) / 255.0);
+        this.updateShader();


ideally don't call updateShader directly, just set a _shaderDirty flag - this makes it easy to add more properties that modify the shader, and also better handle the case where the property is changed multi times per frame. (not that this is typical, but happens).

querielo · 2024-06-17T11:21:54Z

I'd suggest to remove the option / API to change the blur type. Box looks better than Gaussian, and so the SSAO should just use Box. Only expose values that the user would benefit from adjusting.

@mvaligursky Are you certain?
Stage: https://engine-m742xowmh-playcanvas.vercel.app/#/graphics/ambient-occlusion
To me, it seems that enlarging the kernel size of Box displaces dark areas from their correct position more quickly than hiding SSAO artifacts. However, Gaussian necessitates a larger kernel to conceal the SSAO pattern.

ssao_blur.mov

mvaligursky · 2024-06-17T11:46:10Z

@mvaligursky Are you certain? Stage: https://engine-m742xowmh-playcanvas.vercel.app/#/graphics/ambient-occlusion To me, it seems that enlarging the kernel size of Box displaces dark areas from their correct position more quickly than hiding SSAO artifacts. However, Gaussian necessitates a larger kernel to conceal the SSAO pattern.

yeah interesting. It almost feel like the the bilateral weight function should be improved

float bilateralWeight(in float depth, in float sampleDepth) {
    float diff = (sampleDepth - depth);
    return max(0.0, 1.0 - diff * diff);
}

to detect the depth discontinuity better, as currently it blurs over those small edges. Maybe we can try something like this to have a control over it by the sigma value (untested, would need more research / testing)

float bilateralWeight(in float depth, in float sampleDepth, in float sigma) {
    float diff = (sampleDepth - depth);
    return exp(-diff * diff / (2.0 * sigma * sigma));
}

but agreed, the Gaussian blur in general should be slightly better, at a higher cost, so maybe leave it in.

mvaligursky · 2024-06-24T15:08:52Z

Hi @querielo - have you had some time to look at these?

mvaligursky · 2024-06-28T12:02:56Z

@querielo - please let me know if you have time to finished this PR. I'm keen to continue on some improvements too (#6658) but would prefer to avoid larger conflicts with the changes.

If you don't have time, I can take over this PR.

MAG-AdrianMeredith · 2024-07-19T13:00:05Z

@querielo - please let me know if you have time to finished this PR. I'm keen to continue on some improvements too (#6658) but would prefer to avoid larger conflicts with the changes.

If you don't have time, I can take over this PR.

guess thats a no...

mvaligursky · 2024-07-19T13:23:59Z

yep, it's on my list to get to in a week or so. Definitely not forgotten.

querielo · 2024-07-21T20:57:09Z

@mvaligursky Yes, you can take it. Sorry about that. I'm too busy right now and don't have time to fix it.

mvaligursky · 2024-08-02T12:20:44Z

closing this due to #6870
Thanks @querielo

querielo added 2 commits June 10, 2024 22:41

SSAO: improve ssao blur performance using 4 small passes instead of 1…

477059a

… big pass (2 gaussian for high frequent signal, 2 interleaved for low frequency)

Merge branch 'main' into kirill-ssao-blur

0e84638

vercel bot deployed to Preview June 10, 2024 21:08 View deployment

Fix TypeScript declarations, remove some comments

70efa2f

vercel bot deployed to Preview June 10, 2024 22:14 View deployment

querielo changed the title ~~SSAO: improve ssao blur performance~~ RenderPassSsao: improve SSAO blur performance Jun 10, 2024

Update some comments

303fe9f

vercel bot deployed to Preview June 11, 2024 09:50 View deployment

willeastcott requested a review from mvaligursky June 11, 2024 12:30

willeastcott added performance Relating to load times or frame rate area: graphics Graphics related issue labels Jun 11, 2024

SSAO blur: fix destroy

0d2d7f0

vercel bot deployed to Preview June 12, 2024 06:16 View deployment

Merge branch 'main' into kirill-ssao-blur

100f1d8

vercel bot deployed to Preview June 12, 2024 06:45 View deployment

Review comment: playcanvas#6684 (comment)

6f0a2f0

vercel bot deployed to Preview June 12, 2024 11:21 View deployment

mvaligursky reviewed Jun 12, 2024

View reviewed changes

src/core/math/math.js Show resolved Hide resolved

mvaligursky reviewed Jun 12, 2024

View reviewed changes

src/core/math/math.js Show resolved Hide resolved

mvaligursky reviewed Jun 12, 2024

View reviewed changes

Review comments: playcanvas#6684 (comment), playcanvas#6684 (comment)

009c936

mvaligursky reviewed Jun 12, 2024

View reviewed changes

vercel bot deployed to Preview June 12, 2024 11:31 View deployment

mvaligursky reviewed Jun 12, 2024

View reviewed changes

Review comments:playcanvas#6684 (comment), playcanvas#6684 (comment)

811490b

vercel bot deployed to Preview June 16, 2024 21:08 View deployment

Fix typing

87c7577

vercel bot deployed to Preview June 16, 2024 21:13 View deployment

Fix linter

90326be

vercel bot deployed to Preview June 16, 2024 21:28 View deployment

Merge branch 'main' into kirill-ssao-blur

c3f2b29

vercel bot deployed to Preview June 17, 2024 07:24 View deployment

mvaligursky reviewed Jun 17, 2024

View reviewed changes

mvaligursky mentioned this pull request Aug 2, 2024

SSAO uses a separable depth aware blur to improve performance #6870

Merged

mvaligursky closed this Aug 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RenderPassSsao: improve SSAO blur performance #6684

RenderPassSsao: improve SSAO blur performance #6684

querielo commented Jun 10, 2024 •

edited

Loading

mvaligursky commented Jun 11, 2024

querielo commented Jun 11, 2024 •

edited

Loading

mvaligursky commented Jun 12, 2024

mvaligursky commented Jun 12, 2024

querielo commented Jun 12, 2024

querielo commented Jun 12, 2024

mvaligursky Jun 12, 2024

mvaligursky Jun 12, 2024

mvaligursky Jun 12, 2024

mvaligursky Jun 12, 2024

mvaligursky commented Jun 12, 2024

mvaligursky commented Jun 17, 2024

mvaligursky Jun 17, 2024

mvaligursky Jun 17, 2024

querielo Jun 17, 2024

mvaligursky Jun 17, 2024

mvaligursky Jun 17, 2024

querielo Jun 17, 2024

mvaligursky Jun 17, 2024

querielo commented Jun 17, 2024 •

edited

Loading

mvaligursky commented Jun 17, 2024 •

edited

Loading

mvaligursky commented Jun 24, 2024

mvaligursky commented Jun 28, 2024

MAG-AdrianMeredith commented Jul 19, 2024

mvaligursky commented Jul 19, 2024

querielo commented Jul 21, 2024 •

edited

Loading

mvaligursky commented Aug 2, 2024

RenderPassSsao: improve SSAO blur performance #6684

RenderPassSsao: improve SSAO blur performance #6684

Conversation

querielo commented Jun 10, 2024 • edited Loading

mvaligursky commented Jun 11, 2024

querielo commented Jun 11, 2024 • edited Loading

mvaligursky commented Jun 12, 2024

mvaligursky commented Jun 12, 2024

querielo commented Jun 12, 2024

querielo commented Jun 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvaligursky commented Jun 12, 2024

mvaligursky commented Jun 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

querielo commented Jun 17, 2024 • edited Loading

mvaligursky commented Jun 17, 2024 • edited Loading

mvaligursky commented Jun 24, 2024

mvaligursky commented Jun 28, 2024

MAG-AdrianMeredith commented Jul 19, 2024

mvaligursky commented Jul 19, 2024

querielo commented Jul 21, 2024 • edited Loading

mvaligursky commented Aug 2, 2024

querielo commented Jun 10, 2024 •

edited

Loading

querielo commented Jun 11, 2024 •

edited

Loading

querielo commented Jun 17, 2024 •

edited

Loading

mvaligursky commented Jun 17, 2024 •

edited

Loading

querielo commented Jul 21, 2024 •

edited

Loading