[dawn][native] Use the new Defer options for MapAsync callbacks.
- By using the new Defer option for the MapAsync callbacks, we can
simplify the code a bit and avoid returning the Event everywhere.
Bug: 412761228
Change-Id: Ic70dcac78a326e9a50d670c585bbf16d35f91218
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/249098
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn/dawn.json b/src/dawn/dawn.json
index 73fdb72..1ad4a9c 100644
--- a/src/dawn/dawn.json
+++ b/src/dawn/dawn.json
@@ -647,8 +647,7 @@
"returns": "buffer map state"
},
{
- "name": "unmap",
- "no autolock": true
+ "name": "unmap"
},
{
"name": "destroy"
diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp
index adffb03..9b85cb9 100644
--- a/src/dawn/native/Buffer.cpp
+++ b/src/dawn/native/Buffer.cpp
@@ -381,15 +381,13 @@
}
void BufferBase::DestroyImpl() {
- Ref<MapAsyncEvent> event;
-
switch (mState.load(std::memory_order::acquire)) {
case BufferState::Mapped:
case BufferState::PendingMap: {
[[maybe_unused]] bool hadError = GetDevice()->ConsumedError(
UnmapInternal(WGPUMapAsyncStatus_Aborted,
"Buffer was destroyed before mapping was resolved."),
- &event, "calling %s.Destroy().", this);
+ "calling %s.Destroy().", this);
break;
}
case BufferState::MappedAtCreation: {
@@ -399,7 +397,7 @@
[[maybe_unused]] bool hadError = GetDevice()->ConsumedError(
UnmapInternal(WGPUMapAsyncStatus_Aborted,
"Buffer was destroyed before mapping was resolved."),
- &event, "calling %s.Destroy().", this);
+ "calling %s.Destroy().", this);
}
break;
}
@@ -407,16 +405,6 @@
break;
}
mState.store(BufferState::Destroyed, std::memory_order::release);
-
- // This is the error cases where re-entrant API calls, specifically Unmap will fail since
- // this function is called in two places, via Buffer::APIDestroy and Device::Destroy, both which
- // currently hold the device-wide lock which we don't yet have a way to circumvent and release
- // before the callback is called (spontaneously). That said, this only happens if a user is
- // calling Unmap in the MapAsync callback even though the callback was Aborted which is an
- // invalid use case.
- if (event) {
- GetInstance()->GetEventManager()->SetFutureReady(event.Get());
- }
}
// static
@@ -692,21 +680,13 @@
}
void BufferBase::APIUnmap() {
- Ref<MapAsyncEvent> event;
- {
- auto deviceGuard = GetDevice()->GetGuard();
- if (GetDevice()->ConsumedError(ValidateUnmap(), "calling %s.Unmap().", this)) {
- return;
- }
- [[maybe_unused]] bool hadError = GetDevice()->ConsumedError(
- UnmapInternal(WGPUMapAsyncStatus_Aborted,
- "Buffer was unmapped before mapping was resolved."),
- &event, "calling %s.Unmap().", this);
+ if (GetDevice()->ConsumedError(ValidateUnmap(), "calling %s.Unmap().", this)) {
+ return;
}
-
- if (event) {
- GetInstance()->GetEventManager()->SetFutureReady(event.Get());
- }
+ [[maybe_unused]] bool hadError = GetDevice()->ConsumedError(
+ UnmapInternal(WGPUMapAsyncStatus_Aborted,
+ "Buffer was unmapped before mapping was resolved."),
+ "calling %s.Unmap().", this);
}
MaybeError BufferBase::Unmap() {
@@ -736,31 +716,33 @@
return {};
}
-ResultOrError<Ref<BufferBase::MapAsyncEvent>> BufferBase::UnmapInternal(WGPUMapAsyncStatus status,
- std::string_view message) {
- Ref<MapAsyncEvent> event;
-
+MaybeError BufferBase::UnmapInternal(WGPUMapAsyncStatus status, std::string_view message) {
BufferState state = mState.load(std::memory_order::acquire);
// If the buffer is already destroyed, we don't need to do anything.
if (state == BufferState::Destroyed) {
- return nullptr;
+ return {};
}
- // For pending maps, set the pending event statuses, and return it. The caller is responsible
- // for setting the event to be ready once we no longer are holding the device-wide lock.
if (state == BufferState::PendingMap) {
// For pending maps, we update the pending event, and only set it to ready after releasing
// the buffer state lock so that spontaneous callbacks with re-entrant calls work properly.
- event = std::get<Ref<MapAsyncEvent>>(std::exchange(mMapData, static_cast<void*>(nullptr)));
+ Ref<MapAsyncEvent> event =
+ std::get<Ref<MapAsyncEvent>>(std::exchange(mMapData, static_cast<void*>(nullptr)));
+
event->UnmapEarly(status, message);
UnmapImpl();
mState.store(BufferState::Unmapped, std::memory_order::release);
- return std::move(event);
+
+ GetDevice()->DeferIfLocked(
+ [eventManager = GetInstance()->GetEventManager(), mapEvent = std::move(event)]() {
+ eventManager->SetFutureReady(mapEvent.Get());
+ });
+ return {};
}
DAWN_TRY(Unmap());
- return nullptr;
+ return {};
}
MaybeError BufferBase::ValidateMapAsync(wgpu::MapMode mode,
diff --git a/src/dawn/native/Buffer.h b/src/dawn/native/Buffer.h
index 41148d0..ac09b4f 100644
--- a/src/dawn/native/Buffer.h
+++ b/src/dawn/native/Buffer.h
@@ -175,11 +175,7 @@
WGPUMapAsyncStatus* status) const;
MaybeError ValidateUnmap() const;
bool CanGetMappedRange(bool writable, size_t offset, size_t size) const;
-
- // Unmaps the buffer and returns a MapAsyncEvent that should be set to ready if the buffer had a
- // pending map event.
- ResultOrError<Ref<MapAsyncEvent>> UnmapInternal(WGPUMapAsyncStatus status,
- std::string_view message);
+ MaybeError UnmapInternal(WGPUMapAsyncStatus status, std::string_view message);
// Updates internal state to reflect that the buffer is now mapped.
void SetMapped(BufferState newState);
