diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp index 302d98115f..50c5236f3a 100644 --- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp +++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp @@ -326,20 +326,12 @@ namespace dawn_native { namespace d3d12 { mInCompute = inCompute_; } - void OnSetPipeline(PipelineBase* pipeline) { - // Invalidate the root sampler tables previously set in the root signature. - // This is because changing the pipeline layout also changes the root signature. - const PipelineLayout* pipelineLayout = ToBackend(pipeline->GetLayout()); - if (mLastAppliedPipelineLayout != pipelineLayout) { - mBoundRootSamplerTables = {}; - } - - Base::OnSetPipeline(pipeline); - } - MaybeError Apply(CommandRecordingContext* commandContext) { BeforeApply(); + ID3D12GraphicsCommandList* commandList = commandContext->GetCommandList(); + UpdateRootSignatureIfNecessary(commandList); + // Bindgroups are allocated in shader-visible descriptor heaps which are managed by a // ringbuffer. There can be a single shader-visible descriptor heap of each type bound // at any given time. This means that when we switch heaps, all other currently bound @@ -358,8 +350,6 @@ namespace dawn_native { namespace d3d12 { } } - ID3D12GraphicsCommandList* commandList = commandContext->GetCommandList(); - if (!didCreateBindGroupViews || !didCreateBindGroupSamplers) { if (!didCreateBindGroupViews) { DAWN_TRY(mViewAllocator->AllocateAndSwitchShaderVisibleHeap()); @@ -406,6 +396,20 @@ namespace dawn_native { namespace d3d12 { } private: + void UpdateRootSignatureIfNecessary(ID3D12GraphicsCommandList* commandList) { + if (mLastAppliedPipelineLayout != mPipelineLayout) { + if (mInCompute) { + commandList->SetComputeRootSignature( + ToBackend(mPipelineLayout)->GetRootSignature()); + } else { + commandList->SetGraphicsRootSignature( + ToBackend(mPipelineLayout)->GetRootSignature()); + } + // Invalidate the root sampler tables previously set in the root signature. + mBoundRootSamplerTables = {}; + } + } + void ApplyBindGroup(ID3D12GraphicsCommandList* commandList, const PipelineLayout* pipelineLayout, BindGroupIndex index, @@ -1036,9 +1040,7 @@ namespace dawn_native { namespace d3d12 { case Command::SetComputePipeline: { SetComputePipelineCmd* cmd = mCommands.NextCommand(); ComputePipeline* pipeline = ToBackend(cmd->pipeline).Get(); - PipelineLayout* layout = ToBackend(pipeline->GetLayout()); - commandList->SetComputeRootSignature(layout->GetRootSignature()); commandList->SetPipelineState(pipeline->GetPipelineState()); bindingTracker->OnSetPipeline(pipeline); @@ -1305,7 +1307,6 @@ namespace dawn_native { namespace d3d12 { } RenderPipeline* lastPipeline = nullptr; - PipelineLayout* lastLayout = nullptr; VertexBufferTracker vertexBufferTracker = {}; auto EncodeRenderBundleCommand = [&](CommandIterator* iter, Command type) -> MaybeError { @@ -1403,16 +1404,13 @@ namespace dawn_native { namespace d3d12 { case Command::SetRenderPipeline: { SetRenderPipelineCmd* cmd = iter->NextCommand(); RenderPipeline* pipeline = ToBackend(cmd->pipeline).Get(); - PipelineLayout* layout = ToBackend(pipeline->GetLayout()); - commandList->SetGraphicsRootSignature(layout->GetRootSignature()); commandList->SetPipelineState(pipeline->GetPipelineState()); commandList->IASetPrimitiveTopology(pipeline->GetD3D12PrimitiveTopology()); bindingTracker->OnSetPipeline(pipeline); lastPipeline = pipeline; - lastLayout = layout; break; } diff --git a/src/tests/end2end/BindGroupTests.cpp b/src/tests/end2end/BindGroupTests.cpp index 5b8c5fe192..90ecbed2d2 100644 --- a/src/tests/end2end/BindGroupTests.cpp +++ b/src/tests/end2end/BindGroupTests.cpp @@ -758,9 +758,6 @@ TEST_P(BindGroupTests, DrawThenChangePipelineAndBindGroup) { // Test for crbug.com/dawn/1049, where setting a pipeline without drawing can prevent // bind groups from being applied later TEST_P(BindGroupTests, DrawThenChangePipelineTwiceAndBindGroup) { - // TODO(crbug.com/dawn/1055) find out why this test fails on Windows Intel D3D12 drivers. - DAWN_SUPPRESS_TEST_IF(IsIntel() && IsWindows() && IsD3D12()); - utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, kRTSize, kRTSize); // Create a bind group layout which uses a single dynamic uniform buffer. @@ -810,7 +807,7 @@ TEST_P(BindGroupTests, DrawThenChangePipelineTwiceAndBindGroup) { wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo); - // Set the pipeline to (uniform, uniform, storage) + // Set the pipeline to (uniform, uniform, uniform) pass.SetPipeline(pipeline0); // Set the first bind group to color0 in the dynamic uniform buffer. @@ -841,8 +838,9 @@ TEST_P(BindGroupTests, DrawThenChangePipelineTwiceAndBindGroup) { // Revert to pipeline 0 pass.SetPipeline(pipeline0); - // Internally this will not re-apply the bind groups, because we already - // drew with this pipeline (setting pipeline 1 did not dirty the bind groups). + // Internally this should re-apply bind group 2. Because we already + // drew with this pipeline, and setting pipeline 1 did not dirty the bind groups, + // bind groups 0 and 1 should still be valid. pass.Draw(3); pass.EndPass();