Receiver plane depth bias #10188

superdump · 2023-10-19T16:16:07Z

Objective

Avoid self-shadowing with larger offsets from the fragment position in light space when doing PCF

Solution

The problem is that the same light space depth at the fragment position is used to compare to offset positions in the shadow map when using the new PCF techniques. As these offsets get further away from the centre of the filter kernel, and if the surface is at a steep angle, the biasing of the fragment position is insufficient to account for the difference in depth at the offset position. (The PDF below has diagrams.)
Use Receiver Plane Depth Bias (slides 36-40) but using TheRealMJP's implementation

Screenshots

Use the shadow_biases example and moving the light to 5,1,4. Use the default biases (press R).

Before

Hardware 2x2:

This looks fine because the sample positions used by hardware 2x2 are close enough to the fragment position.

Castano13 (The Witness):

Note the bands of self-shadowing on the right side on the ground plane.

Jimenez14 (Call of Duty: Advanced Warfare):

The bands are present here too.

After

Hardware 2x2:

It is the same, but here for completeness.

Castano13:

Jimenez14:

Changelog

Fixed: Prevent self-shadowing with directional lights where the receiving mesh plane is at a steep angle to the light by using a Receiver Plane Depth Bias.

…ground plane

robtfm · 2023-10-19T18:06:03Z

crates/bevy_pbr/src/render/shadow_sampling.wgsl

-    sum += uw1 * vw2 * sample_shadow_map_hardware(base_uv + (vec2(u1, v2) * inv_shadow_map_size), depth, array_index);
-    sum += uw2 * vw2 * sample_shadow_map_hardware(base_uv + (vec2(u2, v2) * inv_shadow_map_size), depth, array_index);
+    let sample_offset_u0_v0 = (vec2(u0, v0) * inv_shadow_map_size);
+    sum += uw0 * vw0 * sample_shadow_map_hardware(base_uv + sample_offset_u0_v0, depth + dot(sample_offset_u0_v0, receiver_plane_depth_bias), array_index);


can we use a sample_shadow_map_offset(uv, offset, depth, bias, index) function?

robtfm · 2023-10-19T18:08:52Z

crates/bevy_pbr/src/render/shadow_sampling.wgsl

-    sum += uw2 * vw2 * sample_shadow_map_hardware(base_uv + (vec2(u2, v2) * inv_shadow_map_size), depth, array_index);
+    let sample_offset_u0_v0 = (vec2(u0, v0) * inv_shadow_map_size);
+    sum += uw0 * vw0 * sample_shadow_map_hardware(base_uv + sample_offset_u0_v0, depth + dot(sample_offset_u0_v0, receiver_plane_depth_bias), array_index);
+    let sample_offset_u1_v0 = (vec2(u1, v0) * inv_shadow_map_size);


not directly related but we could make this a loop by putting uw, u, vw, v into vec3s (vecs don't have the constant index requirement apparently).

robtfm · 2023-10-19T18:14:18Z

~~also please add the receiver bias to the demo params~~ oops disregard, it has no user input

robtfm · 2023-10-19T18:19:10Z

i'm getting black lines at cascade boundaries with castano and jimenez (not present on main)

robtfm · 2023-10-19T19:00:29Z

i'm getting black lines at cascade boundaries with castano and jimenez (not present on main)

i can remove the black lines by always requesting the next cascade texture sample:

--- a/crates/bevy_pbr/src/render/shadows.wgsl
+++ b/crates/bevy_pbr/src/render/shadows.wgsl
@@ -156,11 +156,11 @@ fn fetch_directional_shadow(light_id: u32, frag_position: vec4<f32>, surface_nor
 
     // Blend with the next cascade, if there is one.
     let next_cascade_index = cascade_index + 1u;
+    let next_shadow = sample_directional_cascade(light_id, min(next_cascade_index, (*light).num_cascades - 1u), frag_position, surface_normal);
     if (next_cascade_index < (*light).num_cascades) {
         let this_far_bound = (*light).cascades[cascade_index].far_bound;
         let next_near_bound = (1.0 - (*light).cascades_overlap_proportion) * this_far_bound;
         if (-view_z >= next_near_bound) {
-            let next_shadow = sample_directional_cascade(light_id, next_cascade_index, frag_position, surface_normal);
             shadow = mix(shadow, next_shadow, (-view_z - next_near_bound) / (this_far_bound - next_near_bound));
         }
     }

this makes sense - dxdy can't be used outside of uniform control flow (link), because the texture won't always be available to the neighbouring pixels, and the second cascade access (for blending) is not-uniform.

robtfm · 2023-10-19T19:22:02Z

but this still probably isn't enough (although it seems to work here), since dxdy will be based off different cascades in the neighbouring pixels. seems like the only way to do this reliably is to sample every cascade upfront in a uniform way, then pass the dxdy down...

robtfm · 2023-10-19T20:31:15Z

diff attached using dpdx/y(fragment_position) instead, which is uniform. i only did directional shadows but spotlights could follow the same approach if we do need to go down this road.
diff.txt

cart · 2023-10-27T01:39:27Z

Making a note that if this isn't fixed ready in time for 0.12, we should do the "quick fix" of raising the normal_bias to 1.8 and lowering the depth_bias to 0.01.
(in the interest of having artifact free shadows by default)

JMS55 · 2023-10-27T04:26:50Z

Better imo to merge @robtfm's patch, and accept the lower perf. We need to validate if spotlights work with that though.

rparrett · 2023-10-27T22:35:40Z

Just want to make this info more visible on GitHub. The latest version of Rob's patch is here:

superdump#40

spotlight.rs and shadow_biases still seem to generally work. I am not able to reproduce the black lines thing either before or after that patch though to test.

cart · 2023-11-03T01:40:16Z

One note: this does add some "self shadowing" artifacts with equivalent settings in 3d_scene:

Main

This PR

@robtfm's Fix

Notice the new artifacts at the base of the cube.

cart · 2023-11-03T01:41:24Z

The scene: https://gist.github.com/cart/03af0c686b3701d5e998ea7e672c32c7

cart · 2023-11-03T01:46:02Z

And here it is with the "sweet spot" normal bias setting of 1.8 (bumped up from 0.6).

Godot has a default normal bias of 2. I'm thinking we shouldn't rush into this impl and just bump our defaults for Bevy 0.12.

cart · 2023-11-03T01:55:57Z

#10346

# Objective Bevy's default bias values for directional and spot lights currently cause significant artifacts. We should fix that so shadows look good by default! This is a less controversial/invasive alternative to #10188, which might enable us to keep the default bias value low, but also has its own sets of concerns and caveats that make it a risky choice for Bevy 0.12. ## Solution Bump the default normal bias from `0.6` to `1.8`. There is precedent for values in this general area as Godot has a default normal bias of `2.0`. ### Before ![image](https://github.com/superdump/bevy/assets/2694663/a5828011-33fc-4427-90ed-f093d7389053) ### After ![image](https://github.com/bevyengine/bevy/assets/2694663/0f2b16b0-c116-41ab-9886-1ace9e00efd6) ## Migration Guide The default `shadow_normal_bias` value for `DirectionalLight` and `SpotLight` has changed to accommodate artifacts introduced with the new shadow PCF changes. It is unlikely (especially given the new PCF shadow behaviors with these values), but you might need to manually tweak this value if your scene requires a lower bias and it relied on the previous default value.

Fix branding inconsistencies don't Implement `Display` for `Val` (bevyengine#10345) - Revert bevyengine#10296 - Avoid implementing `Display` without a justification - `Display` implementation is a guarantee without a direct use, takes additional time to compile and require work to maintain - `Debug`, `Reflect` or `Serialize` should cover all needs Combine visibility queries in check_visibility_system (bevyengine#10196) Alternative to bevyengine#7310 Implemented the suggestion from bevyengine#7310 (comment) I am guessing that these were originally split as an optimization, but I am not sure since I believe the original author of the code is the one speculating about combining them up there. I ran three benchmarks to compare main, this PR, and the approach from ([updated](https://github.com/rparrett/bevy/commits/rebased-parallel-check-visibility) to the same commit on main). This seems to perform slightly better than main in scenarios where most entities have AABBs, and a bit worse when they don't (`many_lights`). That seems to make sense to me. Either way, the difference is ~-20 microseconds in the more common scenarios or ~+100 microseconds in the less common scenario. I would speculate that this might perform **very slightly** worse in single-threaded scenarios. Benches were run in release mode for 2000 frames while capturing a trace with tracy. | bench | commit | check_visibility_system mean μs | | -- | -- | -- | | many_cubes | main | 929.5 | | many_cubes | this | 914.0 | | many_cubes | 7310 | 1003.5 | | | | | many_foxes | main | 191.6 | | many_foxes | this | 173.2 | | many_foxes | 7310 | 167.9 | | | | | many_lights | main | 619.3 | | many_lights | this | 703.7 | | many_lights | 7310 | 842.5 | Technically this behaves slightly differently -- prior to this PR, view visibility was determined even for entities without `GlobalTransform`. I don't think this has any practical impact though. IMO, I don't think we need to do this. But I opened a PR because it seemed like the handiest way to share the code / benchmarks. I have done some rudimentary testing with the examples above, but I can do some screenshot diffing if it seems like we want to do this. Make VERTEX_COLORS usable in prepass shader, if available (bevyengine#10341) I was working with forward rendering prepass fragment shaders and ran into an issue of not being able to access vertex colors in the prepass. I was able to access vertex colors in regular fragment shaders as well as in deferred shaders. It seems like this `if` was nested unintentionally as moving it outside of the `deferred` block works. --- Enable vertex colors in forward rendering prepass fragment shaders allow DeferredPrepass to work without other prepass markers (bevyengine#10223) fix crash / misbehaviour when `DeferredPrepass` is used without `DepthPrepass`. - Deferred lighting requires the depth prepass texture to be present, so that the depth texture is available for binding. without it the deferred lighting pass will use 0 for depth of all meshes. - When `DeferredPrepass` is used without other prepass markers, and with any materials that use `OpaqueRenderMode::Forward`, those entities will try to queue to the `Opaque3dPrepass` render phase, which doesn't exist, causing a crash. - check if the prepass phases exist before queueing - generate prepass textures if `Opaque3dDeferred` is present - add a note to the DeferredPrepass marker to note that DepthPrepass is also required by the default deferred lighting pass - also changed some `With<T>.is_some()`s to `Has<T>`s UI batching Fix (bevyengine#9610) Reimplement bevyengine#8793 on top of the recent rendering changes. The batch creation logic is quite convoluted, but I tested it on enough examples to convince myself that it works. The initial value of `batch_image_handle` is changed from `HandleId::Id(Uuid::nil(), u64::MAX)` to `DEFAULT_IMAGE_HANDLE.id()`, which allowed me to make the if-block simpler I think. The default image from `DEFAULT_IMAGE_HANDLE` is always inserted into `UiImageBindGroups` even if it's not used. I tried to add a check so that it would be only inserted when there is only one batch using the default image but this crashed. --- `prepare_uinodes` * Changed the initial value of `batch_image_handle` to `DEFAULT_IMAGE_HANDLE.id()`. * The default image is added to the UI image bind groups before assembling the batches. * A new `UiBatch` isn't created when the next `ExtractedUiNode`s image is set to `DEFAULT_IMAGE_HANDLE` (unless it is the first item in the UI phase items list). Increase default normal bias to avoid common artifacts (bevyengine#10346) Bevy's default bias values for directional and spot lights currently cause significant artifacts. We should fix that so shadows look good by default! This is a less controversial/invasive alternative to bevyengine#10188, which might enable us to keep the default bias value low, but also has its own sets of concerns and caveats that make it a risky choice for Bevy 0.12. Bump the default normal bias from `0.6` to `1.8`. There is precedent for values in this general area as Godot has a default normal bias of `2.0`. ![image](https://github.com/superdump/bevy/assets/2694663/a5828011-33fc-4427-90ed-f093d7389053) ![image](https://github.com/bevyengine/bevy/assets/2694663/0f2b16b0-c116-41ab-9886-1ace9e00efd6) The default `shadow_normal_bias` value for `DirectionalLight` and `SpotLight` has changed to accommodate artifacts introduced with the new shadow PCF changes. It is unlikely (especially given the new PCF shadow behaviors with these values), but you might need to manually tweak this value if your scene requires a lower bias and it relied on the previous default value. Make `DirectionalLight` `Cascades` computation generic over `CameraProjection` (bevyengine#9226) Fixes bevyengine#9077 (see this issue for motivations) Implement 1 and 2 of the "How to fix it" section of bevyengine#9077 `update_directional_light_cascades` is split into `clear_directional_light_cascades` and a generic `build_directional_light_cascades`, to clear once and potentially insert many times. --- `DirectionalLight`'s computation is now generic over `CameraProjection` and can work with custom camera projections. If you have a component `MyCustomProjection` that implements `CameraProjection`: - You need to implement a new required associated method, `get_frustum_corners`, returning an array of the corners of a subset of the frustum with given `z_near` and `z_far`, in local camera space. - You can now add the `build_directional_light_cascades::<MyCustomProjection>` system in `SimulationLightSystems::UpdateDirectionalLightCascades` after `clear_directional_light_cascades` for your projection to work with directional lights. --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com> Update default `ClearColor` to better match Bevy's branding (bevyengine#10339) - Changes the default clear color to match the code block color on Bevy's website. - Changed the clear color, updated text in examples to ensure adequate contrast. Inconsistent usage of white text color set to use the default color instead, which is already white. - Additionally, updated the `3d_scene` example to make it look a bit better, and use bevy's branding colors. ![image](https://github.com/bevyengine/bevy/assets/2632925/540a22c0-826c-4c33-89aa-34905e3e313a) Corrected incorrect doc comment on read_asset_bytes (bevyengine#10352) Fixes bevyengine#10302 - Removed the incorrect comment. Allow AccessKit to react to WindowEvents before they reach the engine (bevyengine#10356) - Adopt bevyengine#10239 to get it in time for the release - Fix accessibility on macOS and linux - call `on_event` from AcccessKit adapter on winit events --------- Co-authored-by: Nolan Darilek <nolan@thewordnerd.info> Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Fix typo in window.rs (bevyengine#10358) Fixes a small typo in `bevy_window/src/window.rs` Change `Should be used instead 'scale_factor' when set.` to `Should be used instead of 'scale_factor' when set.` Add UI Materials (bevyengine#9506) - Add Ui Materials so that UI can render more complex and animated widgets. - Fixes bevyengine#5607 - Create a UiMaterial trait for specifying a Shader Asset and Bind Group Layout/Data. - Create a pipeline for rendering these Materials inside the Ui layout/tree. - Create a MaterialNodeBundle for simple spawning. - Created a `UiMaterial` trait for specifying a Shader asset and Bind Group. - Created a `UiMaterialPipeline` for rendering said Materials. - Added Example [`ui_material` ](https://github.com/MarkusTheOrt/bevy/blob/ui_material/examples/ui/ui_material.rs) for example usage. - Created [`UiVertexOutput`](https://github.com/MarkusTheOrt/bevy/blob/ui_material/crates/bevy_ui/src/render/ui_vertex_output.wgsl) export as VertexData for shaders. - Created [`material_ui`](https://github.com/MarkusTheOrt/bevy/blob/ui_material/crates/bevy_ui/src/render/ui_material.wgsl) shader as default for both Vertex and Fragment shaders. --------- Co-authored-by: ickshonpe <david.curthoys@googlemail.com> Co-authored-by: François <mockersf@gmail.com> support file operations in single threaded context (bevyengine#10312) - Fixes bevyengine#10209 - Assets should work in single threaded - In single threaded mode, don't use `async_fs` but fallback on `std::fs` with a thin layer to mimic the async API - file `file_asset.rs` is the async imps from `mod.rs` - file `sync_file_asset.rs` is the same with `async_fs` APIs replaced by `std::fs` - which module is used depends on the `multi-threaded` feature --------- Co-authored-by: Carter Anderson <mcanders1@gmail.com> Fix gizmo crash when prepass enabled (bevyengine#10360) - Fix gizmo crash when prepass enabled - Add the prepass to the view key Fixes: bevyengine#10347

) # Objective Bevy's default bias values for directional and spot lights currently cause significant artifacts. We should fix that so shadows look good by default! This is a less controversial/invasive alternative to bevyengine#10188, which might enable us to keep the default bias value low, but also has its own sets of concerns and caveats that make it a risky choice for Bevy 0.12. ## Solution Bump the default normal bias from `0.6` to `1.8`. There is precedent for values in this general area as Godot has a default normal bias of `2.0`. ### Before ![image](https://github.com/superdump/bevy/assets/2694663/a5828011-33fc-4427-90ed-f093d7389053) ### After ![image](https://github.com/bevyengine/bevy/assets/2694663/0f2b16b0-c116-41ab-9886-1ace9e00efd6) ## Migration Guide The default `shadow_normal_bias` value for `DirectionalLight` and `SpotLight` has changed to accommodate artifacts introduced with the new shadow PCF changes. It is unlikely (especially given the new PCF shadow behaviors with these values), but you might need to manually tweak this value if your scene requires a lower bias and it relied on the previous default value.

superdump and others added 4 commits October 19, 2023 16:02

shadow_biases: Support moving the light position and resetting biases

a8782ac

Simplifications

72e5f9c

Use transparent background for text and every other sphere is on the …

4393d36

…ground plane

Implement Receiver Plane Depth Bias [Isidoro06]

d791872

JMS55 approved these changes Oct 19, 2023

View reviewed changes

alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Oct 19, 2023

robtfm reviewed Oct 19, 2023

View reviewed changes

superdump added this to the 0.12 milestone Oct 21, 2023

cart mentioned this pull request Nov 3, 2023

Increase default normal bias to avoid common artifacts #10346

Merged

cart removed this from the 0.12 milestone Nov 3, 2023

superdump mentioned this pull request Nov 5, 2023

Percentage Closer Filtering for shadows #3628

Closed

JMS55 added this to the 0.13 milestone Nov 23, 2023

JMS55 mentioned this pull request Nov 25, 2023

Implement minimal reflection probes. #10057

Merged

alice-i-cecile removed this from the 0.13 milestone Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Receiver plane depth bias #10188

Receiver plane depth bias #10188

superdump commented Oct 19, 2023 •

edited

Loading

robtfm Oct 19, 2023

robtfm Oct 19, 2023

robtfm commented Oct 19, 2023 •

edited

Loading

robtfm commented Oct 19, 2023

robtfm commented Oct 19, 2023

robtfm commented Oct 19, 2023

robtfm commented Oct 19, 2023

cart commented Oct 27, 2023 •

edited

Loading

JMS55 commented Oct 27, 2023

rparrett commented Oct 27, 2023

cart commented Nov 3, 2023

cart commented Nov 3, 2023

cart commented Nov 3, 2023 •

edited

Loading

cart commented Nov 3, 2023

Receiver plane depth bias #10188

Are you sure you want to change the base?

Receiver plane depth bias #10188

Conversation

superdump commented Oct 19, 2023 • edited Loading

Objective

Solution

Screenshots

Before

After

Changelog

robtfm Oct 19, 2023

Choose a reason for hiding this comment

robtfm Oct 19, 2023

Choose a reason for hiding this comment

robtfm commented Oct 19, 2023 • edited Loading

robtfm commented Oct 19, 2023

robtfm commented Oct 19, 2023

robtfm commented Oct 19, 2023

robtfm commented Oct 19, 2023

cart commented Oct 27, 2023 • edited Loading

JMS55 commented Oct 27, 2023

rparrett commented Oct 27, 2023

cart commented Nov 3, 2023

Main

This PR

@robtfm's Fix

cart commented Nov 3, 2023

cart commented Nov 3, 2023 • edited Loading

cart commented Nov 3, 2023

superdump commented Oct 19, 2023 •

edited

Loading

robtfm commented Oct 19, 2023 •

edited

Loading

cart commented Oct 27, 2023 •

edited

Loading

cart commented Nov 3, 2023 •

edited

Loading