d3d12: Lazily apply IASetVertexBuffers before draw

This fixes a bug where D3D12 SetVertexBuffers was using the input info
from the last set pipeline's InputState. IASetVertexBuffers needs to be
reapplied if the input state changes.

Bug: dawn:91
Change-Id: I7d0b308ea20cee6d595f6b29548f57d82c8e47a4
Reviewed-on: https://dawn-review.googlesource.com/c/3860
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/d3d12/CommandBufferD3D12.cpp

@@ -420,6 +420,47 @@ namespace dawn_native { namespace d3d12 {
         }
     }
 
+    void CommandBuffer::FlushSetVertexBuffers(ComPtr<ID3D12GraphicsCommandList> commandList,
+                                              VertexBuffersInfo* vertexBuffersInfo,
+                                              const InputState* inputState) {
+        DAWN_ASSERT(vertexBuffersInfo != nullptr);
+        DAWN_ASSERT(inputState != nullptr);
+
+        auto inputsMask = inputState->GetInputsSetMask();
+
+        uint32_t startSlot = vertexBuffersInfo->startSlot;
+        uint32_t endSlot = vertexBuffersInfo->endSlot;
+
+        // If the input state has changed, we need to update the StrideInBytes
+        // for the D3D12 buffer views. We also need to extend the dirty range to
+        // touch all these slots because the stride may have changed.
+        if (vertexBuffersInfo->lastInputState != inputState) {
+            vertexBuffersInfo->lastInputState = inputState;
+
+            for (uint32_t slot : IterateBitSet(inputsMask)) {
+                startSlot = std::min(startSlot, slot);
+                endSlot = std::max(endSlot, slot + 1);
+                vertexBuffersInfo->d3d12BufferViews[slot].StrideInBytes =
+                    inputState->GetInput(slot).stride;
+            }
+        }
+
+        if (endSlot <= startSlot) {
+            return;
+        }
+
+        // d3d12BufferViews is kept up to date with the most recent data passed
+        // to SetVertexBuffers. This makes it correct to only track the start
+        // and end of the dirty range. When FlushSetVertexBuffers is called,
+        // we will at worst set non-dirty vertex buffers in duplicate.
+        uint32_t count = endSlot - startSlot;
+        commandList->IASetVertexBuffers(startSlot, count,
+                                        &vertexBuffersInfo->d3d12BufferViews[startSlot]);
+
+        vertexBuffersInfo->startSlot = kMaxVertexInputs;
+        vertexBuffersInfo->endSlot = 0;
+    }
+
     void CommandBuffer::RecordComputePass(ComPtr<ID3D12GraphicsCommandList> commandList,
                                           BindGroupStateTracker* bindingTracker) {
         PipelineLayout* lastLayout = nullptr;
@@ -532,6 +573,8 @@ namespace dawn_native { namespace d3d12 {
 
         RenderPipeline* lastPipeline = nullptr;
         PipelineLayout* lastLayout = nullptr;
+        InputState* lastInputState = nullptr;
+        VertexBuffersInfo vertexBuffersInfo = {};
 
         Command type;
         while (mCommands.NextCommandId(&type)) {
@@ -543,12 +586,16 @@ namespace dawn_native { namespace d3d12 {
 
                 case Command::Draw: {
                     DrawCmd* draw = mCommands.NextCommand<DrawCmd>();
+
+                    FlushSetVertexBuffers(commandList, &vertexBuffersInfo, lastInputState);
                     commandList->DrawInstanced(draw->vertexCount, draw->instanceCount,
                                                draw->firstVertex, draw->firstInstance);
                 } break;
 
                 case Command::DrawIndexed: {
                     DrawIndexedCmd* draw = mCommands.NextCommand<DrawIndexedCmd>();
+
+                    FlushSetVertexBuffers(commandList, &vertexBuffersInfo, lastInputState);
                     commandList->DrawIndexedInstanced(draw->indexCount, draw->instanceCount,
                                                       draw->firstIndex, draw->baseVertex,
                                                       draw->firstInstance);
@@ -558,6 +605,7 @@ namespace dawn_native { namespace d3d12 {
                     SetRenderPipelineCmd* cmd = mCommands.NextCommand<SetRenderPipelineCmd>();
                     RenderPipeline* pipeline = ToBackend(cmd->pipeline).Get();
                     PipelineLayout* layout = ToBackend(pipeline->GetLayout());
+                    InputState* inputState = ToBackend(pipeline->GetInputState());
 
                     commandList->SetGraphicsRootSignature(layout->GetRootSignature().Get());
                     commandList->SetPipelineState(pipeline->GetPipelineState().Get());
@@ -567,6 +615,7 @@ namespace dawn_native { namespace d3d12 {
 
                     lastPipeline = pipeline;
                     lastLayout = layout;
+                    lastInputState = inputState;
                 } break;
 
                 case Command::SetStencilReference: {
@@ -617,19 +666,19 @@ namespace dawn_native { namespace d3d12 {
                     auto buffers = mCommands.NextData<Ref<BufferBase>>(cmd->count);
                     auto offsets = mCommands.NextData<uint32_t>(cmd->count);
 
-                    auto inputState = ToBackend(lastPipeline->GetInputState());
+                    vertexBuffersInfo.startSlot =
+                        std::min(vertexBuffersInfo.startSlot, cmd->startSlot);
+                    vertexBuffersInfo.endSlot =
+                        std::max(vertexBuffersInfo.endSlot, cmd->startSlot + cmd->count);
 
-                    std::array<D3D12_VERTEX_BUFFER_VIEW, kMaxVertexInputs> d3d12BufferViews;
                     for (uint32_t i = 0; i < cmd->count; ++i) {
-                        auto input = inputState->GetInput(cmd->startSlot + i);
                         Buffer* buffer = ToBackend(buffers[i].Get());
-                        d3d12BufferViews[i].BufferLocation = buffer->GetVA() + offsets[i];
+                        auto* d3d12BufferView =
-                        d3d12BufferViews[i].StrideInBytes = input.stride;
+                            &vertexBuffersInfo.d3d12BufferViews[cmd->startSlot + i];
-                        d3d12BufferViews[i].SizeInBytes = buffer->GetSize() - offsets[i];
+                        d3d12BufferView->BufferLocation = buffer->GetVA() + offsets[i];
+                        d3d12BufferView->SizeInBytes = buffer->GetSize() - offsets[i];
+                        // The bufferView stride is set based on the input state before a draw.
                     }
-
-                    commandList->IASetVertexBuffers(cmd->startSlot, cmd->count,
-                                                    d3d12BufferViews.data());
                 } break;
 
                 default: { UNREACHABLE(); } break;

src/dawn_native/d3d12/CommandBufferD3D12.h

@@ -18,6 +18,7 @@
 #include "dawn_native/CommandAllocator.h"
 #include "dawn_native/CommandBuffer.h"
 
+#include "dawn_native/d3d12/InputStateD3D12.h"
 #include "dawn_native/d3d12/d3d12_platform.h"
 
 namespace dawn_native { namespace d3d12 {
@@ -27,6 +28,17 @@ namespace dawn_native { namespace d3d12 {
 
     struct BindGroupStateTracker;
 
+    struct VertexBuffersInfo {
+        // startSlot and endSlot indicate the range of dirty vertex buffers.
+        // If there are multiple calls to SetVertexBuffers, the start and end
+        // represent the union of the dirty ranges (the union may have non-dirty
+        // data in the middle of the range).
+        const InputState* lastInputState = nullptr;
+        uint32_t startSlot = kMaxVertexInputs;
+        uint32_t endSlot = 0;
+        std::array<D3D12_VERTEX_BUFFER_VIEW, kMaxVertexInputs> d3d12BufferViews = {};
+    };
+
     class CommandBuffer : public CommandBufferBase {
       public:
         CommandBuffer(CommandBufferBuilder* builder);
@@ -35,6 +47,9 @@ namespace dawn_native { namespace d3d12 {
         void RecordCommands(ComPtr<ID3D12GraphicsCommandList> commandList, uint32_t indexInSubmit);
 
       private:
+        void FlushSetVertexBuffers(ComPtr<ID3D12GraphicsCommandList> commandList,
+                                   VertexBuffersInfo* vertexBuffersInfo,
+                                   const InputState* inputState);
         void RecordComputePass(ComPtr<ID3D12GraphicsCommandList> commandList,
                                BindGroupStateTracker* bindingTracker);
         void RecordRenderPass(ComPtr<ID3D12GraphicsCommandList> commandList,

src/tests/end2end/InputStateTests.cpp

@@ -185,6 +185,10 @@ class InputStateTest : public DawnTest {
             dawn::CommandBuffer commands = builder.GetResult();
             queue.Submit(1, &commands);
 
+            CheckResult(triangles, instances);
+        }
+
+        void CheckResult(unsigned int triangles, unsigned int instances) {
             // Check that the center of each triangle is pure green, so that if a single vertex shader
             // instance fails, linear interpolation makes the pixel check fail.
             for (unsigned int triangle = 0; triangle < 4; triangle++) {
@@ -423,6 +427,89 @@ TEST_P(InputStateTest, MixedEverything) {
     DoTestDraw(pipeline, 1, 1, {{0, &buffer0}, {1, &buffer1}});
 }
 
+// Test input state is unaffected by unused vertex slot
+TEST_P(InputStateTest, UnusedVertexSlot) {
+    // Instance input state, using slot 1
+    dawn::InputState instanceInputState =
+        MakeInputState({{1, 4 * sizeof(float), InputStepMode::Instance}},
+                       {{0, 1, 0, VertexFormat::FloatR32G32B32A32}});
+    dawn::RenderPipeline instancePipeline = MakeTestPipeline(
+        instanceInputState, 1, {{0, VertexFormat::FloatR32G32B32A32, InputStepMode::Instance}});
+
+    dawn::Buffer buffer = MakeVertexBuffer<float>({
+        0, 1, 2, 3,
+        1, 2, 3, 4,
+        2, 3, 4, 5,
+        3, 4, 5, 6,
+    });
+
+    dawn::CommandBufferBuilder builder = device.CreateCommandBufferBuilder();
+
+    dawn::RenderPassEncoder pass = builder.BeginRenderPass(renderPass.renderPassInfo);
+
+    uint32_t zeroOffset = 0;
+    pass.SetVertexBuffers(0, 1, &buffer, &zeroOffset);
+    pass.SetVertexBuffers(1, 1, &buffer, &zeroOffset);
+
+    pass.SetPipeline(instancePipeline);
+    pass.Draw(1 * 3, 4, 0, 0);
+
+    pass.EndPass();
+
+    dawn::CommandBuffer commands = builder.GetResult();
+    queue.Submit(1, &commands);
+
+    CheckResult(1, 4);
+}
+
+// Test setting a different pipeline with a different input state.
+// This was a problem with the D3D12 backend where SetVertexBuffers
+// was getting the input from the last set pipeline, not the current.
+// SetVertexBuffers should be reapplied when the input state changes.
+TEST_P(InputStateTest, MultiplePipelinesMixedInputState) {
+    // Basic input state, using slot 0
+    dawn::InputState vertexInputState =
+        MakeInputState({{0, 4 * sizeof(float), InputStepMode::Vertex}},
+                       {{0, 0, 0, VertexFormat::FloatR32G32B32A32}});
+    dawn::RenderPipeline vertexPipeline = MakeTestPipeline(
+        vertexInputState, 1, {{0, VertexFormat::FloatR32G32B32A32, InputStepMode::Vertex}});
+
+    // Instance input state, using slot 1
+    dawn::InputState instanceInputState =
+        MakeInputState({{1, 4 * sizeof(float), InputStepMode::Instance}},
+                       {{0, 1, 0, VertexFormat::FloatR32G32B32A32}});
+    dawn::RenderPipeline instancePipeline = MakeTestPipeline(
+        instanceInputState, 1, {{0, VertexFormat::FloatR32G32B32A32, InputStepMode::Instance}});
+
+    dawn::Buffer buffer = MakeVertexBuffer<float>({
+        0, 1, 2, 3,
+        1, 2, 3, 4,
+        2, 3, 4, 5,
+        3, 4, 5, 6,
+    });
+
+    dawn::CommandBufferBuilder builder = device.CreateCommandBufferBuilder();
+
+    dawn::RenderPassEncoder pass = builder.BeginRenderPass(renderPass.renderPassInfo);
+
+    uint32_t zeroOffset = 0;
+    pass.SetVertexBuffers(0, 1, &buffer, &zeroOffset);
+    pass.SetVertexBuffers(1, 1, &buffer, &zeroOffset);
+
+    pass.SetPipeline(vertexPipeline);
+    pass.Draw(1 * 3, 1, 0, 0);
+
+    pass.SetPipeline(instancePipeline);
+    pass.Draw(1 * 3, 4, 0, 0);
+
+    pass.EndPass();
+
+    dawn::CommandBuffer commands = builder.GetResult();
+    queue.Submit(1, &commands);
+
+    CheckResult(1, 4);
+}
+
 DAWN_INSTANTIATE_TEST(InputStateTest, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend)
 
 // TODO for the input state: