Record zero-attribute vertex buffer when creating render pipeline

Currently Dawn ignores all zero-attribute vertex buffer in the given
pipeline descriptor when creating RenderPipelineBase because
zero-attribute vertex buffer is treated as unused slot, however the spec
doesn't state that zero-attribute vertex buffer should be ignored.

To support zero-attribute vertex buffer, this commit has the following
changes.

1. Add VertexBufferNotUsed enum value to wgpu::VertexStepMode to
   represent unused slots
2. Ignore VertexBufferNotUsed  step mode buffers when creating
   RenderPipelineBase and add tests to check it
3. Record zero-attribute vertex buffers when creating RenderPipelineBase
   and add tests to check it
4. Fix VertexStateTest::LastAllowedVertexBuffer broken by the above
   changes

Temporarily we set the enum value of
wgpu::VertexStepMode::VertexBufferNotUsed to 0 to pass the CTS tests
because currently empty vertex buffer slots step mode can be
zero-initialized. We will make a CL to Blink to explicitly set
wgpu::VertexStepMode::VertexBufferNotUsed for empty slots and change
the enum value to 2.

Bug: dawn:1000
Change-Id: Ibd4ab87f2c922e8e460f2311547f13d58f1d5611
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/89340
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Takahiro <hogehoge@gachapin.jp>

dawn.json

@@ -1479,8 +1479,9 @@
     "vertex step mode": {
         "category": "enum",
         "values": [
-            {"value": 0, "name": "vertex"},
+            {"value": 0, "name": "vertex buffer not used"},
-            {"value": 1, "name": "instance"}
+            {"value": 1, "name": "vertex"},
+            {"value": 2, "name": "instance"}
         ]
     },
     "load op": {

src/dawn/native/RenderPipeline.cpp

@@ -498,7 +498,8 @@ RenderPipelineBase::RenderPipelineBase(DeviceBase* device,
     mVertexBufferCount = descriptor->vertex.bufferCount;
     const VertexBufferLayout* buffers = descriptor->vertex.buffers;
     for (uint8_t slot = 0; slot < mVertexBufferCount; ++slot) {
-        if (buffers[slot].attributeCount == 0) {
+        // Skip unused slots
+        if (buffers[slot].stepMode == wgpu::VertexStepMode::VertexBufferNotUsed) {
             continue;
         }
 

src/dawn/native/ShaderModule.cpp

@@ -112,8 +112,9 @@ tint::transform::VertexStepMode ToTintVertexStepMode(wgpu::VertexStepMode mode)
             return tint::transform::VertexStepMode::kVertex;
         case wgpu::VertexStepMode::Instance:
             return tint::transform::VertexStepMode::kInstance;
+        case wgpu::VertexStepMode::VertexBufferNotUsed:
+            UNREACHABLE();
     }
-    UNREACHABLE();
 }
 
 ResultOrError<SingleShaderStage> TintPipelineStageToShaderStage(tint::ast::PipelineStage stage) {

src/dawn/native/d3d12/RenderPipelineD3D12.cpp

@@ -106,6 +106,8 @@ D3D12_INPUT_CLASSIFICATION VertexStepModeFunction(wgpu::VertexStepMode mode) {
             return D3D12_INPUT_CLASSIFICATION_PER_VERTEX_DATA;
         case wgpu::VertexStepMode::Instance:
             return D3D12_INPUT_CLASSIFICATION_PER_INSTANCE_DATA;
+        case wgpu::VertexStepMode::VertexBufferNotUsed:
+            UNREACHABLE();
     }
 }
 

src/dawn/native/metal/RenderPipelineMTL.mm

@@ -98,6 +98,8 @@ MTLVertexStepFunction VertexStepModeFunction(wgpu::VertexStepMode mode) {
             return MTLVertexStepFunctionPerVertex;
         case wgpu::VertexStepMode::Instance:
             return MTLVertexStepFunctionPerInstance;
+        case wgpu::VertexStepMode::VertexBufferNotUsed:
+            UNREACHABLE();
     }
 }
 

src/dawn/native/opengl/RenderPipelineGL.cpp

@@ -276,6 +276,8 @@ void RenderPipeline::CreateVAOForVertexState() {
                 case wgpu::VertexStepMode::Instance:
                     gl.VertexAttribDivisor(glAttrib, 1);
                     break;
+                case wgpu::VertexStepMode::VertexBufferNotUsed:
+                    UNREACHABLE();
             }
         }
     }

src/dawn/native/vulkan/RenderPipelineVk.cpp

@@ -39,8 +39,9 @@ VkVertexInputRate VulkanInputRate(wgpu::VertexStepMode stepMode) {
             return VK_VERTEX_INPUT_RATE_VERTEX;
         case wgpu::VertexStepMode::Instance:
             return VK_VERTEX_INPUT_RATE_INSTANCE;
+        case wgpu::VertexStepMode::VertexBufferNotUsed:
+            UNREACHABLE();
     }
-    UNREACHABLE();
 }
 
 VkFormat VulkanVertexFormat(wgpu::VertexFormat format) {

src/dawn/tests/end2end/VertexStateTests.cpp

@@ -550,6 +550,10 @@ TEST_P(VertexStateTest, LastAllowedVertexBuffer) {
     vertexState.cAttributes[0].offset = 0;
     vertexState.cAttributes[0].format = VertexFormat::Float32x4;
 
+    for (uint32_t i = 0; i < kBufferIndex; i++) {
+        vertexState.cVertexBuffers[i].stepMode = VertexStepMode::VertexBufferNotUsed;
+    }
+
     wgpu::RenderPipeline pipeline =
         MakeTestPipeline(vertexState, 1, {{0, VertexFormat::Float32x4, VertexStepMode::Vertex}});
 

src/dawn/tests/unittests/validation/DrawVertexAndIndexBufferOOBValidationTests.cpp

@@ -440,6 +440,50 @@ TEST_F(DrawVertexAndIndexBufferOOBValidationTests, DrawVertexBufferOutOfBoundWit
     }
 }
 
+// Verify zero-attribute vertex buffer OOB for non-instanced Draw are caught in command encoder
+TEST_F(DrawVertexAndIndexBufferOOBValidationTests, ZeroAttribute) {
+    // Create a render pipeline with zero-attribute vertex buffer description
+    wgpu::RenderPipeline pipeline =
+        CreateRenderPipelineWithBufferDesc({{kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}}});
+
+    wgpu::Buffer vertexBuffer = CreateBuffer(3 * kFloat32x4Stride);
+
+    {
+        // Valid
+        VertexBufferList vertexBufferList = {{0, vertexBuffer, 0, wgpu::kWholeSize}};
+        TestRenderPassDraw(pipeline, vertexBufferList, 3, 1, 0, 0, true);
+    }
+
+    {
+        // OOB
+        VertexBufferList vertexBufferList = {{0, vertexBuffer, 0, 0}};
+        TestRenderPassDraw(pipeline, vertexBufferList, 3, 1, 0, 0, false);
+    }
+}
+
+// Verify vertex buffers don't need to be set to unused (hole) slots for Draw
+TEST_F(DrawVertexAndIndexBufferOOBValidationTests, UnusedSlots) {
+    // Set vertex buffer only to the second slot
+    wgpu::Buffer vertexBuffer = CreateBuffer(3 * kFloat32x4Stride);
+    VertexBufferList vertexBufferList = {{1, vertexBuffer, 0, wgpu::kWholeSize}};
+
+    {
+        // The first slot it unused so valid even if vertex buffer is not set to it.
+        wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+            {{0, wgpu::VertexStepMode::VertexBufferNotUsed, {}},
+             {kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}}});
+        TestRenderPassDraw(pipeline, vertexBufferList, 3, 1, 0, 0, true);
+    }
+
+    {
+        // The first slot it used so invalid if vertex buffer is not set to it.
+        wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+            {{0, wgpu::VertexStepMode::Vertex, {}},
+             {kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}}});
+        TestRenderPassDraw(pipeline, vertexBufferList, 3, 1, 0, 0, false);
+    }
+}
+
 // Control case for DrawIndexed
 TEST_F(DrawVertexAndIndexBufferOOBValidationTests, DrawIndexedBasic) {
     wgpu::RenderPipeline pipeline = CreateBasicRenderPipeline();
@@ -556,6 +600,75 @@ TEST_F(DrawVertexAndIndexBufferOOBValidationTests, DrawIndexedVertexBufferOOB) {
     }
 }
 
+// Verify zero-attribute instance step mode vertex buffer OOB for instanced DrawIndex
+// are caught in command encoder
+TEST_F(DrawVertexAndIndexBufferOOBValidationTests, DrawIndexedZeroAttribute) {
+    // Create a render pipeline with a vertex step mode and a zero-attribute
+    // instance step mode vertex buffer description
+    wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+        {{kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {{0, wgpu::VertexFormat::Float32x4}}},
+         {kFloat32x2Stride, wgpu::VertexStepMode::Instance, {}}});
+
+    // Build index buffer for 12 indexes
+    wgpu::Buffer indexBuffer = CreateBuffer(12 * sizeof(uint32_t), wgpu::BufferUsage::Index);
+    IndexBufferDesc indexBufferDesc = {indexBuffer, wgpu::IndexFormat::Uint32};
+
+    // Build vertex buffer for 3 vertices
+    wgpu::Buffer vertexBuffer = CreateBuffer(3 * kFloat32x4Stride);
+
+    // Build vertex buffer for 3 instances
+    wgpu::Buffer instanceBuffer = CreateBuffer(3 * kFloat32x2Stride);
+
+    {
+        // Valid
+        VertexBufferList vertexBufferList = {{0, vertexBuffer, 0, wgpu::kWholeSize},
+                                             {1, vertexBuffer, 0, wgpu::kWholeSize}};
+        TestRenderPassDrawIndexed(pipeline, indexBufferDesc, vertexBufferList, 12, 3, 0, 0, 0,
+                                  true);
+    }
+
+    {
+        // OOB
+        VertexBufferList vertexBufferList = {{0, vertexBuffer, 0, wgpu::kWholeSize},
+                                             {1, vertexBuffer, 0, 0}};
+        TestRenderPassDrawIndexed(pipeline, indexBufferDesc, vertexBufferList, 12, 3, 0, 0, 0,
+                                  false);
+    }
+}
+
+// Verify vertex buffers don't need to be set to unused (hole) slots for DrawIndexed
+TEST_F(DrawVertexAndIndexBufferOOBValidationTests, DrawIndexedUnusedSlots) {
+    // Build index buffer
+    wgpu::Buffer indexBuffer = CreateBuffer(12 * sizeof(uint32_t), wgpu::BufferUsage::Index);
+    IndexBufferDesc indexBufferDesc = {indexBuffer, wgpu::IndexFormat::Uint32};
+
+    // Set vertex buffers only to the second and third slots
+    wgpu::Buffer vertexBuffer = CreateBuffer(3 * kFloat32x4Stride);
+    wgpu::Buffer instanceBuffer = CreateBuffer(3 * kFloat32x2Stride);
+    VertexBufferList vertexBufferList = {{1, vertexBuffer, 0, wgpu::kWholeSize},
+                                         {2, instanceBuffer, 0, wgpu::kWholeSize}};
+
+    {
+        // The first slot it unused so valid even if vertex buffer is not set to it.
+        wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+            {{0, wgpu::VertexStepMode::VertexBufferNotUsed, {}},
+             {kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}},
+             {kFloat32x2Stride, wgpu::VertexStepMode::Instance, {}}});
+        TestRenderPassDrawIndexed(pipeline, indexBufferDesc, vertexBufferList, 12, 3, 0, 0, 0,
+                                  true);
+    }
+
+    {
+        // The first slot it used so invalid if vertex buffer is not set to it.
+        wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+            {{0, wgpu::VertexStepMode::Vertex, {}},
+             {kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}},
+             {kFloat32x2Stride, wgpu::VertexStepMode::Instance, {}}});
+        TestRenderPassDrawIndexed(pipeline, indexBufferDesc, vertexBufferList, 12, 3, 0, 0, 0,
+                                  false);
+    }
+}
+
 // Verify zero array stride vertex buffer OOB for Draw and DrawIndexed are caught in command encoder
 // This test only test cases that strideCount > 0. Cases of strideCount == 0 are tested in
 // ZeroStrideCountVertexBufferNeverOOB.