From 300ec34c72722e38c5bf61cb0febc677384d7276 Mon Sep 17 00:00:00 2001 From: Peng Huang Date: Fri, 5 May 2023 21:50:39 +0000 Subject: [PATCH] d3d11: fix indirect draw and dispatch Bug: dawn:1716 Bug: dawn:1791 Change-Id: I6027a9ab8b33bfaf80d32300eed11fe7df5135e3 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/131760 Reviewed-by: Austin Eng Kokoro: Kokoro Commit-Queue: Peng Huang --- src/dawn/native/d3d11/CommandBufferD3D11.cpp | 52 +++++++++++++++---- .../native/d3d11/ComputePipelineD3D11.cpp | 4 ++ src/dawn/native/d3d11/ComputePipelineD3D11.h | 2 + .../tests/end2end/ComputeDispatchTests.cpp | 7 ++- .../end2end/DrawIndexedIndirectTests.cpp | 4 ++ 5 files changed, 54 insertions(+), 15 deletions(-) diff --git a/src/dawn/native/d3d11/CommandBufferD3D11.cpp b/src/dawn/native/d3d11/CommandBufferD3D11.cpp index 3b92b3a4f6..d70f2993f2 100644 --- a/src/dawn/native/d3d11/CommandBufferD3D11.cpp +++ b/src/dawn/native/d3d11/CommandBufferD3D11.cpp @@ -309,16 +309,21 @@ MaybeError CommandBuffer::ExecuteComputePass(CommandRecordingContext* commandCon } case Command::DispatchIndirect: { - // TODO(1716): figure how to update num workgroups builtins DispatchIndirectCmd* dispatch = mCommands.NextCommand(); DAWN_TRY(bindGroupTracker.Apply()); - uint64_t indirectBufferOffset = dispatch->indirectOffset; Buffer* indirectBuffer = ToBackend(dispatch->indirectBuffer.Get()); + if (lastPipeline->UsesNumWorkgroups()) { + // Copy indirect args into the uniform buffer for built-in workgroup variables. + DAWN_TRY(Buffer::Copy(commandContext, indirectBuffer, dispatch->indirectOffset, + sizeof(uint32_t) * 3, commandContext->GetUniformBuffer(), + 0)); + } + commandContext->GetD3D11DeviceContext()->DispatchIndirect( - indirectBuffer->GetD3D11Buffer(), indirectBufferOffset); + indirectBuffer->GetD3D11Buffer(), dispatch->indirectOffset); break; } @@ -470,29 +475,49 @@ MaybeError CommandBuffer::ExecuteRenderPass(BeginRenderPassCmd* renderPass, } case Command::DrawIndirect: { - // TODO(dawn:1716): figure how to setup built-in variables for indirect draw. DrawIndirectCmd* draw = iter->NextCommand(); - DAWN_TRY(bindGroupTracker.Apply()); - uint64_t indirectBufferOffset = draw->indirectOffset; Buffer* indirectBuffer = ToBackend(draw->indirectBuffer.Get()); ASSERT(indirectBuffer != nullptr); + DAWN_TRY(bindGroupTracker.Apply()); + + if (lastPipeline->GetUsesVertexOrInstanceIndex()) { + // Copy StartVertexLocation and StartInstanceLocation into the uniform buffer + // for built-in variables. + uint64_t offset = + draw->indirectOffset + + offsetof(D3D11_DRAW_INSTANCED_INDIRECT_ARGS, StartVertexLocation); + DAWN_TRY(Buffer::Copy(commandContext, indirectBuffer, offset, + sizeof(uint32_t) * 2, commandContext->GetUniformBuffer(), + 0)); + } + commandContext->GetD3D11DeviceContext()->DrawInstancedIndirect( - indirectBuffer->GetD3D11Buffer(), indirectBufferOffset); + indirectBuffer->GetD3D11Buffer(), draw->indirectOffset); break; } case Command::DrawIndexedIndirect: { - // TODO(dawn:1716): figure how to setup built-in variables for indirect draw. DrawIndexedIndirectCmd* draw = iter->NextCommand(); - DAWN_TRY(bindGroupTracker.Apply()); - Buffer* indirectBuffer = ToBackend(draw->indirectBuffer.Get()); ASSERT(indirectBuffer != nullptr); + DAWN_TRY(bindGroupTracker.Apply()); + + if (lastPipeline->GetUsesVertexOrInstanceIndex()) { + // Copy StartVertexLocation and StartInstanceLocation into the uniform buffer + // for built-in variables. + uint64_t offset = + draw->indirectOffset + + offsetof(D3D11_DRAW_INDEXED_INSTANCED_INDIRECT_ARGS, BaseVertexLocation); + DAWN_TRY(Buffer::Copy(commandContext, indirectBuffer, offset, + sizeof(uint32_t) * 2, commandContext->GetUniformBuffer(), + 0)); + } + commandContext->GetD3D11DeviceContext()->DrawIndexedInstancedIndirect( indirectBuffer->GetD3D11Buffer(), draw->indirectOffset); @@ -701,7 +726,12 @@ MaybeError CommandBuffer::RecordFirstIndexOffset(RenderPipeline* renderPipeline, MaybeError CommandBuffer::RecordNumWorkgroupsForDispatch(ComputePipeline* computePipeline, CommandRecordingContext* commandContext, DispatchCmd* dispatchCmd) { - // TODO(dawn:1705): only update the uniform buffer when the value changes. + if (!computePipeline->UsesNumWorkgroups()) { + // Workgroup size is not used in shader, so we don't need to update the uniform buffer. The + // original value in the uniform buffer will not be used, so we don't need to clear it. + return {}; + } + uint32_t data[4] = { dispatchCmd->x, dispatchCmd->y, diff --git a/src/dawn/native/d3d11/ComputePipelineD3D11.cpp b/src/dawn/native/d3d11/ComputePipelineD3D11.cpp index 1d50d59d76..d648d33c04 100644 --- a/src/dawn/native/d3d11/ComputePipelineD3D11.cpp +++ b/src/dawn/native/d3d11/ComputePipelineD3D11.cpp @@ -88,4 +88,8 @@ void ComputePipeline::InitializeAsync(Ref computePipeline, CreateComputePipelineAsyncTask::RunAsync(std::move(asyncTask)); } +bool ComputePipeline::UsesNumWorkgroups() const { + return GetStage(SingleShaderStage::Compute).metadata->usesNumWorkgroups; +} + } // namespace dawn::native::d3d11 diff --git a/src/dawn/native/d3d11/ComputePipelineD3D11.h b/src/dawn/native/d3d11/ComputePipelineD3D11.h index fe4713f72f..dd0759bf6b 100644 --- a/src/dawn/native/d3d11/ComputePipelineD3D11.h +++ b/src/dawn/native/d3d11/ComputePipelineD3D11.h @@ -36,6 +36,8 @@ class ComputePipeline final : public ComputePipelineBase { MaybeError Initialize() override; + bool UsesNumWorkgroups() const; + private: using ComputePipelineBase::ComputePipelineBase; ~ComputePipeline() override; diff --git a/src/dawn/tests/end2end/ComputeDispatchTests.cpp b/src/dawn/tests/end2end/ComputeDispatchTests.cpp index 802ae741d6..005d32cb91 100644 --- a/src/dawn/tests/end2end/ComputeDispatchTests.cpp +++ b/src/dawn/tests/end2end/ComputeDispatchTests.cpp @@ -107,9 +107,6 @@ class ComputeDispatchTests : public DawnTest { void IndirectTest(std::vector indirectBufferData, uint64_t indirectOffset, bool useNumWorkgroups = true) { - // TODO(dawn:1791): fix indirect dispatch on D3D11 - DAWN_SUPPRESS_TEST_IF(IsD3D11()); - // Set up dst storage buffer to contain dispatch x, y, z wgpu::Buffer dst = utils::CreateBufferFromData( device, @@ -118,7 +115,7 @@ class ComputeDispatchTests : public DawnTest { wgpu::Buffer indirectBuffer = utils::CreateBufferFromData( device, &indirectBufferData[0], indirectBufferData.size() * sizeof(uint32_t), - wgpu::BufferUsage::Indirect); + wgpu::BufferUsage::Indirect | wgpu::BufferUsage::CopySrc); uint32_t indirectStart = indirectOffset / sizeof(uint32_t); @@ -174,6 +171,8 @@ class ComputeDispatchTests : public DawnTest { indirectBufferData.begin() + indirectStart + 3); } + // Verify the indirect buffer is not modified + EXPECT_BUFFER_U32_RANGE_EQ(&indirectBufferData[0], indirectBuffer, 0, 3); // Verify the dispatch got called with group counts in indirect buffer if all group counts // are not zero EXPECT_BUFFER_U32_RANGE_EQ(&expected[0], dst, 0, 3); diff --git a/src/dawn/tests/end2end/DrawIndexedIndirectTests.cpp b/src/dawn/tests/end2end/DrawIndexedIndirectTests.cpp index f87994f94a..6e1b17c9d8 100644 --- a/src/dawn/tests/end2end/DrawIndexedIndirectTests.cpp +++ b/src/dawn/tests/end2end/DrawIndexedIndirectTests.cpp @@ -303,6 +303,9 @@ TEST_P(DrawIndexedIndirectTest, ValidateMultipleDraws) { // TODO(dawn:1549) Fails on Qualcomm-based Android devices. DAWN_SUPPRESS_TEST_IF(IsAndroid() && IsQualcomm()); + // TODO(dawn:1791): Test fails with D3D11. + DAWN_SUPPRESS_TEST_IF(IsD3D11()); + // It doesn't make sense to test invalid inputs when validation is disabled. DAWN_SUPPRESS_TEST_IF(HasToggleEnabled("skip_validation")); @@ -710,6 +713,7 @@ TEST_P(DrawIndexedIndirectTest, ValidateReusedBundleWithChangingParams) { } DAWN_INSTANTIATE_TEST(DrawIndexedIndirectTest, + D3D11Backend(), D3D12Backend(), MetalBackend(), OpenGLBackend(),