EEVEE-Next: Add shadow bias based on depth reconstruction precision #111478

Miguel Pozo · 2023-08-24T16:56:12+02:00

Miguel Pozo commented

2023-08-24 16:56:12 +02:00

Avoid self-shadowing artifacts by adding shadow bias based on world position reconstruction precision.
This also updates the Renderbuffers depth texture to use a 32 bit format for extra precision, and to match the HiZ texture.

Before	After

Avoid self-shadowing artifacts by adding shadow bias based on world position reconstruction precision. This also updates the Renderbuffers depth texture to use a 32 bit format for extra precision, and to match the HiZ texture. Before | After ---|--- ![imagen](/attachments/bafee904-7650-4808-9872-b992b1546f3d)|![imagen](/attachments/025f8c41-2c06-4b36-b475-d9461e1821a8)

imagen.png

240 KiB

imagen.png

243 KiB

❤️ 1 🎉 1 🚀 1

Miguel Pozo added the

Module

EEVEE & Viewport

label 2023-08-24 16:56:12 +02:00

Miguel Pozo added 2 commits 2023-08-24 16:56:26 +02:00

a3959f1d42 Convert depth_tx to 32f

b5980d51a3 Add shadow bias based on depth reconstruction precision

Miguel Pozo added this to the EEVEE & Viewport project 2023-08-24 16:56:39 +02:00

Miguel Pozo requested review from Clément Foucault 2023-08-24 16:56:54 +02:00

Clément Foucault reviewed 2023-08-24 17:06:08 +02:00

source/blender/draw/engines/eevee_next/shaders/eevee_deferred_light_frag.glsl Outdated

						
				@ -14,1 +14,4 @@

				float depth_reconstruction_precision(float depth)

				{

				  vec3 P = get_world_space_from_depth(uvcoordsvar.xy, depth);

Clément Foucault commented

2023-08-24 17:06:01 +02:00

Can't you have the same result with get_view_space_from_depth? That would save 2 matrix multiply.

Can't you have the same result with `get_view_space_from_depth`? That would save 2 matrix multiply.

pragma37 marked this conversation as resolved

Miguel Pozo added 1 commit 2023-08-25 18:39:27 +02:00

ce26a3003c Reuse already computed P

Clément Foucault requested changes 2023-08-29 12:31:10 +02:00

source/blender/draw/engines/eevee_next/shaders/eevee_shadow_lib.glsl Outdated

						
				@ -171,12 +175,17 @@ ShadowSample shadow_punctual_sample_get(

				  float radius_divisor = receiver_dist / abs(lP.z);

				  samp.occluder_dist = occluder_z.x * radius_divisor;

				  samp.bias = (occluder_z.y - occluder_z.x) * radius_divisor;

				  samp.bias += bias;

Clément Foucault commented

2023-08-29 12:31:05 +02:00

I would guard this behind a #ifdef DEFERRED_EVAL directive to avoid the cost in other passes.
I don't know if the Dead Code Elimination would be good enough on all platform to eliminate the + 0.0.

I would guard this behind a `#ifdef DEFERRED_EVAL` directive to avoid the cost in other passes. I don't know if the Dead Code Elimination would be good enough on all platform to eliminate the `+ 0.0`.

Miguel Pozo commented

2023-08-29 16:19:58 +02:00

If we do this maybe we should move the shadow bias parameter to a global variable?
It would be quite weird to ignore a function parameter based on a preprocessor define.

That said, even if the compiler doesn't ignore the add, is that going to cause any measurable difference?

If we do this maybe we should move the shadow bias parameter to a global variable? It would be quite weird to ignore a function parameter based on a preprocessor define. That said, even if the compiler doesn't ignore the add, is that going to cause any measurable difference?

Clément Foucault commented

2023-08-29 16:36:51 +02:00

It is a value that takes one register and is long lived.

pragma37 marked this conversation as resolved

Miguel Pozo added 1 commit 2023-08-30 13:45:04 +02:00

c0d16b0cb0 Merge branch 'main' into pull-eevee-next-shadow-bias

Miguel Pozo force-pushed pull-eevee-next-shadow-bias from 9732d02bcc to 01c1d86c0a

2023-08-30 13:46:23 +02:00

Compare

Miguel Pozo added 1 commit 2024-03-08 17:25:43 +01:00

c83c40a795 Merge branch 'main'

Miguel Pozo referenced this pull request

2024-03-08 17:32:26 +01:00

EEVEE-Next: Shadows become a glitchy mess at distance. #119124

Miguel Pozo requested review from Clément Foucault 2024-03-11 12:28:30 +01:00

Clément Foucault requested changes 2024-03-18 17:38:06 +01:00

Clément Foucault left a comment

I am not convinced by this approach. I find it a bit too complicated for what it tries to solve.

The aliasing is caused by 2 factor:

Scene depth buffer quantization:
This can be fixed by added a small constant bias to the depth buffer value right after reading it (the nextafter2). This nextafter only works if depth buffer is using float32 format to have the same quantization bias as the format (otherwise you need to bias to 4.8e-7 or a bit higher. ref http://terathon.com/gdc07_lengyel.pdf slide 26 or search for 2.4e-7 or 4.8e-7 in the code (this should definitely be put into a utility somewhere) ).
But do note that float32 depth format offer no benefit with conventional projection matrix (https://developer.nvidia.com/content/depth-precision-visualized). So while I'm ok with enable it (MacOS already enables it by default) I would like to see a comparison.
I would suggest to just bias the shading point reconstruction instead of computing the error. This is done in many other passes the same way and it doesn't create much difference on the final image.
Shadowmap aliasing:
This is already mitigated by the slope bias in the shadow fragment shader (float f_depth = gl_FragCoord.z + fwidth(gl_FragCoord.z);) and u_depth += 2;.
But that doesn't solve projective or perspective aliasing (i.e: https://i.stack.imgur.com/7GPmi.jpg). That is why we have the manual normal bias. However, since this scales with the shadow resolution it needs to be adjusted with the view distance just as much as shadow resolution changes with distance (that's what the /* TODO(fclem): Scale based on depth. */ is about).
I would not consider scaling it differently for each light but simply scaling it with the distance the same way the shadow LOD request does it. This would improve the situation quite a lot.

I am not convinced by this approach. I find it a bit too complicated for what it tries to solve. The aliasing is caused by 2 factor: - Scene depth buffer quantization: This can be fixed by added a small constant bias to the depth buffer value right after reading it (the `nextafter2`). This `nextafter` only works if depth buffer is using float32 format to have the same quantization bias as the format (otherwise you need to bias to 4.8e-7 or a bit higher. ref http://terathon.com/gdc07_lengyel.pdf slide 26 or search for `2.4e-7` or `4.8e-7` in the code (this should definitely be put into a utility somewhere) ). But do note that float32 depth format offer no benefit with conventional projection matrix (https://developer.nvidia.com/content/depth-precision-visualized). So while I'm ok with enable it (MacOS already enables it by default) I would like to see a comparison. I would suggest to just bias the shading point reconstruction instead of computing the error. This is done in many other passes the same way and it doesn't create much difference on the final image. - Shadowmap aliasing: This is already mitigated by the slope bias in the shadow fragment shader (`float f_depth = gl_FragCoord.z + fwidth(gl_FragCoord.z);`) and `u_depth += 2;`. But that doesn't solve projective or perspective aliasing (i.e: https://i.stack.imgur.com/7GPmi.jpg). That is why we have the manual normal bias. However, since this scales with the shadow resolution it needs to be adjusted with the view distance just as much as shadow resolution changes with distance (that's what the `/* TODO(fclem): Scale based on depth. */` is about). I would not consider scaling it differently for each light but simply scaling it with the distance the same way the shadow LOD request does it. This would improve the situation quite a lot.

source/blender/draw/engines/eevee_next/shaders/eevee_deferred_light_frag.glsl

						
				@ -27,3 +35,4 @@

				  vec3 V = drw_world_incident_vector(P);

				  float vPz = dot(drw_view_forward(), P) - dot(drw_view_forward(), drw_view_position());

				  g_depth_reconstructionn_bias = depth_reconstruction_precision(vP, depth);

Clément Foucault commented

2024-03-18 15:39:55 +01:00

g_depth_reconstructionn_bias typo, double n

`g_depth_reconstructionn_bias` typo, double `n`

This pull request has changes conflicting with the target branch.

source/blender/draw/engines/eevee_next/shaders/eevee_deferred_light_frag.glsl
source/blender/draw/engines/eevee_next/shaders/eevee_shadow_tracing_lib.glsl

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u pull-eevee-next-shadow-bias:pragma37-pull-eevee-next-shadow-bias

git checkout pragma37-pull-eevee-next-shadow-bias

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

EEVEE-Next: Add shadow bias based on depth reconstruction precision #111478

Checkout