Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate VertexBufferLayout::array_stride #6799

Open
abey79 opened this issue Dec 21, 2024 · 1 comment · May be fixed by #6804
Open

Validate VertexBufferLayout::array_stride #6799

abey79 opened this issue Dec 21, 2024 · 1 comment · May be fixed by #6804
Labels
area: validation Issues related to validation, diagnostics, and error handling

Comments

@abey79
Copy link

abey79 commented Dec 21, 2024

Context

The WebGPU spec constraints what the array stride of a VertexBufferLayout might be. In particular, it forbids it to be smaller or equal than the max(offset + size) over all attributes:

image

One might be tempted to use a stride smaller than the attributes to implement some kind of "sliding window" over a vertex buffer (I certainly was). This is however non-compliant as per above, and may variously lead to at least:

  • recent chrome versions to reject your shader
  • wgpu 23.0.0+ to emit incorrect metal shader (including out-of-bound array access)
  • possibly other phenomenons (I have only tested mac, WebGPU, and WebGL, the latter seemingly being unaffected)

Issue

Currently, wgpu doesn't warn/error on non-compliant stride values. Since 23.0.0, it also silently generate erroneous metal shader when provided with non-compliant stride.

Desired change

wgpu should error (or warn loudly) with a reference to the spec whenever a non-compliant stride is detected.

Reference

@sagudev
Copy link
Contributor

sagudev commented Dec 21, 2024

Indeed, we are missing check somewhere around here:

for (i, vb_state) in desc.vertex.buffers.iter().enumerate() {
let mut last_stride = 0;
for attribute in vb_state.attributes.iter() {
last_stride = last_stride.max(attribute.offset + attribute.format.size());
}
vertex_steps.push(pipeline::VertexStep {
stride: vb_state.array_stride,

EDIT: We are missing multiple checks.

@Wumpf Wumpf added the area: validation Issues related to validation, diagnostics, and error handling label Dec 22, 2024
@sagudev sagudev linked a pull request Dec 22, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: validation Issues related to validation, diagnostics, and error handling
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

3 participants