D3D11: Fix bugs in Queue::WriteBuffer for mappable buffers.

Previously when Queue::WriteBuffer was called on a mappable buffer,
D3D11's GPUUsableBuffer would:
1. Mark the buffer as being used by the GPU.
2. Map the mappable storage.
3. Call WriteInternal().
  - WriteInternal() checks if the buffer's last usage serial is less
    than the completed serial to decide whether to map.
  - This check would always fail due to step 1, causing WriteInternal()
    to use a non-mappable storage instead of the mapped one.
  - This resulted in both mappable and non-mappable storages being
    modified in the same function, causing an assertion failure.

Fix by deferring MarkUsedInPendingCommands() to the end of Write(). This
ensures WriteInternal() can successfully map the buffer. Also remove
redundant mapping code from the base class' Write() & Clear() since
WriteInternal() already handles mapping.

Additionally, fix a bug where WriteBuffer with a full buffer
overwrite would skip storage sync but fail to update the revision
number, leaving the storage in an inconsistent state.

Finally, fix a bug in Queue::ForceEventualFlushOfCommands() where it
previously did nothing.

Bug: 345471009
Change-Id: I20d5dbbc386960853783dd1e75ed0cab24fd33d3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/285235
Commit-Queue: Quyen Le <lehoangquyen@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn/native/d3d11/BufferD3D11.cpp

diff --git a/src/dawn/native/d3d11/BufferD3D11.cpp b/src/dawn/native/d3d11/BufferD3D11.cpp
index 642aea1..3bbc8df 100644
--- a/src/dawn/native/d3d11/BufferD3D11.cpp
+++ b/src/dawn/native/d3d11/BufferD3D11.cpp
@@ -650,13 +650,9 @@
         return {};
     }
 
-    // Map the buffer if it is possible, so EnsureDataInitializedAsDestination() and ClearInternal()
-    // can write the mapped memory directly.
-    ScopedMap scopedMap;
-    DAWN_TRY_ASSIGN(scopedMap, ScopedMap::Create(commandContext, this, wgpu::MapMode::Write));
-
     // For non-staging buffers, we can use UpdateSubresource to write the data.
     DAWN_TRY(EnsureDataInitializedAsDestination(commandContext, offset, size));
+
     return ClearInternal(commandContext, clearValue, offset, size);
 }
 
@@ -695,14 +691,9 @@
                          size_t size) {
     DAWN_ASSERT(size != 0);
 
-    MarkUsedInPendingCommands();
-    // Map the buffer if it is possible, so EnsureDataInitializedAsDestination() and WriteInternal()
-    // can write the mapped memory directly.
-    ScopedMap scopedMap;
-    DAWN_TRY_ASSIGN(scopedMap, ScopedMap::Create(commandContext, this, wgpu::MapMode::Write));
-
     // For non-staging buffers, we can use UpdateSubresource to write the data.
     DAWN_TRY(EnsureDataInitializedAsDestination(commandContext, offset, size));
+
     return WriteInternal(commandContext, offset, data, size);
 }
 
@@ -1424,12 +1415,16 @@
     // transfer the data to constant buffer.
     Ref<BufferBase> stagingBuffer;
     DAWN_TRY_ASSIGN(stagingBuffer, ToBackend(GetDevice())->GetStagingBuffer(commandContext, size));
-    stagingBuffer->MarkUsedInPendingCommands();
     DAWN_TRY(ToBackend(stagingBuffer)->WriteInternal(commandContext, 0, data, size));
     DAWN_TRY(ToBackend(stagingBuffer.Get())
                  ->CopyToInternal(commandContext,
                                   /*sourceOffset=*/0,
                                   /*size=*/size, this, offset));
+    // WriteInternal() might not call MarkUsedInPendingCommands() if the staging buffer is mappable.
+    // But we need to mark buffer as being used for CopyToInternal().
+    // TODO(crbug.com/345471009): Consider whether it's OK to change CopyToInternal() to
+    // automatically trigger MarkUsedInPendingCommands().
+    stagingBuffer->MarkUsedInPendingCommands();
     ToBackend(GetDevice())->ReturnStagingBuffer(std::move(stagingBuffer));
 
     return {};
@@ -1455,6 +1450,10 @@
         return {};
     }
 
+    // Mark the buffer as used in pending commands if the mapping path above wasn't taken.
+    // Mapped writes complete synchronously and don't require tracking.
+    MarkUsedInPendingCommands();
+
     // WriteInternal() can be called with GetAllocatedSize(). We treat it as a full buffer write
     // as well.
     bool fullSizeWrite = size >= GetSize() && offset == 0;
@@ -1468,7 +1467,12 @@
         DAWN_TRY_ASSIGN(gpuCopyableStorage, GetOrCreateDstCopyableStorage());
     }
 
-    if (!fullSizeWrite) {
+    if (fullSizeWrite) {
+        // If this is a full overwrite, no need to copy the old content.
+        // Just need to copy the revision number.
+        DAWN_ASSERT(mLastUpdatedStorage);
+        gpuCopyableStorage->SetRevision(mLastUpdatedStorage->GetRevision());
+    } else {
         DAWN_TRY(SyncStorage(commandContext, gpuCopyableStorage));
     }
 

src/dawn/native/d3d11/QueueD3D11.cpp

diff --git a/src/dawn/native/d3d11/QueueD3D11.cpp b/src/dawn/native/d3d11/QueueD3D11.cpp
index 2b298e9..a6a9769 100644
--- a/src/dawn/native/d3d11/QueueD3D11.cpp
+++ b/src/dawn/native/d3d11/QueueD3D11.cpp
@@ -393,7 +393,9 @@
     return mPendingCommandsNeedSubmit.load(std::memory_order_acquire);
 }
 
-void Queue::ForceEventualFlushOfCommands() {}
+void Queue::ForceEventualFlushOfCommands() {
+    mPendingCommandsNeedSubmit.store(true, std::memory_order_release);
+}
 
 MaybeError Queue::WaitForIdleForDestructionImpl() {
     if (!mPendingCommands->IsValid()) {

src/dawn/tests/end2end/BufferTests.cpp

diff --git a/src/dawn/tests/end2end/BufferTests.cpp b/src/dawn/tests/end2end/BufferTests.cpp
index 8318b8c..f05e45c 100644
--- a/src/dawn/tests/end2end/BufferTests.cpp
+++ b/src/dawn/tests/end2end/BufferTests.cpp
@@ -812,7 +812,9 @@
 
     // With Vulkan Queue::WriteBuffers() doesn't encode any commands which need to be waited on so
     // MapAsync() can happen immediately. On other platforms that isn't the case.
-    bool mapCompletesFirst = IsVulkan() && !IsWebGPUOnWebGPU();
+    // Similarly, for MapRead buffers, D3D11's Queue::WriteBuffers() also doesn't encode any
+    // commands.
+    bool mapCompletesFirst = (IsVulkan() || IsD3D11()) && !IsWebGPUOnWebGPU();
 
     // 1. submission without using buffer.
     SubmitCommandBuffer({});
@@ -1607,6 +1609,21 @@
     }
 }
 
+// Test that Queue.WriteBuffer works to update a mappable buffer.
+TEST_P(BufferMapExtendedUsagesTests, QueueWriteMappableBuffer) {
+    wgpu::BufferDescriptor descriptor;
+    descriptor.size = 4;
+    descriptor.usage = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopyDst |
+                       wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::Uniform;
+    wgpu::Buffer buffer = device.CreateBuffer(&descriptor);
+
+    uint32_t myData = 0x12345678;
+    constexpr size_t kSize = sizeof(myData);
+    queue.WriteBuffer(buffer, 0, &myData, kSize);
+
+    EXPECT_BUFFER_U32_EQ(myData, buffer, 0);
+}
+
 // Test that mapping a vertex buffer, modifying the data then draw with the buffer works.
 TEST_P(BufferMapExtendedUsagesTests, MapWriteVertexBufferAndDraw) {
     const utils::RGBA8 kReds[] = {utils::RGBA8::kRed, utils::RGBA8::kRed, utils::RGBA8::kRed};