commit 3f06587542d5859aa8031facf1801efd79f57e65
author Stephen White <senorblanco@chromium.org> Fri Oct 26 11:49:12 2018 +0000
committer Commit Bot service account <commit-bot@chromium.org> Fri Oct 26 11:49:12 2018 +0000
tree 51a8faf52d0c981006df54420c0dee5123a2fef2
parent 56e5974c2c973d2450f32671810ea8db6414718f

Fix D3D12 Buffer::UnmapImpl() for MapWriteAsync().

d3d12::Buffer::UnmapImpl() was passing an empty range to
ID3D12Resource::Unmap().  This causes it to be a no-op when writing a
buffer.  The fix is to store the entire mapped range in the Buffer,
and use it during Unmap().

Note: this doesn't seem to actually cause an issue on the D3D12 drivers I
have, so I have been unable to write a test for it. However, it does cause
an issue in RenderDoc: the buffer data appears unmodifed after the
Map/memcpy/Unmap calls. The hypothesis is that RenderDoc notices the empty
range at the API level, and leaves its shadow buffer contents unmodified.

Because I feel this change is correct regardless, I'm landing it without
a test.

Bug: dawn:25
Change-Id: Ie5dd5fcfedbe1d80c75a3d8094c97af27653ee00
Reviewed-on: https://dawn-review.googlesource.com/c/1920
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>

src/dawn_native/d3d12/BufferD3D12.cpp

diff --git a/src/dawn_native/d3d12/BufferD3D12.cpp b/src/dawn_native/d3d12/BufferD3D12.cpp
index dd67467..71b6b40 100644
--- a/src/dawn_native/d3d12/BufferD3D12.cpp
+++ b/src/dawn_native/d3d12/BufferD3D12.cpp
@@ -169,6 +169,7 @@
     }
 
     void Buffer::MapReadAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) {
+        mWrittenMappedRange = {};
         D3D12_RANGE readRange = {start, start + count};
         char* data = nullptr;
         ASSERT_SUCCESS(mResource->Map(0, &readRange, reinterpret_cast<void**>(&data)));
@@ -180,9 +181,9 @@
     }
 
     void Buffer::MapWriteAsyncImpl(uint32_t serial, uint32_t start, uint32_t count) {
-        D3D12_RANGE readRange = {start, start + count};
+        mWrittenMappedRange = {start, start + count};
         char* data = nullptr;
-        ASSERT_SUCCESS(mResource->Map(0, &readRange, reinterpret_cast<void**>(&data)));
+        ASSERT_SUCCESS(mResource->Map(0, &mWrittenMappedRange, reinterpret_cast<void**>(&data)));
 
         // There is no need to transition the resource to a new state: D3D12 seems to make the CPU
         // writes available on queue submission.
@@ -191,11 +192,9 @@
     }
 
     void Buffer::UnmapImpl() {
-        // TODO(enga@google.com): When MapWrite is implemented, this should state the range that was
-        // modified
-        D3D12_RANGE writeRange = {};
-        mResource->Unmap(0, &writeRange);
+        mResource->Unmap(0, &mWrittenMappedRange);
         ToBackend(GetDevice())->GetResourceAllocator()->Release(mResource);
+        mWrittenMappedRange = {};
     }
 
     BufferView::BufferView(BufferViewBuilder* builder) : BufferViewBase(builder) {

src/dawn_native/d3d12/BufferD3D12.h

diff --git a/src/dawn_native/d3d12/BufferD3D12.h b/src/dawn_native/d3d12/BufferD3D12.h
index 4ff93ba..9965d49 100644
--- a/src/dawn_native/d3d12/BufferD3D12.h
+++ b/src/dawn_native/d3d12/BufferD3D12.h
@@ -47,6 +47,7 @@
         ComPtr<ID3D12Resource> mResource;
         bool mFixedResourceState = false;
         dawn::BufferUsageBit mLastUsage = dawn::BufferUsageBit::None;
+        D3D12_RANGE mWrittenMappedRange;
     };
 
     class BufferView : public BufferViewBase {