Distinguish Map request callback fired by device

Currently Buffer doesn't distinguish Map request callback fired
by device. For example if buffer.MapAsync(), buffer.Unmap(),
and buffer.MapAsync() are called in this order before the first
MapAsync() finishes the MapAsync callback provided by application
for the first MapAsync() is fired when Map request callback
for the first MapAsync() is fired by device although the first
MapAsync callback provided by application shouldn't be fired
because it is already unmapped.

This commit resolves this problem by assigning MapRequestId to
Map request and distinguishing the callback fired by device.

Change-Id: Ic29b02d27cffb254616dc7b48a60151c39f667e2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/113222
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Kokoro: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
This commit is contained in:
Takahiro 2022-12-08 04:49:32 +00:00 committed by Dawn LUCI CQ
parent 5f764d8527
commit e536775b40
3 changed files with 33 additions and 21 deletions

View File

@ -37,24 +37,25 @@ namespace dawn::native {
namespace {
struct MapRequestTask : TrackTaskCallback {
MapRequestTask(dawn::platform::Platform* platform, Ref<BufferBase> buffer)
: TrackTaskCallback(platform), buffer(std::move(buffer)) {}
MapRequestTask(dawn::platform::Platform* platform, Ref<BufferBase> buffer, MapRequestID id)
: TrackTaskCallback(platform), buffer(std::move(buffer)), id(id) {}
void Finish() override {
ASSERT(mSerial != kMaxExecutionSerial);
TRACE_EVENT1(mPlatform, General, "Buffer::TaskInFlight::Finished", "serial",
uint64_t(mSerial));
buffer->OnMapRequestCompleted(WGPUBufferMapAsyncStatus_Success);
buffer->OnMapRequestCompleted(id, WGPUBufferMapAsyncStatus_Success);
}
void HandleDeviceLoss() override {
buffer->OnMapRequestCompleted(WGPUBufferMapAsyncStatus_DeviceLost);
buffer->OnMapRequestCompleted(id, WGPUBufferMapAsyncStatus_DeviceLost);
}
void HandleShutDown() override {
buffer->OnMapRequestCompleted(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
buffer->OnMapRequestCompleted(id, WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
}
~MapRequestTask() override = default;
private:
Ref<BufferBase> buffer;
MapRequestID id;
};
class ErrorBuffer final : public BufferBase {
@ -316,9 +317,9 @@ MaybeError BufferBase::ValidateCanUseOnQueueNow() const {
UNREACHABLE();
}
void BufferBase::CallMapCallback(WGPUBufferMapAsyncStatus status) {
void BufferBase::CallMapCallback(MapRequestID mapID, WGPUBufferMapAsyncStatus status) {
ASSERT(!IsError());
if (mMapCallback != nullptr) {
if (mMapCallback != nullptr && mapID == mLastMapID) {
// Tag the callback as fired before firing it, otherwise it could fire a second time if
// for example buffer.Unmap() is called inside the application-provided callback.
WGPUBufferMapCallback callback = mMapCallback;
@ -357,6 +358,7 @@ void BufferBase::APIMapAsync(wgpu::MapMode mode,
}
ASSERT(!IsError());
mLastMapID++;
mMapMode = mode;
mMapOffset = offset;
mMapSize = size;
@ -365,11 +367,11 @@ void BufferBase::APIMapAsync(wgpu::MapMode mode,
mState = BufferState::PendingMap;
if (GetDevice()->ConsumedError(MapAsyncImpl(mode, offset, size))) {
CallMapCallback(WGPUBufferMapAsyncStatus_DeviceLost);
CallMapCallback(mLastMapID, WGPUBufferMapAsyncStatus_DeviceLost);
return;
}
std::unique_ptr<MapRequestTask> request =
std::make_unique<MapRequestTask>(GetDevice()->GetPlatform(), this);
std::make_unique<MapRequestTask>(GetDevice()->GetPlatform(), this, mLastMapID);
TRACE_EVENT1(GetDevice()->GetPlatform(), General, "Buffer::APIMapAsync", "serial",
uint64_t(GetDevice()->GetPendingCommandSerial()));
GetDevice()->GetQueue()->TrackTask(std::move(request));
@ -439,7 +441,7 @@ void BufferBase::Unmap() {
void BufferBase::UnmapInternal(WGPUBufferMapAsyncStatus callbackStatus) {
if (mState == BufferState::PendingMap) {
CallMapCallback(callbackStatus);
CallMapCallback(mLastMapID, callbackStatus);
UnmapImpl();
} else if (mState == BufferState::Mapped) {
UnmapImpl();
@ -552,11 +554,12 @@ MaybeError BufferBase::ValidateUnmap() const {
return {};
}
void BufferBase::OnMapRequestCompleted(WGPUBufferMapAsyncStatus status) {
if (status == WGPUBufferMapAsyncStatus_Success && mState == BufferState::PendingMap) {
void BufferBase::OnMapRequestCompleted(MapRequestID mapID, WGPUBufferMapAsyncStatus status) {
if (mapID == mLastMapID && status == WGPUBufferMapAsyncStatus_Success &&
mState == BufferState::PendingMap) {
mState = BufferState::Mapped;
}
CallMapCallback(status);
CallMapCallback(mapID, status);
}
bool BufferBase::NeedsInitialization() const {

View File

@ -63,7 +63,7 @@ class BufferBase : public ApiObjectBase {
wgpu::BufferUsage GetUsageExternalOnly() const;
MaybeError MapAtCreation();
void OnMapRequestCompleted(WGPUBufferMapAsyncStatus status);
void OnMapRequestCompleted(MapRequestID mapID, WGPUBufferMapAsyncStatus status);
MaybeError ValidateCanUseOnQueueNow() const;
@ -110,7 +110,7 @@ class BufferBase : public ApiObjectBase {
virtual bool IsCPUWritableAtCreation() const = 0;
MaybeError CopyFromStagingBuffer();
void CallMapCallback(WGPUBufferMapAsyncStatus status);
void CallMapCallback(MapRequestID mapID, WGPUBufferMapAsyncStatus status);
MaybeError ValidateMapAsync(wgpu::MapMode mode,
size_t offset,
@ -129,6 +129,7 @@ class BufferBase : public ApiObjectBase {
WGPUBufferMapCallback mMapCallback = nullptr;
void* mMapUserdata = 0;
MapRequestID mLastMapID = MapRequestID(0);
wgpu::MapMode mMapMode = wgpu::MapMode::None;
size_t mMapOffset = 0;
size_t mMapSize = 0;

View File

@ -406,32 +406,40 @@ TEST_F(BufferValidationTest, MapAsync_UnmapBeforeResult) {
// works as expected and we don't get the cancelled request's data.
TEST_F(BufferValidationTest, MapAsync_UnmapBeforeResultAndMapAgain) {
{
wgpu::Buffer buf = CreateMapReadBuffer(4);
buf.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, this + 0);
wgpu::Buffer buf = CreateMapReadBuffer(16);
buf.MapAsync(wgpu::MapMode::Read, 0, 8, ToMockBufferMapAsyncCallback, this + 0);
EXPECT_CALL(*mockBufferMapAsyncCallback,
Call(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback, this + 0))
.Times(1);
buf.Unmap();
buf.MapAsync(wgpu::MapMode::Read, 0, 4, ToMockBufferMapAsyncCallback, this + 1);
buf.MapAsync(wgpu::MapMode::Read, 8, 8, ToMockBufferMapAsyncCallback, this + 1);
EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this + 1))
.Times(1);
WaitForAllOperations(device);
// Check that only the second MapAsync had an effect
ASSERT_EQ(nullptr, buf.GetConstMappedRange(0));
ASSERT_NE(nullptr, buf.GetConstMappedRange(8));
}
{
wgpu::Buffer buf = CreateMapWriteBuffer(4);
buf.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, this + 0);
wgpu::Buffer buf = CreateMapWriteBuffer(16);
buf.MapAsync(wgpu::MapMode::Write, 0, 8, ToMockBufferMapAsyncCallback, this + 0);
EXPECT_CALL(*mockBufferMapAsyncCallback,
Call(WGPUBufferMapAsyncStatus_UnmappedBeforeCallback, this + 0))
.Times(1);
buf.Unmap();
buf.MapAsync(wgpu::MapMode::Write, 0, 4, ToMockBufferMapAsyncCallback, this + 1);
buf.MapAsync(wgpu::MapMode::Write, 8, 8, ToMockBufferMapAsyncCallback, this + 1);
EXPECT_CALL(*mockBufferMapAsyncCallback, Call(WGPUBufferMapAsyncStatus_Success, this + 1))
.Times(1);
WaitForAllOperations(device);
// Check that only the second MapAsync had an effect
ASSERT_EQ(nullptr, buf.GetConstMappedRange(0));
ASSERT_NE(nullptr, buf.GetConstMappedRange(8));
}
}