Dawn: Fix FeatureState of some features

This CL fix FeatureState for some experimental features to prepare for
refactoring adapter creation with adapter toggles. This CL also fix the
related unittests.

Bug: dawn:1495
Change-Id: Ibf043ed74c0bfc79c64986f2f96135d92adf3930
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/116841
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: Zhaoming Jiang <zhaoming.jiang@intel.com>
Commit-Queue: Zhaoming Jiang <zhaoming.jiang@intel.com>

src/dawn/native/Features.cpp

@@ -47,10 +47,12 @@ static constexpr FeatureEnumAndInfoList kFeatureNameAndInfoList = {{
       "https://bugs.chromium.org/p/dawn/issues/detail?id=955", FeatureInfo::FeatureState::Stable}},
     {Feature::PipelineStatisticsQuery,
      {"pipeline-statistics-query", "Support Pipeline Statistics Query",
-      "https://bugs.chromium.org/p/dawn/issues/detail?id=434", FeatureInfo::FeatureState::Stable}},
+      "https://bugs.chromium.org/p/dawn/issues/detail?id=434",
+      FeatureInfo::FeatureState::Experimental}},
     {Feature::TimestampQuery,
      {"timestamp-query", "Support Timestamp Query",
-      "https://bugs.chromium.org/p/dawn/issues/detail?id=434", FeatureInfo::FeatureState::Stable}},
+      "https://bugs.chromium.org/p/dawn/issues/detail?id=434",
+      FeatureInfo::FeatureState::Experimental}},
     {Feature::TimestampQueryInsidePasses,
      {"timestamp-query-inside-passes", "Support Timestamp Query inside render/compute pass",
       "https://bugs.chromium.org/p/dawn/issues/detail?id=434",

src/dawn/tests/unittests/FeatureTests.cpp

@@ -55,43 +55,98 @@ TEST_F(FeatureTests, AdapterWithRequiredFeatureDisabled) {
         mAdapterBase.SetSupportedFeatures(featureNamesWithoutOne);
         dawn::native::Adapter adapterWithoutFeature(&mAdapterBase);
 
-        wgpu::DeviceDescriptor deviceDescriptor;
+        // Test that creating device with and without DisallowUnsafeApis toggle disabled will both
-        wgpu::FeatureName featureName = FeatureEnumToAPIFeature(notSupportedFeature);
+        // failed.
-        deviceDescriptor.requiredFeatures = &featureName;
-        deviceDescriptor.requiredFeaturesCount = 1;
 
-        WGPUDevice deviceWithFeature = adapterWithoutFeature.CreateDevice(
+        // Without disabling DisallowUnsafeApis toggle
-            reinterpret_cast<const WGPUDeviceDescriptor*>(&deviceDescriptor));
+        {
-        ASSERT_EQ(nullptr, deviceWithFeature);
+            wgpu::DeviceDescriptor deviceDescriptor;
+            wgpu::FeatureName featureName = FeatureEnumToAPIFeature(notSupportedFeature);
+            deviceDescriptor.requiredFeatures = &featureName;
+            deviceDescriptor.requiredFeaturesCount = 1;
+
+            WGPUDevice deviceWithFeature = adapterWithoutFeature.CreateDevice(
+                reinterpret_cast<const WGPUDeviceDescriptor*>(&deviceDescriptor));
+            ASSERT_EQ(nullptr, deviceWithFeature);
+        }
+
+        // Disabling DisallowUnsafeApis toggle
+        {
+            wgpu::DeviceDescriptor unsafeDeviceDescriptor;
+            wgpu::FeatureName featureName = FeatureEnumToAPIFeature(notSupportedFeature);
+            unsafeDeviceDescriptor.requiredFeatures = &featureName;
+            unsafeDeviceDescriptor.requiredFeaturesCount = 1;
+
+            wgpu::DawnTogglesDeviceDescriptor togglesDesc;
+            unsafeDeviceDescriptor.nextInChain = &togglesDesc;
+            const char* toggle = "disallow_unsafe_apis";
+            togglesDesc.forceDisabledToggles = &toggle;
+            togglesDesc.forceDisabledTogglesCount = 1;
+
+            WGPUDevice deviceWithFeature = adapterWithoutFeature.CreateDevice(
+                reinterpret_cast<const WGPUDeviceDescriptor*>(&unsafeDeviceDescriptor));
+            ASSERT_EQ(nullptr, deviceWithFeature);
+        }
     }
 }
 
-// Test Device.GetEnabledFeatures() can return the names of the enabled features correctly.
+// Test creating device requiring a supported feature can succeed (with DisallowUnsafeApis guarded
-TEST_F(FeatureTests, GetEnabledFeatures) {
+// 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::FeaturesInfo featuresInfo;
+
     for (size_t i = 0; i < kTotalFeaturesCount; ++i) {
         dawn::native::Feature feature = static_cast<dawn::native::Feature>(i);
         wgpu::FeatureName featureName = FeatureEnumToAPIFeature(feature);
 
-        wgpu::DeviceDescriptor deviceDescriptor;
+        // Test with DisallowUnsafeApis not disabled.
-        deviceDescriptor.requiredFeatures = &featureName;
+        {
-        deviceDescriptor.requiredFeaturesCount = 1;
+            wgpu::DeviceDescriptor deviceDescriptor;
+            deviceDescriptor.requiredFeatures = &featureName;
+            deviceDescriptor.requiredFeaturesCount = 1;
 
-        // Some features may require DisallowUnsafeApis toggle disabled, otherwise CreateDevice may
+            dawn::native::DeviceBase* deviceBase = dawn::native::FromAPI(adapter.CreateDevice(
-        // failed.
+                reinterpret_cast<const WGPUDeviceDescriptor*>(&deviceDescriptor)));
-        const char* const disableToggles[] = {"disallow_unsafe_apis"};
-        wgpu::DawnTogglesDeviceDescriptor toggleDesc;
-        toggleDesc.forceDisabledToggles = disableToggles;
-        toggleDesc.forceDisabledTogglesCount = 1;
-        deviceDescriptor.nextInChain = &toggleDesc;
 
-        dawn::native::DeviceBase* deviceBase = dawn::native::FromAPI(
+            // Device creation should fail if requiring experimental features without disabling
-            adapter.CreateDevice(reinterpret_cast<const WGPUDeviceDescriptor*>(&deviceDescriptor)));
+            // DisallowUnsafeApis
+            if (featuresInfo.GetFeatureInfo(featureName)->featureState ==
+                dawn::native::FeatureInfo::FeatureState::Experimental) {
+                ASSERT_EQ(nullptr, deviceBase);
+            } else {
+                // Requiring stable features should succeed.
+                ASSERT_NE(nullptr, deviceBase);
+                ASSERT_EQ(1u, deviceBase->APIEnumerateFeatures(nullptr));
+                wgpu::FeatureName enabledFeature;
+                deviceBase->APIEnumerateFeatures(&enabledFeature);
+                EXPECT_EQ(enabledFeature, featureName);
+                deviceBase->APIRelease();
+            }
+        }
 
-        ASSERT_EQ(1u, deviceBase->APIEnumerateFeatures(nullptr));
+        // Test with DisallowUnsafeApis disabled, creating device should always succeed.
-        wgpu::FeatureName enabledFeature;
+        {
-        deviceBase->APIEnumerateFeatures(&enabledFeature);
+            wgpu::DeviceDescriptor unsafeDeviceDescriptor;
-        EXPECT_EQ(enabledFeature, featureName);
+            unsafeDeviceDescriptor.requiredFeatures = &featureName;
-        deviceBase->APIRelease();
+            unsafeDeviceDescriptor.requiredFeaturesCount = 1;
+
+            const char* const disableToggles[] = {"disallow_unsafe_apis"};
+            wgpu::DawnTogglesDeviceDescriptor toggleDesc;
+            toggleDesc.forceDisabledToggles = disableToggles;
+            toggleDesc.forceDisabledTogglesCount = 1;
+            unsafeDeviceDescriptor.nextInChain = &toggleDesc;
+
+            dawn::native::DeviceBase* deviceBase = dawn::native::FromAPI(adapter.CreateDevice(
+                reinterpret_cast<const WGPUDeviceDescriptor*>(&unsafeDeviceDescriptor)));
+
+            ASSERT_NE(nullptr, deviceBase);
+            ASSERT_EQ(1u, deviceBase->APIEnumerateFeatures(nullptr));
+            wgpu::FeatureName enabledFeature;
+            deviceBase->APIEnumerateFeatures(&enabledFeature);
+            EXPECT_EQ(enabledFeature, featureName);
+            deviceBase->APIRelease();
+        }
     }
 }

src/dawn/tests/unittests/validation/QueryValidationTests.cpp

@@ -43,6 +43,14 @@ TEST_F(QuerySetValidationTest, CreationWithoutFeatures) {
     CreateQuerySet(device, wgpu::QueryType::Occlusion, 1);
 
     // Creating a query set for other types of queries fails without features enabled.
+    ASSERT_DEVICE_ERROR(CreateQuerySet(device, wgpu::QueryType::PipelineStatistics, 1,
+                                       {wgpu::PipelineStatisticName::VertexShaderInvocations}));
+    ASSERT_DEVICE_ERROR(CreateQuerySet(device, wgpu::QueryType::PipelineStatistics, 1,
+                                       {wgpu::PipelineStatisticName::ClipperPrimitivesOut}));
+    ASSERT_DEVICE_ERROR(CreateQuerySet(device, wgpu::QueryType::PipelineStatistics, 1,
+                                       {wgpu::PipelineStatisticName::ComputeShaderInvocations}));
+    ASSERT_DEVICE_ERROR(CreateQuerySet(device, wgpu::QueryType::PipelineStatistics, 1,
+                                       {wgpu::PipelineStatisticName::FragmentShaderInvocations}));
     ASSERT_DEVICE_ERROR(CreateQuerySet(device, wgpu::QueryType::PipelineStatistics, 1,
                                        {wgpu::PipelineStatisticName::VertexShaderInvocations}));
     ASSERT_DEVICE_ERROR(CreateQuerySet(device, wgpu::QueryType::Timestamp, 1));

src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp

@@ -37,65 +37,6 @@ class UnsafeAPIValidationTest : public ValidationTest {
     }
 };
 
-class UnsafeQueryAPIValidationTest : public ValidationTest {
-  protected:
-    WGPUDevice CreateTestDevice(dawn::native::Adapter dawnAdapter) override {
-        wgpu::DeviceDescriptor descriptor;
-        wgpu::FeatureName requiredFeatures[2] = {wgpu::FeatureName::PipelineStatisticsQuery,
-                                                 wgpu::FeatureName::TimestampQuery};
-        descriptor.requiredFeatures = requiredFeatures;
-        descriptor.requiredFeaturesCount = 2;
-
-        wgpu::DawnTogglesDeviceDescriptor togglesDesc;
-        descriptor.nextInChain = &togglesDesc;
-        const char* toggle = "disallow_unsafe_apis";
-        togglesDesc.forceEnabledToggles = &toggle;
-        togglesDesc.forceEnabledTogglesCount = 1;
-
-        return dawnAdapter.CreateDevice(&descriptor);
-    }
-};
-
-// Check that pipeline statistics query are disallowed.
-TEST_F(UnsafeQueryAPIValidationTest, PipelineStatisticsDisallowed) {
-    wgpu::QuerySetDescriptor descriptor;
-    descriptor.count = 1;
-
-    // Control case: occlusion query creation is allowed.
-    {
-        descriptor.type = wgpu::QueryType::Occlusion;
-        device.CreateQuerySet(&descriptor);
-    }
-
-    // Error case: pipeline statistics query creation is disallowed.
-    {
-        descriptor.type = wgpu::QueryType::PipelineStatistics;
-        std::vector<wgpu::PipelineStatisticName> pipelineStatistics = {
-            wgpu::PipelineStatisticName::VertexShaderInvocations};
-        descriptor.pipelineStatistics = pipelineStatistics.data();
-        descriptor.pipelineStatisticsCount = pipelineStatistics.size();
-        ASSERT_DEVICE_ERROR(device.CreateQuerySet(&descriptor));
-    }
-}
-
-// Check timestamp queries are disallowed.
-TEST_F(UnsafeQueryAPIValidationTest, TimestampQueryDisallowed) {
-    wgpu::QuerySetDescriptor descriptor;
-    descriptor.count = 1;
-
-    // Control case: occlusion query creation is allowed.
-    {
-        descriptor.type = wgpu::QueryType::Occlusion;
-        device.CreateQuerySet(&descriptor);
-    }
-
-    // Error case: timestamp query creation is disallowed.
-    {
-        descriptor.type = wgpu::QueryType::Timestamp;
-        ASSERT_DEVICE_ERROR(device.CreateQuerySet(&descriptor));
-    }
-}
-
 // Check chromium_disable_uniformity_analysis is an unsafe API.
 TEST_F(UnsafeAPIValidationTest, chromium_disable_uniformity_analysis) {
     ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(