From a94c9acd0175e5a9bb0c67585d0f8c9360e96ba4 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 15 Mar 2021 14:52:53 +0000 Subject: [PATCH] Fix the vertex attribute offset alignment rule. Bug: dawn:130 Change-Id: Ib5fa24bf5520cb1ffe1653f1504dff244df928b4 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/44300 Auto-Submit: Corentin Wallez Reviewed-by: Austin Eng Reviewed-by: Jiawei Shao Commit-Queue: Jiawei Shao --- src/dawn_native/RenderPipeline.cpp | 5 +++-- .../validation/VertexStateValidationTests.cpp | 21 ++++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index 484d0bb663..6cfaccf4c4 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/src/dawn_native/RenderPipeline.cpp @@ -60,8 +60,9 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("Setting attribute offset out of bounds"); } - if (attribute->offset % 4 != 0) { - return DAWN_VALIDATION_ERROR("Attribute offset needs to be a multiple of 4 bytes"); + if (attribute->offset % dawn::VertexFormatComponentSize(attribute->format) != 0) { + return DAWN_VALIDATION_ERROR( + "Attribute offset needs to be a multiple of the size format's components"); } if ((*attributesSetMask)[attribute->shaderLocation]) { diff --git a/src/tests/unittests/validation/VertexStateValidationTests.cpp b/src/tests/unittests/validation/VertexStateValidationTests.cpp index 0ece5ea396..a60073d320 100644 --- a/src/tests/unittests/validation/VertexStateValidationTests.cpp +++ b/src/tests/unittests/validation/VertexStateValidationTests.cpp @@ -292,17 +292,32 @@ TEST_F(VertexStateTest, SetAttributeOffsetOutOfBounds) { CreatePipeline(false, state, kDummyVertexShader); } -// Check multiple of 4 bytes constraint on offset +// Check the "component byte size" alignment constraint for the offset. TEST_F(VertexStateTest, SetOffsetNotAligned) { - // Control case, setting offset 4 bytes. + // Control case, setting the offset at the correct alignments. utils::ComboVertexStateDescriptor state; state.vertexBufferCount = 1; state.cVertexBuffers[0].attributeCount = 1; + + state.cAttributes[0].format = wgpu::VertexFormat::Float32; state.cAttributes[0].offset = 4; CreatePipeline(true, state, kDummyVertexShader); - // Test offset not multiple of 4 bytes + state.cAttributes[0].format = wgpu::VertexFormat::Snorm16x2; state.cAttributes[0].offset = 2; + CreatePipeline(true, state, kDummyVertexShader); + + state.cAttributes[0].format = wgpu::VertexFormat::Uint8x2; + state.cAttributes[0].offset = 1; + CreatePipeline(true, state, kDummyVertexShader); + + // Test offset not multiple of the component byte size. + state.cAttributes[0].format = wgpu::VertexFormat::Float32; + state.cAttributes[0].offset = 2; + CreatePipeline(false, state, kDummyVertexShader); + + state.cAttributes[0].format = wgpu::VertexFormat::Snorm16x2; + state.cAttributes[0].offset = 1; CreatePipeline(false, state, kDummyVertexShader); }