SSAO: New implementation #15243
SSAO: New implementation #15243
Conversation
|
Sweet! |
|
Many thanks! |
|
On my laptop it looks great:
But on my mobile it's very jagged:
Is it dependant on device resolution? Also, "Beauty" output is just the diffuse color, right? Is there a reason why it's called "Beauty" and not "Diffuse"? |
The render result gets better when the device has a higher resolution, sure. The demo does not currently set the pixel device ratio since
Yes. It's actually a common term in context of post processing to name the default render pass "beauty pass". I think this is actually better than "diffuse" since "diffuse" is already used in context of lighting models. BTW: You find the term "Beauty" in other demos, too (e.g. here). |
OK, makes sense, although the choice of the word "Beauty" still seems a little random to me in this context.
My mobile (Moto G6) has a pixel ratio of 3, so that's probably the issue here. |
| // render normals | ||
|
|
||
| this.uniforms[ 'radius' ].value = value; | ||
| this.renderOverride( renderer, this.normalMaterial, this.normalRenderTarget, 0x7777ff, 1.0 ); |
There was a problem hiding this comment.
This is changing the background color and not being reset to the original value, apparently.
There was a problem hiding this comment.
.renderOverride() should actually reset the original state at the end of the method. Is this not the case?
BTW: It's the same approach like in the SAO demo.
There was a problem hiding this comment.
To demonstrate, in webgl_postprocessing_ssao.html, remove the scene background:
//scene.background = new THREE.Color( 0xa0a0a0 );and add a call to render
effectComposer.render();
renderer.render( scene, camera );There was a problem hiding this comment.
I think I've found the problem right here. WebGLRenderer.getClearColor() returns no copy of the color. If the color is later modified, it also affects the original value. A simple fix would be:
this.originalClearColor.copy( renderer.getClearColor() );and later
renderer.setClearColor( this.originalClearColor );I'll add a fix for SAOPass and SSAOPass.
There was a problem hiding this comment.
Would #15212 have been helpful in this case, or are there problems with that, too?
There was a problem hiding this comment.
Not sure. This seems app-level logic to me. Maybe it's better if the app saves and restores these values.
There was a problem hiding this comment.
This seems app-level logic to me.
Granted. I just see the same patterns used over and over...
There was a problem hiding this comment.
If these methods are added, users could ask for the possibility to save multiple states (think of vertex array objects that represent binding states) and restore/load them with a ID or a name. I think stuff like that would bloat the API of WebGLRenderer.
There was a problem hiding this comment.
If these methods are added, users could ask for the possibility...
Anything could happen. Sorry, I'm not buying that as a very compelling argument.
This seems app-level logic to me.
That's a reasonable argument.
| THREE.Pass.call( this ); | ||
|
|
||
| this.width = ( width !== undefined ) ? width : 512; | ||
| this.height = ( height !== undefined ) ? height : 512; |
There was a problem hiding this comment.
this.width and this.height are set here, but the code below still uses normal width and height to setup the WebGLRenderTargets and the resolution uniforms. This causes WebGLRenderTarget to be created with undefined dimensions on start before getting resized by onWindowResize.
Could that cause any strange problems on devices with bad WebGL implementations later on (the jaggedness on mobile devices for example)?
There was a problem hiding this comment.
Good catch! Let me fix this. The code should actually use the this reference.
5 participants
This PR introduces a new screen space ambient occlusion (SSAO) shader. Most of the implementation is based on John Chapman's article about SSAO.
The code hopefully fixes #8576 since it's provides a more or less real ambient occlusion effect.
As mentioned by a user in the linked article, the implemented blur shader produces a thin halo of non-occlusion when there are high frequencies in the depth buffer (steep changes of depth values). I guess the usage of
DepthLimitedBlurShadermight solve this issue, but it's a more complex blur and thus computationally more expensive (and in general not necessary in all use cases).