From c2542cde3e70aa2f4066b4cdbea91e4195f85c44 Mon Sep 17 00:00:00 2001 From: Yunchao He Date: Mon, 11 May 2020 19:12:12 +0000 Subject: [PATCH] Fix a bug for multi-writes in ResourceUsageTrackingTests.cpp It is valid to have race condition for multiple writes on the same resource in some situations in render pass. These situations are: 1) multple storage buffer bindings on the same buffer, 2) multiple writeonly storage texture bindings on the same texture. This change fixed a bug in tests and added a new test, in order to make sure that validation code in Dawn allows this kind race condition. Bug: dawn:407 Change-Id: I42332418bea5b6e608f6730e42f60c1c12b0b025 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/21361 Reviewed-by: Corentin Wallez Reviewed-by: Austin Eng Commit-Queue: Yunchao He --- .../validation/ResourceUsageTrackingTests.cpp | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp b/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp index e24af1ffba..33727fe8e1 100644 --- a/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp +++ b/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp @@ -183,8 +183,7 @@ namespace { } } - // Test that it is invalid to use the same buffer as multiple writable usages in the same - // render pass. It is invalid in the same dispatch in compute pass. + // Test using multiple writable usages on the same buffer in a single pass/dispatch TEST_F(ResourceUsageTrackingTest, BufferWithMultipleWriteUsage) { // Create buffer and bind group wgpu::Buffer buffer = CreateBuffer(512, wgpu::BufferUsage::Storage); @@ -199,14 +198,13 @@ namespace { // test render pass { - // It is invalid to use the buffer as multiple writeable usages in render pass + // It is valid to use multiple storage usages on the same buffer in render pass wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); DummyRenderPass dummyRenderPass(device); wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&dummyRenderPass); - pass.SetIndexBuffer(buffer); pass.SetBindGroup(0, bg); pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); + encoder.Finish(); } // test compute pass @@ -967,8 +965,7 @@ namespace { } } - // Test that it is invalid to use the same texture as multiple writable usages in the same - // render pass. It is invalid in the same dispatch in compute pass. + // Test using multiple writable usages on the same texture in a single pass/dispatch TEST_F(ResourceUsageTrackingTest, TextureWithMultipleWriteUsage) { // Test render pass { @@ -986,16 +983,31 @@ namespace { kFormat}}); wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, view}}); - // Create a bind group to use the texture as render target - utils::ComboRenderPassDescriptor renderPass({view}); - // It is invalid to use the texture as both writeonly storage and render target in // the same pass - wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); - wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); - pass.SetBindGroup(0, bg); - pass.EndPass(); - ASSERT_DEVICE_ERROR(encoder.Finish()); + { + utils::ComboRenderPassDescriptor renderPass({view}); + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass); + pass.SetBindGroup(0, bg); + pass.EndPass(); + ASSERT_DEVICE_ERROR(encoder.Finish()); + } + + // It is valid to use multiple writeonly storage usages on the same texture in render + // pass + { + wgpu::BindGroup bg1 = utils::MakeBindGroup(device, bgl, {{0, view}}); + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + DummyRenderPass dummyRenderPass(device); + wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&dummyRenderPass); + pass.SetBindGroup(0, bg); + pass.SetBindGroup(1, bg1); + pass.EndPass(); + encoder.Finish(); + } } // Test compute pass