Size the visibility range binding for the buffer type it was given - #25687
Open
glashoff wants to merge 1 commit into
Open
Size the visibility range binding for the buffer type it was given#25687glashoff wants to merge 1 commit into
glashoff wants to merge 1 commit into
Conversation
Reported as bevyengine#21309: the `visibility_range` example, built for `wasm32-unknown-unknown` with the `webgl2` feature, quits before it draws a frame: In Device::create_render_pipeline, label = 'pbr_opaque_mesh_pipeline' Error matching ShaderStages(VERTEX) shader requirements against the pipeline Shader global ResourceBinding { group: 0, binding: 14 } is not available in the pipeline layout Buffer structure size 1024, added to one element of an unbound array, if it's the last field, ended up greater than the given `min_binding_size`, which is 16 Binding 14 is declared two ways in `mesh_view_bindings.wesl`: a runtime-sized `array<vec4<f32>>` where six storage buffers are available, and a fixed `array<vec4<f32>, VISIBILITY_RANGE_UNIFORM_BUFFER_SIZE>` where they are not -- WebGL2. `layout_entries` promised `Vec4::min_size()` for both. Sixteen bytes is the minimum only for the runtime-sized one; the fixed one needs all 1024. Only the layout changes. The buffer bound there is already 1024 bytes, because `write_render_visibility_ranges` pads it to exactly that many elements on this path. The size now follows the binding type. `VISIBILITY_RANGE_UNIFORM_BUFFER_SIZE` was private to `bevy_render`, so export it rather than write the number out again here. The gap was found by Claude Opus 5.
Contributor
|
Welcome, new contributor! Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly ✨ |
beicause
approved these changes
Sep 10, 2026
beicause
left a comment
Member
There was a problem hiding this comment.
Looks correct. I didn't tested it.
| false, | ||
| Some(Vec4::min_size()), | ||
| Some(match visibility_ranges_buffer_binding_type { | ||
| BufferBindingType::Uniform => Vec4::min_size().saturating_mul( |
Member
There was a problem hiding this comment.
I think it's better to use checked_mul instead of saturating_mul, though they have no different here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
On WebGL2, a mesh with a crossfading
VisibilityRangegets a pipeline that fails validation:Fixes #21309.
Solution
Binding 14 is the visibility range buffer, and it is declared two ways depending on how many
storage buffers the device has.
mesh_view_bindings.wesl:layout_entriesgave that bindingSome(Vec4::min_size())— 16 bytes — whichever branch hadbeen taken. That is right only for the storage declaration, where the array is runtime-sized and
one element genuinely is the minimum the shader can be said to require. The uniform declaration
is fixed at
VISIBILITY_RANGE_UNIFORM_BUFFER_SIZEelements, so the shader requires all 1024bytes, and a layout promising 16 is one a 16-byte buffer could satisfy.
Only the layout changes. The buffer bound there is already 1024 bytes, because
write_render_visibility_rangespads it to exactlyVISIBILITY_RANGE_UNIFORM_BUFFER_SIZEelements on this path, so the larger
min_binding_sizeis one it already satisfies.The size now follows the binding type.
VISIBILITY_RANGE_UNIFORM_BUFFER_SIZEwas private tobevy_render, so this exports it rather than writing the number out again here.Testing
visibility_rangeexample from Visibility Range example doesn't work in webgl2 #21309, built forwasm32-unknown-unknownwithwebgl2at this commit and at its parent: it fails on the parent and runs here.AI usage
The gap was found by Claude Opus 5, and the change and the verification runs above were drafted
and carried out with it.
I have read every line of this change and stand behind it.