diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index a0b97bc867..c4b05126b8 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/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 || - descA.stepMode != descB.stepMode) { + if (descA.stride != descB.stride || descA.stepMode != descB.stepMode) { return false; } } diff --git a/src/dawn_native/RenderPipeline.h b/src/dawn_native/RenderPipeline.h index 5e945b1d9e..0f33fa6718 100644 --- a/src/dawn_native/RenderPipeline.h +++ b/src/dawn_native/RenderPipeline.h @@ -46,7 +46,6 @@ namespace dawn_native { }; struct VertexBufferInfo { - uint32_t inputSlot; uint64_t stride; dawn::InputStepMode stepMode; }; diff --git a/src/dawn_native/metal/RenderPipelineMTL.mm b/src/dawn_native/metal/RenderPipelineMTL.mm index 95ac0b9eaa..7d7fa53f34 100644 --- a/src/dawn_native/metal/RenderPipelineMTL.mm +++ b/src/dawn_native/metal/RenderPipelineMTL.mm @@ -415,8 +415,8 @@ namespace dawn_native { namespace metal { [attribDesc release]; } - for (uint32_t i : IterateBitSet(GetInputsSetMask())) { - const VertexBufferInfo& info = GetInput(i); + for (uint32_t vbInputSlot : IterateBitSet(GetInputsSetMask())) { + 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; diff --git a/src/tests/end2end/VertexInputTests.cpp b/src/tests/end2end/VertexInputTests.cpp index 41f98f4cf9..e909614cb2 100644 --- a/src/tests/end2end/VertexInputTests.cpp +++ b/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}}); diff --git a/src/tests/unittests/validation/VertexInputValidationTests.cpp b/src/tests/unittests/validation/VertexInputValidationTests.cpp index f855cedcd5..9e52589a52 100644 --- a/src/tests/unittests/validation/VertexInputValidationTests.cpp +++ b/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;