[WGPUFuture] Always return a future, and use enum instead of bitmask.
Bug: dawn:2052
Change-Id: Ie04462a43da5d29754d2a6b23fca1538b1f124d9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/151532
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
diff --git a/dawn.json b/dawn.json
index dcb8d10..b460237 100644
--- a/dawn.json
+++ b/dawn.json
@@ -1999,12 +1999,11 @@
]
},
"callback mode": {
- "_comment": "TODO(crbug.com/dawn/2052): Change this to an enum, and always return a future.",
- "category": "bitmask",
+ "category": "enum",
"values": [
- {"name": "future", "value": 1},
- {"name": "process events", "value": 2},
- {"name": "spontaneous", "value": 4}
+ {"name": "wait any only", "value": 0},
+ {"name": "allow process events", "value": 1},
+ {"name": "allow spontaneous", "value": 2}
]
},
"future": {
diff --git a/src/dawn/common/BUILD.gn b/src/dawn/common/BUILD.gn
index 5271208..04df3d2 100644
--- a/src/dawn/common/BUILD.gn
+++ b/src/dawn/common/BUILD.gn
@@ -242,7 +242,6 @@
"CoreFoundationRef.h",
"DynamicLib.cpp",
"DynamicLib.h",
- "FutureUtils.cpp",
"FutureUtils.h",
"GPUInfo.cpp",
"GPUInfo.h",
diff --git a/src/dawn/common/CMakeLists.txt b/src/dawn/common/CMakeLists.txt
index 6ebe743..a037d77 100644
--- a/src/dawn/common/CMakeLists.txt
+++ b/src/dawn/common/CMakeLists.txt
@@ -45,7 +45,6 @@
"CoreFoundationRef.h"
"DynamicLib.cpp"
"DynamicLib.h"
- "FutureUtils.cpp"
"FutureUtils.h"
"GPUInfo.cpp"
"GPUInfo.h"
diff --git a/src/dawn/common/FutureUtils.cpp b/src/dawn/common/FutureUtils.cpp
deleted file mode 100644
index 6c46b38..0000000
--- a/src/dawn/common/FutureUtils.cpp
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright 2023 The Dawn Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "dawn/common/FutureUtils.h"
-
-#include "dawn/common/Assert.h"
-
-namespace dawn {
-
-// TODO(crbug.com/dawn/2052) Remove this when we use an enum instead of a bitmask.
-CallbackMode ValidateAndFlattenCallbackMode(WGPUCallbackModeFlags mode) {
- switch (mode) {
- case WGPUCallbackMode_Spontaneous:
- return CallbackMode::Spontaneous;
- case WGPUCallbackMode_Future:
- return CallbackMode::Future;
- case WGPUCallbackMode_Future | WGPUCallbackMode_Spontaneous:
- return CallbackMode::FutureOrSpontaneous;
- case WGPUCallbackMode_ProcessEvents:
- return CallbackMode::ProcessEvents;
- case WGPUCallbackMode_ProcessEvents | WGPUCallbackMode_Spontaneous:
- return CallbackMode::ProcessEventsOrSpontaneous;
- default:
- // These cases are undefined behaviors according to the API contract.
- DAWN_ASSERT(false);
- return CallbackMode::Spontaneous;
- }
-}
-
-} // namespace dawn
diff --git a/src/dawn/common/FutureUtils.h b/src/dawn/common/FutureUtils.h
index c08c4bd..6b6d91e 100644
--- a/src/dawn/common/FutureUtils.h
+++ b/src/dawn/common/FutureUtils.h
@@ -34,17 +34,6 @@
Shutdown,
};
-// Flattened version of the wgpu::CallbackMode flags.
-// TODO(crbug.com/dawn/2052) Remove when API changes to use an enum instead of flags.
-enum class [[nodiscard]] CallbackMode {
- Spontaneous,
- Future,
- FutureOrSpontaneous,
- ProcessEvents,
- ProcessEventsOrSpontaneous,
-};
-CallbackMode ValidateAndFlattenCallbackMode(WGPUCallbackModeFlags mode);
-
} // namespace dawn
#endif // SRC_DAWN_COMMON_FUTUREUTILS_H_
diff --git a/src/dawn/native/EventManager.cpp b/src/dawn/native/EventManager.cpp
index 3c52b11..87ce188 100644
--- a/src/dawn/native/EventManager.cpp
+++ b/src/dawn/native/EventManager.cpp
@@ -29,6 +29,7 @@
namespace {
+// We can replace the std::vector& when std::span is available via C++20.
wgpu::WaitStatus WaitImpl(std::vector<TrackedFutureWaitInfo>& futures, Nanoseconds timeout) {
// Sort the futures by how they'll be waited (their GetWaitDevice).
// This lets us do each wait on a slice of the array.
@@ -82,11 +83,11 @@
// EventManager
EventManager::EventManager() {
- mTrackers.emplace(); // Construct the non-movable inner struct.
+ mEvents.emplace(); // Construct the non-movable inner struct.
}
EventManager::~EventManager() {
- DAWN_ASSERT(!mTrackers.has_value());
+ DAWN_ASSERT(!mEvents.has_value());
}
MaybeError EventManager::Initialize(const InstanceDescriptor* descriptor) {
@@ -105,73 +106,66 @@
}
void EventManager::ShutDown() {
- mTrackers.reset();
+ mEvents.reset();
}
FutureID EventManager::TrackEvent(wgpu::CallbackMode mode, Ref<TrackedEvent>&& future) {
- // TODO(crbug.com/dawn/2052) Can remove the validation on the mode once it's an enum.
- switch (ValidateAndFlattenCallbackMode(static_cast<WGPUCallbackModeFlags>(mode))) {
- case CallbackMode::Spontaneous:
- // We don't need to track the future because some other code is responsible for
- // completing it, and we aren't returning an ID so we don't need to be able to query it.
- return kNullFutureID;
- case CallbackMode::Future:
- case CallbackMode::FutureOrSpontaneous: {
- FutureID futureID = mNextFutureID++;
- if (mTrackers.has_value()) {
- mTrackers->futures->emplace(futureID, std::move(future));
- }
- return futureID;
- }
- case CallbackMode::ProcessEvents:
- case CallbackMode::ProcessEventsOrSpontaneous: {
- FutureID futureID = mNextFutureID++;
- if (mTrackers.has_value()) {
- mTrackers->pollEvents->emplace(futureID, std::move(future));
- }
- // Return null future, because the user didn't actually ask for a future.
- return kNullFutureID;
- }
+ FutureID futureID = mNextFutureID++;
+ if (!mEvents.has_value()) {
+ return futureID;
}
+
+ mEvents->Use([&](auto events) { events->emplace(futureID, std::move(future)); });
+ return futureID;
}
void EventManager::ProcessPollEvents() {
- DAWN_ASSERT(mTrackers.has_value());
+ DAWN_ASSERT(mEvents.has_value());
std::vector<TrackedFutureWaitInfo> futures;
- mTrackers->pollEvents.Use([&](auto trackedPollEvents) {
- futures.reserve(trackedPollEvents->size());
-
- for (auto& [futureID, event] : *trackedPollEvents) {
- futures.push_back(
- TrackedFutureWaitInfo{futureID, TrackedEvent::WaitRef{event.Get()}, 0, false});
+ mEvents->Use([&](auto events) {
+ // Iterate all events and record poll events and spontaneous events since they are both
+ // allowed to be completed in the ProcessPoll call. Note that spontaneous events are allowed
+ // to trigger anywhere which is why we include them in the call.
+ futures.reserve(events->size());
+ for (auto& [futureID, event] : *events) {
+ if (event->mCallbackMode != wgpu::CallbackMode::WaitAnyOnly) {
+ futures.push_back(
+ TrackedFutureWaitInfo{futureID, TrackedEvent::WaitRef{event.Get()}, 0, false});
+ }
}
+ });
- // The WaitImpl is inside of the lock to prevent any two ProcessEvents calls from
- // calling competing OS wait syscalls at the same time.
+ {
+ // There cannot be two competing ProcessEvent calls, so we use a lock to prevent it.
+ std::lock_guard<std::mutex> lock(mProcessEventLock);
wgpu::WaitStatus waitStatus = WaitImpl(futures, Nanoseconds(0));
if (waitStatus == wgpu::WaitStatus::TimedOut) {
return;
}
DAWN_ASSERT(waitStatus == wgpu::WaitStatus::Success);
+ }
+ // For all the futures we are about to complete, first ensure they're untracked. It's OK if
+ // something actually isn't tracked anymore (because it completed elsewhere while waiting.)
+ mEvents->Use([&](auto events) {
for (TrackedFutureWaitInfo& future : futures) {
if (future.ready) {
- trackedPollEvents->erase(future.futureID);
+ events->erase(future.futureID);
}
}
});
+ // Finally, call callbacks.
for (TrackedFutureWaitInfo& future : futures) {
if (future.ready) {
- DAWN_ASSERT(future.event->mCallbackMode & wgpu::CallbackMode::ProcessEvents);
future.event->EnsureComplete(EventCompletionType::Ready);
}
}
}
wgpu::WaitStatus EventManager::WaitAny(size_t count, FutureWaitInfo* infos, Nanoseconds timeout) {
- DAWN_ASSERT(mTrackers.has_value());
+ DAWN_ASSERT(mEvents.has_value());
// Validate for feature support.
if (timeout > Nanoseconds(0)) {
@@ -192,7 +186,7 @@
std::vector<TrackedFutureWaitInfo> futures;
futures.reserve(count);
bool anyCompleted = false;
- mTrackers->futures.Use([&](auto trackedFutures) {
+ mEvents->Use([&](auto events) {
FutureID firstInvalidFutureID = mNextFutureID;
for (size_t i = 0; i < count; ++i) {
FutureID futureID = infos[i].future.id;
@@ -203,11 +197,14 @@
// TakeWaitRef below will catch if the future is waited twice at the
// same time (unless it's already completed).
- auto it = trackedFutures->find(futureID);
- if (it == trackedFutures->end()) {
+ // Try to find the event.
+ auto it = events->find(futureID);
+ if (it == events->end()) {
infos[i].completed = true;
anyCompleted = true;
} else {
+ // TakeWaitRef below will catch if the future is waited twice at the same time
+ // (unless it's already completed).
infos[i].completed = false;
TrackedEvent* event = it->second.Get();
futures.push_back(
@@ -229,10 +226,10 @@
// For any futures that we're about to complete, first ensure they're untracked. It's OK if
// something actually isn't tracked anymore (because it completed elsewhere while waiting.)
- mTrackers->futures.Use([&](auto trackedFutures) {
+ mEvents->Use([&](auto events) {
for (const TrackedFutureWaitInfo& future : futures) {
if (future.ready) {
- trackedFutures->erase(future.futureID);
+ events->erase(future.futureID);
}
}
});
@@ -243,7 +240,6 @@
// Set completed before calling the callback.
infos[future.indexInInfos].completed = true;
// TODO(crbug.com/dawn/2066): Guarantee the event ordering from the JS spec.
- DAWN_ASSERT(future.event->mCallbackMode & wgpu::CallbackMode::Future);
future.event->EnsureComplete(EventCompletionType::Ready);
}
}
@@ -278,7 +274,7 @@
}
void EventManager::TrackedEvent::CompleteIfSpontaneous() {
- if (mCallbackMode & wgpu::CallbackMode::Spontaneous) {
+ if (mCallbackMode == wgpu::CallbackMode::AllowSpontaneous) {
bool alreadyComplete = mCompleted.exchange(true);
// If it was already complete, but there was an error, we have no place
// to report it, so DAWN_ASSERT. This shouldn't happen.
diff --git a/src/dawn/native/EventManager.h b/src/dawn/native/EventManager.h
index 122f376..180ec03 100644
--- a/src/dawn/native/EventManager.h
+++ b/src/dawn/native/EventManager.h
@@ -43,8 +43,7 @@
// There are various ways to optimize ProcessEvents/WaitAny:
// - TODO(crbug.com/dawn/2064) Only pay attention to the earliest serial on each queue.
// - TODO(crbug.com/dawn/2059) Spontaneously set events as "early-ready" in other places when we see
-// serials advance, e.g.
-// Submit, or when checking a later wait before an earlier wait.
+// serials advance, e.g. Submit, or when checking a later wait before an earlier wait.
// - TODO(crbug.com/dawn/2049) For thread-driven events (async pipeline compilation and Metal queue
// events), defer tracking for ProcessEvents until the event is already completed.
// - TODO(crbug.com/dawn/2051) Avoid creating OS events until they're actually needed (see the todo
@@ -69,20 +68,17 @@
Nanoseconds timeout);
private:
- struct Trackers : dawn::NonMovable {
- // Tracks Futures (used by WaitAny).
- MutexProtected<std::unordered_map<FutureID, Ref<TrackedEvent>>> futures;
- // Tracks events polled by ProcessEvents.
- MutexProtected<std::unordered_map<FutureID, Ref<TrackedEvent>>> pollEvents;
- };
-
bool mTimedWaitAnyEnable = false;
size_t mTimedWaitAnyMaxCount = kTimedWaitAnyMaxCountDefault;
std::atomic<FutureID> mNextFutureID = 1;
+ // Only 1 thread is allowed to call ProcessEvents at a time. This lock ensures that.
+ std::mutex mProcessEventLock;
+
// Freed once the user has dropped their last ref to the Instance, so can't call WaitAny or
// ProcessEvents anymore. This breaks reference cycles.
- std::optional<Trackers> mTrackers;
+ using EventMap = std::unordered_map<FutureID, Ref<TrackedEvent>>;
+ std::optional<MutexProtected<EventMap>> mEvents;
};
// Base class for the objects that back WGPUFutures. TrackedEvent is responsible for the lifetime
@@ -154,8 +150,10 @@
// A Ref<TrackedEvent>, but ASSERTing that a future isn't used concurrently in multiple
// WaitAny/ProcessEvents call (by checking that there's never more than one WaitRef for a
-// TrackedEvent). For WaitAny, this checks the embedder's behavior, but for ProcessEvents this is
-// only an internal DAWN_ASSERT (it's supposed to be synchronized so that this never happens).
+// TrackedEvent). While concurrent calls on the same futures are not explicitly disallowed, they are
+// generally unintentional, and hence this can help to identify potential bugs. Note that for
+// WaitAny, this checks the embedder's behavior, but for ProcessEvents this is only an internal
+// DAWN_ASSERT (it's supposed to be synchronized so that this never happens).
class EventManager::TrackedEvent::WaitRef : dawn::NonCopyable {
public:
WaitRef(WaitRef&& rhs) = default;
diff --git a/src/dawn/tests/end2end/EventTests.cpp b/src/dawn/tests/end2end/EventTests.cpp
index ab04f781..c185535 100644
--- a/src/dawn/tests/end2end/EventTests.cpp
+++ b/src/dawn/tests/end2end/EventTests.cpp
@@ -66,28 +66,28 @@
};
enum class WaitTypeAndCallbackMode {
- TimedWaitAny_Future,
- TimedWaitAny_FutureSpontaneous,
- SpinWaitAny_Future,
- SpinWaitAny_FutureSpontaneous,
- SpinProcessEvents_ProcessEvents,
- SpinProcessEvents_ProcessEventsSpontaneous,
+ TimedWaitAny_WaitAnyOnly,
+ TimedWaitAny_AllowSpontaneous,
+ SpinWaitAny_WaitAnyOnly,
+ SpinWaitAny_AllowSpontaneous,
+ SpinProcessEvents_AllowProcessEvents,
+ SpinProcessEvents_AllowSpontaneous,
};
std::ostream& operator<<(std::ostream& o, WaitTypeAndCallbackMode waitMode) {
switch (waitMode) {
- case WaitTypeAndCallbackMode::TimedWaitAny_Future:
- return o << "TimedWaitAny_Future";
- case WaitTypeAndCallbackMode::SpinWaitAny_Future:
- return o << "SpinWaitAny_Future";
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEvents:
- return o << "SpinProcessEvents_ProcessEvents";
- case WaitTypeAndCallbackMode::TimedWaitAny_FutureSpontaneous:
- return o << "TimedWaitAny_FutureSpontaneous";
- case WaitTypeAndCallbackMode::SpinWaitAny_FutureSpontaneous:
- return o << "SpinWaitAny_FutureSpontaneous";
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEventsSpontaneous:
- return o << "SpinProcessEvents_ProcessEventsSpontaneous";
+ case WaitTypeAndCallbackMode::TimedWaitAny_WaitAnyOnly:
+ return o << "TimedWaitAny_WaitAnyOnly";
+ case WaitTypeAndCallbackMode::SpinWaitAny_WaitAnyOnly:
+ return o << "SpinWaitAny_WaitAnyOnly";
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowProcessEvents:
+ return o << "SpinProcessEvents_AllowProcessEvents";
+ case WaitTypeAndCallbackMode::TimedWaitAny_AllowSpontaneous:
+ return o << "TimedWaitAny_AllowSpontaneous";
+ case WaitTypeAndCallbackMode::SpinWaitAny_AllowSpontaneous:
+ return o << "SpinWaitAny_AllowSpontaneous";
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowSpontaneous:
+ return o << "SpinProcessEvents_AllowSpontaneous";
}
}
@@ -107,9 +107,9 @@
DawnTestWithParams::SetUp();
WaitTypeAndCallbackMode mode = GetParam().mWaitTypeAndCallbackMode;
if (UsesWire()) {
- DAWN_TEST_UNSUPPORTED_IF(mode == WaitTypeAndCallbackMode::TimedWaitAny_Future ||
+ DAWN_TEST_UNSUPPORTED_IF(mode == WaitTypeAndCallbackMode::TimedWaitAny_WaitAnyOnly ||
mode ==
- WaitTypeAndCallbackMode::TimedWaitAny_FutureSpontaneous);
+ WaitTypeAndCallbackMode::TimedWaitAny_AllowSpontaneous);
}
testInstance = GetInstance();
testDevice = device;
@@ -141,34 +141,32 @@
wgpu::CallbackMode GetCallbackMode() {
switch (GetParam().mWaitTypeAndCallbackMode) {
- case WaitTypeAndCallbackMode::TimedWaitAny_Future:
- case WaitTypeAndCallbackMode::SpinWaitAny_Future:
- return wgpu::CallbackMode::Future;
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEvents:
- return wgpu::CallbackMode::ProcessEvents;
- case WaitTypeAndCallbackMode::TimedWaitAny_FutureSpontaneous:
- case WaitTypeAndCallbackMode::SpinWaitAny_FutureSpontaneous:
- return wgpu::CallbackMode::Future | wgpu::CallbackMode::Spontaneous;
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEventsSpontaneous:
- return wgpu::CallbackMode::ProcessEvents | wgpu::CallbackMode::Spontaneous;
+ case WaitTypeAndCallbackMode::TimedWaitAny_WaitAnyOnly:
+ case WaitTypeAndCallbackMode::SpinWaitAny_WaitAnyOnly:
+ return wgpu::CallbackMode::WaitAnyOnly;
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowProcessEvents:
+ return wgpu::CallbackMode::AllowProcessEvents;
+ case WaitTypeAndCallbackMode::TimedWaitAny_AllowSpontaneous:
+ case WaitTypeAndCallbackMode::SpinWaitAny_AllowSpontaneous:
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowSpontaneous:
+ return wgpu::CallbackMode::AllowSpontaneous;
}
}
- bool IsSpontaneous() { return GetCallbackMode() & wgpu::CallbackMode::Spontaneous; }
+ bool IsSpontaneous() { return GetCallbackMode() == wgpu::CallbackMode::AllowSpontaneous; }
void TrackForTest(wgpu::Future future) {
mCallbacksIssuedCount++;
switch (GetParam().mWaitTypeAndCallbackMode) {
- case WaitTypeAndCallbackMode::TimedWaitAny_Future:
- case WaitTypeAndCallbackMode::TimedWaitAny_FutureSpontaneous:
- case WaitTypeAndCallbackMode::SpinWaitAny_Future:
- case WaitTypeAndCallbackMode::SpinWaitAny_FutureSpontaneous:
+ case WaitTypeAndCallbackMode::TimedWaitAny_WaitAnyOnly:
+ case WaitTypeAndCallbackMode::TimedWaitAny_AllowSpontaneous:
+ case WaitTypeAndCallbackMode::SpinWaitAny_WaitAnyOnly:
+ case WaitTypeAndCallbackMode::SpinWaitAny_AllowSpontaneous:
mFutures.push_back(wgpu::FutureWaitInfo{future, false});
break;
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEvents:
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEventsSpontaneous:
- ASSERT_EQ(future.id, 0ull);
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowProcessEvents:
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowSpontaneous:
break;
}
}
@@ -195,27 +193,27 @@
void TestWaitAll(bool loopOnlyOnce = false) {
switch (GetParam().mWaitTypeAndCallbackMode) {
- case WaitTypeAndCallbackMode::TimedWaitAny_Future:
- case WaitTypeAndCallbackMode::TimedWaitAny_FutureSpontaneous:
+ case WaitTypeAndCallbackMode::TimedWaitAny_WaitAnyOnly:
+ case WaitTypeAndCallbackMode::TimedWaitAny_AllowSpontaneous:
return TestWaitImpl(WaitType::TimedWaitAny);
- case WaitTypeAndCallbackMode::SpinWaitAny_Future:
- case WaitTypeAndCallbackMode::SpinWaitAny_FutureSpontaneous:
+ case WaitTypeAndCallbackMode::SpinWaitAny_WaitAnyOnly:
+ case WaitTypeAndCallbackMode::SpinWaitAny_AllowSpontaneous:
return TestWaitImpl(WaitType::SpinWaitAny);
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEvents:
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEventsSpontaneous:
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowProcessEvents:
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowSpontaneous:
return TestWaitImpl(WaitType::SpinProcessEvents);
}
}
void TestWaitIncorrectly() {
switch (GetParam().mWaitTypeAndCallbackMode) {
- case WaitTypeAndCallbackMode::TimedWaitAny_Future:
- case WaitTypeAndCallbackMode::TimedWaitAny_FutureSpontaneous:
- case WaitTypeAndCallbackMode::SpinWaitAny_Future:
- case WaitTypeAndCallbackMode::SpinWaitAny_FutureSpontaneous:
+ case WaitTypeAndCallbackMode::TimedWaitAny_WaitAnyOnly:
+ case WaitTypeAndCallbackMode::TimedWaitAny_AllowSpontaneous:
+ case WaitTypeAndCallbackMode::SpinWaitAny_WaitAnyOnly:
+ case WaitTypeAndCallbackMode::SpinWaitAny_AllowSpontaneous:
return TestWaitImpl(WaitType::SpinProcessEvents);
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEvents:
- case WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEventsSpontaneous:
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowProcessEvents:
+ case WaitTypeAndCallbackMode::SpinProcessEvents_AllowSpontaneous:
return TestWaitImpl(WaitType::SpinWaitAny);
}
}
@@ -374,8 +372,7 @@
// WorkDone events waited in reverse order.
TEST_P(EventCompletionTests, WorkDoneOutOfOrder) {
// With ProcessEvents or Spontaneous we can't control the order of completion.
- DAWN_TEST_UNSUPPORTED_IF(GetCallbackMode() &
- (wgpu::CallbackMode::ProcessEvents | wgpu::CallbackMode::Spontaneous));
+ DAWN_TEST_UNSUPPORTED_IF(GetCallbackMode() != wgpu::CallbackMode::WaitAnyOnly);
TrivialSubmit();
wgpu::Future f1 = OnSubmittedWorkDone(WGPUQueueWorkDoneStatus_Success);
@@ -452,12 +449,12 @@
// TODO(crbug.com/dawn/2058): Enable tests for the rest of the backends.
{MetalBackend()},
{
- WaitTypeAndCallbackMode::TimedWaitAny_Future,
- WaitTypeAndCallbackMode::TimedWaitAny_FutureSpontaneous,
- WaitTypeAndCallbackMode::SpinWaitAny_Future,
- WaitTypeAndCallbackMode::SpinWaitAny_FutureSpontaneous,
- WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEvents,
- WaitTypeAndCallbackMode::SpinProcessEvents_ProcessEventsSpontaneous,
+ WaitTypeAndCallbackMode::TimedWaitAny_WaitAnyOnly,
+ WaitTypeAndCallbackMode::TimedWaitAny_AllowSpontaneous,
+ WaitTypeAndCallbackMode::SpinWaitAny_WaitAnyOnly,
+ WaitTypeAndCallbackMode::SpinWaitAny_AllowSpontaneous,
+ WaitTypeAndCallbackMode::SpinProcessEvents_AllowProcessEvents,
+ WaitTypeAndCallbackMode::SpinProcessEvents_AllowSpontaneous,
// TODO(crbug.com/dawn/2059): The cases with the Spontaneous flag
// enabled were added before we implemented all of the spontaneous
@@ -502,7 +499,8 @@
for (uint64_t timeout : {uint64_t(1), uint64_t(0), UINT64_MAX}) {
wgpu::FutureWaitInfo info{device2.GetQueue().OnSubmittedWorkDoneF(
- {nullptr, wgpu::CallbackMode::Future, [](WGPUQueueWorkDoneStatus, void*) {}, nullptr})};
+ {nullptr, wgpu::CallbackMode::WaitAnyOnly, [](WGPUQueueWorkDoneStatus, void*) {},
+ nullptr})};
wgpu::WaitStatus status = instance2.WaitAny(1, &info, timeout);
if (timeout == 0) {
ASSERT_TRUE(status == wgpu::WaitStatus::Success ||
@@ -521,7 +519,7 @@
std::vector<wgpu::FutureWaitInfo> infos;
for (size_t i = 0; i < count; ++i) {
infos.push_back(
- {queue.OnSubmittedWorkDoneF({nullptr, wgpu::CallbackMode::Future,
+ {queue.OnSubmittedWorkDoneF({nullptr, wgpu::CallbackMode::WaitAnyOnly,
[](WGPUQueueWorkDoneStatus, void*) {}, nullptr})});
}
wgpu::WaitStatus status = GetInstance().WaitAny(infos.size(), infos.data(), timeout);
@@ -545,9 +543,9 @@
wgpu::Queue queue2 = device2.GetQueue();
for (uint64_t timeout : {uint64_t(0), uint64_t(1)}) {
std::vector<wgpu::FutureWaitInfo> infos{{
- {queue.OnSubmittedWorkDoneF({nullptr, wgpu::CallbackMode::Future,
+ {queue.OnSubmittedWorkDoneF({nullptr, wgpu::CallbackMode::WaitAnyOnly,
[](WGPUQueueWorkDoneStatus, void*) {}, nullptr})},
- {queue2.OnSubmittedWorkDoneF({nullptr, wgpu::CallbackMode::Future,
+ {queue2.OnSubmittedWorkDoneF({nullptr, wgpu::CallbackMode::WaitAnyOnly,
[](WGPUQueueWorkDoneStatus, void*) {}, nullptr})},
}};
wgpu::WaitStatus status = GetInstance().WaitAny(infos.size(), infos.data(), timeout);
diff --git a/src/dawn/wire/client/EventManager.cpp b/src/dawn/wire/client/EventManager.cpp
index 744b6b2..fa80b6d 100644
--- a/src/dawn/wire/client/EventManager.cpp
+++ b/src/dawn/wire/client/EventManager.cpp
@@ -26,9 +26,7 @@
EventManager::EventManager(Client* client) : mClient(client) {}
-FutureID EventManager::TrackEvent(WGPUCallbackModeFlags mode, EventCallback&& callback) {
- DAWN_UNUSED(ValidateAndFlattenCallbackMode(mode));
-
+FutureID EventManager::TrackEvent(WGPUCallbackMode mode, EventCallback&& callback) {
FutureID futureID = mNextFutureID++;
if (mClient->IsDisconnected()) {
@@ -84,7 +82,9 @@
mTrackedEvents.Use([&](auto trackedEvents) {
for (auto it = trackedEvents->begin(); it != trackedEvents->end();) {
TrackedEvent& event = it->second;
- bool shouldRemove = (event.mMode & WGPUCallbackMode_ProcessEvents) && event.mReady;
+ bool shouldRemove = (event.mMode == WGPUCallbackMode_AllowProcessEvents ||
+ event.mMode == WGPUCallbackMode_AllowSpontaneous) &&
+ event.mReady;
if (!shouldRemove) {
++it;
continue;
@@ -135,7 +135,6 @@
}
TrackedEvent& event = it->second;
- DAWN_ASSERT(event.mMode & WGPUCallbackMode_Future);
// Early update .completed, in prep to complete the callback if ready.
infos[i].completed = event.mReady;
if (event.mReady) {
@@ -161,7 +160,7 @@
// EventManager::TrackedEvent
-EventManager::TrackedEvent::TrackedEvent(WGPUCallbackModeFlags mode, EventCallback&& callback)
+EventManager::TrackedEvent::TrackedEvent(WGPUCallbackMode mode, EventCallback&& callback)
: mMode(mode), mCallback(callback) {}
EventManager::TrackedEvent::~TrackedEvent() {
diff --git a/src/dawn/wire/client/EventManager.h b/src/dawn/wire/client/EventManager.h
index e74f20b..e2aae83 100644
--- a/src/dawn/wire/client/EventManager.h
+++ b/src/dawn/wire/client/EventManager.h
@@ -43,7 +43,7 @@
explicit EventManager(Client*);
~EventManager() = default;
- FutureID TrackEvent(WGPUCallbackModeFlags mode, EventCallback&& callback);
+ FutureID TrackEvent(WGPUCallbackMode mode, EventCallback&& callback);
void ShutDown();
void SetFutureReady(FutureID futureID);
void ProcessPollEvents();
@@ -51,13 +51,13 @@
private:
struct TrackedEvent : dawn::NonCopyable {
- TrackedEvent(WGPUCallbackModeFlags mode, EventCallback&& callback);
+ TrackedEvent(WGPUCallbackMode mode, EventCallback&& callback);
~TrackedEvent();
TrackedEvent(TrackedEvent&&) = default;
TrackedEvent& operator=(TrackedEvent&&) = default;
- WGPUCallbackModeFlags mMode;
+ WGPUCallbackMode mMode;
// Callback. Falsey if already called.
EventCallback mCallback;
// These states don't need to be atomic because they're always protected by
diff --git a/src/dawn/wire/client/Queue.cpp b/src/dawn/wire/client/Queue.cpp
index e369275..26f662b 100644
--- a/src/dawn/wire/client/Queue.cpp
+++ b/src/dawn/wire/client/Queue.cpp
@@ -85,9 +85,7 @@
cmd.requestSerial = serial;
client->SerializeCommand(cmd);
-
- FutureID futureID = (callbackInfo.mode & WGPUCallbackMode_Future) ? futureIDInternal : 0;
- return {futureID};
+ return {futureIDInternal};
}
void Queue::WriteBuffer(WGPUBuffer cBuffer, uint64_t bufferOffset, const void* data, size_t size) {