Update VertexInput (InputState) to match the spec - Part 4

This patch added support for null vertex buffer(s).

BUG=dawn:80, dawn:107

Change-Id: I3fb9427fcd4323c6df441be7011223088b40ec53
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/7900
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Yunchao He <yunchao.he@intel.com>

src/dawn_native/RenderPipeline.cpp

@@ -347,8 +347,11 @@ namespace dawn_native {
           mFragmentEntryPoint(descriptor->fragmentStage->entryPoint),
           mIsBlueprint(blueprint) {
         for (uint32_t slot = 0; slot < mVertexInput.bufferCount; ++slot) {
+            if (mVertexInput.buffers[slot].attributeCount == 0) {
+                continue;
+            }
+
             mInputsSetMask.set(slot);
-            mInputInfos[slot].inputSlot = slot;
             mInputInfos[slot].stride = mVertexInput.buffers[slot].stride;
             mInputInfos[slot].stepMode = mVertexInput.buffers[slot].stepMode;
 
@@ -566,7 +569,7 @@ namespace dawn_native {
         HashCombine(&hash, pipeline->mInputsSetMask);
         for (uint32_t i : IterateBitSet(pipeline->mInputsSetMask)) {
             const VertexBufferInfo& desc = pipeline->GetInput(i);
-            HashCombine(&hash, desc.inputSlot, desc.stride, desc.stepMode);
+            HashCombine(&hash, desc.stride, desc.stepMode);
         }
 
         HashCombine(&hash, pipeline->mVertexInput.indexFormat);
@@ -666,8 +669,7 @@ namespace dawn_native {
         for (uint32_t i : IterateBitSet(a->mInputsSetMask)) {
             const VertexBufferInfo& descA = a->GetInput(i);
             const VertexBufferInfo& descB = b->GetInput(i);
-            if (descA.inputSlot != descB.inputSlot || descA.stride != descB.stride ||
+            if (descA.stride != descB.stride || descA.stepMode != descB.stepMode) {
-                descA.stepMode != descB.stepMode) {
                 return false;
             }
         }

src/dawn_native/RenderPipeline.h

@@ -46,7 +46,6 @@ namespace dawn_native {
     };
 
     struct VertexBufferInfo {
-        uint32_t inputSlot;
         uint64_t stride;
         dawn::InputStepMode stepMode;
     };

src/dawn_native/metal/RenderPipelineMTL.mm

@@ -415,8 +415,8 @@ namespace dawn_native { namespace metal {
             [attribDesc release];
         }
 
-        for (uint32_t i : IterateBitSet(GetInputsSetMask())) {
+        for (uint32_t vbInputSlot : IterateBitSet(GetInputsSetMask())) {
-            const VertexBufferInfo& info = GetInput(i);
+            const VertexBufferInfo& info = GetInput(vbInputSlot);
 
             auto layoutDesc = [MTLVertexBufferLayoutDescriptor new];
             if (info.stride == 0) {
@@ -427,7 +427,7 @@ namespace dawn_native { namespace metal {
                 for (uint32_t attribIndex : IterateBitSet(GetAttributesSetMask())) {
                     const VertexAttributeInfo& attrib = GetAttribute(attribIndex);
                     // Only use the attributes that use the current input
-                    if (attrib.inputSlot != info.inputSlot) {
+                    if (attrib.inputSlot != vbInputSlot) {
                         continue;
                     }
                     max_stride = std::max(max_stride,
@@ -444,7 +444,7 @@ namespace dawn_native { namespace metal {
                 layoutDesc.stride = info.stride;
             }
             // TODO(cwallez@chromium.org): make the offset depend on the pipeline layout
-            mtlVertexDescriptor.layouts[kMaxBindingsPerGroup + i] = layoutDesc;
+            mtlVertexDescriptor.layouts[kMaxBindingsPerGroup + vbInputSlot] = layoutDesc;
             [layoutDesc release];
         }
         return mtlVertexDescriptor;

src/tests/end2end/VertexInputTests.cpp

@@ -408,10 +408,9 @@ TEST_P(VertexInputTest, MixedEverything) {
 // Test input state is unaffected by unused vertex slot
 TEST_P(VertexInputTest, UnusedVertexSlot) {
     // Instance input state, using slot 1
-    // TODO(yunchao.he@intel.com): This is not actually testing slot 1 right now,
-    // need to allow null for buffer[0].
     utils::ComboVertexInputDescriptor instanceVertexInput = MakeVertexInput(
-        {{4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}});
+        {{0, InputStepMode::Vertex, {}},
+         {4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}});
     dawn::RenderPipeline instancePipeline = MakeTestPipeline(
         instanceVertexInput, 1, {{0, VertexFormat::Float4, InputStepMode::Instance}});
 
@@ -453,10 +452,9 @@ TEST_P(VertexInputTest, MultiplePipelinesMixedVertexInput) {
         MakeTestPipeline(vertexVertexInput, 1, {{0, VertexFormat::Float4, InputStepMode::Vertex}});
 
     // Instance input state, using slot 1
-    // TODO(yunchao.he@intel.com): This is not actually testing slot 1 right now,
-    // need to allow null for buffer[0].
     utils::ComboVertexInputDescriptor instanceVertexInput = MakeVertexInput(
-        {{4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}});
+        {{0, InputStepMode::Instance, {}},
+         {4 * sizeof(float), InputStepMode::Instance, {{0, 0, VertexFormat::Float4}}}});
     dawn::RenderPipeline instancePipeline = MakeTestPipeline(
         instanceVertexInput, 1, {{0, VertexFormat::Float4, InputStepMode::Instance}});
 

src/tests/unittests/validation/VertexInputValidationTests.cpp

@@ -58,6 +58,50 @@ TEST_F(VertexInputTest, EmptyIsOk) {
     )");
 }
 
+// Check null buffer is valid
+TEST_F(VertexInputTest, NullBufferIsOk) {
+    utils::ComboVertexInputDescriptor state;
+    // One null buffer (buffer[0]) is OK
+    state.bufferCount = 1;
+    state.cBuffers[0].stride = 0;
+    state.cBuffers[0].attributeCount = 0;
+    state.cBuffers[0].attributes = nullptr;
+    CreatePipeline(true, state, R"(
+        #version 450
+        void main() {
+            gl_Position = vec4(0.0);
+        }
+    )");
+
+    // One null buffer (buffer[0]) followed by a buffer (buffer[1]) is OK
+    state.bufferCount = 2;
+    state.cBuffers[1].stride = 0;
+    state.cBuffers[1].attributeCount = 1;
+    state.cBuffers[1].attributes = &state.cAttributes[0];
+    state.cAttributes[0].shaderLocation = 0;
+    CreatePipeline(true, state, R"(
+        #version 450
+        void main() {
+            gl_Position = vec4(0.0);
+        }
+    )");
+
+    // Null buffer (buffer[2]) sitting between buffers (buffer[1] and buffer[3]) is OK
+    state.bufferCount = 4;
+    state.cBuffers[2].attributeCount = 0;
+    state.cBuffers[2].attributes = nullptr;
+    state.cBuffers[3].attributeCount = 1;
+    state.cBuffers[3].attributes = &state.cAttributes[1];
+    state.cAttributes[1].shaderLocation = 1;
+    CreatePipeline(true, state, R"(
+        #version 450
+        void main() {
+            gl_Position = vec4(0.0);
+        }
+    )");
+}
+
+// Check validation that pipeline vertex buffers are backed by attributes in the vertex input
 // Check validation that pipeline vertex buffers are backed by attributes in the vertex input
 TEST_F(VertexInputTest, PipelineCompatibility) {
     utils::ComboVertexInputDescriptor state;