From 1b01b665d7cf987b31642e8d159fc7e133f6fe2c Mon Sep 17 00:00:00 2001
From: jchen10 <jie.a.chen@intel.com>
Date: Tue, 2 May 2023 08:36:10 +0000
Subject: [PATCH] Workaround for UpdateSubresource1 16-byte alignment

In case of misalignment, we write to a temp staging buffer first,
and then copy to the dest buffer using CopySubresourceRegion.

Bug: dawn:1776
Bug: dawn:1705
Change-Id: Iba44dd79d9ee4b02f8cc83263bcfc911e1cce136
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/130140
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Jie A Chen <jie.a.chen@intel.com>
---
 src/dawn/native/d3d11/BufferD3D11.cpp     | 56 +++++++++++++++++------
 src/dawn/tests/end2end/BindGroupTests.cpp |  6 ---
 2 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/src/dawn/native/d3d11/BufferD3D11.cpp b/src/dawn/native/d3d11/BufferD3D11.cpp
index b7e894c080..b399870bef 100644
--- a/src/dawn/native/d3d11/BufferD3D11.cpp
+++ b/src/dawn/native/d3d11/BufferD3D11.cpp
@@ -445,23 +445,51 @@ MaybeError Buffer::WriteInternal(CommandRecordingContext* commandContext,
         return {};
     }
 
-    D3D11_BOX dstBox;
-    dstBox.left = offset;
-    dstBox.right = offset + size;
-    dstBox.top = 0;
-    dstBox.bottom = 1;
-    dstBox.front = 0;
-    dstBox.back = 1;
+    D3D11_BOX box;
+    box.left = offset;
+    box.right = offset + size;
+    box.top = 0;
+    box.bottom = 1;
+    box.front = 0;
+    box.back = 1;
 
-    // TODO(dawn:1739): check whether driver supports partial update of uniform buffer.
     if ((GetUsage() & wgpu::BufferUsage::Uniform)) {
-        d3d11DeviceContext1->UpdateSubresource1(GetD3D11Buffer(), /*DstSubresource=*/0, &dstBox,
-                                                data,
-                                                /*SrcRowPitch=*/0,
-                                                /*SrcDepthPitch*/ 0, D3D11_COPY_NO_OVERWRITE);
+        if (!IsAligned(box.left, 16) || !IsAligned(box.right, 16)) {
+            // Create a temp staging buffer to workaround the alignment issue.
+            BufferDescriptor descriptor;
+            descriptor.size = box.right - box.left;
+            DAWN_ASSERT(IsAligned(descriptor.size, 4));
+            descriptor.usage = wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc;
+            descriptor.mappedAtCreation = false;
+            descriptor.label = "temp staging buffer";
+            Ref<BufferBase> stagingBufferBase;
+            DAWN_TRY_ASSIGN(stagingBufferBase, GetDevice()->CreateBuffer(&descriptor));
+            Ref<Buffer> stagingBuffer;
+            stagingBuffer = ToBackend(std::move(stagingBufferBase));
+            {
+                ScopedMap scopedMap;
+                DAWN_TRY_ASSIGN(scopedMap, ScopedMap::Create(stagingBuffer.Get()));
+                uint8_t* mappedData = scopedMap.GetMappedData();
+                DAWN_ASSERT(mappedData);
+                memcpy(mappedData, data, size);
+            }
+            box.left = 0;
+            box.right = descriptor.size;
+            commandContext->GetD3D11DeviceContext()->CopySubresourceRegion(
+                GetD3D11Buffer(), /*DstSubresource=*/0, /*DstX=*/offset,
+                /*DstY=*/0,
+                /*DstZ=*/0, stagingBuffer->GetD3D11Buffer(), /*SrcSubresource=*/0, &box);
+            stagingBuffer = nullptr;
+
+        } else {
+            // TODO(dawn:1739): check whether driver supports partial update of uniform buffer.
+            d3d11DeviceContext1->UpdateSubresource1(GetD3D11Buffer(), /*DstSubresource=*/0, &box,
+                                                    data,
+                                                    /*SrcRowPitch=*/0,
+                                                    /*SrcDepthPitch*/ 0, D3D11_COPY_NO_OVERWRITE);
+        }
     } else {
-        d3d11DeviceContext1->UpdateSubresource(GetD3D11Buffer(), /*DstSubresource=*/0, &dstBox,
-                                               data,
+        d3d11DeviceContext1->UpdateSubresource(GetD3D11Buffer(), /*DstSubresource=*/0, &box, data,
                                                /*SrcRowPitch=*/0,
                                                /*SrcDepthPitch*/ 0);
     }
diff --git a/src/dawn/tests/end2end/BindGroupTests.cpp b/src/dawn/tests/end2end/BindGroupTests.cpp
index 999023aa1f..f4551e7284 100644
--- a/src/dawn/tests/end2end/BindGroupTests.cpp
+++ b/src/dawn/tests/end2end/BindGroupTests.cpp
@@ -994,9 +994,6 @@ TEST_P(BindGroupTests, DrawThenChangePipelineTwiceAndBindGroup) {
 // Regression test for crbug.com/dawn/408 where dynamic offsets were applied in the wrong order.
 // Dynamic offsets should be applied in increasing order of binding number.
 TEST_P(BindGroupTests, DynamicOffsetOrder) {
-    // TODO(dawn:1776): Fix the UpdateSubresource1 16-byte alignment.
-    DAWN_SUPPRESS_TEST_IF(IsD3D11());
-
     // We will put the following values and the respective offsets into a buffer.
     // The test will ensure that the correct dynamic offset is applied to each buffer by reading the
     // value from an offset binding.
@@ -1082,9 +1079,6 @@ TEST_P(BindGroupTests, DynamicAndNonDynamicBindingsDoNotConflictAfterRemapping)
     // // TODO(crbug.com/dawn/1106): Test output is wrong on D3D12 using WARP.
     DAWN_SUPPRESS_TEST_IF(IsWARP());
 
-    // TODO(dawn:1776): Fix the UpdateSubresource1 16-byte alignment.
-    DAWN_SUPPRESS_TEST_IF(IsD3D11());
-
     auto RunTestWith = [&](bool dynamicBufferFirst) {
         uint32_t dynamicBufferBindingNumber = dynamicBufferFirst ? 0 : 1;
         uint32_t bufferBindingNumber = dynamicBufferFirst ? 1 : 0;