From 7cbcacc712cd93a941dcaf635a2b731466a45319 Mon Sep 17 00:00:00 2001 From: Hao Li Date: Wed, 9 Jun 2021 08:47:37 +0000 Subject: [PATCH] Disallow pipeline statistics query as UnsafeAPIs - Pipeline statistics query is not fully implemented, disallow its creation as unsafe - Add pipeline statistics creation in UnsafeAPIsTest, because it needs enable extension, add a separate test class for it. BUG: chromium:1177506 Change-Id: Ic77e04c9c854b396e7240674bd9deb0caf97a513 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/53889 Commit-Queue: Jiawei Shao Reviewed-by: Corentin Wallez Reviewed-by: Jiawei Shao --- src/dawn_native/QuerySet.cpp | 8 +++++ .../validation/QueryValidationTests.cpp | 7 ++-- .../validation/UnsafeAPIValidationTests.cpp | 32 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/dawn_native/QuerySet.cpp b/src/dawn_native/QuerySet.cpp index 26e9c6d1c9..e8e3c9083c 100644 --- a/src/dawn_native/QuerySet.cpp +++ b/src/dawn_native/QuerySet.cpp @@ -58,6 +58,14 @@ namespace dawn_native { break; case wgpu::QueryType::PipelineStatistics: { + // TODO(crbug.com/1177506): Pipeline statistics query is not fully implemented. + // Disallow it as unsafe until the implementaion is completed. + if (device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) { + return DAWN_VALIDATION_ERROR( + "Pipeline statistics query is disallowed because it's not fully " + "implemented"); + } + if (!device->IsExtensionEnabled(Extension::PipelineStatisticsQuery)) { return DAWN_VALIDATION_ERROR( "The pipeline statistics query feature is not supported"); diff --git a/src/tests/unittests/validation/QueryValidationTests.cpp b/src/tests/unittests/validation/QueryValidationTests.cpp index 1ee0134765..53a650f56e 100644 --- a/src/tests/unittests/validation/QueryValidationTests.cpp +++ b/src/tests/unittests/validation/QueryValidationTests.cpp @@ -226,7 +226,7 @@ class TimestampQueryValidationTest : public QuerySetValidationTest { protected: WGPUDevice CreateTestDevice() override { dawn_native::DeviceDescriptor descriptor; - descriptor.requiredExtensions = {"timestamp_query"}; + descriptor.requiredExtensions.push_back("timestamp_query"); return adapter.CreateDevice(&descriptor); } }; @@ -429,7 +429,10 @@ class PipelineStatisticsQueryValidationTest : public QuerySetValidationTest { protected: WGPUDevice CreateTestDevice() override { dawn_native::DeviceDescriptor descriptor; - descriptor.requiredExtensions = {"pipeline_statistics_query"}; + descriptor.requiredExtensions.push_back("pipeline_statistics_query"); + // TODO(crbug.com/1177506): Pipeline statistic query is an unsafe API, disable disallowing + // unsafe APIs to test it. + descriptor.forceDisabledToggles.push_back("disallow_unsafe_apis"); return adapter.CreateDevice(&descriptor); } }; diff --git a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp index 6eebeb0a26..02e55fd761 100644 --- a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp +++ b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp @@ -201,3 +201,35 @@ TEST_F(UnsafeAPIValidationTest, DynamicStorageBuffer) { ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&desc)); } } + +class UnsafeQueryAPIValidationTest : public ValidationTest { + protected: + WGPUDevice CreateTestDevice() override { + dawn_native::DeviceDescriptor descriptor; + descriptor.requiredExtensions.push_back("pipeline_statistics_query"); + descriptor.forceEnabledToggles.push_back("disallow_unsafe_apis"); + return adapter.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 pipelineStatistics = { + wgpu::PipelineStatisticName::VertexShaderInvocations}; + descriptor.pipelineStatistics = pipelineStatistics.data(); + descriptor.pipelineStatisticsCount = pipelineStatistics.size(); + ASSERT_DEVICE_ERROR(device.CreateQuerySet(&descriptor)); + } +}