Factor ErrorSink out of DeviceBase
ErrorSink has lots of nice error-handling niceities that would be
useful on the Instance as well.
Bug: dawn:1833
Change-Id: I2cfa20335eb6cc037d1910c190da72cdd8d8963d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/181720
Reviewed-by: Loko Kung <lokokung@google.com>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/dawn/native/Adapter.cpp b/src/dawn/native/Adapter.cpp
index 801c7f6..7d19538 100644
--- a/src/dawn/native/Adapter.cpp
+++ b/src/dawn/native/Adapter.cpp
@@ -117,17 +117,17 @@
if (unpacked.Get<AdapterPropertiesMemoryHeaps>() != nullptr &&
!mSupportedFeatures.IsEnabled(wgpu::FeatureName::AdapterPropertiesMemoryHeaps)) {
- instance->ConsumedError(
+ [[maybe_unused]] bool hadError = instance->ConsumedError(
DAWN_VALIDATION_ERROR("Feature AdapterPropertiesMemoryHeaps is not available."));
}
if (unpacked.Get<AdapterPropertiesD3D>() != nullptr &&
!mSupportedFeatures.IsEnabled(wgpu::FeatureName::AdapterPropertiesD3D)) {
- instance->ConsumedError(
+ [[maybe_unused]] bool hadError = instance->ConsumedError(
DAWN_VALIDATION_ERROR("Feature AdapterPropertiesD3D is not available."));
}
if (unpacked.Get<AdapterPropertiesVk>() != nullptr &&
!mSupportedFeatures.IsEnabled(wgpu::FeatureName::AdapterPropertiesVk)) {
- instance->ConsumedError(
+ [[maybe_unused]] bool hadError = instance->ConsumedError(
DAWN_VALIDATION_ERROR("Feature AdapterPropertiesVk is not available."));
}
@@ -197,12 +197,11 @@
descriptor = &kDefaultDesc;
}
- auto result = CreateDevice(descriptor);
- if (result.IsError()) {
- mPhysicalDevice->GetInstance()->ConsumedError(result.AcquireError());
+ Ref<DeviceBase> device;
+ if (mPhysicalDevice->GetInstance()->ConsumedError(CreateDevice(descriptor), &device)) {
return nullptr;
}
- return ReturnToAPI(result.AcquireSuccess());
+ return ReturnToAPI(std::move(device));
}
ResultOrError<Ref<DeviceBase>> AdapterBase::CreateDevice(const DeviceDescriptor* rawDescriptor) {
@@ -334,7 +333,7 @@
FormatCapabilities* capabilities) {
InstanceBase* instance = mPhysicalDevice->GetInstance();
if (!mSupportedFeatures.IsEnabled(wgpu::FeatureName::FormatCapabilities)) {
- instance->ConsumedError(
+ [[maybe_unused]] bool hadError = instance->ConsumedError(
DAWN_VALIDATION_ERROR("Feature FormatCapabilities is not available."));
return false;
}
@@ -347,7 +346,7 @@
if (unpacked.Get<DrmFormatCapabilities>() != nullptr &&
!mSupportedFeatures.IsEnabled(wgpu::FeatureName::DrmFormatCapabilities)) {
- instance->ConsumedError(
+ [[maybe_unused]] bool hadError = instance->ConsumedError(
DAWN_VALIDATION_ERROR("Feature DrmFormatCapabilities is not available."));
return false;
}
diff --git a/src/dawn/native/BUILD.gn b/src/dawn/native/BUILD.gn
index d8c126c..55036fb 100644
--- a/src/dawn/native/BUILD.gn
+++ b/src/dawn/native/BUILD.gn
@@ -276,6 +276,7 @@
"ErrorInjector.h",
"ErrorScope.cpp",
"ErrorScope.h",
+ "ErrorSink.h",
"EventManager.cpp",
"EventManager.h",
"ExecutionQueue.cpp",
diff --git a/src/dawn/native/CMakeLists.txt b/src/dawn/native/CMakeLists.txt
index 9a3d8d5..325d741 100644
--- a/src/dawn/native/CMakeLists.txt
+++ b/src/dawn/native/CMakeLists.txt
@@ -131,6 +131,7 @@
"ErrorInjector.h"
"ErrorScope.cpp"
"ErrorScope.h"
+ "ErrorSink.h"
"EventManager.cpp"
"EventManager.h"
"Features.cpp"
diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h
index 663ce83..48301a4 100644
--- a/src/dawn/native/Device.h
+++ b/src/dawn/native/Device.h
@@ -42,6 +42,7 @@
#include "dawn/native/Commands.h"
#include "dawn/native/ComputePipeline.h"
#include "dawn/native/Error.h"
+#include "dawn/native/ErrorSink.h"
#include "dawn/native/ExecutionQueue.h"
#include "dawn/native/Features.h"
#include "dawn/native/Format.h"
@@ -76,7 +77,7 @@
struct InternalPipelineStore;
struct ShaderModuleParseResult;
-class DeviceBase : public RefCountedWithExternalCount {
+class DeviceBase : public ErrorSink, public RefCountedWithExternalCount {
public:
DeviceBase(AdapterBase* adapter,
const UnpackedPtr<DeviceDescriptor>& descriptor,
@@ -93,78 +94,6 @@
InternalErrorType additionalAllowedErrors = InternalErrorType::None,
WGPUDeviceLostReason lost_reason = WGPUDeviceLostReason_Undefined);
- // Variants of ConsumedError must use the returned boolean to handle failure cases since an
- // error may cause a device loss and further execution may be undefined. This is especially
- // true for the ResultOrError variants.
- [[nodiscard]] bool ConsumedError(
- MaybeError maybeError,
- InternalErrorType additionalAllowedErrors = InternalErrorType::None) {
- if (DAWN_UNLIKELY(maybeError.IsError())) {
- ConsumeError(maybeError.AcquireError(), additionalAllowedErrors);
- return true;
- }
- return false;
- }
- template <typename... Args>
- [[nodiscard]] bool ConsumedError(MaybeError maybeError,
- InternalErrorType additionalAllowedErrors,
- const char* formatStr,
- const Args&... args) {
- if (DAWN_UNLIKELY(maybeError.IsError())) {
- std::unique_ptr<ErrorData> error = maybeError.AcquireError();
- if (error->GetType() == InternalErrorType::Validation) {
- error->AppendContext(formatStr, args...);
- }
- ConsumeError(std::move(error), additionalAllowedErrors);
- return true;
- }
- return false;
- }
- template <typename... Args>
- [[nodiscard]] bool ConsumedError(MaybeError maybeError,
- const char* formatStr,
- const Args&... args) {
- return ConsumedError(std::move(maybeError), InternalErrorType::None, formatStr, args...);
- }
-
- template <typename T>
- [[nodiscard]] bool ConsumedError(
- ResultOrError<T> resultOrError,
- T* result,
- InternalErrorType additionalAllowedErrors = InternalErrorType::None) {
- if (DAWN_UNLIKELY(resultOrError.IsError())) {
- ConsumeError(resultOrError.AcquireError(), additionalAllowedErrors);
- return true;
- }
- *result = resultOrError.AcquireSuccess();
- return false;
- }
- template <typename T, typename... Args>
- [[nodiscard]] bool ConsumedError(ResultOrError<T> resultOrError,
- T* result,
- InternalErrorType additionalAllowedErrors,
- const char* formatStr,
- const Args&... args) {
- if (DAWN_UNLIKELY(resultOrError.IsError())) {
- std::unique_ptr<ErrorData> error = resultOrError.AcquireError();
- if (error->GetType() == InternalErrorType::Validation) {
- error->AppendContext(formatStr, args...);
- }
- ConsumeError(std::move(error), additionalAllowedErrors);
- return true;
- }
- *result = resultOrError.AcquireSuccess();
- return false;
- }
- template <typename T, typename... Args>
- [[nodiscard]] bool ConsumedError(ResultOrError<T> resultOrError,
- T* result,
- const char* formatStr,
- const Args&... args) {
- return ConsumedError(std::move(resultOrError), result, InternalErrorType::None, formatStr,
- args...);
- }
-
MaybeError ValidateObject(const ApiObjectBase* object) const;
InstanceBase* GetInstance() const;
@@ -559,8 +488,9 @@
void SetWGSLExtensionAllowList();
+ // ErrorSink implementation
void ConsumeError(std::unique_ptr<ErrorData> error,
- InternalErrorType additionalAllowedErrors = InternalErrorType::None);
+ InternalErrorType additionalAllowedErrors = InternalErrorType::None) override;
bool HasPendingTasks();
bool IsDeviceIdle();
diff --git a/src/dawn/native/ErrorSink.h b/src/dawn/native/ErrorSink.h
new file mode 100644
index 0000000..325691d
--- /dev/null
+++ b/src/dawn/native/ErrorSink.h
@@ -0,0 +1,126 @@
+// Copyright 2024 The Dawn & Tint Authors
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// 1. Redistributions of source code must retain the above copyright notice, this
+// list of conditions and the following disclaimer.
+//
+// 2. Redistributions in binary form must reproduce the above copyright notice,
+// this list of conditions and the following disclaimer in the documentation
+// and/or other materials provided with the distribution.
+//
+// 3. Neither the name of the copyright holder nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#ifndef SRC_DAWN_NATIVE_ERRORSINK_H_
+#define SRC_DAWN_NATIVE_ERRORSINK_H_
+
+#include <memory>
+#include <utility>
+
+#include "dawn/native/ErrorData.h"
+
+namespace dawn::native {
+
+class ErrorSink {
+ public:
+ virtual ~ErrorSink() = default;
+
+ // Variants of ConsumedError must use the returned boolean to handle failure cases since an
+ // error may cause a fatal error and further execution may be undefined. This is especially
+ // true for the ResultOrError variants.
+ [[nodiscard]] bool ConsumedError(
+ MaybeError maybeError,
+ InternalErrorType additionalAllowedErrors = InternalErrorType::None) {
+ if (DAWN_UNLIKELY(maybeError.IsError())) {
+ ConsumeError(maybeError.AcquireError(), additionalAllowedErrors);
+ return true;
+ }
+ return false;
+ }
+
+ template <typename... Args>
+ [[nodiscard]] bool ConsumedError(MaybeError maybeError,
+ InternalErrorType additionalAllowedErrors,
+ const char* formatStr,
+ const Args&... args) {
+ if (DAWN_UNLIKELY(maybeError.IsError())) {
+ std::unique_ptr<ErrorData> error = maybeError.AcquireError();
+ if (error->GetType() == InternalErrorType::Validation) {
+ error->AppendContext(formatStr, args...);
+ }
+ ConsumeError(std::move(error), additionalAllowedErrors);
+ return true;
+ }
+ return false;
+ }
+
+ template <typename... Args>
+ [[nodiscard]] bool ConsumedError(MaybeError maybeError,
+ const char* formatStr,
+ const Args&... args) {
+ return ConsumedError(std::move(maybeError), InternalErrorType::None, formatStr, args...);
+ }
+
+ template <typename T>
+ [[nodiscard]] bool ConsumedError(
+ ResultOrError<T> resultOrError,
+ T* result,
+ InternalErrorType additionalAllowedErrors = InternalErrorType::None) {
+ if (DAWN_UNLIKELY(resultOrError.IsError())) {
+ ConsumeError(resultOrError.AcquireError(), additionalAllowedErrors);
+ return true;
+ }
+ *result = resultOrError.AcquireSuccess();
+ return false;
+ }
+
+ template <typename T, typename... Args>
+ [[nodiscard]] bool ConsumedError(ResultOrError<T> resultOrError,
+ T* result,
+ InternalErrorType additionalAllowedErrors,
+ const char* formatStr,
+ const Args&... args) {
+ if (DAWN_UNLIKELY(resultOrError.IsError())) {
+ std::unique_ptr<ErrorData> error = resultOrError.AcquireError();
+ if (error->GetType() == InternalErrorType::Validation) {
+ error->AppendContext(formatStr, args...);
+ }
+ ConsumeError(std::move(error), additionalAllowedErrors);
+ return true;
+ }
+ *result = resultOrError.AcquireSuccess();
+ return false;
+ }
+
+ template <typename T, typename... Args>
+ [[nodiscard]] bool ConsumedError(ResultOrError<T> resultOrError,
+ T* result,
+ const char* formatStr,
+ const Args&... args) {
+ return ConsumedError(std::move(resultOrError), result, InternalErrorType::None, formatStr,
+ args...);
+ }
+
+ private:
+ virtual void ConsumeError(
+ std::unique_ptr<ErrorData> error,
+ InternalErrorType additionalAllowedErrors = InternalErrorType::None) = 0;
+};
+
+} // namespace dawn::native
+
+#endif // SRC_DAWN_NATIVE_ERRORSINK_H_
diff --git a/src/dawn/native/Instance.cpp b/src/dawn/native/Instance.cpp
index 171e234..a0a7b41 100644
--- a/src/dawn/native/Instance.cpp
+++ b/src/dawn/native/Instance.cpp
@@ -478,14 +478,6 @@
return discoveredPhysicalDevices;
}
-bool InstanceBase::ConsumedError(MaybeError maybeError) {
- if (maybeError.IsError()) {
- ConsumeError(maybeError.AcquireError());
- return true;
- }
- return false;
-}
-
bool InstanceBase::ConsumedErrorAndWarnOnce(MaybeError maybeErr) {
if (!maybeErr.IsError()) {
return false;
@@ -593,7 +585,10 @@
return &mEventManager;
}
-void InstanceBase::ConsumeError(std::unique_ptr<ErrorData> error) {
+void InstanceBase::ConsumeError(std::unique_ptr<ErrorData> error,
+ InternalErrorType additionalAllowedErrors) {
+ // Note: `additionalAllowedErrors` is ignored. The instance considers every type of error to be
+ // an error that is logged.
DAWN_ASSERT(error != nullptr);
if (mLoggingCallback) {
std::string messageStr = error->GetFormattedMessage();
diff --git a/src/dawn/native/Instance.h b/src/dawn/native/Instance.h
index 1f106ed..7aeda22 100644
--- a/src/dawn/native/Instance.h
+++ b/src/dawn/native/Instance.h
@@ -42,6 +42,7 @@
#include "dawn/common/ityp_bitset.h"
#include "dawn/native/Adapter.h"
#include "dawn/native/BackendConnection.h"
+#include "dawn/native/ErrorSink.h"
#include "dawn/native/EventManager.h"
#include "dawn/native/Features.h"
#include "dawn/native/Forward.h"
@@ -72,7 +73,7 @@
// This is called InstanceBase for consistency across the frontend, even if the backends don't
// specialize this class.
-class InstanceBase final : public RefCountedWithExternalCount {
+class InstanceBase final : public ErrorSink, public RefCountedWithExternalCount {
public:
static ResultOrError<Ref<InstanceBase>> Create(const InstanceDescriptor* descriptor = nullptr);
@@ -89,19 +90,6 @@
size_t GetPhysicalDeviceCountForTesting() const;
- // Used to handle error that happen up to device creation.
- bool ConsumedError(MaybeError maybeError);
-
- template <typename T>
- bool ConsumedError(ResultOrError<T> resultOrError, T* result) {
- if (resultOrError.IsError()) {
- ConsumeError(resultOrError.AcquireError());
- return true;
- }
- *result = resultOrError.AcquireSuccess();
- return false;
- }
-
// Consume an error and log its warning at most once. This is useful for
// physical device creation errors that happen because the backend is not
// supported or doesn't meet the required capabilities.
@@ -202,7 +190,10 @@
wgpu::PowerPreference powerPreference) const;
void GatherWGSLFeatures(const DawnWGSLBlocklist* wgslBlocklist);
- void ConsumeError(std::unique_ptr<ErrorData> error);
+
+ // ErrorSink implementation
+ void ConsumeError(std::unique_ptr<ErrorData> error,
+ InternalErrorType additionalAllowedErrors = InternalErrorType::None) override;
absl::flat_hash_set<std::string> mWarningMessages;
diff --git a/src/dawn/native/PhysicalDevice.cpp b/src/dawn/native/PhysicalDevice.cpp
index 6f8bd55..b90c77c 100644
--- a/src/dawn/native/PhysicalDevice.cpp
+++ b/src/dawn/native/PhysicalDevice.cpp
@@ -200,7 +200,7 @@
}
void PhysicalDeviceBase::ResetInternalDeviceForTesting() {
- mInstance->ConsumedError(ResetInternalDeviceForTestingImpl());
+ [[maybe_unused]] bool hadError = mInstance->ConsumedError(ResetInternalDeviceForTestingImpl());
}
MaybeError PhysicalDeviceBase::ResetInternalDeviceForTestingImpl() {
diff --git a/src/dawn/native/vulkan/BackendVk.cpp b/src/dawn/native/vulkan/BackendVk.cpp
index 4dd3597..823ae86 100644
--- a/src/dawn/native/vulkan/BackendVk.cpp
+++ b/src/dawn/native/vulkan/BackendVk.cpp
@@ -573,10 +573,12 @@
if (!mVulkanInstancesCreated[icd]) {
mVulkanInstancesCreated.set(icd);
- instance->ConsumedErrorAndWarnOnce([&]() -> MaybeError {
- DAWN_TRY_ASSIGN(mVulkanInstances[icd], VulkanInstance::Create(instance, icd));
- return {};
- }());
+ [[maybe_unused]] bool hadError =
+ instance->ConsumedErrorAndWarnOnce([&]() -> MaybeError {
+ DAWN_TRY_ASSIGN(mVulkanInstances[icd],
+ VulkanInstance::Create(instance, icd));
+ return {};
+ }());
}
if (mVulkanInstances[icd] == nullptr) {
