From 9bb32ba330d0133ac558c11313d851d235a4d004 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Fri, 23 Jul 2021 14:28:47 +0000 Subject: [PATCH] Make vertex attrib offsets require min(4, formatSize) Bug: dawn:1010 Change-Id: Ib7e7fe7f9fb7fbf1a7701b72ed5a1c75d5c01209 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/59282 Reviewed-by: Kai Ninomiya Commit-Queue: Corentin Wallez --- src/dawn_native/RenderPipeline.cpp | 4 +-- .../validation/VertexStateValidationTests.cpp | 29 +++++++++++++------ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index 7392f1d9b8..da78253aad 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/src/dawn_native/RenderPipeline.cpp @@ -57,9 +57,9 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("Setting attribute offset out of bounds"); } - if (attribute->offset % formatInfo.componentByteSize != 0) { + if (attribute->offset % std::min(4u, formatInfo.byteSize) != 0) { return DAWN_VALIDATION_ERROR( - "Attribute offset needs to be a multiple of the size format's components"); + "Attribute offset needs to be a multiple of min(4, formatSize)"); } if (metadata.usedVertexInputs[location] && diff --git a/src/tests/unittests/validation/VertexStateValidationTests.cpp b/src/tests/unittests/validation/VertexStateValidationTests.cpp index 974dacb6b0..771956180f 100644 --- a/src/tests/unittests/validation/VertexStateValidationTests.cpp +++ b/src/tests/unittests/validation/VertexStateValidationTests.cpp @@ -291,33 +291,44 @@ TEST_F(VertexStateTest, SetAttributeOffsetOutOfBounds) { CreatePipeline(false, state, kDummyVertexShader); } -// Check the "component byte size" alignment constraint for the offset. +// Check the min(4, formatSize) alignment constraint for the offset. TEST_F(VertexStateTest, SetOffsetNotAligned) { // Control case, setting the offset at the correct alignments. utils::ComboVertexStateDescriptor state; state.vertexBufferCount = 1; state.cVertexBuffers[0].attributeCount = 1; + // Test that for small formats, the offset must be aligned to the format size. state.cAttributes[0].format = wgpu::VertexFormat::Float32; state.cAttributes[0].offset = 4; CreatePipeline(true, state, kDummyVertexShader); + state.cAttributes[0].offset = 2; + CreatePipeline(false, state, kDummyVertexShader); state.cAttributes[0].format = wgpu::VertexFormat::Snorm16x2; - state.cAttributes[0].offset = 2; + state.cAttributes[0].offset = 4; CreatePipeline(true, state, kDummyVertexShader); + state.cAttributes[0].offset = 2; + CreatePipeline(false, state, kDummyVertexShader); state.cAttributes[0].format = wgpu::VertexFormat::Unorm8x2; + state.cAttributes[0].offset = 2; + CreatePipeline(true, state, kDummyVertexShader); state.cAttributes[0].offset = 1; + CreatePipeline(false, state, kDummyVertexShader); + + // Test that for large formts the offset only needs to be aligned to 4. + state.cAttributes[0].format = wgpu::VertexFormat::Snorm16x4; + state.cAttributes[0].offset = 4; 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::Uint32x3; + state.cAttributes[0].offset = 4; + CreatePipeline(true, state, kDummyVertexShader); - state.cAttributes[0].format = wgpu::VertexFormat::Snorm16x2; - state.cAttributes[0].offset = 1; - CreatePipeline(false, state, kDummyVertexShader); + state.cAttributes[0].format = wgpu::VertexFormat::Sint32x4; + state.cAttributes[0].offset = 4; + CreatePipeline(true, state, kDummyVertexShader); } // Check attribute offset overflow