diff --git a/include/dawn/native/DawnNative.h b/include/dawn/native/DawnNative.h index 6b7090d06d..5c6ffadeee 100644 --- a/include/dawn/native/DawnNative.h +++ b/include/dawn/native/DawnNative.h @@ -58,7 +58,7 @@ struct FeatureInfo { const char* description; const char* url; // The enum of feature state, could be stable or experimental. Using an experimental feature - // requires DisallowUnsafeAPIs toggle being disabled. + // requires the AllowUnsafeAPIs toggle to be enabled. enum class FeatureState { Stable = 0, Experimental }; FeatureState featureState; }; diff --git a/src/dawn/native/Adapter.cpp b/src/dawn/native/Adapter.cpp index 6b494505a1..0e9aa04044 100644 --- a/src/dawn/native/Adapter.cpp +++ b/src/dawn/native/Adapter.cpp @@ -63,4 +63,9 @@ const TogglesState& AdapterBase::GetTogglesState() const { return mPhysicalDevice->GetTogglesState(); } +bool AdapterBase::AllowUnsafeAPIs() const { + return GetTogglesState().IsEnabled(Toggle::AllowUnsafeAPIs) || + !GetTogglesState().IsEnabled(Toggle::DisallowUnsafeAPIs); +} + } // namespace dawn::native diff --git a/src/dawn/native/Adapter.h b/src/dawn/native/Adapter.h index b9b03c2caf..f56e4b2393 100644 --- a/src/dawn/native/Adapter.h +++ b/src/dawn/native/Adapter.h @@ -48,6 +48,11 @@ class AdapterBase : public RefCounted { // Get the actual toggles state of the adapter. const TogglesState& GetTogglesState() const; + // Temporary wrapper to decide whether unsafe APIs are allowed while in the process of + // deprecating DisallowUnsafeAPIs toggle. + // TODO(dawn:1685): Remove wrapper once DisallowUnsafeAPIs is fully removed. + bool AllowUnsafeAPIs() const; + private: Ref mPhysicalDevice; }; diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index d3a87d4361..797773c460 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -1395,7 +1395,7 @@ void DeviceBase::SetWGSLExtensionAllowList() { if (mEnabledFeatures.IsEnabled(Feature::ShaderF16)) { mWGSLExtensionAllowList.insert("f16"); } - if (!IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) { + if (AllowUnsafeAPIs()) { mWGSLExtensionAllowList.insert("chromium_disable_uniformity_analysis"); } } @@ -1412,6 +1412,11 @@ bool DeviceBase::IsRobustnessEnabled() const { return !IsToggleEnabled(Toggle::DisableRobustness); } +bool DeviceBase::AllowUnsafeAPIs() const { + // TODO(dawn:1685) Currently allows if either toggle are set accordingly. + return IsToggleEnabled(Toggle::AllowUnsafeAPIs) || !IsToggleEnabled(Toggle::DisallowUnsafeAPIs); +} + size_t DeviceBase::GetLazyClearCountForTesting() { return mLazyClearCountForTesting; } diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h index 364786ffb6..486efce927 100644 --- a/src/dawn/native/Device.h +++ b/src/dawn/native/Device.h @@ -359,6 +359,7 @@ class DeviceBase : public RefCountedWithExternalCount { bool IsToggleEnabled(Toggle toggle) const; bool IsValidationEnabled() const; bool IsRobustnessEnabled() const; + bool AllowUnsafeAPIs() const; size_t GetLazyClearCountForTesting(); void IncrementLazyClearCountForTesting(); size_t GetDeprecationWarningCountForTesting(); diff --git a/src/dawn/native/Instance.cpp b/src/dawn/native/Instance.cpp index 7bfad4d2d8..d6a899ef05 100644 --- a/src/dawn/native/Instance.cpp +++ b/src/dawn/native/Instance.cpp @@ -132,10 +132,11 @@ Ref InstanceBase::Create(const InstanceDescriptor* descriptor) { // 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 + // By default disable the AllowUnsafeAPIs 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. + // TODO(dawn:1685): Remove DisallowUnsafeAPIs. instanceToggles.Default(Toggle::DisallowUnsafeAPIs, true); + instanceToggles.Default(Toggle::AllowUnsafeAPIs, false); Ref instance = AcquireRef(new InstanceBase(instanceToggles)); if (instance->ConsumedError(instance->Initialize(descriptor))) { diff --git a/src/dawn/native/PhysicalDevice.cpp b/src/dawn/native/PhysicalDevice.cpp index c69318a09d..f6e6ebaca2 100644 --- a/src/dawn/native/PhysicalDevice.cpp +++ b/src/dawn/native/PhysicalDevice.cpp @@ -235,11 +235,12 @@ MaybeError PhysicalDeviceBase::ValidateFeatureSupportedWithToggles( "Requested feature %s is not supported.", feature); const FeatureInfo* featureInfo = GetInstance()->GetFeatureInfo(feature); - // Experimental features are guarded by toggle DisallowUnsafeAPIs. + // Experimental features are guarded by the AllowUnsafeAPIs toggle. if (featureInfo->featureState == FeatureInfo::FeatureState::Experimental) { - // DisallowUnsafeAPIs toggle is by default enabled if not explicitly disabled. - DAWN_INVALID_IF(toggles.IsEnabled(Toggle::DisallowUnsafeAPIs), - "Feature %s is guarded by toggle disallow_unsafe_apis.", featureInfo->name); + // AllowUnsafeAPIs toggle is by default disabled if not explicitly enabled. + DAWN_INVALID_IF(toggles.IsEnabled(Toggle::DisallowUnsafeAPIs) && + !toggles.IsEnabled(Toggle::AllowUnsafeAPIs), + "Feature %s is guarded by toggle allow_unsafe_apis.", featureInfo->name); } // Do backend-specific validation. diff --git a/src/dawn/native/QuerySet.cpp b/src/dawn/native/QuerySet.cpp index 8f4047583f..f7b31cc512 100644 --- a/src/dawn/native/QuerySet.cpp +++ b/src/dawn/native/QuerySet.cpp @@ -54,8 +54,8 @@ MaybeError ValidateQuerySetDescriptor(DeviceBase* device, const QuerySetDescript case wgpu::QueryType::PipelineStatistics: { // TODO(crbug.com/1177506): Pipeline statistics query is not fully implemented. - // Disallow it as unsafe until the implementaion is completed. - DAWN_INVALID_IF(device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs), + // Allow it only as unsafe until the implementaion is completed. + DAWN_INVALID_IF(!device->AllowUnsafeAPIs(), "Pipeline statistics queries are disallowed because they are not " "fully implemented"); @@ -78,7 +78,7 @@ MaybeError ValidateQuerySetDescriptor(DeviceBase* device, const QuerySetDescript } break; case wgpu::QueryType::Timestamp: - DAWN_INVALID_IF(device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs), + DAWN_INVALID_IF(!device->AllowUnsafeAPIs(), "Timestamp queries are disallowed because they may expose precise " "timing information."); diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index 50d7f4fd02..f806c773c7 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp @@ -177,6 +177,11 @@ static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{ "Produces validation errors on API entry points or parameter combinations that aren't " "considered secure yet.", "http://crbug.com/1138528", ToggleStage::Instance}}, + {Toggle::AllowUnsafeAPIs, + {"allow_unsafe_apis", + "Suppresses validation errors on API entry points or parameter combinations that aren't " + "considered secure yet.", + "http://crbug.com/1138528", ToggleStage::Instance}}, {Toggle::FlushBeforeClientWaitSync, {"flush_before_client_wait_sync", "Call glFlush before glClientWaitSync to work around bugs in the latter", @@ -450,6 +455,10 @@ TogglesState TogglesState::CreateFromTogglesDescriptor(const DawnTogglesDescript TogglesInfo togglesInfo; for (uint32_t i = 0; i < togglesDesc->enabledTogglesCount; ++i) { Toggle toggle = togglesInfo.ToggleNameToEnum(togglesDesc->enabledToggles[i]); + if (toggle == Toggle::DisallowUnsafeAPIs) { + dawn::WarningLog() << "Enabling the disallow_unsafe_apis toggle is deprecated, disable " + "allow_unsafe_apis toggle instead."; + } if (toggle != Toggle::InvalidEnum) { const ToggleInfo* toggleInfo = togglesInfo.GetToggleInfo(toggle); // Accept the required toggles of current and earlier stage to allow override @@ -462,6 +471,10 @@ TogglesState TogglesState::CreateFromTogglesDescriptor(const DawnTogglesDescript } for (uint32_t i = 0; i < togglesDesc->disabledTogglesCount; ++i) { Toggle toggle = togglesInfo.ToggleNameToEnum(togglesDesc->disabledToggles[i]); + if (toggle == Toggle::DisallowUnsafeAPIs) { + dawn::WarningLog() << "Disabling the disallow_unsafe_apis toggle is deprecated, enable " + "allow_unsafe_apis toggle instead."; + } if (toggle != Toggle::InvalidEnum) { const ToggleInfo* toggleInfo = togglesInfo.GetToggleInfo(toggle); // Accept the required toggles of current and earlier stage to allow override diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index e5e348338a..a8c4e10c13 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h @@ -57,6 +57,7 @@ enum class Toggle { DisableRobustness, MetalEnableVertexPulling, DisallowUnsafeAPIs, + AllowUnsafeAPIs, FlushBeforeClientWaitSync, UseTempBufferInSmallFormatTextureToTextureCopyFromGreaterToLessMipLevel, EmitHLSLDebugSymbols, diff --git a/src/dawn/tests/DawnNativeTest.cpp b/src/dawn/tests/DawnNativeTest.cpp index e8f06289b2..7a3a6c14c3 100644 --- a/src/dawn/tests/DawnNativeTest.cpp +++ b/src/dawn/tests/DawnNativeTest.cpp @@ -45,14 +45,14 @@ DawnNativeTest::~DawnNativeTest() { } void DawnNativeTest::SetUp() { - // Create an instance with toggle DisallowUnsafeApis disabled, which would be inherited to + // Create an instance with toggle AllowUnsafeAPIs enabled, 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"; + const char* allowUnsafeApisToggle = "allow_unsafe_apis"; WGPUDawnTogglesDescriptor instanceToggles = {}; instanceToggles.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; - instanceToggles.disabledTogglesCount = 1; - instanceToggles.disabledToggles = &disallowUnsafeApisToggle; + instanceToggles.enabledTogglesCount = 1; + instanceToggles.enabledToggles = &allowUnsafeApisToggle; WGPUInstanceDescriptor instanceDesc = {}; instanceDesc.nextInChain = &instanceToggles.chain; diff --git a/src/dawn/tests/DawnTest.cpp b/src/dawn/tests/DawnTest.cpp index 3ed7ebd232..6f09b27c75 100644 --- a/src/dawn/tests/DawnTest.cpp +++ b/src/dawn/tests/DawnTest.cpp @@ -374,14 +374,14 @@ void DawnTestEnvironment::ParseArgs(int argc, char** argv) { } std::unique_ptr DawnTestEnvironment::CreateInstanceAndDiscoverAdapters() { - // Create an instance with toggle DisallowUnsafeApis disabled, which would be inherited to + // Create an instance with toggle AllowUnsafeAPIs enabled, 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"; + const char* allowUnsafeApisToggle = "allow_unsafe_apis"; WGPUDawnTogglesDescriptor instanceToggles = {}; instanceToggles.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; - instanceToggles.disabledTogglesCount = 1; - instanceToggles.disabledToggles = &disallowUnsafeApisToggle; + instanceToggles.enabledTogglesCount = 1; + instanceToggles.enabledToggles = &allowUnsafeApisToggle; WGPUInstanceDescriptor instanceDesc = {}; instanceDesc.nextInChain = &instanceToggles.chain; @@ -993,7 +993,7 @@ WGPUDevice DawnTestBase::CreateDeviceImpl(std::string isolationKey) { deviceDescriptor.nextInChain = &cacheDesc; cacheDesc.isolationKey = isolationKey.c_str(); - // Note that DisallowUnsafeApis is disabled when creating testing instance and would be + // Note that AllowUnsafeAPIs is enabled when creating testing instance and would be // inherited to all adapters' toggles set. ParamTogglesHelper deviceTogglesHelper(mParam, dawn::native::ToggleStage::Device); cacheDesc.nextInChain = &deviceTogglesHelper.togglesDesc; diff --git a/src/dawn/tests/end2end/ExperimentalDP4aTests.cpp b/src/dawn/tests/end2end/ExperimentalDP4aTests.cpp index 073b896501..ae642a2f77 100644 --- a/src/dawn/tests/end2end/ExperimentalDP4aTests.cpp +++ b/src/dawn/tests/end2end/ExperimentalDP4aTests.cpp @@ -85,10 +85,10 @@ TEST_P(ExperimentalDP4aTests, BasicDP4aFeaturesTest) { GetParam().mRequestDP4aExtension && // Adapter support the feature IsDP4aSupportedOnAdapter() && - // Proper toggle, disallow_unsafe_apis and use_dxc if d3d12 - // Note that "disallow_unsafe_apis" is always disabled in + // Proper toggle, allow_unsafe_apis and use_dxc if d3d12 + // Note that "allow_unsafe_apis" is always enabled in // DawnTestEnvironment::CreateInstanceAndDiscoverAdapters. - !HasToggleEnabled("disallow_unsafe_apis") && UseDxcEnabledOrNonD3D12(); + HasToggleEnabled("allow_unsafe_apis") && UseDxcEnabledOrNonD3D12(); const bool deviceSupportDP4AFeature = device.HasFeature(wgpu::FeatureName::ChromiumExperimentalDp4a); EXPECT_EQ(deviceSupportDP4AFeature, shouldDP4AFeatureSupportedByDevice); @@ -128,7 +128,7 @@ TEST_P(ExperimentalDP4aTests, BasicDP4aFeaturesTest) { EXPECT_BUFFER_U32_RANGE_EQ(expected, bufferOut, 0, 4); } -// DawnTestBase::CreateDeviceImpl always disable disallow_unsafe_apis toggle. +// DawnTestBase::CreateDeviceImpl always enables allow_unsafe_apis toggle. DAWN_INSTANTIATE_TEST_P(ExperimentalDP4aTests, { D3D12Backend(), diff --git a/src/dawn/tests/end2end/ShaderF16Tests.cpp b/src/dawn/tests/end2end/ShaderF16Tests.cpp index 20ee517986..9c72ff0cfd 100644 --- a/src/dawn/tests/end2end/ShaderF16Tests.cpp +++ b/src/dawn/tests/end2end/ShaderF16Tests.cpp @@ -99,9 +99,9 @@ TEST_P(ShaderF16Tests, BasicShaderF16FeaturesTest) { GetParam().mRequireShaderF16Feature && // Adapter support the feature IsShaderF16SupportedOnAdapter() && - // Proper toggle, disallow_unsafe_apis and use_dxc if d3d12 - // Note that "disallow_unsafe_apis" is always disabled in DawnTestBase::CreateDeviceImpl. - !HasToggleEnabled("disallow_unsafe_apis") && UseDxcEnabledOrNonD3D12(); + // Proper toggle, allow_unsafe_apis and use_dxc if d3d12 + // Note that "allow_unsafe_apis" is always enabled in DawnTestBase::CreateDeviceImpl. + HasToggleEnabled("allow_unsafe_apis") && UseDxcEnabledOrNonD3D12(); const bool deviceSupportShaderF16Feature = device.HasFeature(wgpu::FeatureName::ShaderF16); EXPECT_EQ(deviceSupportShaderF16Feature, shouldShaderF16FeatureSupportedByDevice); @@ -440,7 +440,7 @@ fn FSMain(@location(0) color : vec4f) -> @location(0) vec4f { } } -// DawnTestBase::CreateDeviceImpl always disable disallow_unsafe_apis toggle. +// DawnTestBase::CreateDeviceImpl always enables allow_unsafe_apis toggle. DAWN_INSTANTIATE_TEST_P(ShaderF16Tests, { D3D12Backend(), diff --git a/src/dawn/tests/unittests/FeatureTests.cpp b/src/dawn/tests/unittests/FeatureTests.cpp index 34c7b2efea..ce954dbc7a 100644 --- a/src/dawn/tests/unittests/FeatureTests.cpp +++ b/src/dawn/tests/unittests/FeatureTests.cpp @@ -26,11 +26,16 @@ class FeatureTests : public testing::Test { : testing::Test(), mInstanceBase(dawn::native::InstanceBase::Create()), mPhysicalDevice(mInstanceBase.Get()), - mUnsafePhysicalDevice( + mUnsafePhysicalDeviceDisallow( mInstanceBase.Get(), dawn::native::TogglesState(dawn::native::ToggleStage::Adapter) .SetForTesting(dawn::native::Toggle::DisallowUnsafeAPIs, false, false)), + mUnsafePhysicalDevice( + mInstanceBase.Get(), + dawn::native::TogglesState(dawn::native::ToggleStage::Adapter) + .SetForTesting(dawn::native::Toggle::AllowUnsafeAPIs, true, true)), mAdapterBase(&mPhysicalDevice), + mUnsafeAdapterBaseDisallow(&mUnsafePhysicalDeviceDisallow), mUnsafeAdapterBase(&mUnsafePhysicalDevice) {} std::vector GetAllFeatureNames() { @@ -48,11 +53,14 @@ class FeatureTests : public testing::Test { // By default DisallowUnsafeAPIs is enabled in this instance. Ref mInstanceBase; dawn::native::null::PhysicalDevice mPhysicalDevice; + // TODO(dawn:1685) Remove duplicated unsafe objects once DisallowUnsafeAPIs is removed. + dawn::native::null::PhysicalDevice mUnsafePhysicalDeviceDisallow; dawn::native::null::PhysicalDevice mUnsafePhysicalDevice; // The adapter that inherit toggles states from the instance, also have DisallowUnsafeAPIs // enabled. dawn::native::AdapterBase mAdapterBase; - // The adapter that override DisallowUnsafeAPIs to disabled in toggles state. + // TODO(dawn:1685) Remove duplicated unsafe objects once DisallowUnsafeAPIs is removed. + dawn::native::AdapterBase mUnsafeAdapterBaseDisallow; dawn::native::AdapterBase mUnsafeAdapterBase; }; @@ -66,7 +74,7 @@ TEST_F(FeatureTests, AdapterWithRequiredFeatureDisabled) { std::vector featureNamesWithoutOne = kAllFeatureNames; featureNamesWithoutOne.erase(featureNamesWithoutOne.begin() + i); - // Test that the adapter with unsafe apis disallowed validate features as expected. + // Test that the default adapter validates features as expected. { mPhysicalDevice.SetSupportedFeaturesForTesting(featureNamesWithoutOne); dawn::native::Adapter adapterWithoutFeature(&mAdapterBase); @@ -81,7 +89,23 @@ TEST_F(FeatureTests, AdapterWithRequiredFeatureDisabled) { ASSERT_EQ(nullptr, deviceWithFeature); } - // Test that the adapter with unsafe apis allowed validate features as expected. + // Test that an adapter with DisallowUnsafeApis disabled validates features as expected. + // TODO(dawn:1685) Remove this block once DisallowUnsafeAPIs is removed. + { + mUnsafePhysicalDeviceDisallow.SetSupportedFeaturesForTesting(featureNamesWithoutOne); + dawn::native::Adapter adapterWithoutFeature(&mUnsafeAdapterBaseDisallow); + + wgpu::DeviceDescriptor deviceDescriptor; + wgpu::FeatureName featureName = FeatureEnumToAPIFeature(notSupportedFeature); + deviceDescriptor.requiredFeatures = &featureName; + deviceDescriptor.requiredFeaturesCount = 1; + + WGPUDevice deviceWithFeature = adapterWithoutFeature.CreateDevice( + reinterpret_cast(&deviceDescriptor)); + ASSERT_EQ(nullptr, deviceWithFeature); + } + + // Test that an adapter with AllowUnsafeApis enabled validates features as expected. { mUnsafePhysicalDevice.SetSupportedFeaturesForTesting(featureNamesWithoutOne); dawn::native::Adapter adapterWithoutFeature(&mUnsafeAdapterBase); @@ -103,7 +127,8 @@ TEST_F(FeatureTests, AdapterWithRequiredFeatureDisabled) { // of the enabled features correctly. TEST_F(FeatureTests, RequireAndGetEnabledFeatures) { dawn::native::Adapter adapter(&mAdapterBase); - dawn::native::Adapter unsafeAdapter(&mUnsafeAdapterBase); + dawn::native::Adapter unsafeAdapterDisallow(&mUnsafeAdapterBaseDisallow); + dawn::native::Adapter unsafeAdapterAllow(&mUnsafeAdapterBase); dawn::native::FeaturesInfo featuresInfo; for (size_t i = 0; i < kTotalFeaturesCount; ++i) { @@ -114,13 +139,13 @@ TEST_F(FeatureTests, RequireAndGetEnabledFeatures) { deviceDescriptor.requiredFeatures = &featureName; deviceDescriptor.requiredFeaturesCount = 1; - // Test with the adapter with DisallowUnsafeApis adapter toggles not disabled. + // Test with the default adapter. { dawn::native::DeviceBase* deviceBase = dawn::native::FromAPI(adapter.CreateDevice( reinterpret_cast(&deviceDescriptor))); - // Creating a device with experimental feature requires the adapter disables - // DisallowUnsafeApis, otherwise a validation error. + // Creating a device with experimental feature requires the adapter enables + // AllowUnsafeAPIs or disables DisallowUnsafeApis, otherwise expect validation error. if (featuresInfo.GetFeatureInfo(featureName)->featureState == dawn::native::FeatureInfo::FeatureState::Experimental) { ASSERT_EQ(nullptr, deviceBase); @@ -137,9 +162,26 @@ TEST_F(FeatureTests, RequireAndGetEnabledFeatures) { // Test with the adapter with DisallowUnsafeApis toggles disabled, creating device should // always succeed. + // TODO(dawn:1685) Remove this block once DisallowUnsafeAPIs is removed. { - dawn::native::DeviceBase* deviceBase = dawn::native::FromAPI(unsafeAdapter.CreateDevice( - reinterpret_cast(&deviceDescriptor))); + dawn::native::DeviceBase* deviceBase = + dawn::native::FromAPI(unsafeAdapterDisallow.CreateDevice( + reinterpret_cast(&deviceDescriptor))); + + ASSERT_NE(nullptr, deviceBase); + ASSERT_EQ(1u, deviceBase->APIEnumerateFeatures(nullptr)); + wgpu::FeatureName enabledFeature; + deviceBase->APIEnumerateFeatures(&enabledFeature); + EXPECT_EQ(enabledFeature, featureName); + deviceBase->APIRelease(); + } + + // Test with the adapter with AllowUnsafeApis toggles enabled, creating device should always + // succeed. + { + dawn::native::DeviceBase* deviceBase = + dawn::native::FromAPI(unsafeAdapterAllow.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 index cc333b7e71..82cb045c2e 100644 --- a/src/dawn/tests/unittests/ToggleTests.cpp +++ b/src/dawn/tests/unittests/ToggleTests.cpp @@ -65,9 +65,10 @@ TEST_F(InstanceToggleTest, InstanceTogglesSet) { { std::unique_ptr instance; - // Create an instance with default toggles, where DisallowUnsafeApis is enabled. + // Create an instance with default toggles, where AllowUnsafeAPIs is disabled and + // DisallowUnsafeAPIs is enabled. instance = std::make_unique(); - validateInstanceToggles(instance.get(), {"disallow_unsafe_apis"}, {}); + validateInstanceToggles(instance.get(), {"disallow_unsafe_apis"}, {"allow_unsafe_apis"}); } // Create instance with empty toggles descriptor @@ -81,9 +82,10 @@ TEST_F(InstanceToggleTest, InstanceTogglesSet) { WGPUInstanceDescriptor instanceDesc = {}; instanceDesc.nextInChain = &instanceTogglesDesc.chain; - // Create an instance with default toggles, where DisallowUnsafeApis is enabled. + // Create an instance with default toggles, where AllowUnsafeAPIs is disabled and + // DisallowUnsafeAPIs is enabled. instance = std::make_unique(&instanceDesc); - validateInstanceToggles(instance.get(), {"disallow_unsafe_apis"}, {}); + validateInstanceToggles(instance.get(), {"disallow_unsafe_apis"}, {"allow_unsafe_apis"}); } // Create instance with DisallowUnsafeAPIs explicitly enabled in toggles descriptor @@ -101,7 +103,7 @@ TEST_F(InstanceToggleTest, InstanceTogglesSet) { // Create an instance with DisallowUnsafeApis explicitly enabled. instance = std::make_unique(&instanceDesc); - validateInstanceToggles(instance.get(), {disallowUnsafeApisToggle}, {}); + validateInstanceToggles(instance.get(), {disallowUnsafeApisToggle}, {"allow_unsafe_apis"}); } // Create instance with DisallowUnsafeAPIs explicitly disabled in toggles descriptor @@ -119,7 +121,45 @@ TEST_F(InstanceToggleTest, InstanceTogglesSet) { // Create an instance with DisallowUnsafeApis explicitly disabled. instance = std::make_unique(&instanceDesc); - validateInstanceToggles(instance.get(), {}, {disallowUnsafeApisToggle}); + validateInstanceToggles(instance.get(), {}, + {disallowUnsafeApisToggle, "allow_unsafe_apis"}); + } + + // Create instance with AllowUnsafeAPIs explicitly enabled in toggles descriptor + { + std::unique_ptr instance; + + const char* allowUnsafeApisToggle = "allow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceTogglesDesc.enabledTogglesCount = 1; + instanceTogglesDesc.enabledToggles = &allowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceTogglesDesc.chain; + + // Create an instance with AllowUnsafeAPIs explicitly enabled. + instance = std::make_unique(&instanceDesc); + validateInstanceToggles(instance.get(), {allowUnsafeApisToggle, "disallow_unsafe_apis"}, + {}); + } + + // Create instance with AllowUnsafeAPIs explicitly disabled in toggles descriptor + { + std::unique_ptr instance; + + const char* allowUnsafeApisToggle = "allow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceTogglesDesc.disabledTogglesCount = 1; + instanceTogglesDesc.disabledToggles = &allowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceTogglesDesc.chain; + + // Create an instance with AllowUnsafeAPIs explicitly disabled. + instance = std::make_unique(&instanceDesc); + validateInstanceToggles(instance.get(), {"disallow_unsafe_apis"}, {allowUnsafeApisToggle}); } } @@ -198,5 +238,41 @@ TEST_F(InstanceToggleTest, InstanceTogglesInheritToAdapterAndDevice) { instance = std::make_unique(&instanceDesc); validateInstanceTogglesInheritedToAdapter(instance.get()); } + + // Create instance with AllowUnsafeAPIs explicitly enabled in toggles descriptor + { + std::unique_ptr instance; + + const char* allowUnsafeApisToggle = "allow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceTogglesDesc.enabledTogglesCount = 1; + instanceTogglesDesc.enabledToggles = &allowUnsafeApisToggle; + + WGPUInstanceDescriptor instanceDesc = {}; + instanceDesc.nextInChain = &instanceTogglesDesc.chain; + + // Create an instance with DisallowUnsafeApis explicitly enabled. + instance = std::make_unique(&instanceDesc); + validateInstanceTogglesInheritedToAdapter(instance.get()); + } + + // Create instance with AllowUnsafeAPIs explicitly disabled in toggles descriptor + { + std::unique_ptr instance; + + const char* allowUnsafeApisToggle = "allow_unsafe_apis"; + WGPUDawnTogglesDescriptor instanceTogglesDesc = {}; + instanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + instanceTogglesDesc.disabledTogglesCount = 1; + instanceTogglesDesc.disabledToggles = &allowUnsafeApisToggle; + + 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 add691af37..84a2101de2 100644 --- a/src/dawn/tests/unittests/native/DeviceCreationTests.cpp +++ b/src/dawn/tests/unittests/native/DeviceCreationTests.cpp @@ -39,19 +39,8 @@ class DeviceCreationTest : public testing::Test { void SetUp() override { dawnProcSetProcs(&dawn::native::GetProcs()); + // Create an instance with default toggles and create an adapter from it. 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()) { @@ -64,8 +53,41 @@ class DeviceCreationTest : public testing::Test { } } - // Create an instance with toggle DisallowUnsafeApis disabled, and create an unsafe adapter + { + // Create an instance with toggle DisallowUnsafeAPIs disabled, and create an unsafe + // adapter from it. + // TODO(dawn:1685) Remove this block once DisallowUnsafeAPIs is fully removed. + 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; + + unsafeInstanceDisallow = std::make_unique(&unsafeInstanceDesc); + unsafeInstanceDisallow->DiscoverDefaultAdapters(); + for (dawn::native::Adapter& nativeAdapter : unsafeInstanceDisallow->GetAdapters()) { + wgpu::AdapterProperties properties; + nativeAdapter.GetProperties(&properties); + + if (properties.backendType == wgpu::BackendType::Null) { + unsafeAdapterDisallow = wgpu::Adapter(nativeAdapter.Get()); + break; + } + } + } + + // Create an instance with toggle AllowUnsafeAPIs enabled, and create an unsafe adapter // from it. + const char* allowUnsafeApisToggle = "allow_unsafe_apis"; + WGPUDawnTogglesDescriptor unsafeInstanceTogglesDesc = {}; + unsafeInstanceTogglesDesc.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; + unsafeInstanceTogglesDesc.enabledTogglesCount = 1; + unsafeInstanceTogglesDesc.enabledToggles = &allowUnsafeApisToggle; + WGPUInstanceDescriptor unsafeInstanceDesc = {}; + unsafeInstanceDesc.nextInChain = &unsafeInstanceTogglesDesc.chain; + unsafeInstance = std::make_unique(&unsafeInstanceDesc); unsafeInstance->DiscoverDefaultAdapters(); for (dawn::native::Adapter& nativeAdapter : unsafeInstance->GetAdapters()) { @@ -79,13 +101,16 @@ class DeviceCreationTest : public testing::Test { } ASSERT_NE(adapter, nullptr); + ASSERT_NE(unsafeAdapterDisallow, nullptr); ASSERT_NE(unsafeAdapter, nullptr); } void TearDown() override { adapter = nullptr; + unsafeAdapterDisallow = nullptr; unsafeAdapter = nullptr; instance = nullptr; + unsafeInstanceDisallow = nullptr; unsafeInstance = nullptr; dawnProcSetProcs(nullptr); } @@ -94,8 +119,10 @@ class DeviceCreationTest : public testing::Test { static_cast(dawn::native::Feature::EnumCount); std::unique_ptr instance; + std::unique_ptr unsafeInstanceDisallow; std::unique_ptr unsafeInstance; wgpu::Adapter adapter; + wgpu::Adapter unsafeAdapterDisallow; wgpu::Adapter unsafeAdapter; dawn::native::FeaturesInfo featuresInfo; }; @@ -132,15 +159,14 @@ TEST_F(DeviceCreationTest, CreateDeviceWithTogglesSuccess) { // Test experimental features are guarded by DisallowUnsafeApis adapter toggle, it is inherited from // instance but can be overriden by device toggles. +// TODO(dawn:1685) Remove clang format disabling after the DisallowUnsafeAPI block below is removed. +// clang-format off 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)); + // Ensure that unsafe apis are disallowed on safe adapter. + ASSERT_FALSE(dawn::native::FromAPI(adapter.Get())->AllowUnsafeAPIs()); + // Ensure that unsafe apis are allowed unsafe adapter(s). + ASSERT_TRUE(dawn::native::FromAPI(unsafeAdapterDisallow.Get())->AllowUnsafeAPIs()); + ASSERT_TRUE(dawn::native::FromAPI(unsafeAdapter.Get())->AllowUnsafeAPIs()); for (size_t i = 0; i < kTotalFeaturesCount; i++) { dawn::native::Feature feature = static_cast(i); @@ -156,16 +182,84 @@ TEST_F(DeviceCreationTest, CreateDeviceRequiringExperimentalFeatures) { deviceDescriptor.requiredFeatures = &featureName; deviceDescriptor.requiredFeaturesCount = 1; - // Test creating device on the adapter with DisallowUnsafeApis toggle enabled would fail. + // Test creating device on default adapter would fail. { wgpu::Device device = adapter.CreateDevice(&deviceDescriptor); EXPECT_EQ(device, nullptr); } - // Test creating device on the adapter with DisallowUnsafeApis toggle disabled would - // succeed. + // TODO(dawn:1685): Remove this block once DisallowUnsafeAPIs is removed. + { + // Test creating device on the adapter with DisallowUnsafeApis toggle disabled would + // succeed. + { + wgpu::Device device = unsafeAdapterDisallow.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; + deviceTogglesDesc.disabledToggles = disableToggles; + deviceTogglesDesc.disabledTogglesCount = 1; + deviceDescriptor.nextInChain = &deviceTogglesDesc; + + // 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, featureName); + } + + // Test on adapter with DisallowUnsafeApis disabled. + { + wgpu::Device device = unsafeAdapterDisallow.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 = unsafeAdapterDisallow.CreateDevice(&deviceDescriptor); + EXPECT_EQ(device, nullptr); + } + } + } + + // Test creating device on the adapter with AllowUnsafeApis toggle enabled would succeed. { - // unsafeAdapter has DisallowUnsafeApis adapter toggle disabled. wgpu::Device device = unsafeAdapter.CreateDevice(&deviceDescriptor); EXPECT_NE(device, nullptr); @@ -175,16 +269,16 @@ TEST_F(DeviceCreationTest, CreateDeviceRequiringExperimentalFeatures) { EXPECT_EQ(enabledFeature, featureName); } - // Test creating device with DisallowUnsafeApis disabled in device toggle descriptor will + // Test creating device with AllowUnsafeApis enabled 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"}; + const char* const enableToggles[] = {"allow_unsafe_apis"}; wgpu::DawnTogglesDescriptor deviceTogglesDesc; - deviceTogglesDesc.disabledToggles = disableToggles; - deviceTogglesDesc.disabledTogglesCount = 1; + deviceTogglesDesc.enabledToggles = enableToggles; + deviceTogglesDesc.enabledTogglesCount = 1; deviceDescriptor.nextInChain = &deviceTogglesDesc; - // Test on adapter with DisallowUnsafeApis enabled. + // Test on adapter with AllowUnsafeApis disabled. { wgpu::Device device = adapter.CreateDevice(&deviceDescriptor); EXPECT_NE(device, nullptr); @@ -195,7 +289,7 @@ TEST_F(DeviceCreationTest, CreateDeviceRequiringExperimentalFeatures) { EXPECT_EQ(enabledFeature, featureName); } - // Test on adapter with DisallowUnsafeApis disabled. + // Test on adapter with AllowUnsafeApis disabled. { wgpu::Device device = unsafeAdapter.CreateDevice(&deviceDescriptor); EXPECT_NE(device, nullptr); @@ -207,13 +301,13 @@ TEST_F(DeviceCreationTest, CreateDeviceRequiringExperimentalFeatures) { } } - // Test creating device with DisallowUnsafeApis enabled in device toggle descriptor will - // fail on both adapter, as device toggles will override the inherited adapter toggles. + // Test creating device with AllowUnsafeApis disabled 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"}; + const char* const disableToggles[] = {"allow_unsafe_apis"}; wgpu::DawnTogglesDescriptor deviceToggleDesc; - deviceToggleDesc.enabledToggles = enableToggles; - deviceToggleDesc.enabledTogglesCount = 1; + deviceToggleDesc.disabledToggles = disableToggles; + deviceToggleDesc.disabledTogglesCount = 1; deviceDescriptor.nextInChain = &deviceToggleDesc; // Test on adapter with DisallowUnsafeApis enabled. @@ -230,6 +324,7 @@ TEST_F(DeviceCreationTest, CreateDeviceRequiringExperimentalFeatures) { } } } +// clang-format on TEST_F(DeviceCreationTest, CreateDeviceWithCacheSuccess) { // Default device descriptor should have the same cache key as a device descriptor with a diff --git a/src/dawn/tests/unittests/validation/QueryValidationTests.cpp b/src/dawn/tests/unittests/validation/QueryValidationTests.cpp index ccf12104f0..52e5aa65de 100644 --- a/src/dawn/tests/unittests/validation/QueryValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/QueryValidationTests.cpp @@ -711,7 +711,7 @@ 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 + // statistic query is an unsafe API, while AllowUnsafeApis instance toggle is enabled // when ValidationTest creating testing instance, so we can test it. wgpu::DeviceDescriptor descriptor; wgpu::FeatureName requiredFeatures[1] = {wgpu::FeatureName::PipelineStatisticsQuery}; diff --git a/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp index 5ecd096a3d..f0335ec64f 100644 --- a/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp @@ -26,17 +26,17 @@ using testing::HasSubstr; class UnsafeAPIValidationTest : public ValidationTest { protected: - // UnsafeAPIValidationTest create the device with toggle DisallowUnsafeApis explicitly enabled, - // which overrides the inheritance. + // UnsafeAPIValidationTest create the device with the AllowUnsafeAPIs toggle explicitly + // disabled, which overrides the inheritance. WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override { - // Enable the DisallowUnsafeAPIs toggles in device toggles descriptor to override the + // Disable the AllowUnsafeAPIs 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; - const char* toggle = "disallow_unsafe_apis"; - deviceTogglesDesc.enabledToggles = &toggle; - deviceTogglesDesc.enabledTogglesCount = 1; + const char* toggle = "allow_unsafe_apis"; + deviceTogglesDesc.disabledToggles = &toggle; + deviceTogglesDesc.disabledTogglesCount = 1; return dawnAdapter.CreateDevice(&descriptor); } }; diff --git a/src/dawn/tests/unittests/validation/ValidationTest.cpp b/src/dawn/tests/unittests/validation/ValidationTest.cpp index b573c39979..2c21dc7eaf 100644 --- a/src/dawn/tests/unittests/validation/ValidationTest.cpp +++ b/src/dawn/tests/unittests/validation/ValidationTest.cpp @@ -125,15 +125,15 @@ ValidationTest::ValidationTest() { } void ValidationTest::SetUp() { - // Create an instance with toggle DisallowUnsafeApis disabled, which would be inherited to + // Create an instance with toggle AllowUnsafeAPIs enabled, 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 + // features). To test device with AllowUnsafeAPIs disabled, require it in device toggles // descriptor to override the inheritance. - const char* disallowUnsafeApisToggle = "disallow_unsafe_apis"; + const char* allowUnsafeApisToggle = "allow_unsafe_apis"; WGPUDawnTogglesDescriptor instanceToggles = {}; instanceToggles.chain.sType = WGPUSType::WGPUSType_DawnTogglesDescriptor; - instanceToggles.disabledTogglesCount = 1; - instanceToggles.disabledToggles = &disallowUnsafeApisToggle; + instanceToggles.enabledTogglesCount = 1; + instanceToggles.enabledToggles = &allowUnsafeApisToggle; WGPUInstanceDescriptor instanceDesc = {}; instanceDesc.nextInChain = &instanceToggles.chain;