Capture: Handle unmap by dirty flag.
We can't capture on Unmap if the buffer is new
(mappedOnCreation) because calling CaptureUnmappedBuffer
will end up calling AddResource which will try to capture
the buffer's contents. We can't capture the buffer's
contents because it's currently mapped.
Instead, mark the buffer as "needs capture" and
capture it again later if it's used.
Bug: 477349135
Change-Id: I6a6a696466378f70100cdb8973761f00d974f5e3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/285935
Reviewed-by: Shrek Shao <shrekshao@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Gregg Tavares <gman@chromium.org>
diff --git a/src/dawn/native/webgpu/BufferWGPU.cpp b/src/dawn/native/webgpu/BufferWGPU.cpp
index c5ee6fb..81070a3 100644
--- a/src/dawn/native/webgpu/BufferWGPU.cpp
+++ b/src/dawn/native/webgpu/BufferWGPU.cpp
@@ -161,11 +161,10 @@
if (IsMappedState(oldState) && MapMode() == wgpu::MapMode::Write &&
newState != BufferState::Destroyed) {
- CaptureContext* captureContext = ToBackend(GetDevice()->GetQueue())->GetCaptureContext();
- if (captureContext != nullptr) {
- [[maybe_unused]] auto result =
- captureContext->CaptureUnmapBuffer(this, MapOffset(), mMappedData, MapSize());
- }
+ // TODO(477349135): Optimize this by tracking the ranges. As it is we'll
+ // capture the entire buffer even if only a few bytes were updated. Instead
+ // of mNeedsCapture we could have mDirtySpans. When size is 0 there's nothing to do.
+ mNeedsCapture = true;
}
if (mInnerHandle) {
@@ -204,16 +203,18 @@
// TODO(451338754): If it's a new resource and we know the buffer is all zero then don't
// capture.
wgpu::BufferUsage usage = GetUsage();
- bool unwritableOnPlayback = usage & wgpu::BufferUsage::MapWrite;
- if (!newResource || unwritableOnPlayback) {
+ if (!mNeedsCapture && !newResource) {
return {};
}
+
// A MapRead buffer is never used as input since it's only allowed CopyDst
// so we don't need its contents.
if (usage & wgpu::BufferUsage::MapRead) {
return {};
}
+ mNeedsCapture = false;
+
return AddContentToCapture(captureContext);
}
diff --git a/src/dawn/native/webgpu/BufferWGPU.h b/src/dawn/native/webgpu/BufferWGPU.h
index fcdc83f..2c1a2a1 100644
--- a/src/dawn/native/webgpu/BufferWGPU.h
+++ b/src/dawn/native/webgpu/BufferWGPU.h
@@ -63,6 +63,7 @@
MaybeError AddContentToCapture(CaptureContext& captureContext);
raw_ptr<void> mMappedData = nullptr;
+ bool mNeedsCapture = true;
};
} // namespace dawn::native::webgpu
diff --git a/src/dawn/native/webgpu/CaptureContext.cpp b/src/dawn/native/webgpu/CaptureContext.cpp
index 17820c2..773a299 100644
--- a/src/dawn/native/webgpu/CaptureContext.cpp
+++ b/src/dawn/native/webgpu/CaptureContext.cpp
@@ -129,25 +129,6 @@
return {};
}
-MaybeError CaptureContext::CaptureUnmapBuffer(Buffer* buffer,
- uint64_t bufferOffset,
- const void* data,
- size_t size) {
- schema::ObjectId id;
- DAWN_TRY_ASSIGN(id, AddResourceAndGetId(buffer));
- schema::RootCommandUnmapBufferCmd cmd{{
- .data = {{
- .bufferId = id,
- .bufferOffset = bufferOffset,
- .size = size,
- }},
- }};
-
- Serialize(*this, cmd);
- WriteContentBytes(data, size);
- return {};
-}
-
MaybeError CaptureContext::CaptureQueueWriteTexture(const TexelCopyTextureInfo& destination,
const void* data,
size_t dataSize,
diff --git a/src/dawn/native/webgpu/CaptureContext.h b/src/dawn/native/webgpu/CaptureContext.h
index 06d2c27..8233f79 100644
--- a/src/dawn/native/webgpu/CaptureContext.h
+++ b/src/dawn/native/webgpu/CaptureContext.h
@@ -177,10 +177,6 @@
uint64_t bufferOffset,
const void* data,
size_t size);
- MaybeError CaptureUnmapBuffer(Buffer* buffer,
- uint64_t bufferOffset,
- const void* data,
- size_t size);
MaybeError CaptureQueueWriteTexture(const TexelCopyTextureInfo& destination,
const void* data,
size_t dataSize,
diff --git a/src/dawn/replay/Replay.cpp b/src/dawn/replay/Replay.cpp
index 8bba788..7942d10 100644
--- a/src/dawn/replay/Replay.cpp
+++ b/src/dawn/replay/Replay.cpp
@@ -222,22 +222,6 @@
return {};
}
-MaybeError MapContentIntoBuffer(ReadHead& readHead,
- wgpu::Device device,
- wgpu::Buffer buffer,
- uint64_t bufferOffset,
- uint64_t size) {
- const uint32_t* data;
- DAWN_TRY_ASSIGN(data, readHead.GetData(size));
-
- // Note: We could call MapAsync here, wait for it to map, put in the data, then unmap.
- // To do so we'd have to change the code in ReplayImpl::CreateBuffer to leave the buffer
- // as MapWrite|CopySrc. That would be more inline with what the user actually did
- // though it might be slower as it would be synchronous.
- device.GetQueue().WriteBuffer(buffer, bufferOffset, data, size);
- return {};
-}
-
MaybeError ReadContentIntoTexture(const ReplayImpl& replay,
ReadHead& readHead,
wgpu::Device device,
@@ -1359,14 +1343,6 @@
mDevice.GetQueue().Submit(commandBuffers.size(), commandBuffers.data());
break;
}
- case schema::RootCommand::UnmapBuffer: {
- schema::RootCommandUnmapBufferCmdData data;
- DAWN_TRY(Deserialize(readHead, &data));
- wgpu::Buffer buffer = GetObjectById<wgpu::Buffer>(data.bufferId);
- DAWN_TRY(MapContentIntoBuffer(contentReadHead, mDevice, buffer, data.bufferOffset,
- data.size));
- break;
- }
case schema::RootCommand::SetLabel: {
schema::RootCommandSetLabelCmdData data;
DAWN_TRY(Deserialize(readHead, &data));
diff --git a/src/dawn/serialization/Schema.h b/src/dawn/serialization/Schema.h
index 0010806..9b630dd 100644
--- a/src/dawn/serialization/Schema.h
+++ b/src/dawn/serialization/Schema.h
@@ -120,7 +120,6 @@
QueueSubmit,
WriteBuffer,
WriteTexture,
- UnmapBuffer,
SetLabel,
InitTexture,
@@ -478,13 +477,6 @@
DAWN_REPLAY_MAKE_ROOT_CMD_AND_CMD_DATA(WriteBuffer, WRITE_BUFFER_CMD_DATA_MEMBER){};
-#define UNMAP_BUFFER_CMD_DATA_MEMBER(X) \
- X(ObjectId, bufferId) \
- X(uint64_t, bufferOffset) \
- X(uint64_t, size)
-
-DAWN_REPLAY_MAKE_ROOT_CMD_AND_CMD_DATA(UnmapBuffer, UNMAP_BUFFER_CMD_DATA_MEMBER){};
-
#define WRITE_TEXTURE_CMD_DATA_MEMBER(X) \
X(TexelCopyTextureInfo, destination) \
X(TexelCopyBufferLayout, layout) \
diff --git a/src/dawn/tests/end2end/CaptureAndReplayTests.cpp b/src/dawn/tests/end2end/CaptureAndReplayTests.cpp
index c976d15..9c15f6e 100644
--- a/src/dawn/tests/end2end/CaptureAndReplayTests.cpp
+++ b/src/dawn/tests/end2end/CaptureAndReplayTests.cpp
@@ -368,25 +368,47 @@
}
}
-// We make a buffer before capture. During capture write map it, put data in it.
-// Then check the data is correct on replay.
+// We make a buffer before capture. During capture
+// write map it, put data in it, copyB2B from it,
+// write map it again, put different data in it, copyB2B from it,
+// then check the data is correct on replay.
TEST_P(CaptureAndReplayTests, MapWrite) {
- const char* label = "myBuffer";
- const uint8_t myData[] = {0x11, 0x22, 0x33, 0x44};
+ const uint8_t myData1[] = {0x11, 0x22, 0x33, 0x44};
+ const uint8_t myData2[] = {0x55, 0x66, 0x77, 0x88};
- wgpu::Buffer buffer =
- CreateBuffer(label, 4, wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc);
+ wgpu::Buffer srcBuffer =
+ CreateBuffer("srcBuffer", 4, wgpu::BufferUsage::MapWrite | wgpu::BufferUsage::CopySrc);
+ wgpu::Buffer dstBuffer = CreateBuffer("dstBuffer", 4, wgpu::BufferUsage::CopyDst);
auto recorder = Recorder::CreateAndStart(device);
- MapAsyncAndWait(buffer, wgpu::MapMode::Write, 0, 4);
- buffer.WriteMappedRange(0, &myData, sizeof(myData));
- buffer.Unmap();
+ MapAsyncAndWait(srcBuffer, wgpu::MapMode::Write, 0, 4);
+ srcBuffer.WriteMappedRange(0, &myData1, sizeof(myData1));
+ srcBuffer.Unmap();
+
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToBuffer(srcBuffer, 0, dstBuffer, 0, 4);
+ wgpu::CommandBuffer commands = encoder.Finish();
+ queue.Submit(1, &commands);
+ }
+
+ MapAsyncAndWait(srcBuffer, wgpu::MapMode::Write, 0, 4);
+ srcBuffer.WriteMappedRange(0, &myData2, sizeof(myData2));
+ srcBuffer.Unmap();
+
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToBuffer(srcBuffer, 0, dstBuffer, 0, 4);
+ wgpu::CommandBuffer commands = encoder.Finish();
+ queue.Submit(1, &commands);
+ }
auto capture = recorder.Finish();
auto replay = capture.Replay(device);
- ExpectBufferEQ(replay.get(), label, myData);
+ ExpectBufferEQ(replay.get(), "srcBuffer", myData2);
+ ExpectBufferEQ(replay.get(), "dstBuffer", myData2);
}
// We make 2 buffers before capture. During capture we map one buffer
