From c2657b21d50938d4676aa6c87e44b3aa50c7f468 Mon Sep 17 00:00:00 2001 From: Zhaoming Jiang Date: Tue, 7 Mar 2023 02:03:54 +0000 Subject: [PATCH] Dawn: Promote DisallowUnsafeAPIs as instance toggle This CL add Instance and Adapter toggle stage, and promote DisallowUnsafeAPIs as an instance toggle, and can be required using DawnTogglesDescriptor chained in instance descriptor when creating instance. The instance's toggles state will get inherited to adapters and devices it create. Related tests are implemented and updated. Toggles inheritance can be overriden if not forced, so requiring DisallowUnsafeAPIs when creating device is still available and working like before. Note that currently we don't have toggle of adapter stage, and can not require toggles when creating adapter, until follow up CLs implement it. Currently the toggles state of a adapter is simply inherited from instance. Bug: dawn:1495 Change-Id: I6bf7aa0f950a99451afcc2cab5322c924b7d9520 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/122021 Commit-Queue: Zhaoming Jiang Reviewed-by: Austin Eng Kokoro: Kokoro Reviewed-by: Corentin Wallez --- dawn.json | 2 +- include/dawn/native/DawnNative.h | 4 +- src/dawn/native/Adapter.cpp | 33 +-- src/dawn/native/Adapter.h | 11 +- src/dawn/native/BackendConnection.cpp | 4 +- src/dawn/native/BackendConnection.h | 7 +- src/dawn/native/Instance.cpp | 42 +++- src/dawn/native/Instance.h | 6 +- src/dawn/native/Toggles.cpp | 43 +++- src/dawn/native/Toggles.h | 10 +- src/dawn/native/d3d12/AdapterD3D12.cpp | 6 +- src/dawn/native/d3d12/AdapterD3D12.h | 4 +- src/dawn/native/d3d12/BackendD3D12.cpp | 19 +- src/dawn/native/d3d12/BackendD3D12.h | 6 +- src/dawn/native/metal/BackendMTL.h | 6 +- src/dawn/native/metal/BackendMTL.mm | 16 +- src/dawn/native/null/DeviceNull.cpp | 12 +- src/dawn/native/null/DeviceNull.h | 3 + src/dawn/native/opengl/AdapterGL.cpp | 6 +- src/dawn/native/opengl/AdapterGL.h | 4 +- src/dawn/native/opengl/BackendGL.cpp | 11 +- src/dawn/native/opengl/BackendGL.h | 6 +- src/dawn/native/vulkan/AdapterVk.cpp | 5 +- src/dawn/native/vulkan/AdapterVk.h | 3 +- src/dawn/native/vulkan/BackendVk.cpp | 11 +- src/dawn/native/vulkan/BackendVk.h | 6 +- src/dawn/tests/BUILD.gn | 1 + src/dawn/tests/DawnNativeTest.cpp | 25 ++- src/dawn/tests/DawnTest.cpp | 118 ++++++---- .../tests/end2end/ExperimentalDP4aTests.cpp | 3 +- src/dawn/tests/unittests/FeatureTests.cpp | 80 ++++--- src/dawn/tests/unittests/ToggleTests.cpp | 202 ++++++++++++++++++ .../unittests/native/DeviceCreationTests.cpp | 136 ++++++++++-- .../validation/QueryValidationTests.cpp | 23 +- .../RenderPipelineValidationTests.cpp | 11 - .../validation/TextureValidationTests.cpp | 8 - .../validation/UnsafeAPIValidationTests.cpp | 4 + .../unittests/validation/ValidationTest.cpp | 29 ++- 38 files changed, 696 insertions(+), 230 deletions(-) create mode 100644 src/dawn/tests/unittests/ToggleTests.cpp diff --git a/dawn.json b/dawn.json index 49eb767ee7..20be7866fb 100644 --- a/dawn.json +++ b/dawn.json @@ -174,7 +174,7 @@ "tags": ["dawn", "native"], "category": "structure", "chained": "in", - "chain roots": ["device descriptor"], + "chain roots": ["instance descriptor", "device descriptor"], "members": [ {"name": "enabled toggles count", "type": "uint32_t", "default": 0}, {"name": "enabled toggles", "type": "char", "annotation": "const*const*", "length": "enabled toggles count"}, diff --git a/include/dawn/native/DawnNative.h b/include/dawn/native/DawnNative.h index 033dee2a59..7e1315c9a5 100644 --- a/include/dawn/native/DawnNative.h +++ b/include/dawn/native/DawnNative.h @@ -51,9 +51,7 @@ struct DAWN_NATIVE_EXPORT DawnDeviceDescriptor { // Each toggle is assigned with a TogglesStage, indicating the validation and earliest usage // time of the toggle. -// TODO(dawn:1495): Currently all toggles are device toggles, i.e. of Device toggle stage. Add -// instance and adapter stages after instance and adapter toggles implemented. -enum class ToggleStage { Device }; +enum class ToggleStage { Instance, Adapter, Device }; // A struct to record the information of a toggle. A toggle is a code path in Dawn device that // can be manually configured to run or not outside Dawn, including workarounds, special diff --git a/src/dawn/native/Adapter.cpp b/src/dawn/native/Adapter.cpp index a23b6e101e..73286e08aa 100644 --- a/src/dawn/native/Adapter.cpp +++ b/src/dawn/native/Adapter.cpp @@ -27,10 +27,11 @@ namespace dawn::native { -AdapterBase::AdapterBase(InstanceBase* instance, wgpu::BackendType backend) - : mInstance(instance), mBackend(backend) { - mSupportedFeatures.EnableFeature(Feature::DawnNative); - mSupportedFeatures.EnableFeature(Feature::DawnInternalUsages); +AdapterBase::AdapterBase(InstanceBase* instance, + wgpu::BackendType backend, + const TogglesState& adapterToggles) + : mInstance(instance), mBackend(backend), mTogglesState(adapterToggles) { + ASSERT(adapterToggles.GetStage() == ToggleStage::Adapter); } AdapterBase::~AdapterBase() = default; @@ -39,6 +40,8 @@ MaybeError AdapterBase::Initialize() { DAWN_TRY_CONTEXT(InitializeImpl(), "initializing adapter (backend=%s)", mBackend); InitializeVendorArchitectureImpl(); + mSupportedFeatures.EnableFeature(Feature::DawnNative); + mSupportedFeatures.EnableFeature(Feature::DawnInternalUsages); InitializeSupportedFeaturesImpl(); DAWN_TRY_CONTEXT( @@ -206,6 +209,10 @@ bool AdapterBase::GetLimits(SupportedLimits* limits) const { return true; } +const TogglesState& AdapterBase::GetTogglesState() const { + return mTogglesState; +} + MaybeError AdapterBase::ValidateFeatureSupportedWithDeviceToggles( wgpu::FeatureName feature, const TogglesState& deviceTogglesState) { @@ -229,8 +236,8 @@ ResultOrError> AdapterBase::CreateDeviceInternal( const DeviceDescriptor* descriptor) { ASSERT(descriptor != nullptr); - // Create device toggles state from required toggles descriptor. - // TODO(dawn:1495): After implementing adapter toggles, also inherite adapter toggles state. + // Create device toggles state from required toggles descriptor and inherited adapter toggles + // state. const DawnTogglesDescriptor* deviceTogglesDesc = nullptr; FindInChain(descriptor->nextInChain, &deviceTogglesDesc); @@ -261,21 +268,21 @@ ResultOrError> AdapterBase::CreateDeviceInternal( deviceTogglesDesc = &convertedDeviceTogglesDesc; } - // Create device toggles state from user-given toggles descriptor, and set up forced and default - // toggles. - // TODO(dawn:1495): After implementing adapter toggles, device toggles state should also inherit - // from adapter toggles state. + // Create device toggles state. TogglesState deviceToggles = TogglesState::CreateFromTogglesDescriptor(deviceTogglesDesc, ToggleStage::Device); + deviceToggles.InheritFrom(mTogglesState); // Default toggles for all backend deviceToggles.Default(Toggle::LazyClearResourceOnFirstUse, true); - deviceToggles.Default(Toggle::DisallowUnsafeAPIs, true); + // Backend-specific forced and default device toggles SetupBackendDeviceToggles(&deviceToggles); // Validate all required features are supported by the adapter and suitable under given toggles. - // TODO(dawn:1495): After implementing adapter toggles, validate supported features using - // adapter toggles instead of device toggles. + // Note that certain toggles in device toggles state may be overriden by user and different from + // the adapter toggles state. + // TODO(dawn:1495): After implementing adapter toggles, decide whether we should validate + // supported features using adapter toggles or device toggles. for (uint32_t i = 0; i < descriptor->requiredFeaturesCount; ++i) { wgpu::FeatureName feature = descriptor->requiredFeatures[i]; DAWN_TRY(ValidateFeatureSupportedWithDeviceToggles(feature, deviceToggles)); diff --git a/src/dawn/native/Adapter.h b/src/dawn/native/Adapter.h index 86f97e1498..08e0f234e9 100644 --- a/src/dawn/native/Adapter.h +++ b/src/dawn/native/Adapter.h @@ -34,7 +34,9 @@ class DeviceBase; class AdapterBase : public RefCounted { public: - AdapterBase(InstanceBase* instance, wgpu::BackendType backend); + AdapterBase(InstanceBase* instance, + wgpu::BackendType backend, + const TogglesState& adapterToggles); ~AdapterBase() override; MaybeError Initialize(); @@ -65,6 +67,9 @@ class AdapterBase : public RefCounted { void SetUseTieredLimits(bool useTieredLimits); + // Get the actual toggles state of the adapter. + const TogglesState& GetTogglesState() const; + virtual bool SupportsExternalImages() const = 0; protected: @@ -113,6 +118,10 @@ class AdapterBase : public RefCounted { virtual MaybeError ResetInternalDeviceForTestingImpl(); Ref mInstance; wgpu::BackendType mBackend; + + // Adapter toggles state, currently only inherited from instance toggles state. + TogglesState mTogglesState; + CombinedLimits mLimits; bool mUseTieredLimits = false; }; diff --git a/src/dawn/native/BackendConnection.cpp b/src/dawn/native/BackendConnection.cpp index 8dcd19fd45..303b8b1d6f 100644 --- a/src/dawn/native/BackendConnection.cpp +++ b/src/dawn/native/BackendConnection.cpp @@ -13,6 +13,7 @@ // limitations under the License. #include "dawn/native/BackendConnection.h" +#include "dawn/native/Instance.h" namespace dawn::native { @@ -28,7 +29,8 @@ InstanceBase* BackendConnection::GetInstance() const { } ResultOrError>> BackendConnection::DiscoverAdapters( - const AdapterDiscoveryOptionsBase* options) { + const AdapterDiscoveryOptionsBase* options, + const TogglesState& adapterToggles) { return DAWN_VALIDATION_ERROR("DiscoverAdapters not implemented for this backend."); } diff --git a/src/dawn/native/BackendConnection.h b/src/dawn/native/BackendConnection.h index 04fe35bec6..8d279df570 100644 --- a/src/dawn/native/BackendConnection.h +++ b/src/dawn/native/BackendConnection.h @@ -20,6 +20,7 @@ #include "dawn/native/Adapter.h" #include "dawn/native/DawnNative.h" +#include "dawn/native/Toggles.h" namespace dawn::native { @@ -35,11 +36,13 @@ class BackendConnection { // Returns all the adapters for the system that can be created by the backend, without extra // options (such as debug adapters, custom driver libraries, etc.) - virtual std::vector> DiscoverDefaultAdapters() = 0; + virtual std::vector> DiscoverDefaultAdapters( + const TogglesState& adapterToggles) = 0; // Returns new adapters created with the backend-specific options. virtual ResultOrError>> DiscoverAdapters( - const AdapterDiscoveryOptionsBase* options); + const AdapterDiscoveryOptionsBase* options, + const TogglesState& adapterToggles); private: InstanceBase* mInstance = nullptr; diff --git a/src/dawn/native/Instance.cpp b/src/dawn/native/Instance.cpp index a2c490da63..7035c6ff85 100644 --- a/src/dawn/native/Instance.cpp +++ b/src/dawn/native/Instance.cpp @@ -23,6 +23,7 @@ #include "dawn/native/ChainUtils_autogen.h" #include "dawn/native/ErrorData.h" #include "dawn/native/Surface.h" +#include "dawn/native/Toggles.h" #include "dawn/native/ValidationUtils_autogen.h" #include "dawn/platform/DawnPlatform.h" @@ -110,18 +111,30 @@ InstanceBase* APICreateInstance(const InstanceDescriptor* descriptor) { // static Ref InstanceBase::Create(const InstanceDescriptor* descriptor) { - Ref instance = AcquireRef(new InstanceBase); static constexpr InstanceDescriptor kDefaultDesc = {}; if (descriptor == nullptr) { descriptor = &kDefaultDesc; } + + const DawnTogglesDescriptor* instanceTogglesDesc = nullptr; + FindInChain(descriptor->nextInChain, &instanceTogglesDesc); + + // Set up the instance toggle state from toggles descriptor + TogglesState instanceToggles = + TogglesState::CreateFromTogglesDescriptor(instanceTogglesDesc, ToggleStage::Instance); + // By default enable the DisallowUnsafeAPIs instance toggle, it will be inherited to adapters + // and devices created by this instance if not overriden. + // TODO(dawn:1685): Rename DisallowUnsafeAPIs to AllowUnsafeAPIs, and change relating logic. + instanceToggles.Default(Toggle::DisallowUnsafeAPIs, true); + + Ref instance = AcquireRef(new InstanceBase(instanceToggles)); if (instance->ConsumedError(instance->Initialize(descriptor))) { return nullptr; } return instance; } -InstanceBase::InstanceBase() = default; +InstanceBase::InstanceBase(const TogglesState& instanceToggles) : mToggles(instanceToggles) {} InstanceBase::~InstanceBase() = default; @@ -138,7 +151,9 @@ void InstanceBase::WillDropLastExternalRef() { // TODO(crbug.com/dawn/832): make the platform an initialization parameter of the instance. MaybeError InstanceBase::Initialize(const InstanceDescriptor* descriptor) { - DAWN_TRY(ValidateSingleSType(descriptor->nextInChain, wgpu::SType::DawnInstanceDescriptor)); + DAWN_TRY(ValidateSTypes(descriptor->nextInChain, {{wgpu::SType::DawnInstanceDescriptor}, + {wgpu::SType::DawnTogglesDescriptor}})); + const DawnInstanceDescriptor* dawnDesc = nullptr; FindInChain(descriptor->nextInChain, &dawnDesc); if (dawnDesc != nullptr) { @@ -280,7 +295,14 @@ void InstanceBase::DiscoverDefaultAdapters() { // Query and merge all default adapters for all backends for (std::unique_ptr& backend : mBackends) { - std::vector> backendAdapters = backend->DiscoverDefaultAdapters(); + // Set up toggles state for default adapters, currently adapter don't have a toggles + // descriptor so just inherit from instance toggles. + // TODO(dawn:1495): Handle the adapter toggles descriptor after implemented. + TogglesState adapterToggles = TogglesState(ToggleStage::Adapter); + adapterToggles.InheritFrom(mToggles); + + std::vector> backendAdapters = + backend->DiscoverDefaultAdapters(adapterToggles); for (Ref& adapter : backendAdapters) { ASSERT(adapter->GetBackendType() == backend->GetType()); @@ -306,6 +328,10 @@ bool InstanceBase::DiscoverAdapters(const AdapterDiscoveryOptionsBase* options) return true; } +const TogglesState& InstanceBase::GetTogglesState() const { + return mToggles; +} + const ToggleInfo* InstanceBase::GetToggleInfo(const char* toggleName) { return mTogglesInfo.GetToggleInfo(toggleName); } @@ -397,8 +423,14 @@ MaybeError InstanceBase::DiscoverAdaptersInternal(const AdapterDiscoveryOptionsB } foundBackend = true; + // Set up toggles state for default adapters, currently adapter don't have a toggles + // descriptor so just inherit from instance toggles. + // TODO(dawn:1495): Handle the adapter toggles descriptor after implemented. + TogglesState adapterToggles = TogglesState(ToggleStage::Adapter); + adapterToggles.InheritFrom(mToggles); + std::vector> newAdapters; - DAWN_TRY_ASSIGN(newAdapters, backend->DiscoverAdapters(options)); + DAWN_TRY_ASSIGN(newAdapters, backend->DiscoverAdapters(options, adapterToggles)); for (Ref& adapter : newAdapters) { ASSERT(adapter->GetBackendType() == backend->GetType()); diff --git a/src/dawn/native/Instance.h b/src/dawn/native/Instance.h index f6d518f1f0..6ae291c5f0 100644 --- a/src/dawn/native/Instance.h +++ b/src/dawn/native/Instance.h @@ -72,6 +72,8 @@ class InstanceBase final : public RefCountedWithExternalCount { return false; } + const TogglesState& GetTogglesState() const; + // Used to query the details of a toggle. Return nullptr if toggleName is not a valid name // of a toggle supported in Dawn. const ToggleInfo* GetToggleInfo(const char* toggleName); @@ -113,7 +115,7 @@ class InstanceBase final : public RefCountedWithExternalCount { Surface* APICreateSurface(const SurfaceDescriptor* descriptor); private: - InstanceBase(); + explicit InstanceBase(const TogglesState& instanceToggles); ~InstanceBase() override; void WillDropLastExternalRef() override; @@ -150,6 +152,8 @@ class InstanceBase final : public RefCountedWithExternalCount { std::vector> mBackends; std::vector> mAdapters; + TogglesState mToggles; + FeaturesInfo mFeaturesInfo; TogglesInfo mTogglesInfo; diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index 39e1fc2c89..028ded6b0b 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp @@ -176,7 +176,7 @@ static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{ {"disallow_unsafe_apis", "Produces validation errors on API entry points or parameter combinations that aren't " "considered secure yet.", - "http://crbug.com/1138528", ToggleStage::Device}}, + "http://crbug.com/1138528", ToggleStage::Instance}}, {Toggle::FlushBeforeClientWaitSync, {"flush_before_client_wait_sync", "Call glFlush before glClientWaitSync to work around bugs in the latter", @@ -447,7 +447,9 @@ TogglesState TogglesState::CreateFromTogglesDescriptor(const DawnTogglesDescript Toggle toggle = togglesInfo.ToggleNameToEnum(togglesDesc->enabledToggles[i]); if (toggle != Toggle::InvalidEnum) { const ToggleInfo* toggleInfo = togglesInfo.GetToggleInfo(toggle); - if (toggleInfo->stage == requiredStage) { + // Accept the required toggles of current and earlier stage to allow override + // inheritance. + if (toggleInfo->stage <= requiredStage) { togglesState.mTogglesSet.Set(toggle, true); togglesState.mEnabledToggles.Set(toggle, true); } @@ -457,7 +459,9 @@ TogglesState TogglesState::CreateFromTogglesDescriptor(const DawnTogglesDescript Toggle toggle = togglesInfo.ToggleNameToEnum(togglesDesc->disabledToggles[i]); if (toggle != Toggle::InvalidEnum) { const ToggleInfo* toggleInfo = togglesInfo.GetToggleInfo(toggle); - if (toggleInfo->stage == requiredStage) { + // Accept the required toggles of current and earlier stage to allow override + // inheritance. + if (toggleInfo->stage <= requiredStage) { togglesState.mTogglesSet.Set(toggle, true); togglesState.mEnabledToggles.Set(toggle, false); } @@ -467,6 +471,30 @@ TogglesState TogglesState::CreateFromTogglesDescriptor(const DawnTogglesDescript return togglesState; } +TogglesState& TogglesState::InheritFrom(const TogglesState& inheritedToggles) { + ASSERT(inheritedToggles.GetStage() < mStage); + + // Do inheritance. All toggles that are force-set in the inherited toggles states would + // be force-set in the result toggles state, and all toggles that are set in the inherited + // toggles states and not required in current toggles state would be set in the result toggles + // state. + for (uint32_t i : inheritedToggles.mTogglesSet.Iterate()) { + const Toggle& toggle = static_cast(i); + ASSERT(TogglesInfo::GetToggleInfo(toggle)->stage < mStage); + bool isEnabled = inheritedToggles.mEnabledToggles.Has(toggle); + bool isForced = inheritedToggles.mForcedToggles.Has(toggle); + // Only inherit a toggle if it is not set by user requirement or is forced in earlier stage. + // In this way we allow user requirement override the inheritance if not forced. + if (!mTogglesSet.Has(toggle) || isForced) { + mTogglesSet.Set(toggle, true); + mEnabledToggles.Set(toggle, isEnabled); + mForcedToggles.Set(toggle, isForced); + } + } + + return *this; +} + // Set a toggle to given state, if the toggle has not been already set. Do nothing otherwise. void TogglesState::Default(Toggle toggle, bool enabled) { ASSERT(toggle != Toggle::InvalidEnum); @@ -492,6 +520,15 @@ void TogglesState::ForceSet(Toggle toggle, bool enabled) { mForcedToggles.Set(toggle, true); } +TogglesState& TogglesState::SetForTesting(Toggle toggle, bool enabled, bool forced) { + ASSERT(toggle != Toggle::InvalidEnum); + mTogglesSet.Set(toggle, true); + mEnabledToggles.Set(toggle, enabled); + mForcedToggles.Set(toggle, forced); + + return *this; +} + bool TogglesState::IsSet(Toggle toggle) const { // Ensure that the toggle never used earlier than its stage. ASSERT(TogglesInfo::GetToggleInfo(toggle)->stage <= mStage); diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index 6d75fff604..064ebf1125 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h @@ -127,17 +127,25 @@ class TogglesState { // Create an empty toggles state of given stage explicit TogglesState(ToggleStage stage); - // Create a RequiredTogglesSet from a DawnTogglesDescriptor, only considering toggles of + // Create a TogglesState from a DawnTogglesDescriptor, only considering toggles of // required toggle stage. static TogglesState CreateFromTogglesDescriptor(const DawnTogglesDescriptor* togglesDesc, ToggleStage requiredStage); + // Inherit from a given toggles state of earlier stage, only inherit the forced and the + // unrequired toggles to allow overriding. Return *this to allow method chaining manner. + TogglesState& InheritFrom(const TogglesState& inheritedToggles); + // Set a toggle of the same stage of toggles state stage if and only if it is not already set. void Default(Toggle toggle, bool enabled); // Force set a toggle of same stage of toggles state stage. A force-set toggle will get // inherited to all later stage as forced. void ForceSet(Toggle toggle, bool enabled); + // Set a toggle of any stage for testing propose. Return *this to allow method chaining + // manner. + TogglesState& SetForTesting(Toggle toggle, bool enabled, bool forced); + // Return whether the toggle is set or not. Force-set is always treated as set. bool IsSet(Toggle toggle) const; // Return true if and only if the toggle is set to true. diff --git a/src/dawn/native/d3d12/AdapterD3D12.cpp b/src/dawn/native/d3d12/AdapterD3D12.cpp index 8fd7993881..bd261c47bb 100644 --- a/src/dawn/native/d3d12/AdapterD3D12.cpp +++ b/src/dawn/native/d3d12/AdapterD3D12.cpp @@ -28,8 +28,10 @@ namespace dawn::native::d3d12 { -Adapter::Adapter(Backend* backend, ComPtr hardwareAdapter) - : AdapterBase(backend->GetInstance(), wgpu::BackendType::D3D12), +Adapter::Adapter(Backend* backend, + ComPtr hardwareAdapter, + const TogglesState& adapterToggles) + : AdapterBase(backend->GetInstance(), wgpu::BackendType::D3D12, adapterToggles), mHardwareAdapter(hardwareAdapter), mBackend(backend) {} diff --git a/src/dawn/native/d3d12/AdapterD3D12.h b/src/dawn/native/d3d12/AdapterD3D12.h index 0535ee1a1c..4309dff416 100644 --- a/src/dawn/native/d3d12/AdapterD3D12.h +++ b/src/dawn/native/d3d12/AdapterD3D12.h @@ -26,7 +26,9 @@ class Backend; class Adapter : public AdapterBase { public: - Adapter(Backend* backend, ComPtr hardwareAdapter); + Adapter(Backend* backend, + ComPtr hardwareAdapter, + const TogglesState& adapterToggles); ~Adapter() override; // AdapterBase Implementation diff --git a/src/dawn/native/d3d12/BackendD3D12.cpp b/src/dawn/native/d3d12/BackendD3D12.cpp index ac2f5f252a..6e5fff584e 100644 --- a/src/dawn/native/d3d12/BackendD3D12.cpp +++ b/src/dawn/native/d3d12/BackendD3D12.cpp @@ -69,10 +69,12 @@ ResultOrError> CreateFactory(const PlatformFunctions* func } ResultOrError> CreateAdapterFromIDXGIAdapter(Backend* backend, - ComPtr dxgiAdapter) { + ComPtr dxgiAdapter, + const TogglesState& adapterToggles) { ComPtr dxgiAdapter3; DAWN_TRY(CheckHRESULT(dxgiAdapter.As(&dxgiAdapter3), "DXGIAdapter retrieval")); - Ref adapter = AcquireRef(new Adapter(backend, std::move(dxgiAdapter3))); + Ref adapter = + AcquireRef(new Adapter(backend, std::move(dxgiAdapter3), adapterToggles)); DAWN_TRY(adapter->Initialize()); return {std::move(adapter)}; @@ -271,9 +273,9 @@ const PlatformFunctions* Backend::GetFunctions() const { return mFunctions.get(); } -std::vector> Backend::DiscoverDefaultAdapters() { +std::vector> Backend::DiscoverDefaultAdapters(const TogglesState& adapterToggles) { AdapterDiscoveryOptions options; - auto result = DiscoverAdapters(&options); + auto result = DiscoverAdapters(&options, adapterToggles); if (result.IsError()) { GetInstance()->ConsumedError(result.AcquireError()); return {}; @@ -282,7 +284,8 @@ std::vector> Backend::DiscoverDefaultAdapters() { } ResultOrError>> Backend::DiscoverAdapters( - const AdapterDiscoveryOptionsBase* optionsBase) { + const AdapterDiscoveryOptionsBase* optionsBase, + const TogglesState& adapterToggles) { ASSERT(optionsBase->backendType == WGPUBackendType_D3D12); const AdapterDiscoveryOptions* options = static_cast(optionsBase); @@ -291,7 +294,8 @@ ResultOrError>> Backend::DiscoverAdapters( if (options->dxgiAdapter != nullptr) { // |dxgiAdapter| was provided. Discover just that adapter. Ref adapter; - DAWN_TRY_ASSIGN(adapter, CreateAdapterFromIDXGIAdapter(this, options->dxgiAdapter)); + DAWN_TRY_ASSIGN(adapter, + CreateAdapterFromIDXGIAdapter(this, options->dxgiAdapter, adapterToggles)); adapters.push_back(std::move(adapter)); return std::move(adapters); } @@ -304,7 +308,8 @@ ResultOrError>> Backend::DiscoverAdapters( } ASSERT(dxgiAdapter != nullptr); - ResultOrError> adapter = CreateAdapterFromIDXGIAdapter(this, dxgiAdapter); + ResultOrError> adapter = + CreateAdapterFromIDXGIAdapter(this, dxgiAdapter, adapterToggles); if (adapter.IsError()) { GetInstance()->ConsumedError(adapter.AcquireError()); continue; diff --git a/src/dawn/native/d3d12/BackendD3D12.h b/src/dawn/native/d3d12/BackendD3D12.h index 4d0184a22b..f2bb13f818 100644 --- a/src/dawn/native/d3d12/BackendD3D12.h +++ b/src/dawn/native/d3d12/BackendD3D12.h @@ -75,9 +75,11 @@ class Backend : public BackendConnection { const PlatformFunctions* GetFunctions() const; - std::vector> DiscoverDefaultAdapters() override; + std::vector> DiscoverDefaultAdapters( + const TogglesState& adapterToggles) override; ResultOrError>> DiscoverAdapters( - const AdapterDiscoveryOptionsBase* optionsBase) override; + const AdapterDiscoveryOptionsBase* optionsBase, + const TogglesState& adapterToggles) override; private: // Acquiring DXC version information and store the result in mDxcVersionInfo. This function diff --git a/src/dawn/native/metal/BackendMTL.h b/src/dawn/native/metal/BackendMTL.h index 75be73040e..b5b9601b03 100644 --- a/src/dawn/native/metal/BackendMTL.h +++ b/src/dawn/native/metal/BackendMTL.h @@ -26,9 +26,11 @@ class Backend : public BackendConnection { explicit Backend(InstanceBase* instance); ~Backend() override; - std::vector> DiscoverDefaultAdapters() override; + std::vector> DiscoverDefaultAdapters( + const TogglesState& adapterToggles) override; ResultOrError>> DiscoverAdapters( - const AdapterDiscoveryOptionsBase* optionsBase) override; + const AdapterDiscoveryOptionsBase* optionsBase, + const TogglesState& adapterToggles) override; }; } // namespace dawn::native::metal diff --git a/src/dawn/native/metal/BackendMTL.mm b/src/dawn/native/metal/BackendMTL.mm index 5a1b166aa8..512ab99d4f 100644 --- a/src/dawn/native/metal/BackendMTL.mm +++ b/src/dawn/native/metal/BackendMTL.mm @@ -255,8 +255,8 @@ DAWN_NOINLINE bool IsGPUCounterSupported(id device, class Adapter : public AdapterBase { public: - Adapter(InstanceBase* instance, id device) - : AdapterBase(instance, wgpu::BackendType::Metal), mDevice(device) { + Adapter(InstanceBase* instance, id device, const TogglesState& requiredAdapterToggle) + : AdapterBase(instance, wgpu::BackendType::Metal, requiredAdapterToggle), mDevice(device) { mName = std::string([[*mDevice name] UTF8String]); PCIIDs ids; @@ -774,9 +774,9 @@ Backend::Backend(InstanceBase* instance) : BackendConnection(instance, wgpu::Bac Backend::~Backend() = default; -std::vector> Backend::DiscoverDefaultAdapters() { +std::vector> Backend::DiscoverDefaultAdapters(const TogglesState& adapterToggles) { AdapterDiscoveryOptions options; - auto result = DiscoverAdapters(&options); + auto result = DiscoverAdapters(&options, adapterToggles); if (result.IsError()) { GetInstance()->ConsumedError(result.AcquireError()); return {}; @@ -785,7 +785,8 @@ std::vector> Backend::DiscoverDefaultAdapters() { } ResultOrError>> Backend::DiscoverAdapters( - const AdapterDiscoveryOptionsBase* optionsBase) { + const AdapterDiscoveryOptionsBase* optionsBase, + const TogglesState& adapterToggles) { ASSERT(optionsBase->backendType == WGPUBackendType_Metal); std::vector> adapters; @@ -793,7 +794,7 @@ ResultOrError>> Backend::DiscoverAdapters( NSRef>> devices = AcquireNSRef(MTLCopyAllDevices()); for (id device in devices.Get()) { - Ref adapter = AcquireRef(new Adapter(GetInstance(), device)); + Ref adapter = AcquireRef(new Adapter(GetInstance(), device, adapterToggles)); if (!GetInstance()->ConsumedError(adapter->Initialize())) { adapters.push_back(std::move(adapter)); } @@ -802,7 +803,8 @@ ResultOrError>> Backend::DiscoverAdapters( // iOS only has a single device so MTLCopyAllDevices doesn't exist there. #if defined(DAWN_PLATFORM_IOS) - Ref adapter = AcquireRef(new Adapter(GetInstance(), MTLCreateSystemDefaultDevice())); + Ref adapter = + AcquireRef(new Adapter(GetInstance(), MTLCreateSystemDefaultDevice(), adapterToggles)); if (!GetInstance()->ConsumedError(adapter->Initialize())) { adapters.push_back(std::move(adapter)); } diff --git a/src/dawn/native/null/DeviceNull.cpp b/src/dawn/native/null/DeviceNull.cpp index c9d32ad3e2..9827bdf267 100644 --- a/src/dawn/native/null/DeviceNull.cpp +++ b/src/dawn/native/null/DeviceNull.cpp @@ -30,7 +30,12 @@ namespace dawn::native::null { // Implementation of pre-Device objects: the null adapter, null backend connection and Connect() -Adapter::Adapter(InstanceBase* instance) : AdapterBase(instance, wgpu::BackendType::Null) { +Adapter::Adapter(InstanceBase* instance) + : Adapter(instance, + TogglesState(ToggleStage::Adapter).InheritFrom(instance->GetTogglesState())) {} + +Adapter::Adapter(InstanceBase* instance, const TogglesState& adapterToggles) + : AdapterBase(instance, wgpu::BackendType::Null, adapterToggles) { mVendorId = 0; mDeviceId = 0; mName = "Null backend"; @@ -85,11 +90,12 @@ class Backend : public BackendConnection { explicit Backend(InstanceBase* instance) : BackendConnection(instance, wgpu::BackendType::Null) {} - std::vector> DiscoverDefaultAdapters() override { + std::vector> DiscoverDefaultAdapters( + const TogglesState& adapterToggles) override { // There is always a single Null adapter because it is purely CPU based and doesn't // depend on the system. std::vector> adapters; - Ref adapter = AcquireRef(new Adapter(GetInstance())); + Ref adapter = AcquireRef(new Adapter(GetInstance(), adapterToggles)); adapters.push_back(std::move(adapter)); return adapters; } diff --git a/src/dawn/native/null/DeviceNull.h b/src/dawn/native/null/DeviceNull.h index b53b036231..d5ea66f5bc 100644 --- a/src/dawn/native/null/DeviceNull.h +++ b/src/dawn/native/null/DeviceNull.h @@ -171,7 +171,10 @@ class Device final : public DeviceBase { class Adapter : public AdapterBase { public: + // Create null adapter without providing toggles state for testing, only inherit instance's + // toggles state explicit Adapter(InstanceBase* instance); + Adapter(InstanceBase* instance, const TogglesState& adapterToggles); ~Adapter() override; // AdapterBase Implementation diff --git a/src/dawn/native/opengl/AdapterGL.cpp b/src/dawn/native/opengl/AdapterGL.cpp index b965e42ef1..4bffcf6840 100644 --- a/src/dawn/native/opengl/AdapterGL.cpp +++ b/src/dawn/native/opengl/AdapterGL.cpp @@ -53,8 +53,10 @@ uint32_t GetVendorIdFromVendors(const char* vendor) { } // anonymous namespace -Adapter::Adapter(InstanceBase* instance, wgpu::BackendType backendType) - : AdapterBase(instance, backendType) {} +Adapter::Adapter(InstanceBase* instance, + wgpu::BackendType backendType, + const TogglesState& adapterToggle) + : AdapterBase(instance, backendType, adapterToggle) {} MaybeError Adapter::InitializeGLFunctions(void* (*getProc)(const char*)) { // Use getProc to populate the dispatch table diff --git a/src/dawn/native/opengl/AdapterGL.h b/src/dawn/native/opengl/AdapterGL.h index 918e439395..22b47f0877 100644 --- a/src/dawn/native/opengl/AdapterGL.h +++ b/src/dawn/native/opengl/AdapterGL.h @@ -23,7 +23,9 @@ namespace dawn::native::opengl { class Adapter : public AdapterBase { public: - Adapter(InstanceBase* instance, wgpu::BackendType backendType); + Adapter(InstanceBase* instance, + wgpu::BackendType backendType, + const TogglesState& adapterToggle); MaybeError InitializeGLFunctions(void* (*getProc)(const char*)); diff --git a/src/dawn/native/opengl/BackendGL.cpp b/src/dawn/native/opengl/BackendGL.cpp index ac4e374e0d..dec797614f 100644 --- a/src/dawn/native/opengl/BackendGL.cpp +++ b/src/dawn/native/opengl/BackendGL.cpp @@ -33,7 +33,7 @@ namespace dawn::native::opengl { Backend::Backend(InstanceBase* instance, wgpu::BackendType backendType) : BackendConnection(instance, backendType) {} -std::vector> Backend::DiscoverDefaultAdapters() { +std::vector> Backend::DiscoverDefaultAdapters(const TogglesState& adapterToggles) { std::vector> adapters; #if DAWN_PLATFORM_IS(WINDOWS) const char* eglLib = "libEGL.dll"; @@ -69,7 +69,7 @@ std::vector> Backend::DiscoverDefaultAdapters() { context->MakeCurrent(); - auto result = DiscoverAdapters(&options); + auto result = DiscoverAdapters(&options, adapterToggles); if (result.IsError()) { GetInstance()->ConsumedError(result.AcquireError()); @@ -84,7 +84,8 @@ std::vector> Backend::DiscoverDefaultAdapters() { } ResultOrError>> Backend::DiscoverAdapters( - const AdapterDiscoveryOptionsBase* optionsBase) { + const AdapterDiscoveryOptionsBase* optionsBase, + const TogglesState& adapterToggles) { // TODO(cwallez@chromium.org): For now only create a single OpenGL adapter because don't // know how to handle MakeCurrent. DAWN_INVALID_IF(mCreatedAdapter, "The OpenGL backend can only create a single adapter."); @@ -95,8 +96,8 @@ ResultOrError>> Backend::DiscoverAdapters( DAWN_INVALID_IF(options->getProc == nullptr, "AdapterDiscoveryOptions::getProc must be set"); - Ref adapter = AcquireRef( - new Adapter(GetInstance(), static_cast(optionsBase->backendType))); + Ref adapter = AcquireRef(new Adapter( + GetInstance(), static_cast(optionsBase->backendType), adapterToggles)); DAWN_TRY(adapter->InitializeGLFunctions(options->getProc)); DAWN_TRY(adapter->Initialize()); diff --git a/src/dawn/native/opengl/BackendGL.h b/src/dawn/native/opengl/BackendGL.h index dc8fea5d6c..e31dd8d586 100644 --- a/src/dawn/native/opengl/BackendGL.h +++ b/src/dawn/native/opengl/BackendGL.h @@ -26,9 +26,11 @@ class Backend : public BackendConnection { public: Backend(InstanceBase* instance, wgpu::BackendType backendType); - std::vector> DiscoverDefaultAdapters() override; + std::vector> DiscoverDefaultAdapters( + const TogglesState& adapterToggles) override; ResultOrError>> DiscoverAdapters( - const AdapterDiscoveryOptionsBase* options) override; + const AdapterDiscoveryOptionsBase* option, + const TogglesState& adapterToggless) override; private: bool mCreatedAdapter = false; diff --git a/src/dawn/native/vulkan/AdapterVk.cpp b/src/dawn/native/vulkan/AdapterVk.cpp index 19556823cb..556f5f8f05 100644 --- a/src/dawn/native/vulkan/AdapterVk.cpp +++ b/src/dawn/native/vulkan/AdapterVk.cpp @@ -58,8 +58,9 @@ gpu_info::DriverVersion DecodeVulkanDriverVersion(uint32_t vendorID, uint32_t ve Adapter::Adapter(InstanceBase* instance, VulkanInstance* vulkanInstance, - VkPhysicalDevice physicalDevice) - : AdapterBase(instance, wgpu::BackendType::Vulkan), + VkPhysicalDevice physicalDevice, + const TogglesState& adapterToggles) + : AdapterBase(instance, wgpu::BackendType::Vulkan, adapterToggles), mPhysicalDevice(physicalDevice), mVulkanInstance(vulkanInstance) {} diff --git a/src/dawn/native/vulkan/AdapterVk.h b/src/dawn/native/vulkan/AdapterVk.h index 861980822f..ebdbfcb150 100644 --- a/src/dawn/native/vulkan/AdapterVk.h +++ b/src/dawn/native/vulkan/AdapterVk.h @@ -29,7 +29,8 @@ class Adapter : public AdapterBase { public: Adapter(InstanceBase* instance, VulkanInstance* vulkanInstance, - VkPhysicalDevice physicalDevice); + VkPhysicalDevice physicalDevice, + const TogglesState& adapterToggles); ~Adapter() override; // AdapterBase Implementation diff --git a/src/dawn/native/vulkan/BackendVk.cpp b/src/dawn/native/vulkan/BackendVk.cpp index e90cee6ab4..7b5c41fc1b 100644 --- a/src/dawn/native/vulkan/BackendVk.cpp +++ b/src/dawn/native/vulkan/BackendVk.cpp @@ -457,9 +457,9 @@ Backend::Backend(InstanceBase* instance) : BackendConnection(instance, wgpu::Bac Backend::~Backend() = default; -std::vector> Backend::DiscoverDefaultAdapters() { +std::vector> Backend::DiscoverDefaultAdapters(const TogglesState& adapterToggles) { AdapterDiscoveryOptions options; - auto result = DiscoverAdapters(&options); + auto result = DiscoverAdapters(&options, adapterToggles); if (result.IsError()) { GetInstance()->ConsumedError(result.AcquireError()); return {}; @@ -468,7 +468,8 @@ std::vector> Backend::DiscoverDefaultAdapters() { } ResultOrError>> Backend::DiscoverAdapters( - const AdapterDiscoveryOptionsBase* optionsBase) { + const AdapterDiscoveryOptionsBase* optionsBase, + const TogglesState& adapterToggles) { ASSERT(optionsBase->backendType == WGPUBackendType_Vulkan); const AdapterDiscoveryOptions* options = @@ -497,8 +498,8 @@ ResultOrError>> Backend::DiscoverAdapters( const std::vector& physicalDevices = mVulkanInstances[icd]->GetPhysicalDevices(); for (uint32_t i = 0; i < physicalDevices.size(); ++i) { - Ref adapter = - AcquireRef(new Adapter(instance, mVulkanInstances[icd].Get(), physicalDevices[i])); + Ref adapter = AcquireRef(new Adapter(instance, mVulkanInstances[icd].Get(), + physicalDevices[i], adapterToggles)); if (instance->ConsumedError(adapter->Initialize())) { continue; } diff --git a/src/dawn/native/vulkan/BackendVk.h b/src/dawn/native/vulkan/BackendVk.h index f912344d31..f8f033990b 100644 --- a/src/dawn/native/vulkan/BackendVk.h +++ b/src/dawn/native/vulkan/BackendVk.h @@ -91,9 +91,11 @@ class Backend : public BackendConnection { MaybeError Initialize(); - std::vector> DiscoverDefaultAdapters() override; + std::vector> DiscoverDefaultAdapters( + const TogglesState& adapterToggles) override; ResultOrError>> DiscoverAdapters( - const AdapterDiscoveryOptionsBase* optionsBase) override; + const AdapterDiscoveryOptionsBase* optionsBase, + const TogglesState& adapterToggles) override; private: ityp::array, 2> mVulkanInstances = {}; diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn index 35e216558e..4c3d14c9da 100644 --- a/src/dawn/tests/BUILD.gn +++ b/src/dawn/tests/BUILD.gn @@ -309,6 +309,7 @@ dawn_test("dawn_unittests") { "unittests/SubresourceStorageTests.cpp", "unittests/SystemUtilsTests.cpp", "unittests/ToBackendTests.cpp", + "unittests/ToggleTests.cpp", "unittests/TypedIntegerTests.cpp", "unittests/UnicodeTests.cpp", "unittests/native/AllowedErrorTests.cpp", diff --git a/src/dawn/tests/DawnNativeTest.cpp b/src/dawn/tests/DawnNativeTest.cpp index bd5cf45945..e8f06289b2 100644 --- a/src/dawn/tests/DawnNativeTest.cpp +++ b/src/dawn/tests/DawnNativeTest.cpp @@ -45,7 +45,19 @@ DawnNativeTest::~DawnNativeTest() { } void DawnNativeTest::SetUp() { - instance = std::make_unique(); + // Create an instance with toggle DisallowUnsafeApis disabled, which would be inherited to + // adapter and device toggles and allow us to test unsafe apis (including experimental + // features). + const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceToggles = {}; + instanceToggles.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceToggles.disabledTogglesCount = 1; + instanceToggles.disabledToggles = &disallowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceToggles.chain; + + instance = std::make_unique(&instanceDesc); instance->EnableAdapterBlocklist(false); platform = CreateTestPlatform(); dawn::native::FromAPI(instance->Get())->SetPlatformForTesting(platform.get()); @@ -78,16 +90,7 @@ std::unique_ptr DawnNativeTest::CreateTestPlatform() { } WGPUDevice DawnNativeTest::CreateTestDevice() { - // Disabled disallowing unsafe APIs so we can test them. - wgpu::DeviceDescriptor deviceDescriptor = {}; - wgpu::DawnTogglesDescriptor deviceTogglesDesc = {}; - deviceDescriptor.nextInChain = &deviceTogglesDesc; - - const char* toggle = "disallow_unsafe_apis"; - deviceTogglesDesc.disabledToggles = &toggle; - deviceTogglesDesc.disabledTogglesCount = 1; - - return adapter.CreateDevice(&deviceDescriptor); + return adapter.CreateDevice(); } // static diff --git a/src/dawn/tests/DawnTest.cpp b/src/dawn/tests/DawnTest.cpp index e70b263584..3765c44dc0 100644 --- a/src/dawn/tests/DawnTest.cpp +++ b/src/dawn/tests/DawnTest.cpp @@ -73,6 +73,56 @@ void printBuffer(testing::AssertionResult& result, const T* buffer, const size_t result << std::endl; } +// A helper class to create DawnTogglesDescriptor from test params +struct ParamTogglesHelper { + std::vector enabledToggles; + std::vector disabledToggles; + wgpu::DawnTogglesDescriptor togglesDesc; + + // Create toggles descriptor for a given stage from test param and global test env + ParamTogglesHelper(const AdapterTestParam& testParam, dawn::native::ToggleStage requiredStage) { + for (const char* requireEnabledWorkaround : testParam.forceEnabledWorkarounds) { + const dawn::native::ToggleInfo* info = + gTestEnv->GetInstance()->GetToggleInfo(requireEnabledWorkaround); + ASSERT(info != nullptr); + if (info->stage == requiredStage) { + enabledToggles.push_back(requireEnabledWorkaround); + } + } + for (const char* requireDisabledWorkaround : testParam.forceDisabledWorkarounds) { + const dawn::native::ToggleInfo* info = + gTestEnv->GetInstance()->GetToggleInfo(requireDisabledWorkaround); + ASSERT(info != nullptr); + if (info->stage == requiredStage) { + disabledToggles.push_back(requireDisabledWorkaround); + } + } + + for (const std::string& toggle : gTestEnv->GetEnabledToggles()) { + const dawn::native::ToggleInfo* info = + gTestEnv->GetInstance()->GetToggleInfo(toggle.c_str()); + ASSERT(info != nullptr); + if (info->stage == requiredStage) { + enabledToggles.push_back(info->name); + } + } + + for (const std::string& toggle : gTestEnv->GetDisabledToggles()) { + const dawn::native::ToggleInfo* info = + gTestEnv->GetInstance()->GetToggleInfo(toggle.c_str()); + ASSERT(info != nullptr); + if (info->stage == requiredStage) { + disabledToggles.push_back(info->name); + } + } + + togglesDesc = {}; + togglesDesc.enabledToggles = enabledToggles.data(); + togglesDesc.enabledTogglesCount = enabledToggles.size(); + togglesDesc.disabledToggles = disabledToggles.data(); + togglesDesc.disabledTogglesCount = disabledToggles.size(); + } +}; } // anonymous namespace DawnTestBase::PrintToStringParamName::PrintToStringParamName(const char* test) : mTest(test) {} @@ -307,7 +357,19 @@ void DawnTestEnvironment::ParseArgs(int argc, char** argv) { } std::unique_ptr DawnTestEnvironment::CreateInstanceAndDiscoverAdapters() { - auto instance = std::make_unique(); + // Create an instance with toggle DisallowUnsafeApis disabled, which would be inherited to + // adapter and device toggles and allow us to test unsafe apis (including experimental + // features). + const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceToggles = {}; + instanceToggles.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceToggles.disabledTogglesCount = 1; + instanceToggles.disabledToggles = &disallowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceToggles.chain; + + auto instance = std::make_unique(&instanceDesc); instance->EnableBeginCaptureOnStartup(mBeginCaptureOnStartup); instance->SetBackendValidationLevel(mBackendValidationLevel); instance->EnableAdapterBlocklist(false); @@ -577,8 +639,9 @@ DawnTestBase::DawnTestBase(const AdapterTestParam& param) : mParam(param) { gCurrentTest = this; DawnProcTable procs = dawn::native::GetProcs(); - // Override procs to provide harness-specific behavior to always select the null adapter, - // and to allow fixture-specific overriding of the test device with CreateDeviceImpl. + // Override procs to provide harness-specific behavior to always select the adapter required in + // testing parameter, and to allow fixture-specific overriding of the test device with + // CreateDeviceImpl. procs.instanceRequestAdapter = [](WGPUInstance instance, const WGPURequestAdapterOptions*, WGPURequestAdapterCallback callback, void* userdata) { ASSERT(gCurrentTest); @@ -872,55 +935,26 @@ bool DawnTestBase::SupportsFeatures(const std::vector& featur WGPUDevice DawnTestBase::CreateDeviceImpl(std::string isolationKey) { // Create the device from the adapter - for (const char* forceEnabledWorkaround : mParam.forceEnabledWorkarounds) { - ASSERT(gTestEnv->GetInstance()->GetToggleInfo(forceEnabledWorkaround) != nullptr); - } - for (const char* forceDisabledWorkaround : mParam.forceDisabledWorkarounds) { - ASSERT(gTestEnv->GetInstance()->GetToggleInfo(forceDisabledWorkaround) != nullptr); - } - - std::vector forceEnabledToggles = mParam.forceEnabledWorkarounds; - std::vector forceDisabledToggles = mParam.forceDisabledWorkarounds; - std::vector requiredFeatures = GetRequiredFeatures(); wgpu::SupportedLimits supportedLimits; mBackendAdapter.GetLimits(reinterpret_cast(&supportedLimits)); wgpu::RequiredLimits requiredLimits = GetRequiredLimits(supportedLimits); - // Disabled disallowing unsafe APIs so we can test them. - forceDisabledToggles.push_back("disallow_unsafe_apis"); - - for (const std::string& toggle : gTestEnv->GetEnabledToggles()) { - const dawn::native::ToggleInfo* info = - gTestEnv->GetInstance()->GetToggleInfo(toggle.c_str()); - ASSERT(info != nullptr); - forceEnabledToggles.push_back(info->name); - } - - for (const std::string& toggle : gTestEnv->GetDisabledToggles()) { - const dawn::native::ToggleInfo* info = - gTestEnv->GetInstance()->GetToggleInfo(toggle.c_str()); - ASSERT(info != nullptr); - forceDisabledToggles.push_back(info->name); - } - wgpu::DeviceDescriptor deviceDescriptor = {}; deviceDescriptor.requiredLimits = &requiredLimits; deviceDescriptor.requiredFeatures = requiredFeatures.data(); deviceDescriptor.requiredFeaturesCount = requiredFeatures.size(); - wgpu::DawnTogglesDescriptor deviceTogglesDesc = {}; - deviceDescriptor.nextInChain = &deviceTogglesDesc; - deviceTogglesDesc.enabledToggles = forceEnabledToggles.data(); - deviceTogglesDesc.enabledTogglesCount = forceEnabledToggles.size(); - deviceTogglesDesc.disabledToggles = forceDisabledToggles.data(); - deviceTogglesDesc.disabledTogglesCount = forceDisabledToggles.size(); - wgpu::DawnCacheDeviceDescriptor cacheDesc = {}; - deviceTogglesDesc.nextInChain = &cacheDesc; + deviceDescriptor.nextInChain = &cacheDesc; cacheDesc.isolationKey = isolationKey.c_str(); + // Note that DisallowUnsafeApis is disabled when creating testing instance and would be + // inherited to all adapters' toggles set. + ParamTogglesHelper deviceTogglesHelper(mParam, dawn::native::ToggleStage::Device); + cacheDesc.nextInChain = &deviceTogglesHelper.togglesDesc; + return mBackendAdapter.CreateDevice(&deviceDescriptor); } @@ -931,7 +965,9 @@ wgpu::Device DawnTestBase::CreateDevice(std::string isolationKey) { // to CreateDeviceImpl. mNextIsolationKeyQueue.push(std::move(isolationKey)); - // This descriptor doesn't matter since device selection is overriden by CreateDeviceImpl. + // RequestDevice is overriden by CreateDeviceImpl and device descriptor is ignored by it. Give + // an empty descriptor. + // TODO(dawn:1684): Replace empty DeviceDescriptor with nullptr after Dawn wire support it. wgpu::DeviceDescriptor deviceDesc = {}; mAdapter.RequestDevice( &deviceDesc, @@ -988,7 +1024,9 @@ void DawnTestBase::SetUp() { "_" + ::testing::UnitTest::GetInstance()->current_test_info()->name(); mWireHelper->BeginWireTrace(traceName.c_str()); - // These options are unused since adapter selection is overriden to use the test params + // RequestAdapter is overriden to ignore RequestAdapterOptions, but dawn_wire requires a valid + // pointer, so give a empty option. + // TODO(dawn:1684): Replace empty RequestAdapterOptions with nullptr after Dawn wire support it. wgpu::RequestAdapterOptions options = {}; mInstance.RequestAdapter( &options, diff --git a/src/dawn/tests/end2end/ExperimentalDP4aTests.cpp b/src/dawn/tests/end2end/ExperimentalDP4aTests.cpp index d77de5bffc..073b896501 100644 --- a/src/dawn/tests/end2end/ExperimentalDP4aTests.cpp +++ b/src/dawn/tests/end2end/ExperimentalDP4aTests.cpp @@ -86,7 +86,8 @@ TEST_P(ExperimentalDP4aTests, BasicDP4aFeaturesTest) { // Adapter support the feature IsDP4aSupportedOnAdapter() && // Proper toggle, disallow_unsafe_apis and use_dxc if d3d12 - // Note that "disallow_unsafe_apis" is always disabled in DawnTestBase::CreateDeviceImpl. + // Note that "disallow_unsafe_apis" is always disabled in + // DawnTestEnvironment::CreateInstanceAndDiscoverAdapters. !HasToggleEnabled("disallow_unsafe_apis") && UseDxcEnabledOrNonD3D12(); const bool deviceSupportDP4AFeature = device.HasFeature(wgpu::FeatureName::ChromiumExperimentalDp4a); diff --git a/src/dawn/tests/unittests/FeatureTests.cpp b/src/dawn/tests/unittests/FeatureTests.cpp index 45351d922b..f879225b2d 100644 --- a/src/dawn/tests/unittests/FeatureTests.cpp +++ b/src/dawn/tests/unittests/FeatureTests.cpp @@ -16,6 +16,7 @@ #include "dawn/native/Features.h" #include "dawn/native/Instance.h" +#include "dawn/native/Toggles.h" #include "dawn/native/null/DeviceNull.h" #include "gtest/gtest.h" @@ -24,7 +25,11 @@ class FeatureTests : public testing::Test { FeatureTests() : testing::Test(), mInstanceBase(dawn::native::InstanceBase::Create()), - mAdapterBase(mInstanceBase.Get()) {} + mAdapterBase(mInstanceBase.Get()), + mUnsafeAdapterBase( + mInstanceBase.Get(), + dawn::native::TogglesState(dawn::native::ToggleStage::Adapter) + .SetForTesting(dawn::native::Toggle::DisallowUnsafeAPIs, false, false)) {} std::vector GetAllFeatureNames() { std::vector allFeatureNames(kTotalFeaturesCount); @@ -38,8 +43,13 @@ class FeatureTests : public testing::Test { static_cast(dawn::native::Feature::EnumCount); protected: + // By default DisallowUnsafeAPIs is enabled in this instance. Ref mInstanceBase; + // The adapter that inherit toggles states from the instance, also have DisallowUnsafeAPIs + // enabled. dawn::native::null::Adapter mAdapterBase; + // The adapter that override DisallowUnsafeAPIs to disabled in toggles state. + dawn::native::null::Adapter mUnsafeAdapterBase; }; // Test the creation of a device will fail if the requested feature is not supported on the @@ -52,14 +62,11 @@ TEST_F(FeatureTests, AdapterWithRequiredFeatureDisabled) { std::vector featureNamesWithoutOne = kAllFeatureNames; featureNamesWithoutOne.erase(featureNamesWithoutOne.begin() + i); - mAdapterBase.SetSupportedFeatures(featureNamesWithoutOne); - dawn::native::Adapter adapterWithoutFeature(&mAdapterBase); - - // Test that creating device with and without DisallowUnsafeApis toggle disabled will both - // failed. - - // Without disabling DisallowUnsafeApis toggle + // Test that the adapter with unsafe apis disallowed validate features as expected. { + mAdapterBase.SetSupportedFeatures(featureNamesWithoutOne); + dawn::native::Adapter adapterWithoutFeature(&mAdapterBase); + wgpu::DeviceDescriptor deviceDescriptor; wgpu::FeatureName featureName = FeatureEnumToAPIFeature(notSupportedFeature); deviceDescriptor.requiredFeatures = &featureName; @@ -70,48 +77,46 @@ TEST_F(FeatureTests, AdapterWithRequiredFeatureDisabled) { ASSERT_EQ(nullptr, deviceWithFeature); } - // Disabling DisallowUnsafeApis toggle + // Test that the adapter with unsafe apis allowed validate features as expected. { - wgpu::DeviceDescriptor unsafeDeviceDescriptor; - wgpu::FeatureName featureName = FeatureEnumToAPIFeature(notSupportedFeature); - unsafeDeviceDescriptor.requiredFeatures = &featureName; - unsafeDeviceDescriptor.requiredFeaturesCount = 1; + mUnsafeAdapterBase.SetSupportedFeatures(featureNamesWithoutOne); + dawn::native::Adapter adapterWithoutFeature(&mUnsafeAdapterBase); - wgpu::DawnTogglesDescriptor deviceTogglesDesc; - unsafeDeviceDescriptor.nextInChain = &deviceTogglesDesc; - const char* toggle = "disallow_unsafe_apis"; - deviceTogglesDesc.disabledToggles = &toggle; - deviceTogglesDesc.disabledTogglesCount = 1; + wgpu::DeviceDescriptor deviceDescriptor; + wgpu::FeatureName featureName = FeatureEnumToAPIFeature(notSupportedFeature); + deviceDescriptor.requiredFeatures = &featureName; + deviceDescriptor.requiredFeaturesCount = 1; WGPUDevice deviceWithFeature = adapterWithoutFeature.CreateDevice( - reinterpret_cast(&unsafeDeviceDescriptor)); + reinterpret_cast(&deviceDescriptor)); ASSERT_EQ(nullptr, deviceWithFeature); } } } -// Test creating device requiring a supported feature can succeed (with DisallowUnsafeApis guarded -// for experimental features), and Device.GetEnabledFeatures() can return the names of the enabled -// features correctly. +// Test creating device requiring a supported feature can succeed (with DisallowUnsafeApis adapter +// toggle disabled for experimental features), and Device.GetEnabledFeatures() can return the names +// of the enabled features correctly. TEST_F(FeatureTests, RequireAndGetEnabledFeatures) { dawn::native::Adapter adapter(&mAdapterBase); + dawn::native::Adapter unsafeAdapter(&mUnsafeAdapterBase); dawn::native::FeaturesInfo featuresInfo; for (size_t i = 0; i < kTotalFeaturesCount; ++i) { dawn::native::Feature feature = static_cast(i); wgpu::FeatureName featureName = FeatureEnumToAPIFeature(feature); - // Test with DisallowUnsafeApis not disabled. - { - wgpu::DeviceDescriptor deviceDescriptor; - deviceDescriptor.requiredFeatures = &featureName; - deviceDescriptor.requiredFeaturesCount = 1; + wgpu::DeviceDescriptor deviceDescriptor; + deviceDescriptor.requiredFeatures = &featureName; + deviceDescriptor.requiredFeaturesCount = 1; + // Test with the adapter with DisallowUnsafeApis adapter toggles not disabled. + { dawn::native::DeviceBase* deviceBase = dawn::native::FromAPI(adapter.CreateDevice( reinterpret_cast(&deviceDescriptor))); - // Device creation should fail if requiring experimental features without disabling - // DisallowUnsafeApis + // Creating a device with experimental feature requires the adapter disables + // DisallowUnsafeApis, otherwise a validation error. if (featuresInfo.GetFeatureInfo(featureName)->featureState == dawn::native::FeatureInfo::FeatureState::Experimental) { ASSERT_EQ(nullptr, deviceBase); @@ -126,20 +131,11 @@ TEST_F(FeatureTests, RequireAndGetEnabledFeatures) { } } - // Test with DisallowUnsafeApis disabled, creating device should always succeed. + // Test with the adapter with DisallowUnsafeApis toggles disabled, creating device should + // always succeed. { - wgpu::DeviceDescriptor unsafeDeviceDescriptor; - unsafeDeviceDescriptor.requiredFeatures = &featureName; - unsafeDeviceDescriptor.requiredFeaturesCount = 1; - - const char* const disableToggles[] = {"disallow_unsafe_apis"}; - wgpu::DawnTogglesDescriptor toggleDesc; - toggleDesc.disabledToggles = disableToggles; - toggleDesc.disabledTogglesCount = 1; - unsafeDeviceDescriptor.nextInChain = &toggleDesc; - - dawn::native::DeviceBase* deviceBase = dawn::native::FromAPI(adapter.CreateDevice( - reinterpret_cast(&unsafeDeviceDescriptor))); + dawn::native::DeviceBase* deviceBase = dawn::native::FromAPI(unsafeAdapter.CreateDevice( + reinterpret_cast(&deviceDescriptor))); ASSERT_NE(nullptr, deviceBase); ASSERT_EQ(1u, deviceBase->APIEnumerateFeatures(nullptr)); diff --git a/src/dawn/tests/unittests/ToggleTests.cpp b/src/dawn/tests/unittests/ToggleTests.cpp new file mode 100644 index 0000000000..51578055e9 --- /dev/null +++ b/src/dawn/tests/unittests/ToggleTests.cpp @@ -0,0 +1,202 @@ +// 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 +#include + +#include "dawn/dawn_proc.h" +#include "dawn/native/Adapter.h" +#include "dawn/native/DawnNative.h" +#include "dawn/native/Device.h" +#include "dawn/native/Instance.h" +#include "dawn/native/Toggles.h" +#include "dawn/native/dawn_platform.h" +#include "dawn/tests/MockCallback.h" +#include "dawn/utils/SystemUtils.h" +#include "dawn/utils/WGPUHelpers.h" +#include "gtest/gtest.h" + +namespace { + +using testing::Contains; +using testing::MockCallback; +using testing::NotNull; +using testing::SaveArg; +using testing::StrEq; + +class InstanceToggleTest : public testing::Test { + protected: + void SetUp() override { dawnProcSetProcs(&dawn::native::GetProcs()); } + + void TearDown() override { dawnProcSetProcs(nullptr); } +}; + +// Test that instance toggles are set by requirement or default as expected. +TEST_F(InstanceToggleTest, InstanceTogglesSet) { + auto validateInstanceToggles = [](const dawn::native::Instance* nativeInstance, + std::initializer_list enableToggles, + std::initializer_list disableToggles) { + const dawn::native::InstanceBase* instance = dawn::native::FromAPI(nativeInstance->Get()); + const dawn::native::TogglesState& instanceTogglesState = instance->GetTogglesState(); + std::vector enabledToggles = instanceTogglesState.GetEnabledToggleNames(); + std::vector disabledToggles = instanceTogglesState.GetDisabledToggleNames(); + EXPECT_EQ(disabledToggles.size(), disableToggles.size()); + EXPECT_EQ(enabledToggles.size(), enableToggles.size()); + for (auto* enableToggle : enableToggles) { + EXPECT_THAT(enabledToggles, Contains(StrEq(enableToggle))); + } + for (auto* disableToggle : disableToggles) { + EXPECT_THAT(disabledToggles, Contains(StrEq(disableToggle))); + } + }; + + // Create instance with no toggles descriptor + { + std::unique_ptr instance; + + // Create an instance with default toggles, where DisallowUnsafeApis is enabled. + instance = std::make_unique(); + validateInstanceToggles(instance.get(), {"disallow_unsafe_apis"}, {}); + } + + // Create instance with empty toggles descriptor + { + std::unique_ptr instance; + + // Make an instance descriptor chaining an empty toggles descriptor + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceTogglesDesc.chain; + + // Create an instance with default toggles, where DisallowUnsafeApis is enabled. + instance = std::make_unique(&instanceDesc); + validateInstanceToggles(instance.get(), {"disallow_unsafe_apis"}, {}); + } + + // Create instance with DisallowUnsafeAPIs explicitly enabled in toggles descriptor + { + std::unique_ptr instance; + + const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceTogglesDesc.enabledTogglesCount = 1; + instanceTogglesDesc.enabledToggles = &disallowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceTogglesDesc.chain; + + // Create an instance with DisallowUnsafeApis explicitly enabled. + instance = std::make_unique(&instanceDesc); + validateInstanceToggles(instance.get(), {disallowUnsafeApisToggle}, {}); + } + + // Create instance with DisallowUnsafeAPIs explicitly disabled in toggles descriptor + { + std::unique_ptr instance; + + const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceTogglesDesc.disabledTogglesCount = 1; + instanceTogglesDesc.disabledToggles = &disallowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceTogglesDesc.chain; + + // Create an instance with DisallowUnsafeApis explicitly disabled. + instance = std::make_unique(&instanceDesc); + validateInstanceToggles(instance.get(), {}, {disallowUnsafeApisToggle}); + } +} + +// Test that instance toggles are inherited to the adapters and devices it creates. +TEST_F(InstanceToggleTest, InstanceTogglesInheritToAdapterAndDevice) { + auto validateInstanceTogglesInheritedToAdapter = [&](dawn::native::Instance* nativeInstance) { + dawn::native::InstanceBase* instance = dawn::native::FromAPI(nativeInstance->Get()); + const dawn::native::TogglesState& instanceTogglesState = instance->GetTogglesState(); + + // Discover adapters with default toggles. + instance->DiscoverDefaultAdapters(); + + // Get the adapter created by instance with default toggles. + dawn::native::AdapterBase* nullAdapter = nullptr; + for (auto& adapter : instance->GetAdapters()) { + if (adapter->GetBackendType() == wgpu::BackendType::Null) { + nullAdapter = adapter.Get(); + break; + } + } + ASSERT_NE(nullAdapter, nullptr); + auto& adapterTogglesState = nullAdapter->GetTogglesState(); + + // Creater a default device. + dawn::native::DeviceBase* nullDevice = nullAdapter->APICreateDevice(); + + // Check instance toggles are inherited by adapter and device. + dawn::native::TogglesInfo togglesInfo; + static_assert(std::is_same_v, int>); + for (int i = 0; i < static_cast(dawn::native::Toggle::EnumCount); i++) { + dawn::native::Toggle toggle = static_cast(i); + if (togglesInfo.GetToggleInfo(toggle)->stage != dawn::native::ToggleStage::Instance) { + continue; + } + EXPECT_EQ(instanceTogglesState.IsSet(toggle), adapterTogglesState.IsSet(toggle)); + EXPECT_EQ(instanceTogglesState.IsEnabled(toggle), + adapterTogglesState.IsEnabled(toggle)); + EXPECT_EQ(instanceTogglesState.IsEnabled(toggle), nullDevice->IsToggleEnabled(toggle)); + } + + nullDevice->Release(); + }; + + // Create instance with DisallowUnsafeAPIs explicitly enabled in toggles descriptor + { + std::unique_ptr instance; + + const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceTogglesDesc.enabledTogglesCount = 1; + instanceTogglesDesc.enabledToggles = &disallowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceTogglesDesc.chain; + + // Create an instance with DisallowUnsafeApis explicitly enabled. + instance = std::make_unique(&instanceDesc); + validateInstanceTogglesInheritedToAdapter(instance.get()); + } + + // Create instance with DisallowUnsafeAPIs explicitly disabled in toggles descriptor + { + std::unique_ptr instance; + + const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceTogglesDesc.disabledTogglesCount = 1; + instanceTogglesDesc.disabledToggles = &disallowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceTogglesDesc.chain; + + // Create an instance with DisallowUnsafeApis explicitly enabled. + instance = std::make_unique(&instanceDesc); + validateInstanceTogglesInheritedToAdapter(instance.get()); + } +} +} // anonymous namespace diff --git a/src/dawn/tests/unittests/native/DeviceCreationTests.cpp b/src/dawn/tests/unittests/native/DeviceCreationTests.cpp index 17a94d7afb..add691af37 100644 --- a/src/dawn/tests/unittests/native/DeviceCreationTests.cpp +++ b/src/dawn/tests/unittests/native/DeviceCreationTests.cpp @@ -16,8 +16,10 @@ #include #include "dawn/dawn_proc.h" +#include "dawn/native/Adapter.h" #include "dawn/native/DawnNative.h" #include "dawn/native/Device.h" +#include "dawn/native/Toggles.h" #include "dawn/native/dawn_platform.h" #include "dawn/tests/MockCallback.h" #include "dawn/utils/SystemUtils.h" @@ -37,7 +39,20 @@ class DeviceCreationTest : public testing::Test { void SetUp() override { dawnProcSetProcs(&dawn::native::GetProcs()); - instance = std::make_unique(); + WGPUInstanceDescriptor safeInstanceDesc = {}; + + const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + WGPUDawnTogglesDescriptor unsafeInstanceTogglesDesc = {}; + unsafeInstanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + unsafeInstanceTogglesDesc.disabledTogglesCount = 1; + unsafeInstanceTogglesDesc.disabledToggles = &disallowUnsafeApisToggle; + + WGPUInstanceDescriptor unsafeInstanceDesc = {}; + unsafeInstanceDesc.nextInChain = &unsafeInstanceTogglesDesc.chain; + + // Create an instance with default toggles, where DisallowUnsafeApis is enabled, and create + // an adapter from it. + instance = std::make_unique(&safeInstanceDesc); instance->DiscoverDefaultAdapters(); for (dawn::native::Adapter& nativeAdapter : instance->GetAdapters()) { wgpu::AdapterProperties properties; @@ -48,17 +63,41 @@ class DeviceCreationTest : public testing::Test { break; } } + + // Create an instance with toggle DisallowUnsafeApis disabled, and create an unsafe adapter + // from it. + unsafeInstance = std::make_unique(&unsafeInstanceDesc); + unsafeInstance->DiscoverDefaultAdapters(); + for (dawn::native::Adapter& nativeAdapter : unsafeInstance->GetAdapters()) { + wgpu::AdapterProperties properties; + nativeAdapter.GetProperties(&properties); + + if (properties.backendType == wgpu::BackendType::Null) { + unsafeAdapter = wgpu::Adapter(nativeAdapter.Get()); + break; + } + } + ASSERT_NE(adapter, nullptr); + ASSERT_NE(unsafeAdapter, nullptr); } void TearDown() override { adapter = nullptr; + unsafeAdapter = nullptr; instance = nullptr; + unsafeInstance = nullptr; dawnProcSetProcs(nullptr); } + static constexpr size_t kTotalFeaturesCount = + static_cast(dawn::native::Feature::EnumCount); + std::unique_ptr instance; + std::unique_ptr unsafeInstance; wgpu::Adapter adapter; + wgpu::Adapter unsafeAdapter; + dawn::native::FeaturesInfo featuresInfo; }; // Test successful call to CreateDevice with no descriptor @@ -91,23 +130,53 @@ TEST_F(DeviceCreationTest, CreateDeviceWithTogglesSuccess) { EXPECT_THAT(toggles, Contains(StrEq(toggle))); } -// Test features guarded by toggles are validated when creating devices. -TEST_F(DeviceCreationTest, CreateDeviceRequiringFeaturesGuardedByToggle) { - std::vector featuresGuardedByToggle = { - wgpu::FeatureName::ShaderF16, wgpu::FeatureName::ChromiumExperimentalDp4a}; +// Test experimental features are guarded by DisallowUnsafeApis adapter toggle, it is inherited from +// instance but can be overriden by device toggles. +TEST_F(DeviceCreationTest, CreateDeviceRequiringExperimentalFeatures) { + // Ensure that DisallowUnsafeApis adapter toggle is enabled on safe adapter. + auto safeAdapterBase = dawn::native::FromAPI(adapter.Get()); + ASSERT_TRUE( + safeAdapterBase->GetTogglesState().IsEnabled(dawn::native::Toggle::DisallowUnsafeAPIs)); + // Ensure that DisallowUnsafeApis adapter toggle is disabled on unsafe adapter. + auto unsafeAdapterBase = dawn::native::FromAPI(unsafeAdapter.Get()); + ASSERT_FALSE( + unsafeAdapterBase->GetTogglesState().IsEnabled(dawn::native::Toggle::DisallowUnsafeAPIs)); + + for (size_t i = 0; i < kTotalFeaturesCount; i++) { + dawn::native::Feature feature = static_cast(i); + wgpu::FeatureName featureName = dawn::native::FeatureEnumToAPIFeature(feature); + + // Only test experimental features. + if (featuresInfo.GetFeatureInfo(featureName)->featureState == + dawn::native::FeatureInfo::FeatureState::Stable) { + continue; + } - for (auto feature : featuresGuardedByToggle) { wgpu::DeviceDescriptor deviceDescriptor; - deviceDescriptor.requiredFeatures = &feature; + deviceDescriptor.requiredFeatures = &featureName; deviceDescriptor.requiredFeaturesCount = 1; - // Test creating device without toggle would fail. + // Test creating device on the adapter with DisallowUnsafeApis toggle enabled would fail. { wgpu::Device device = adapter.CreateDevice(&deviceDescriptor); EXPECT_EQ(device, nullptr); } - // Test creating device without DisallowUnsafeApis toggle disabled. + // Test creating device on the adapter with DisallowUnsafeApis toggle disabled would + // succeed. + { + // unsafeAdapter has DisallowUnsafeApis adapter toggle disabled. + wgpu::Device device = unsafeAdapter.CreateDevice(&deviceDescriptor); + EXPECT_NE(device, nullptr); + + ASSERT_EQ(1u, device.EnumerateFeatures(nullptr)); + wgpu::FeatureName enabledFeature; + device.EnumerateFeatures(&enabledFeature); + EXPECT_EQ(enabledFeature, featureName); + } + + // Test creating device with DisallowUnsafeApis disabled in device toggle descriptor will + // success on both adapter, as device toggles will override the inherited adapter toggles. { const char* const disableToggles[] = {"disallow_unsafe_apis"}; wgpu::DawnTogglesDescriptor deviceTogglesDesc; @@ -115,14 +184,49 @@ TEST_F(DeviceCreationTest, CreateDeviceRequiringFeaturesGuardedByToggle) { deviceTogglesDesc.disabledTogglesCount = 1; deviceDescriptor.nextInChain = &deviceTogglesDesc; - wgpu::Device device = adapter.CreateDevice(&deviceDescriptor); - EXPECT_NE(device, nullptr); + // Test on adapter with DisallowUnsafeApis enabled. + { + wgpu::Device device = adapter.CreateDevice(&deviceDescriptor); + EXPECT_NE(device, nullptr); - ASSERT_EQ(1u, device.EnumerateFeatures(nullptr)); - wgpu::FeatureName enabledFeature; - device.EnumerateFeatures(&enabledFeature); - EXPECT_EQ(enabledFeature, feature); - device.Release(); + ASSERT_EQ(1u, device.EnumerateFeatures(nullptr)); + wgpu::FeatureName enabledFeature; + device.EnumerateFeatures(&enabledFeature); + EXPECT_EQ(enabledFeature, featureName); + } + + // Test on adapter with DisallowUnsafeApis disabled. + { + wgpu::Device device = unsafeAdapter.CreateDevice(&deviceDescriptor); + EXPECT_NE(device, nullptr); + + ASSERT_EQ(1u, device.EnumerateFeatures(nullptr)); + wgpu::FeatureName enabledFeature; + device.EnumerateFeatures(&enabledFeature); + EXPECT_EQ(enabledFeature, featureName); + } + } + + // Test creating device with DisallowUnsafeApis enabled in device toggle descriptor will + // fail on both adapter, as device toggles will override the inherited adapter toggles. + { + const char* const enableToggles[] = {"disallow_unsafe_apis"}; + wgpu::DawnTogglesDescriptor deviceToggleDesc; + deviceToggleDesc.enabledToggles = enableToggles; + deviceToggleDesc.enabledTogglesCount = 1; + deviceDescriptor.nextInChain = &deviceToggleDesc; + + // Test on adapter with DisallowUnsafeApis enabled. + { + wgpu::Device device = adapter.CreateDevice(&deviceDescriptor); + EXPECT_EQ(device, nullptr); + } + + // Test on adapter with DisallowUnsafeApis disabled. + { + wgpu::Device device = unsafeAdapter.CreateDevice(&deviceDescriptor); + EXPECT_EQ(device, nullptr); + } } } } diff --git a/src/dawn/tests/unittests/validation/QueryValidationTests.cpp b/src/dawn/tests/unittests/validation/QueryValidationTests.cpp index ea2e378b84..ccf12104f0 100644 --- a/src/dawn/tests/unittests/validation/QueryValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/QueryValidationTests.cpp @@ -279,12 +279,6 @@ class TimestampQueryValidationTest : public QuerySetValidationTest { descriptor.requiredFeatures = requiredFeatures; descriptor.requiredFeaturesCount = 1; - wgpu::DawnTogglesDescriptor deviceTogglesDesc; - descriptor.nextInChain = &deviceTogglesDesc; - const char* disabledToggles[1] = {"disallow_unsafe_apis"}; - deviceTogglesDesc.disabledToggles = disabledToggles; - deviceTogglesDesc.disabledTogglesCount = 1; - return dawnAdapter.CreateDevice(&descriptor); } @@ -582,12 +576,6 @@ class TimestampQueryInsidePassesValidationTest : public QuerySetValidationTest { descriptor.requiredFeatures = requiredFeatures; descriptor.requiredFeaturesCount = 2; - wgpu::DawnTogglesDescriptor deviceTogglesDesc; - descriptor.nextInChain = &deviceTogglesDesc; - const char* disabledToggles[1] = {"disallow_unsafe_apis"}; - deviceTogglesDesc.disabledToggles = disabledToggles; - deviceTogglesDesc.disabledTogglesCount = 1; - return dawnAdapter.CreateDevice(&descriptor); } }; @@ -722,19 +710,14 @@ TEST_F(TimestampQueryInsidePassesValidationTest, WriteTimestampOnRenderPassEncod class PipelineStatisticsQueryValidationTest : public QuerySetValidationTest { protected: WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override { + // Create a device with pipeline statistic query feature required. Note that Pipeline + // statistic query is an unsafe API, while DisallowUnsafeApis instance toggle is disabled + // when ValidationTest creating testing instance, so we can test it. wgpu::DeviceDescriptor descriptor; wgpu::FeatureName requiredFeatures[1] = {wgpu::FeatureName::PipelineStatisticsQuery}; descriptor.requiredFeatures = requiredFeatures; descriptor.requiredFeaturesCount = 1; - // TODO(crbug.com/1177506): Pipeline statistic query is an unsafe API, disable disallowing - // unsafe APIs to test it. - wgpu::DawnTogglesDescriptor deviceTogglesDesc; - descriptor.nextInChain = &deviceTogglesDesc; - const char* disabledToggles[1] = {"disallow_unsafe_apis"}; - deviceTogglesDesc.disabledToggles = disabledToggles; - deviceTogglesDesc.disabledTogglesCount = 1; - return dawnAdapter.CreateDevice(&descriptor); } }; diff --git a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp index 811de6e093..2aadaf71c3 100644 --- a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -26,22 +26,11 @@ class RenderPipelineValidationTest : public ValidationTest { protected: WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override { - // Disabled disallowing unsafe APIs so we can test ShaderF16 feature. - const char* disabledToggle[] = {"disallow_unsafe_apis"}; - wgpu::DeviceDescriptor descriptor; wgpu::FeatureName requiredFeatures[1] = {wgpu::FeatureName::ShaderF16}; descriptor.requiredFeatures = requiredFeatures; descriptor.requiredFeaturesCount = 1; - wgpu::DawnTogglesDescriptor deviceTogglesDesc; - descriptor.nextInChain = &deviceTogglesDesc; - - deviceTogglesDesc.enabledToggles = nullptr; - deviceTogglesDesc.enabledTogglesCount = 0; - deviceTogglesDesc.disabledToggles = disabledToggle; - deviceTogglesDesc.disabledTogglesCount = 1; - return dawnAdapter.CreateDevice(&descriptor); } diff --git a/src/dawn/tests/unittests/validation/TextureValidationTests.cpp b/src/dawn/tests/unittests/validation/TextureValidationTests.cpp index 5cc3d9cc8b..bc3f33c140 100644 --- a/src/dawn/tests/unittests/validation/TextureValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/TextureValidationTests.cpp @@ -729,14 +729,6 @@ class CompressedTextureFormatsValidationTests : public TextureValidationTest { descriptor.requiredFeatures = requiredFeatures; descriptor.requiredFeaturesCount = 3; - // TODO(dawn:814): Remove when 1D texture support is complete. - const char* kDisallowUnsafeApis = "disallow_unsafe_apis"; - wgpu::DawnTogglesDescriptor deviceTogglesDesc; - deviceTogglesDesc.disabledToggles = &kDisallowUnsafeApis; - deviceTogglesDesc.disabledTogglesCount = 1; - - descriptor.nextInChain = &deviceTogglesDesc; - return dawnAdapter.CreateDevice(&descriptor); } diff --git a/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp index c422eae6b8..5ecd096a3d 100644 --- a/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp @@ -26,7 +26,11 @@ using testing::HasSubstr; class UnsafeAPIValidationTest : public ValidationTest { protected: + // UnsafeAPIValidationTest create the device with toggle DisallowUnsafeApis explicitly enabled, + // which overrides the inheritance. WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override { + // Enable the DisallowUnsafeAPIs toggles in device toggles descriptor to override the + // inheritance and create a device disallowing unsafe apis. wgpu::DeviceDescriptor descriptor; wgpu::DawnTogglesDescriptor deviceTogglesDesc; descriptor.nextInChain = &deviceTogglesDesc; diff --git a/src/dawn/tests/unittests/validation/ValidationTest.cpp b/src/dawn/tests/unittests/validation/ValidationTest.cpp index 4556bebc7f..708da78481 100644 --- a/src/dawn/tests/unittests/validation/ValidationTest.cpp +++ b/src/dawn/tests/unittests/validation/ValidationTest.cpp @@ -84,11 +84,14 @@ ValidationTest::ValidationTest() { gCurrentTest = this; DawnProcTable procs = dawn::native::GetProcs(); - // Override procs to provide harness-specific behavior to always select the null adapter, - // and to allow fixture-specific overriding of the test device with CreateTestDevice. + + // Override procs to provide harness-specific behavior to always select the null adapter, with + // adapter toggles inherited from instance toggles state. WGPURequestAdapterOptions is ignored + // here. procs.instanceRequestAdapter = [](WGPUInstance instance, const WGPURequestAdapterOptions*, WGPURequestAdapterCallback callback, void* userdata) { ASSERT(gCurrentTest); + std::vector adapters = gCurrentTest->mDawnInstance->GetAdapters(); // Validation tests run against the null backend, find the corresponding adapter for (auto& adapter : adapters) { @@ -121,7 +124,21 @@ ValidationTest::ValidationTest() { } void ValidationTest::SetUp() { - mDawnInstance = std::make_unique(); + // Create an instance with toggle DisallowUnsafeApis disabled, which would be inherited to + // adapter and device toggles and allow us to test unsafe apis (including experimental + // features). To test device of DisallowUnsafeApis enabled, require it in device toggles + // descriptor to override the inheritance. + const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceToggles = {}; + instanceToggles.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceToggles.disabledTogglesCount = 1; + instanceToggles.disabledToggles = &disallowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceToggles.chain; + + mDawnInstance = std::make_unique(&instanceDesc); + mDawnInstance->DiscoverDefaultAdapters(); mInstance = mWireHelper->RegisterInstance(mDawnInstance->Get()); @@ -130,7 +147,8 @@ void ValidationTest::SetUp() { "_" + ::testing::UnitTest::GetInstance()->current_test_info()->name(); mWireHelper->BeginWireTrace(traceName.c_str()); - // These options are unused since validation tests always select the null adapter + // RequestAdapter is overriden to ignore RequestAdapterOptions and always select the null + // adapter. wgpu::RequestAdapterOptions options = {}; mInstance.RequestAdapter( &options, @@ -263,9 +281,8 @@ dawn::native::Adapter& ValidationTest::GetBackendAdapter() { } WGPUDevice ValidationTest::CreateTestDevice(dawn::native::Adapter dawnAdapter) { - // Disabled disallowing unsafe APIs so we can test them. std::vector enabledToggles; - std::vector disabledToggles = {"disallow_unsafe_apis"}; + std::vector disabledToggles; for (const std::string& toggle : gToggleParser->GetEnabledToggles()) { enabledToggles.push_back(toggle.c_str());