From 22bd21ef74f534149bbb6bc46724018bee0d32e0 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Thu, 13 Jan 2022 10:53:36 +0000 Subject: [PATCH] Add support for creating 1D textures. However there are a lot more things to implement for full support: - Copies from/to buffers. - Copies from/to other textures. - WriteTexture - Lazy initialization (if needed) - Anything using views and 1D textures in shaders So they are currently marked as unsafe API. Bug: dawn:814 Change-Id: I3f1aac87bd5bc27f710d58e525938c1226d093d8 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/64542 Reviewed-by: Austin Eng Commit-Queue: Corentin Wallez Auto-Submit: Corentin Wallez --- src/dawn_native/BindGroupLayout.cpp | 3 + src/dawn_native/CommandValidation.cpp | 7 ++ src/dawn_native/Texture.cpp | 56 +++++++---- src/dawn_native/d3d12/TextureD3D12.cpp | 5 +- src/dawn_native/metal/TextureMTL.mm | 15 ++- src/dawn_native/vulkan/TextureVk.cpp | 9 +- .../validation/TextureValidationTests.cpp | 94 ++++++++++++++++++- .../validation/UnsafeAPIValidationTests.cpp | 17 ++++ 8 files changed, 172 insertions(+), 34 deletions(-) diff --git a/src/dawn_native/BindGroupLayout.cpp b/src/dawn_native/BindGroupLayout.cpp index cea3a067f2..8cd4a4d64f 100644 --- a/src/dawn_native/BindGroupLayout.cpp +++ b/src/dawn_native/BindGroupLayout.cpp @@ -90,11 +90,13 @@ namespace dawn::native { allowedStages &= ~wgpu::ShaderStage::Vertex; } } + if (entry.sampler.type != wgpu::SamplerBindingType::Undefined) { bindingMemberCount++; bindingType = BindingInfoType::Sampler; DAWN_TRY(ValidateSamplerBindingType(entry.sampler.type)); } + if (entry.texture.sampleType != wgpu::TextureSampleType::Undefined) { bindingMemberCount++; bindingType = BindingInfoType::Texture; @@ -113,6 +115,7 @@ namespace dawn::native { "View dimension (%s) for a multisampled texture bindings was not %s.", viewDimension, wgpu::TextureViewDimension::e2D); } + if (entry.storageTexture.access != wgpu::StorageTextureAccess::Undefined) { bindingMemberCount++; bindingType = BindingInfoType::StorageTexture; diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp index e7a0feb905..1e307e59b3 100644 --- a/src/dawn_native/CommandValidation.cpp +++ b/src/dawn_native/CommandValidation.cpp @@ -267,6 +267,13 @@ namespace dawn::native { const Extent3D& copySize) { const TextureBase* texture = textureCopy.texture; DAWN_TRY(device->ValidateObject(texture)); + + // TODO(https://crbug.com/dawn/814): Disallow 1D texture copies until they are implemented. + DAWN_INVALID_IF(texture->GetDimension() == wgpu::TextureDimension::e1D, + "%s is used in a copy and has dimension %s. Copies with %s aren't " + "implemented (yet) and are disallowed. See https://crbug.com/dawn/814.", + texture, wgpu::TextureDimension::e1D, wgpu::TextureDimension::e1D); + DAWN_INVALID_IF(textureCopy.mipLevel >= texture->GetNumMipLevels(), "MipLevel (%u) is greater than the number of mip levels (%u) in %s.", textureCopy.mipLevel, texture->GetNumMipLevels(), texture); diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp index 092f7a1357..999d550024 100644 --- a/src/dawn_native/Texture.cpp +++ b/src/dawn_native/Texture.cpp @@ -181,6 +181,9 @@ namespace dawn::native { const CombinedLimits& limits = device->GetLimits(); Extent3D maxExtent; switch (descriptor->dimension) { + case wgpu::TextureDimension::e1D: + maxExtent = {limits.v1.maxTextureDimension1D, 1, 1}; + break; case wgpu::TextureDimension::e2D: maxExtent = {limits.v1.maxTextureDimension2D, limits.v1.maxTextureDimension2D, limits.v1.maxTextureArrayLayers}; @@ -189,9 +192,6 @@ namespace dawn::native { maxExtent = {limits.v1.maxTextureDimension3D, limits.v1.maxTextureDimension3D, limits.v1.maxTextureDimension3D}; break; - case wgpu::TextureDimension::e1D: - default: - UNREACHABLE(); } DAWN_INVALID_IF(descriptor->size.width > maxExtent.width || descriptor->size.height > maxExtent.height || @@ -199,18 +199,33 @@ namespace dawn::native { "Texture size (%s) exceeded maximum texture size (%s).", &descriptor->size, &maxExtent); - uint32_t maxMippedDimension = descriptor->size.width; - if (descriptor->dimension != wgpu::TextureDimension::e1D) { - maxMippedDimension = std::max(maxMippedDimension, descriptor->size.height); + switch (descriptor->dimension) { + case wgpu::TextureDimension::e1D: + DAWN_INVALID_IF( + descriptor->mipLevelCount != 1, + "Texture mip level count (%u) is more than 1 when its dimension is %s.", + descriptor->mipLevelCount, wgpu::TextureDimension::e1D); + break; + case wgpu::TextureDimension::e2D: { + uint32_t maxMippedDimension = + std::max(descriptor->size.width, descriptor->size.height); + DAWN_INVALID_IF( + Log2(maxMippedDimension) + 1 < descriptor->mipLevelCount, + "Texture mip level count (%u) exceeds the maximum (%u) for its size (%s).", + descriptor->mipLevelCount, Log2(maxMippedDimension) + 1, &descriptor->size); + break; + } + case wgpu::TextureDimension::e3D: { + uint32_t maxMippedDimension = std::max( + descriptor->size.width, + std::max(descriptor->size.height, descriptor->size.depthOrArrayLayers)); + DAWN_INVALID_IF( + Log2(maxMippedDimension) + 1 < descriptor->mipLevelCount, + "Texture mip level count (%u) exceeds the maximum (%u) for its size (%s).", + descriptor->mipLevelCount, Log2(maxMippedDimension) + 1, &descriptor->size); + break; + } } - if (descriptor->dimension == wgpu::TextureDimension::e3D) { - maxMippedDimension = - std::max(maxMippedDimension, descriptor->size.depthOrArrayLayers); - } - DAWN_INVALID_IF( - Log2(maxMippedDimension) + 1 < descriptor->mipLevelCount, - "Texture mip level count (%u) exceeds the maximum (%u) for its size (%s).", - descriptor->mipLevelCount, Log2(maxMippedDimension) + 1, &descriptor->size); if (format->isCompressed) { const TexelBlockInfo& blockInfo = @@ -271,13 +286,18 @@ namespace dawn::native { const DawnTextureInternalUsageDescriptor* internalUsageDesc = nullptr; FindInChain(descriptor->nextInChain, &internalUsageDesc); - DAWN_INVALID_IF(descriptor->dimension == wgpu::TextureDimension::e1D, - "1D textures aren't supported (yet)."); - DAWN_INVALID_IF( internalUsageDesc != nullptr && !device->IsFeatureEnabled(Feature::DawnInternalUsages), "The dawn-internal-usages feature is not enabled"); + // Support for 1D textures is not complete so they are currently unsafe to use. + DAWN_INVALID_IF( + device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs) && + descriptor->dimension == wgpu::TextureDimension::e1D, + "Texture with dimension %s are disallowed because they are partially implemented. See " + "https://crbug.com/dawn/814", + wgpu::TextureDimension::e1D); + const Format* format; DAWN_TRY_ASSIGN(format, device->GetInternalFormat(descriptor->format)); @@ -526,8 +546,6 @@ namespace dawn::native { } uint32_t TextureBase::GetArrayLayers() const { ASSERT(!IsError()); - // TODO(crbug.com/dawn/814): Update for 1D textures when they are supported. - ASSERT(mDimension != wgpu::TextureDimension::e1D); if (mDimension == wgpu::TextureDimension::e3D) { return 1; } diff --git a/src/dawn_native/d3d12/TextureD3D12.cpp b/src/dawn_native/d3d12/TextureD3D12.cpp index 85ceabc13b..d958daf830 100644 --- a/src/dawn_native/d3d12/TextureD3D12.cpp +++ b/src/dawn_native/d3d12/TextureD3D12.cpp @@ -101,13 +101,12 @@ namespace dawn::native::d3d12 { D3D12_RESOURCE_DIMENSION D3D12TextureDimension(wgpu::TextureDimension dimension) { switch (dimension) { + case wgpu::TextureDimension::e1D: + return D3D12_RESOURCE_DIMENSION_TEXTURE1D; case wgpu::TextureDimension::e2D: return D3D12_RESOURCE_DIMENSION_TEXTURE2D; case wgpu::TextureDimension::e3D: return D3D12_RESOURCE_DIMENSION_TEXTURE3D; - - case wgpu::TextureDimension::e1D: - UNREACHABLE(); } } diff --git a/src/dawn_native/metal/TextureMTL.mm b/src/dawn_native/metal/TextureMTL.mm index a3e9745e6e..5230423f13 100644 --- a/src/dawn_native/metal/TextureMTL.mm +++ b/src/dawn_native/metal/TextureMTL.mm @@ -423,7 +423,6 @@ namespace dawn::native::metal { MTLTextureDescriptor* mtlDesc = mtlDescRef.Get(); mtlDesc.width = GetWidth(); - mtlDesc.height = GetHeight(); mtlDesc.sampleCount = GetSampleCount(); // Metal only allows format reinterpretation to happen on swizzle pattern or conversion // between linear space and sRGB. For example, creating bgra8Unorm texture view on @@ -438,9 +437,17 @@ namespace dawn::native::metal { // Choose the correct MTLTextureType and paper over differences in how the array layer count // is specified. switch (GetDimension()) { - case wgpu::TextureDimension::e2D: + case wgpu::TextureDimension::e1D: + mtlDesc.arrayLength = 1; mtlDesc.depth = 1; + ASSERT(mtlDesc.sampleCount == 1); + mtlDesc.textureType = MTLTextureType1D; + break; + + case wgpu::TextureDimension::e2D: + mtlDesc.height = GetHeight(); mtlDesc.arrayLength = GetArrayLayers(); + mtlDesc.depth = 1; if (mtlDesc.arrayLength > 1) { ASSERT(mtlDesc.sampleCount == 1); mtlDesc.textureType = MTLTextureType2DArray; @@ -451,14 +458,12 @@ namespace dawn::native::metal { } break; case wgpu::TextureDimension::e3D: + mtlDesc.height = GetHeight(); mtlDesc.depth = GetDepth(); mtlDesc.arrayLength = 1; ASSERT(mtlDesc.sampleCount == 1); mtlDesc.textureType = MTLTextureType3D; break; - - case wgpu::TextureDimension::e1D: - UNREACHABLE(); } return mtlDescRef; diff --git a/src/dawn_native/vulkan/TextureVk.cpp b/src/dawn_native/vulkan/TextureVk.cpp index 83982e8e11..8d08f9e1aa 100644 --- a/src/dawn_native/vulkan/TextureVk.cpp +++ b/src/dawn_native/vulkan/TextureVk.cpp @@ -196,6 +196,12 @@ namespace dawn::native::vulkan { // Fill in the image type, and paper over differences in how the array layer count is // specified between WebGPU and Vulkan. switch (texture.GetDimension()) { + case wgpu::TextureDimension::e1D: + info->imageType = VK_IMAGE_TYPE_1D; + info->extent = {size.width, 1, 1}; + info->arrayLayers = 1; + break; + case wgpu::TextureDimension::e2D: info->imageType = VK_IMAGE_TYPE_2D; info->extent = {size.width, size.height, 1}; @@ -207,9 +213,6 @@ namespace dawn::native::vulkan { info->extent = {size.width, size.height, size.depthOrArrayLayers}; info->arrayLayers = 1; break; - - case wgpu::TextureDimension::e1D: - UNREACHABLE(); } } diff --git a/src/tests/unittests/validation/TextureValidationTests.cpp b/src/tests/unittests/validation/TextureValidationTests.cpp index 1835cd5206..e81cd0b1ca 100644 --- a/src/tests/unittests/validation/TextureValidationTests.cpp +++ b/src/tests/unittests/validation/TextureValidationTests.cpp @@ -291,6 +291,22 @@ namespace { ASSERT_DEVICE_ERROR(device.CreateTexture(&descriptor)); } + + // 1D textures can only have a single mip level. + { + wgpu::TextureDescriptor descriptor = defaultDescriptor; + descriptor.dimension = wgpu::TextureDimension::e1D; + descriptor.size.width = 32; + descriptor.size.height = 1; + + // Having a single mip level is allowed. + descriptor.mipLevelCount = 1; + device.CreateTexture(&descriptor); + + // Having more than 1 is an error. + descriptor.mipLevelCount = 2; + ASSERT_DEVICE_ERROR(device.CreateTexture(&descriptor)); + } } // Test the validation of array layer count @@ -321,6 +337,51 @@ namespace { } } + // Test the validation of 1D texture size + TEST_F(TextureValidationTest, 1DTextureSize) { + wgpu::Limits supportedLimits = GetSupportedLimits().limits; + + wgpu::TextureDescriptor defaultDescriptor; + defaultDescriptor.size = {4, 1, 1}; + defaultDescriptor.dimension = wgpu::TextureDimension::e1D; + defaultDescriptor.usage = wgpu::TextureUsage::CopySrc; + defaultDescriptor.format = wgpu::TextureFormat::RGBA8Unorm; + + // Width must be in [1, kMaxTextureDimension1D] + { + wgpu::TextureDescriptor desc = defaultDescriptor; + desc.size.width = 0; + ASSERT_DEVICE_ERROR(device.CreateTexture(&desc)); + desc.size.width = 1; + device.CreateTexture(&desc); + + desc.size.width = supportedLimits.maxTextureDimension1D; + device.CreateTexture(&desc); + desc.size.width = supportedLimits.maxTextureDimension1D + 1u; + ASSERT_DEVICE_ERROR(device.CreateTexture(&desc)); + } + + // Height must be 1 + { + wgpu::TextureDescriptor desc = defaultDescriptor; + desc.size.height = 2; + ASSERT_DEVICE_ERROR(device.CreateTexture(&desc)); + + desc.size.height = 0; + ASSERT_DEVICE_ERROR(device.CreateTexture(&desc)); + } + + // DepthOrArrayLayers must be 1 + { + wgpu::TextureDescriptor desc = defaultDescriptor; + desc.size.depthOrArrayLayers = 2; + ASSERT_DEVICE_ERROR(device.CreateTexture(&desc)); + + desc.size.depthOrArrayLayers = 0; + ASSERT_DEVICE_ERROR(device.CreateTexture(&desc)); + } + } + // Test the validation of 2D texture size TEST_F(TextureValidationTest, 2DTextureSize) { wgpu::TextureDescriptor defaultDescriptor = CreateDefaultTextureDescriptor(); @@ -427,8 +488,8 @@ namespace { } } - // Test that depth/stencil formats are invalid for 3D texture - TEST_F(TextureValidationTest, DepthStencilFormatsFor3D) { + // Test that depth/stencil formats are invalid for 1D and 3D texture + TEST_F(TextureValidationTest, DepthStencilFormatsFor1DAnd3D) { wgpu::TextureDescriptor descriptor = CreateDefaultTextureDescriptor(); wgpu::TextureFormat depthStencilFormats[] = { @@ -638,8 +699,6 @@ namespace { } } - // TODO(jiawei.shao@intel.com): add tests to verify we cannot create 1D or 3D textures with - // compressed texture formats. class CompressedTextureFormatsValidationTests : public TextureValidationTest { protected: WGPUDevice CreateTestDevice() override { @@ -649,6 +708,15 @@ namespace { wgpu::FeatureName::TextureCompressionASTC}; descriptor.requiredFeatures = requiredFeatures; descriptor.requiredFeaturesCount = 3; + + // TODO(dawn:814): Remove when 1D texture support is complete. + const char* kDisallowUnsafeApis = "disallow_unsafe_apis"; + wgpu::DawnTogglesDeviceDescriptor togglesDesc; + togglesDesc.forceDisabledToggles = &kDisallowUnsafeApis; + togglesDesc.forceDisabledTogglesCount = 1; + + descriptor.nextInChain = &togglesDesc; + return adapter.CreateDevice(&descriptor); } @@ -718,6 +786,24 @@ namespace { } } + // Test that it is not allowed to create a 1D texture in compressed formats. + TEST_F(CompressedTextureFormatsValidationTests, 1DTexture) { + for (wgpu::TextureFormat format : utils::kCompressedFormats) { + wgpu::TextureDescriptor descriptor = CreateDefaultTextureDescriptor(); + descriptor.format = format; + // Unfortunately we can't use the block height here otherwise validation for the max + // texture 1D size will trigger. We check the error message below to make sure the + // correct code path is covered. + descriptor.size.height = 1; + descriptor.size.depthOrArrayLayers = 1; + descriptor.dimension = wgpu::TextureDimension::e1D; + ASSERT_DEVICE_ERROR( + device.CreateTexture(&descriptor), + testing::HasSubstr( + "The dimension (TextureDimension::e1D) of a texture with a compressed format")); + } + } + // Test that it is not allowed to create a 3D texture in compressed formats. TEST_F(CompressedTextureFormatsValidationTests, 3DTexture) { for (wgpu::TextureFormat format : utils::kCompressedFormats) { diff --git a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp index e1ab1ae1d6..b115a5d7b8 100644 --- a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp +++ b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp @@ -32,6 +32,23 @@ class UnsafeAPIValidationTest : public ValidationTest { } }; +// Check that 1D textures are disallowed as part of unsafe APIs. +// TODO(dawn:814): Remove when 1D texture support is complete. +TEST_F(UnsafeAPIValidationTest, 1DTextures) { + wgpu::TextureDescriptor desc; + desc.size = {1, 1, 1}; + desc.format = wgpu::TextureFormat::RGBA8Unorm; + desc.usage = wgpu::TextureUsage::CopyDst; + + // Control case: 2D textures are allowed. + desc.dimension = wgpu::TextureDimension::e2D; + device.CreateTexture(&desc); + + // Error case: 1D textures are disallowed. + desc.dimension = wgpu::TextureDimension::e1D; + ASSERT_DEVICE_ERROR(device.CreateTexture(&desc)); +} + // Check that pipeline overridable constants are disallowed as part of unsafe APIs. // TODO(dawn:1041) Remove when implementation for all backend is added TEST_F(UnsafeAPIValidationTest, PipelineOverridableConstants) {