Fix occasional flickering by ensuring that each invocation of the indirect parameters building shader only writes to the batch range it's supposed to. by pcwalton · Pull Request #24971 · bevyengine/bevy

pcwalton · 2026-07-13T03:48:26Z

Since PR #23662, the shadows in the 3d_shapes demo occasionally flicker for a frame or two. This typically happens only when (a) there are many views and (b) objects move from one view to another. 3d_shapes fulfills requirement (a), as it contains a point light, which extracts to 6 shadow views; it also fulfills requirement (b), as the rotation of the objects changes their OBBs, which pushes them into and out of view of the different cubemap sides.

Correct rendering:

Incorrect rendering:

The cause is quite subtle. For a long time, the indirect parameters building shader (build_indirect_params), which we dispatch right before rendering each render phase of each view, has been preparing the entire shared buffer of indirect draw parameters. This is wasteful, as there's no need to prepare the entire shared buffer of indirect draw parameters for all phases of all views when we're only about to render a portion of that buffer. Now up until #23662 landed, this was harmless to correctness, as the CPU prepared the indirect parameters metadata buffer that the indirect parameters building shader used before any of the GPU shaders ran. So the indirect parameters metadata buffer was always fully up to date, making the indirect parameters building correct, if inefficient. But, after #23662, which moved the logic that populates the indirect parameters metadata buffer from the CPU to the GPU, only the portion of the indirect parameters metadata corresponding to the batch ranges that are about to be drawn has been filled in by the time the indirect parameters building shader runs. Thus, when the indirect parameters building shader runs for mid-frame views and phases, some of the indirect parameters metadata buffer corresponds to data from this frame, and some of the indirect parameters metadata buffer corresponds to data from the previous frame. Because the indirect parameters metadata describes allocations within the indirect parameters buffer that must be nonoverlapping, this violates invariants, causing pieces of indirect parameter data to crash into and corrupt one another.

This commit fixes the issue by restricting the build_indirect_params shader to only work on the range of batches it's supposed to. This requires some extra CPU bookkeeping in order to pack the ranges of batch indices into a dynamic uniform buffer for that shader to read, as well as to ensure the batch range makes its way to the render graph system so that the correct number of workgroups can be dispatched. Fortunately, this fix is not only a correctness fix but also a performance improvement: it means that the GPU writes to each set of indirect draw parameters only once per frame instead of once per phase per view.

indirect parameters building shader only writes to the batch range it's supposed to. Since PR bevyengine#23662, the shadows in the `3d_shapes` demo occasionally flicker for a frame or two. This typically happens only when (a) there are many views and (b) objects move from one view to another. `3d_shapes` fulfills requirement (a), as it contains a point light, which extracts to 6 shadow views; it also fulfills requirement (b), as the rotation of the objects changes their OBBs, which pushes them into and out of view of the different cubemap sides. The cause is quite subtle. For a long time, the indirect parameters building shader (`build_indirect_params`), which we dispatch right before rendering each render phase of each view, has been preparing the *entire* shared buffer of indirect draw parameters. This is wasteful, as there's no need to prepare the entire shared buffer of indirect draw parameters for *all* phases of all views when we're only about to render a portion of that buffer. Now up until bevyengine#23662 landed, this was harmless to correctness, as the CPU prepared the *indirect parameters metadata* buffer that the indirect parameters building shader used before any of the GPU shaders ran. So the indirect parameters metadata buffer was always fully up to date, making the indirect parameters building correct, if inefficient. But, after bevyengine#23662, which moved the logic that populates the indirect parameters metadata buffer from the CPU to the GPU, only the portion of the indirect parameters metadata *corresponding to the batch ranges that are about to be drawn* has been filled in by the time the indirect parameters building shader runs. Thus, when the indirect parameters building shader runs for mid-frame views and phases, some of the indirect parameters metadata buffer corresponds to data from this frame, and some of the indirect parameters metadata buffer corresponds to data from the *previous* frame. Because the indirect parameters metadata describes allocations within the indirect parameters buffer that must be nonoverlapping, this violates invariants, causing pieces of indirect parameter data to crash into and corrupt one another. This commit fixes the issue by restricting the `build_indirect_params` shader to only work on the range of batches it's supposed to. This requires some extra CPU bookkeeping in order to pack the ranges of batch indices into a dynamic uniform buffer for that shader to read, as well as to ensure the batch range makes its way to the render graph system so that the correct number of workgroups can be dispatched. Fortunately, this fix is not only a correctness fix but also a performance improvement: it means that the GPU writes to each set of indirect draw parameters only once per frame instead of once per phase per view.

pcwalton · 2026-07-13T03:50:16Z

Please note that this doesn't fix the crash in Bistro that #23662 also surfaced. That will be a separate PR.

github-actions · 2026-07-13T03:55:33Z

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-24971

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.

tychedelia

Looks like there's some alignment error on macOS. The pixel eagle failure is real:

Caused by:
  In a CommandEncoder
    In a set_bind_group command
      Dynamic binding index 0 (targeting BindGroup with 'build_indexed_indirect_parameters_bind_group' label 0, binding 4) with value 704, does not respect device's requested `min_uniform_buffer_offset_alignment` limit: 256

2026-07-13T04:09:18.184217Z ERROR bevy_render::error_handler: Caught rendering error: Validation Error

I think you need to check min_uniform_buffer_offset_alignment for the stride if you want to use RawBufferVec.

beicause · 2026-07-13T04:29:48Z

+    pad_b: u32,
+    pad_c: vec4<u32>,
+    pad_d: vec4<u32>,
+    pad_e: vec4<u32>,


Do these pads exist due to dynamic offset?

The default limit min_uniform_buffer_offset_alignment is 256 bytes

The struct size doesn't need to be equal to alignment. It just need to be written aligned.

You can use DynamicUniformBuffer

DynamicUniformBuffer uses encase so it’s slow.

stuartparmenter · 2026-07-13T19:39:48Z

+
+        // Record the batch ranges for this phase for the indirect batch set
+        // building shader to use.
+        indexed_preparer.flush_batch_range_to(


Sorted phases don't seem like they'd get build jobs at all, since only batch_and_prepare_binned_render_phase records batch ranges, and even there, unbatchables are allocated outside the recorded range.

I’m confused, what did you want me to change here?

The way I read this PR, build_indirect_params used to process every batch in the metadata buffer (the old arrayLength bound), and now it only processes the ranges recorded in view_to_indirect_parameters_batch_range. So anything that never gets a range recorded never gets its indirect parameters built.

The only place ranges get recorded is this flush_batch_range_to call, which just covers what the MultidrawableBatchSetPreparer allocated. But a couple of other places allocate metadata in the same buffers and used to rely on the whole-buffer dispatch:

Sorted phases — batch_and_prepare_sorted_render_phase writes metadata for its batches (e.g. Transparent3d) but never records a range, so those views/phases never get a build job at all and run_build_indirect_parameters just skips them.

Unbatchables (and batchable-but-not-multidraw batches) in binned phases. Those get allocated in the first per-view loop, before the multidrawables loop runs, and the preparer captures its starting index after that. So they land outside every recorded range too.

Unless I'm missing something that fills those in, transparent and unbatchable meshes would end up drawing with stale or garbage indirect parameters. Seems like those allocation sites need to record ranges too, or the per-view range needs to be widened to cover them.

Good point, and this was showing up in the testbed as missing transparent meshes! I fixed this by pushing indirect parameter ranges for these types of meshes too.

github-actions · 2026-07-15T02:11:45Z

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-24971

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-07-16T08:44:33Z

+        // We aren't using `DynamicUniformBuffer` because it uses `encase`,
+        // which is slow.
+        debug_assert_eq!(
+            self.buffer_alignment as usize % size_of::<IndirectParametersBuildJob>(),


Handling alignment with RawBufferVec looks very fragile. And it should reserve memory before pushing in loop below.

I prefer to use DynamicUniformBuffer::get_writer for now which is simpler and directly writes to GPU buffer. I don't think it's slow as the struct is very small.

We can consider removing encase in future.

Or just pad the struct to 256 bytes? I'm not sure if there is a performance impact.

I saw GpuBinUnpackingMetadata and GpuUniformAllocationMetadata are already padded to 256 bytes.

OK, I padded the struct to 256 bytes.

github-actions · 2026-07-16T19:27:50Z

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-24971

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.

tychedelia · 2026-07-16T20:48:38Z

Just kicked off another example run but can confirm the failure on the testbed still

tychedelia · 2026-07-22T22:50:14Z

+            let uniform_offset = indirect_parameters_build_jobs.push(
+                IndirectParametersBuildJob::new(indirect_parameters_range.clone()),
+            );
+            build_indirect_parameters_metadata


Minor: I think we might leak retained view entities here as I don't see the BuildIndirectParametersMetadata being cleared?

tychedelia

Ope, unfortunately the pixel eagle failures are real though:

pcwalton added S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Rendering Drawing game state to the screen labels Jul 13, 2026

github-project-automation Bot added this to Rendering Jul 13, 2026

github-project-automation Bot moved this to Needs SME Triage in Rendering Jul 13, 2026

pcwalton requested review from Aceeri, IceSentry, kristoff3r and tychedelia July 13, 2026 03:50

pcwalton added C-Bug An unexpected or incorrect behavior P-Regression Functionality that used to work but no longer does. Add a test for this! labels Jul 13, 2026

tychedelia suggested changes Jul 13, 2026

View reviewed changes

beicause reviewed Jul 13, 2026

View reviewed changes

tychedelia 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 Jul 13, 2026

github-actions Bot mentioned this pull request Jul 13, 2026

24971 bevyengine/bevy-example-runner#246

Closed

stuartparmenter reviewed Jul 13, 2026

View reviewed changes

pcwalton added 2 commits July 14, 2026 18:16

Merge remote-tracking branch 'origin/main' into flickering-fix

22cc2b0

Fix dynamic uniform alignment problem

534d947

pcwalton 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 Jul 15, 2026

Merge remote-tracking branch 'origin/main' into flickering-fix

4292981

beicause reviewed Jul 16, 2026

View reviewed changes

pcwalton added 2 commits July 16, 2026 12:13

Fix sorted render phases

03b0ecc

Merge remote-tracking branch 'origin/main' into flickering-fix

b1ab711

github-actions Bot mentioned this pull request Jul 16, 2026

24971 bevyengine/bevy-example-runner#248

Closed

pcwalton added 4 commits July 16, 2026 14:22

Merge remote-tracking branch 'origin/main' into flickering-fix

23e62b6

Fix the shadows in the 3D testbed

e67fe9b

Merge remote-tracking branch 'origin/main' into flickering-fix

188566c

Switch to a 256 byte aligned structure

e067b5a

pcwalton requested review from beicause, stuartparmenter and tychedelia July 17, 2026 04:13

stuartparmenter approved these changes Jul 17, 2026

View reviewed changes

Doc check police

137fdda

This was referenced Jul 17, 2026

24971 bevyengine/bevy-example-runner#249

Closed

24971 bevyengine/bevy-example-runner#250

Closed

tychedelia approved these changes Jul 22, 2026

View reviewed changes

tychedelia suggested changes Jul 22, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Uh oh!

pcwalton commented Jul 13, 2026

Uh oh!

github-actions Bot commented Jul 13, 2026

Uh oh!

tychedelia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartparmenter Jul 15, 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!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 16, 2026

Uh oh!

tychedelia commented Jul 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tychedelia left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pcwalton commented Jul 13, 2026 •

edited

Loading

stuartparmenter Jul 15, 2026 •

edited

Loading