Conversation
ValidateRenderPipelineDescriptor had no vertex buffer checks, so any arrayStride reached the HAL and the native backends disagreed about it: software drops a stride-0 draw, GLES reads 0 as tightly packed, Metal sets a zero stride with no constant step function, and a stride of 30 passes on Vulkan while failing in the browser and on GLES. Add the WebGPU GPUVertexBufferLayout checks: arrayStride % 4 == 0, arrayStride <= maxVertexBufferArrayStride (skipped when the limit is unset), and offset + format size within arrayStride (or within the limit when arrayStride is 0). arrayStride 0 is rejected explicitly until a native backend emulates broadcast. Four new CreateRenderPipelineErrorKind values, appended after CreateRenderPipelineErrorHAL, and fields naming the buffer, stride and attribute.
8 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
core.ValidateRenderPipelineDescriptorchecks the vertex module and entry point, the fragment stage, the colour and depth formats and the sample count, but it has no vertex buffer check at all. AnyarrayStridetherefore reaches the HAL, and the native backends disagree about what it means.arrayStride: 0is legal WebGPU and means "broadcast": every vertex reads the same offset. The CTS tests it as a pass case (vertex_state.spec.ts,vertex_buffer_array_stride_limit_alignment). Dawn and Rust wgpu check% 4and the limit, and they emulate stride 0 on Metal with a constant step function. Here, in v0.34.5:hal/software/draw.go:461-464return nil: the whole draw is droppedhal/software/draw.go:664-667continue: the attribute stays zero, not broadcasthal/metal/device.go:949-951setStride:0,setStepRate:1, no constant step functionhal/gles/command.go:1413-1424glVertexAttribPointer, where 0 means tightly packedhal/vulkan/pipeline.go:104-107hal/dx12/device.go:2514internal/browser/convert_resources.go:488A misaligned stride behaves the same way: a 30-byte stride succeeds on Vulkan and fails in the browser and on GLES.
What this changes
Validation only. No HAL file changes and no existing signature changes.
ValidateRenderPipelineDescriptorruns before any HAL on the native path (device_native.go), so one check covers every native backend. For each entry indesc.Vertex.Buffers:ArrayStride % 4 == 0.ArrayStride <= limits.MaxVertexBufferArrayStride, skipped when the limit is zero, as the file already does for other limits.Offset + format size <= ArrayStride. For stride 0 it is bounded byMaxVertexBufferArrayStrideinstead, per the spec's special case, so it stays right once check 4 is lifted.ArrayStride == 0is rejected explicitly, with a message saying broadcast is not supported on native backends yet. Rejecting beats emulating per backend here: failing loudly on every path is better than working on one. The check can be removed once a backend emulates broadcast.New exported API, additive only:
CreateRenderPipelineErrorKindvalues, appended afterCreateRenderPipelineErrorHALso existing values do not move:CreateRenderPipelineErrorVertexStrideMisaligned,CreateRenderPipelineErrorVertexStrideTooLarge,CreateRenderPipelineErrorVertexAttributeOutOfStride,CreateRenderPipelineErrorVertexStrideZero.CreateRenderPipelineErrorfields naming the failure:BufferIndex,ArrayStride,MaxArrayStride,AttributeIndex,AttributeOffset,AttributeFormat.The browser path is unchanged, because browser WebGPU already validates this.
Tests
Table tests in
core/validate_test.go:MaxVertexBufferArrayStride;go test ./...passes.