From 3f06587542d5859aa8031facf1801efd79f57e65 Mon Sep 17 00:00:00 2001 From: Stephen White Date: Fri, 26 Oct 2018 11:49:12 +0000 Subject: [PATCH] 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 Reviewed-by: Corentin Wallez Commit-Queue: Stephen White --- src/dawn_native/d3d12/BufferD3D12.cpp | 11 +++++------ src/dawn_native/d3d12/BufferD3D12.h | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dawn_native/d3d12/BufferD3D12.cpp b/src/dawn_native/d3d12/BufferD3D12.cpp index dd67467d0a..71b6b400e5 100644 --- a/src/dawn_native/d3d12/BufferD3D12.cpp +++ b/src/dawn_native/d3d12/BufferD3D12.cpp @@ -169,6 +169,7 @@ namespace dawn_native { namespace d3d12 { } 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(&data))); @@ -180,9 +181,9 @@ namespace dawn_native { namespace d3d12 { } 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(&data))); + ASSERT_SUCCESS(mResource->Map(0, &mWrittenMappedRange, reinterpret_cast(&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 @@ namespace dawn_native { namespace d3d12 { } 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) { diff --git a/src/dawn_native/d3d12/BufferD3D12.h b/src/dawn_native/d3d12/BufferD3D12.h index 4ff93ba4b1..9965d49e77 100644 --- a/src/dawn_native/d3d12/BufferD3D12.h +++ b/src/dawn_native/d3d12/BufferD3D12.h @@ -47,6 +47,7 @@ namespace dawn_native { namespace d3d12 { ComPtr mResource; bool mFixedResourceState = false; dawn::BufferUsageBit mLastUsage = dawn::BufferUsageBit::None; + D3D12_RANGE mWrittenMappedRange; }; class BufferView : public BufferViewBase {