Fix: Send `CurrentView`/ `PointAndSpotLightShadowPrimaryCamera` ’s world position via Immediates for gpu vis range culling by kfc35 · Pull Request #24197 · bevyengine/bevy

kfc35 · 2026-05-08T14:15:12Z

Objective

Also fixes Shadows disappear abruptly in visibility range example #23991
(Better?) Alternate to Fix: Use the auxiliary entity (associated camera view)’s position via ViewUniform for gpu visibility range culling #24133 . Decisions made can be mixed and matched between the two, just let me know
@JMS55 took a look and suggested using Immediates to avoid bloating ViewUniforms. There was also a concern about using RetainedViewEntity to pass information about what camera to use along. This is that alternate implementation path.

Solution

Rely on the CurrentView mechanism to get the current view’s world position, used when preprocessing directional light shadow views. This will be used for gpu vis range culling dir light shadows.
For Point/Spot shadow cameras (aka RootNonCameraViews), a marker component has been added to denote the camera to use: PointAndSpotLightShadowPrimaryCamera. There must only be one camera with this component. This camera’s position will be used for gpu vis range culling point/spotlight shadows.
- This component could probably be expanded later in functionality if desired, but I just went with what’s simplest.
- If we don’t want to have the PointAndSpotLightShadowPrimaryCamera component, we can revert
  ac87f8e — the previous logic just queried for extracted cameras and took the world position of the first one returned by the query.

Note

Copying this comment from @JMS55:
With this PR, we lose out on PreprocessingOnly support for WebGPU, until immediates are added to the spec and wgpu supports it.

Testing

cargo run --example visibility_range -- --no-cpu-culling works with a directional light as desired
I updated the light in the visibility_range example to be a Spot or PointLight like so:

SpotLight {
            intensity: 1_000_000_000.0, // lumens
            range: 110.0,
            color: Color::WHITE,
            shadow_maps_enabled: true,
            radius: 10.0,
            ..default()
        },
        Transform::from_rotation(Quat::from_array([
            0.6539259,
            -0.34646285,
            0.36505926,
            -0.5648683,
        ]))
        .with_translation(vec3(57.693, 34.334, -6.422)),

And added the PointAndSpotLightShadowPrimaryCamera to the Camera3d in the example, and then ran the example. The shadows of spot and point lights also look normal.

… culling

JMS55

Note: With this PR, we lose out on PreprocessingOnly support for WebGPU, until immediates are added to the spec and wgpu supports it.

kfc35 · 2026-05-09T16:40:26Z

I will add a marker component for the camera to use for gpu vis culling spot and point lights shadows, that should be ready to review in a few hours

github-actions · 2026-05-09T21:32:22Z

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-24197

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

beicause · 2026-05-11T23:28:28Z

I'm not sure if PointAndSpotLightShadowPrimaryCamera is correct. The original implementation #12916 says:

Cascaded shadow maps show the HLOD level of the view they're associated with. Point light and spot light shadow maps, which have no CSMs, display all HLOD levels that are visible in any view.

kfc35 · 2026-05-12T00:29:30Z

Thanks for finding that @beicause

My thoughts given that information:

I agree that the new component I made is probably not correct then with what is originally intended. I’ll revert it after finalizing what to do.
It sounds like I should be passing in a list of camera world positions to the preprocessor shader code for the point and spot light shadows case then, to check if the shadow is within range of any extracted camera view. If it’s not in range of any camera, it should be culled.
Does this information change whether we prefer to put this information in Immediates vs a Uniform? Immediates are 64 bytes guaranteed at minimum according to the spec. That’s 4 camera positions max (4 x vec3, one u32 is already taken for occlusion culling). I assume I need to set a max number on the list anyway (Since you can’t have variable length arrays in wgsl? I’m not familiar but that’s the impression I’ve been getting). In this case, for this Immediates PR specifically, I’m thinking that I’ll send max 4 camera view world positions to be tested for gpu visibility range culling for point and spot light shadow views.

I’ll get to implementing this logic in about 18 hours unless there are any objections (and may work on it sooner if I get some timely affirmations)

beicause · 2026-05-12T00:58:22Z

IMO we should consider reverting #23115. This may introduce extra overhead on GPU and regression on WebGPU before immediates is supported.

~~And I saw lights are still checking visibility range on CPU:~~ Edit: I misread it

bevy/crates/bevy_light/src/lib.rs

Line 429 in 2002a18

!visible_entity_ranges.entity_is_in_range_of_view(entity, *view)

JMS55 · 2026-05-12T01:11:31Z

It sounds like I should be passing in a list of camera world positions to the preprocessor shader code for the point and spot light shadows case then, to check if the shadow is within range of any extracted camera view. If it’s not in range of any camera, it should be culled.

Seems seems quite expensive 😬 . Also I don't know what that would even look like, I feel like the shadows would be all weird.

kfc35 · 2026-05-12T01:18:51Z

Seems seems quite expensive 😬 . Also I don't know what that would even look like, I feel like the shadows would be all weird.

I agree that it would look weird 😢 I’m not sure how to interpret the original intent for visibility ranges in this context then...

IMO we should consider reverting #23115.

Reverting is definitely an option at this point… NoCpuCulling could use some more time to develop for 0.20

I’ll just keep this PR as is for now. Thanks for the quick input!

stuartparmenter · 2026-05-12T18:18:23Z

+            } else {
+                // There is no single designated primary camera to use for vis range culling of the shadows.
+                // Do not render them.
+                continue;


Doesn't this continue skip the entire preprocess dispatch for the view?

For the PointLight/SpotLight shadow view, yes

But I would consider that a user configuration error at this point, so the shadows just wouldn’t be rendered.

(However we do not need to continue this conversation because I will close this PR! It seems the consensus is converging on reverting the offending PR)

…CpuCulling` is specified. (#23115) (#24252) This reverts commit ebfbc3f. # Objective - A third PR that fixes #23991 ## Solution - Reverts the commit that causes the issue. - This can be considered if the other solutions (#24197 & #24133) I’ve put up are not satisfactory, and we need more time to come up with a good solution (i.e. post 0.19 rc) that: - Implements PointLight and SpotLight shadow view behavior for gpu vis range culling that is agreed upon and looks good - Finalizes an appropriate way of sending this camera view world position data to the gpu (Immediates vs ViewUniformBuffer or other Uniform) - Note: Meshes that are tagged with `NoCpuCulling` will **not** undergo cpu visibility range culling. That was implemented in a separate PR: https://github.com/bevyengine/bevy/pull/23107/changes#diff-fec33d34072b7b4be336571ceecf2c10fc9bafd8366c1b29531089b7e3ef621cL745-L748. If you want to use `VisibilityRange` culling, you will still need to have CPU culling enabled for the mesh. ## Testing - `cargo run --example visibility_range` works normal now. - Tested some other 3d examples make sure the pipeline is ok — atmosphere, pccm

Right now, visibility ranges are always resolved relative to the view. This is incorrect for shadow maps in two ways: 1. Visibility ranges for meshes in directional light shadow maps should be resolved relative to the camera that the cascades are associated with. 2. Visibility ranges for meshes in point and spot light shadow maps should be resolved relative to *some* camera. To properly solve (2), this commit introduces the notion of a *primary camera*. The primary camera is the one that visibility ranges are relative to, when rendering views not associated with any camera. Point and spot light shadow maps are currently not associated with a camera, and therefore we need this extra notion in order to properly evaluate visibility ranges. (As a follow-up, we should introduce the notion of *own shadow maps*, which will allow each camera to have separate shadow maps for point and spot lights. That feature is however out of scope for *this* patch, which simply seeks to make the existing semantics consistent.) A new component, `PrimaryCamera`, has been added, which allows the developer to customize which camera is primary. In the absence of this component, this PR implements a simple heuristic to determine the primary camera: to prefer cameras that render to a window. This heuristic should suffice in the vast majority of cases, so developers will rarely have to manually use the `PrimaryCamera` component. A new field, `lod_view_world_position`, has been added to `View` to supply the position of the primary camera to the GPU. This is much simpler than introducing a new uniform or using immediates, as bevyengine#24197 tried to do. This commit is the proper fix for bevyengine#23991. PR bevyengine#24252 attempted to fix this problem by reverting bevyengine#23115. However, this didn't actually fix the issue, because the semantics were still inconsistent. This commit constitutes the correct fix for the issue. I verified that, after un-reverting bevyengine#23115 on top of this patch and modifying it to use the new `lod_view_world_position`, that the issue reported in bevyengine#23991 disappears.

Right now, visibility ranges are always resolved relative to the view. This is incorrect for shadow maps in two ways: 1. Visibility ranges for meshes in directional light shadow maps should be resolved relative to the camera that the cascades are associated with. 2. Visibility ranges for meshes in point and spot light shadow maps should be resolved relative to *some* camera. To properly solve (2), this commit introduces the notion of a *shadow LOD origin*. The shadow LOD origin is the point that visibility ranges are relative to, when rendering views not associated with any camera. Point and spot light shadow maps are currently not associated with a camera, and therefore we need this extra notion in order to properly evaluate visibility ranges. (As a follow-up, we should introduce the notion of *own shadow maps*, which will allow each camera to have separate shadow maps for point and spot lights. That feature is however out of scope for *this* patch, which simply seeks to make the existing semantics consistent.) A new component, `ShadowLodOrigin`, has been added, which allows the developer to customize the shadow LOD origin. In the absence of this component, this PR implements a simple heuristic to determine the shadow LOD origin: to prefer an origin that coincides with cameras that render to a window. This heuristic should suffice in the vast majority of cases, so developers will rarely have to manually use the `ShadowLodOrigin` component. A new field, `lod_view_world_position`, has been added to `View` to supply the position of the shadow LOD origin to the GPU. This is much simpler than introducing a new uniform or using immediates, as bevyengine#24197 tried to do. This commit is the proper fix for bevyengine#23991. PR bevyengine#24252 attempted to fix this problem by reverting bevyengine#23115. However, this didn't actually fix the issue, because the semantics were still inconsistent. This commit constitutes the correct fix for the issue. I verified that, after un-reverting bevyengine#23115 on top of this patch and modifying it to use the new `lod_view_world_position`, that the issue reported in bevyengine#23991 disappears.

Right now, visibility ranges are always resolved relative to the view. This is incorrect for shadow maps in two ways: 1. Visibility ranges for meshes in directional light shadow maps should be resolved relative to the camera that the cascades are associated with. 2. Visibility ranges for meshes in point and spot light shadow maps should be resolved relative to *some* camera. To properly solve (2), this commit introduces the notion of a *shadow LOD origin*. The shadow LOD origin is the point that visibility ranges are relative to, when rendering views not associated with any camera. Point and spot light shadow maps are currently not associated with a camera, and therefore we need this extra notion in order to properly evaluate visibility ranges. (As a follow-up, we should introduce the notion of *own shadow maps*, which will allow each camera to have separate shadow maps for point and spot lights. That feature is however out of scope for *this* patch, which simply seeks to make the existing semantics consistent.) A new component, `ShadowLodOrigin`, has been added, which allows the developer to customize the shadow LOD origin. In the absence of this component, this PR implements a simple heuristic to determine the shadow LOD origin: to prefer an origin that coincides with cameras that render to a window. This heuristic should suffice in the vast majority of cases, so developers will rarely have to manually use the `ShadowLodOrigin` component. A new field, `lod_view_world_position`, has been added to `View` to supply the position of the shadow LOD origin to the GPU. This is much simpler than introducing a new uniform or using immediates, as #24197 tried to do. This commit is the proper fix for #23991. PR #24252 attempted to fix this problem by reverting #23115. However, this didn't actually fix the issue, because the semantics were still inconsistent. This commit constitutes the correct fix for the issue. I verified that, after un-reverting #23115 on top of this patch and modifying it to use the new `lod_view_world_position`, that the issue reported in #23991 disappears.

Fix: Send CurrentView world position via immediates for gpu vis range…

8ea52c8

… culling

kfc35 requested review from JMS55 and pcwalton May 8, 2026 14:15

kfc35 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 8, 2026

github-project-automation Bot added this to Rendering May 8, 2026

github-project-automation Bot moved this to Needs SME Triage in Rendering May 8, 2026

kfc35 mentioned this pull request May 8, 2026

Fix: Use the auxiliary entity (associated camera view)’s position via ViewUniform for gpu visibility range culling #24133

Closed

kfc35 added this to the 0.19 milestone May 8, 2026

Zeophlite approved these changes May 8, 2026

View reviewed changes

kfc35 added 3 commits May 8, 2026 10:33

remove changes to late gpu preprocess - it only does occlusion culling?

37fac2e

move frustum culling out of PreprocessOnly

f3b3681

fmt

147ba07

JMS55 suggested changes May 8, 2026

View reviewed changes

Comment thread crates/bevy_pbr/src/render/gpu_preprocess.rs

Comment thread crates/bevy_render/src/batching/gpu_preprocessing.rs

Comment thread crates/bevy_pbr/src/render/gpu_preprocess.rs

kfc35 added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 8, 2026

kfc35 added 4 commits May 8, 2026 19:39

docs: state reason behind logic for directional shadow cascades

7f7d031

Add back code that I thought I had to remove heh

2f8e21a

PreprocessingOnly requires Immediates

31dc456

add ifndef just in case...

edc1df0

kfc35 added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 9, 2026

kfc35 requested a review from JMS55 May 9, 2026 00:15

kfc35 added 2 commits May 8, 2026 21:30

remove unnecessary comment

c6f162b

more comments in wgsl

f1fc851

kfc35 force-pushed the 23991_gpu_culling_immediates branch from 34ad77f to 3a800cd Compare May 9, 2026 21:31

kfc35 force-pushed the 23991_gpu_culling_immediates branch 3 times, most recently from a767075 to 3f8e38b Compare May 9, 2026 21:53

Add marker component for point and spot light shadow primary camera

ac87f8e

kfc35 force-pushed the 23991_gpu_culling_immediates branch from 3f8e38b to ac87f8e Compare May 9, 2026 21:59

kfc35 changed the title ~~Fix: Send CurrentView/ an ExtractedCamera’s world position via immediates for gpu vis range culling~~ Fix: Send CurrentView/ PointAndSpotLightShadowPrimaryCamera ’s world position via Immediates for gpu vis range culling May 10, 2026

kfc35 mentioned this pull request May 12, 2026

Revert "Move visibility range checking from the CPU to the GPU if NoCpuCulling is specified. (#23115) #24252

Merged

stuartparmenter reviewed May 12, 2026

View reviewed changes

Zeophlite modified the milestones: 0.19, 0.20 May 13, 2026

kfc35 removed the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label May 13, 2026

kfc35 added the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label May 13, 2026

pcwalton mentioned this pull request May 14, 2026

Correctly handle visibility ranges in shadow maps. #24289

Merged

kfc35 closed this May 15, 2026

github-project-automation Bot moved this from Needs SME Triage to Done in Rendering May 15, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix: Send `CurrentView`/ `PointAndSpotLightShadowPrimaryCamera` ’s world position via Immediates for gpu vis range culling#24197

Fix: Send `CurrentView`/ `PointAndSpotLightShadowPrimaryCamera` ’s world position via Immediates for gpu vis range culling#24197
kfc35 wants to merge 11 commits into
bevyengine:mainfrom
kfc35:23991_gpu_culling_immediates

kfc35 commented May 8, 2026 •

edited

Loading

Uh oh!

JMS55 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kfc35 commented May 9, 2026

Uh oh!

github-actions Bot commented May 9, 2026

Uh oh!

beicause commented May 11, 2026

Uh oh!

kfc35 commented May 12, 2026

Uh oh!

beicause commented May 12, 2026 •

edited

Loading

Uh oh!

JMS55 commented May 12, 2026 •

edited

Loading

Uh oh!

kfc35 commented May 12, 2026

Uh oh!

stuartparmenter May 12, 2026

Uh oh!

kfc35 May 13, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

kfc35 commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Testing

Uh oh!

JMS55 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kfc35 commented May 9, 2026

Uh oh!

github-actions Bot commented May 9, 2026

Uh oh!

beicause commented May 11, 2026

Uh oh!

kfc35 commented May 12, 2026

Uh oh!

beicause commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JMS55 commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kfc35 commented May 12, 2026

Uh oh!

stuartparmenter May 12, 2026

Choose a reason for hiding this comment

Uh oh!

kfc35 May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kfc35 commented May 8, 2026 •

edited

Loading

beicause commented May 12, 2026 •

edited

Loading

JMS55 commented May 12, 2026 •

edited

Loading

kfc35 May 13, 2026 •

edited

Loading