From 0bdf2da9b8b8655d85bc4d12195f36c3f1ab21fc Mon Sep 17 00:00:00 2001 From: Gregg Tavares Date: Wed, 24 May 2023 12:39:46 +0000 Subject: [PATCH] Compat: Reject CubeArrays via Validation Bug: dawn:1825 Change-Id: I32917a479c3eea457c8f315cfc089351b64292b6 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/133320 Commit-Queue: Gregg Tavares Reviewed-by: Austin Eng Kokoro: Kokoro --- dawn_wire.json | 2 +- src/dawn/native/Device.cpp | 4 ++ src/dawn/native/Device.h | 1 + src/dawn/native/Texture.cpp | 11 +++- src/dawn/tests/BUILD.gn | 1 + src/dawn/tests/DawnTest.cpp | 33 ++++++++--- src/dawn/tests/DawnTest.h | 8 ++- src/dawn/tests/end2end/TextureViewTests.cpp | 2 +- .../validation/CompatValidationTests.cpp | 58 +++++++++++++++++++ .../unittests/validation/ValidationTest.cpp | 11 +++- .../unittests/validation/ValidationTest.h | 2 + 11 files changed, 114 insertions(+), 19 deletions(-) create mode 100644 src/dawn/tests/unittests/validation/CompatValidationTests.cpp diff --git a/dawn_wire.json b/dawn_wire.json index e5cf4f6070..aa96202272 100644 --- a/dawn_wire.json +++ b/dawn_wire.json @@ -89,7 +89,7 @@ { "name": "instance id", "type": "ObjectId", "id_type": "instance" }, { "name": "request serial", "type": "uint64_t" }, { "name": "adapter object handle", "type": "ObjectHandle", "handle_type": "adapter"}, - { "name": "options", "type": "request adapter options", "annotation": "const*" } + { "name": "options", "type": "request adapter options", "annotation": "const*", "optional": true } ], "adapter request device": [ { "name": "adapter id", "type": "ObjectId", "id_type": "adapter" }, diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp index 8d306fd2fd..4d774568e7 100644 --- a/src/dawn/native/Device.cpp +++ b/src/dawn/native/Device.cpp @@ -1434,6 +1434,10 @@ bool DeviceBase::IsRobustnessEnabled() const { return !IsToggleEnabled(Toggle::DisableRobustness); } +bool DeviceBase::IsCompatibilityMode() const { + return mAdapter->GetFeatureLevel() == FeatureLevel::Compatibility; +} + bool DeviceBase::AllowUnsafeAPIs() const { // TODO(dawn:1685) Currently allows if either toggle are set accordingly. return IsToggleEnabled(Toggle::AllowUnsafeAPIs) || !IsToggleEnabled(Toggle::DisallowUnsafeAPIs); diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h index 3643566f99..28a7f2a440 100644 --- a/src/dawn/native/Device.h +++ b/src/dawn/native/Device.h @@ -360,6 +360,7 @@ class DeviceBase : public RefCountedWithExternalCount { bool IsToggleEnabled(Toggle toggle) const; bool IsValidationEnabled() const; bool IsRobustnessEnabled() const; + bool IsCompatibilityMode() const; bool AllowUnsafeAPIs() const; size_t GetLazyClearCountForTesting(); void IncrementLazyClearCountForTesting(); diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp index fece48f445..0747b2cc4a 100644 --- a/src/dawn/native/Texture.cpp +++ b/src/dawn/native/Texture.cpp @@ -20,6 +20,7 @@ #include "dawn/common/Assert.h" #include "dawn/common/Constants.h" #include "dawn/common/Math.h" +#include "dawn/native/Adapter.h" #include "dawn/native/ChainUtils_autogen.h" #include "dawn/native/Device.h" #include "dawn/native/EnumMaskIterator.h" @@ -178,7 +179,8 @@ MaybeError ValidateSampleCount(const TextureDescriptor* descriptor, return {}; } -MaybeError ValidateTextureViewDimensionCompatibility(const TextureBase* texture, +MaybeError ValidateTextureViewDimensionCompatibility(const DeviceBase* device, + const TextureBase* texture, const TextureViewDescriptor* descriptor) { DAWN_INVALID_IF(!IsArrayLayerValidForTextureViewDimension(descriptor->dimension, descriptor->arrayLayerCount), @@ -207,8 +209,11 @@ MaybeError ValidateTextureViewDimensionCompatibility(const TextureBase* texture, "(%u) and height (%u) are not equal.", descriptor->dimension, texture, texture->GetSize().width, texture->GetSize().height); + DAWN_INVALID_IF(descriptor->dimension == wgpu::TextureViewDimension::CubeArray && + device->IsCompatibilityMode(), + "A %s texture view for %s is not supported in compatibility mode", + descriptor->dimension, texture); break; - case wgpu::TextureViewDimension::e1D: case wgpu::TextureViewDimension::e2D: case wgpu::TextureViewDimension::e2DArray: @@ -436,7 +441,7 @@ MaybeError ValidateTextureViewDescriptor(const DeviceBase* device, descriptor->baseMipLevel, descriptor->mipLevelCount, texture->GetNumMipLevels()); DAWN_TRY(ValidateCanViewTextureAs(device, texture, *viewFormat, descriptor->aspect)); - DAWN_TRY(ValidateTextureViewDimensionCompatibility(texture, descriptor)); + DAWN_TRY(ValidateTextureViewDimensionCompatibility(device, texture, descriptor)); return {}; } diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn index 0d387472b1..40a922e3d6 100644 --- a/src/dawn/tests/BUILD.gn +++ b/src/dawn/tests/BUILD.gn @@ -327,6 +327,7 @@ dawn_test("dawn_unittests") { "unittests/validation/BindGroupValidationTests.cpp", "unittests/validation/BufferValidationTests.cpp", "unittests/validation/CommandBufferValidationTests.cpp", + "unittests/validation/CompatValidationTests.cpp", "unittests/validation/ComputeIndirectValidationTests.cpp", "unittests/validation/ComputeValidationTests.cpp", "unittests/validation/CopyCommandsValidationTests.cpp", diff --git a/src/dawn/tests/DawnTest.cpp b/src/dawn/tests/DawnTest.cpp index 423a5ac2db..a2d94f03c5 100644 --- a/src/dawn/tests/DawnTest.cpp +++ b/src/dawn/tests/DawnTest.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -128,10 +129,13 @@ struct ParamTogglesHelper { DawnTestBase::PrintToStringParamName::PrintToStringParamName(const char* test) : mTest(test) {} -std::string DawnTestBase::PrintToStringParamName::SanitizeParamName(std::string paramName, - size_t index) const { +std::string DawnTestBase::PrintToStringParamName::SanitizeParamName( + std::string paramName, + const wgpu::AdapterProperties& properties, + size_t index) const { // Sanitize the adapter name for GoogleTest - std::string sanitizedName = std::regex_replace(paramName, std::regex("[^a-zA-Z0-9]+"), "_"); + std::string sanitizedName = std::regex_replace(paramName, std::regex("[^a-zA-Z0-9]+"), "_") + + (properties.compatibilityMode ? "_compat" : ""); // Strip trailing underscores, if any. while (sanitizedName.back() == '_') { @@ -437,11 +441,15 @@ void DawnTestEnvironment::SelectPreferredAdapterProperties(const dawn::native::I } } - std::set> adapterNameSet; + std::set> adapterNameSet; for (const dawn::native::Adapter& adapter : instance->GetAdapters()) { wgpu::AdapterProperties properties; adapter.GetProperties(&properties); + if (properties.compatibilityMode) { + continue; + } + // All adapters are selected by default. bool selected = true; // The adapter is deselected if: @@ -479,8 +487,8 @@ void DawnTestEnvironment::SelectPreferredAdapterProperties(const dawn::native::I // In Windows Remote Desktop sessions we may be able to discover multiple adapters that // have the same name and backend type. We will just choose one adapter from them in our // tests. - const auto adapterTypeAndName = - std::make_pair(properties.backendType, std::string(properties.name)); + const auto adapterTypeAndName = std::tuple( + properties.backendType, std::string(properties.name), properties.compatibilityMode); if (adapterNameSet.find(adapterTypeAndName) == adapterNameSet.end()) { adapterNameSet.insert(adapterTypeAndName); mAdapterProperties.emplace_back(properties, selected); @@ -493,10 +501,11 @@ std::vector DawnTestEnvironment::GetAvailableAdapterTestParams size_t numParams) { std::vector testParams; for (size_t i = 0; i < numParams; ++i) { + const auto& backendTestParams = params[i]; for (const auto& adapterProperties : mAdapterProperties) { - if (params[i].backendType == adapterProperties.backendType && + if (backendTestParams.backendType == adapterProperties.backendType && adapterProperties.selected) { - testParams.push_back(AdapterTestParam(params[i], adapterProperties)); + testParams.push_back(AdapterTestParam(backendTestParams, adapterProperties)); } } } @@ -590,7 +599,8 @@ void DawnTestEnvironment::PrintTestConfigurationAndAdapterInfo( << properties.adapterName << "\" - \"" << properties.driverDescription << (properties.selected ? " [Selected]" : "") << "\"\n" << " type: " << properties.AdapterTypeName() - << ", backend: " << properties.ParamName() << "\n" + << ", backend: " << properties.ParamName() + << ", compatibilityMode: " << (properties.compatibilityMode ? "true" : "false") << "\n" << " vendorId: 0x" << vendorId.str() << ", deviceId: 0x" << deviceId.str() << "\n"; if (strlen(properties.vendorName) || strlen(properties.architecture)) { @@ -684,6 +694,7 @@ DawnTestBase::DawnTestBase(const AdapterTestParam& param) : mParam(param) { const auto& param = gCurrentTest->mParam; return (param.adapterProperties.selected && + properties.compatibilityMode == param.adapterProperties.compatibilityMode && properties.deviceID == param.adapterProperties.deviceID && properties.vendorID == param.adapterProperties.vendorID && properties.adapterType == param.adapterProperties.adapterType && @@ -881,6 +892,10 @@ bool DawnTestBase::IsFullBackendValidationEnabled() const { return gTestEnv->GetBackendValidationLevel() == dawn::native::BackendValidationLevel::Full; } +bool DawnTestBase::IsCompatibilityMode() const { + return mParam.adapterProperties.compatibilityMode; +} + bool DawnTestBase::RunSuppressedTests() const { return gTestEnv->RunSuppressedTests(); } diff --git a/src/dawn/tests/DawnTest.h b/src/dawn/tests/DawnTest.h index ea5dc00e10..c10284e26b 100644 --- a/src/dawn/tests/DawnTest.h +++ b/src/dawn/tests/DawnTest.h @@ -258,6 +258,7 @@ class DawnTestBase { bool IsImplicitDeviceSyncEnabled() const; bool IsBackendValidationEnabled() const; bool IsFullBackendValidationEnabled() const; + bool IsCompatibilityMode() const; bool RunSuppressedTests() const; bool IsDXC() const; @@ -283,11 +284,14 @@ class DawnTestBase { struct PrintToStringParamName { explicit PrintToStringParamName(const char* test); - std::string SanitizeParamName(std::string paramName, size_t index) const; + std::string SanitizeParamName(std::string paramName, + const wgpu::AdapterProperties& properties, + size_t index) const; template std::string operator()(const ::testing::TestParamInfo& info) const { - return SanitizeParamName(::testing::PrintToStringParamName()(info), info.index); + return SanitizeParamName(::testing::PrintToStringParamName()(info), + info.param.adapterProperties, info.index); } std::string mTest; diff --git a/src/dawn/tests/end2end/TextureViewTests.cpp b/src/dawn/tests/end2end/TextureViewTests.cpp index ae52dd14a3..e2e918bca4 100644 --- a/src/dawn/tests/end2end/TextureViewTests.cpp +++ b/src/dawn/tests/end2end/TextureViewTests.cpp @@ -319,7 +319,7 @@ class TextureViewSamplingTest : public DawnTest { uint32_t textureViewLayerCount, bool isCubeMapArray) { // TODO(crbug.com/dawn/1300): OpenGLES does not support cube map arrays. - DAWN_TEST_UNSUPPORTED_IF(isCubeMapArray && IsOpenGLES()); + DAWN_TEST_UNSUPPORTED_IF(isCubeMapArray && IsCompatibilityMode()); constexpr uint32_t kMipLevels = 1u; InitTexture(textureArrayLayers, kMipLevels); diff --git a/src/dawn/tests/unittests/validation/CompatValidationTests.cpp b/src/dawn/tests/unittests/validation/CompatValidationTests.cpp new file mode 100644 index 0000000000..8990236fc4 --- /dev/null +++ b/src/dawn/tests/unittests/validation/CompatValidationTests.cpp @@ -0,0 +1,58 @@ +// 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/tests/unittests/validation/ValidationTest.h" +#include "dawn/utils/ComboRenderPipelineDescriptor.h" +#include "dawn/utils/WGPUHelpers.h" + +namespace dawn { +namespace { + +class CompatValidationTest : public ValidationTest { + protected: + bool UseCompatibilityMode() const override { return true; } +}; + +TEST_F(CompatValidationTest, CanNotCreateCubeArrayTextureView) { + wgpu::TextureDescriptor descriptor; + descriptor.size = {1, 1, 6}; + descriptor.dimension = wgpu::TextureDimension::e2D; + descriptor.format = wgpu::TextureFormat::RGBA8Unorm; + descriptor.usage = wgpu::TextureUsage::TextureBinding; + wgpu::Texture cubeTexture = device.CreateTexture(&descriptor); + + { + wgpu::TextureViewDescriptor cubeViewDescriptor; + cubeViewDescriptor.dimension = wgpu::TextureViewDimension::Cube; + cubeViewDescriptor.format = wgpu::TextureFormat::RGBA8Unorm; + + cubeTexture.CreateView(&cubeViewDescriptor); + } + + { + wgpu::TextureViewDescriptor cubeArrayViewDescriptor; + cubeArrayViewDescriptor.dimension = wgpu::TextureViewDimension::CubeArray; + cubeArrayViewDescriptor.format = wgpu::TextureFormat::RGBA8Unorm; + + ASSERT_DEVICE_ERROR(cubeTexture.CreateView(&cubeArrayViewDescriptor)); + } + + cubeTexture.Destroy(); +} + +} // anonymous namespace +} // namespace dawn diff --git a/src/dawn/tests/unittests/validation/ValidationTest.cpp b/src/dawn/tests/unittests/validation/ValidationTest.cpp index d3b4ba538f..310021eba7 100644 --- a/src/dawn/tests/unittests/validation/ValidationTest.cpp +++ b/src/dawn/tests/unittests/validation/ValidationTest.cpp @@ -99,10 +99,12 @@ ValidationTest::ValidationTest() { wgpu::AdapterProperties adapterProperties; adapter.GetProperties(&adapterProperties); - if (adapterProperties.backendType == wgpu::BackendType::Null) { + if (adapterProperties.backendType == wgpu::BackendType::Null && + adapterProperties.compatibilityMode == gCurrentTest->UseCompatibilityMode()) { gCurrentTest->mBackendAdapter = adapter; WGPUAdapter cAdapter = adapter.Get(); ASSERT(cAdapter); + dawn::native::GetProcs().adapterReference(cAdapter); callback(WGPURequestAdapterStatus_Success, cAdapter, nullptr, userdata); return; @@ -153,9 +155,8 @@ void ValidationTest::SetUp() { // RequestAdapter is overriden to ignore RequestAdapterOptions and always select the null // adapter. - wgpu::RequestAdapterOptions options = {}; mInstance.RequestAdapter( - &options, + nullptr, [](WGPURequestAdapterStatus, WGPUAdapter cAdapter, const char*, void* userdata) { *static_cast(userdata) = wgpu::Adapter::Acquire(cAdapter); }, @@ -311,6 +312,10 @@ WGPUDevice ValidationTest::CreateTestDevice(dawn::native::Adapter dawnAdapter, return dawnAdapter.CreateDevice(&deviceDescriptor); } +bool ValidationTest::UseCompatibilityMode() const { + return false; +} + // static void ValidationTest::OnDeviceError(WGPUErrorType type, const char* message, void* userdata) { ASSERT(type != WGPUErrorType_NoError); diff --git a/src/dawn/tests/unittests/validation/ValidationTest.h b/src/dawn/tests/unittests/validation/ValidationTest.h index 7088755cc3..318e833d52 100644 --- a/src/dawn/tests/unittests/validation/ValidationTest.h +++ b/src/dawn/tests/unittests/validation/ValidationTest.h @@ -140,6 +140,8 @@ class ValidationTest : public testing::Test { wgpu::Device RequestDeviceSync(const wgpu::DeviceDescriptor& deviceDesc); + virtual bool UseCompatibilityMode() const; + wgpu::Device device; wgpu::Adapter adapter; WGPUDevice backendDevice;