From 4f98dd4706743f077a64b7ff4697d53f72c6498a Mon Sep 17 00:00:00 2001 From: Loko Kung Date: Wed, 8 Mar 2023 02:45:50 +0000 Subject: [PATCH] Add maxFragmentCombinedOutputResources and validation for it. - Suppresses maxStorageTexturesPerShaderStage CTS test because it needs to be modified to adhere to the new limit as well. Bug: dawn:1665 Change-Id: I66c62bd94b613059633888210ec7e7b42dc3a1dc Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/122461 Commit-Queue: Loko Kung Reviewed-by: Austin Eng Kokoro: Kokoro --- dawn.json | 3 +- src/dawn/native/Limits.cpp | 3 +- src/dawn/native/RenderPipeline.cpp | 30 +++++ src/dawn/tests/end2end/MaxLimitTests.cpp | 17 ++- .../RenderPipelineValidationTests.cpp | 114 ++++++++++++++++++ .../utils/ComboRenderPipelineDescriptor.cpp | 4 + .../utils/ComboRenderPipelineDescriptor.h | 1 + webgpu-cts/expectations.txt | 7 +- 8 files changed, 174 insertions(+), 5 deletions(-) diff --git a/dawn.json b/dawn.json index 20be7866fb..0d81e52297 100644 --- a/dawn.json +++ b/dawn.json @@ -1312,7 +1312,8 @@ {"name": "max compute workgroup size x", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max compute workgroup size y", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, {"name": "max compute workgroup size z", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, - {"name": "max compute workgroups per dimension", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"} + {"name": "max compute workgroups per dimension", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"}, + {"name": "max fragment combined output resources", "type": "uint32_t", "default": "WGPU_LIMIT_U32_UNDEFINED"} ] }, "required limits": { diff --git a/src/dawn/native/Limits.cpp b/src/dawn/native/Limits.cpp index 3e593aeb18..bbdf464c79 100644 --- a/src/dawn/native/Limits.cpp +++ b/src/dawn/native/Limits.cpp @@ -73,7 +73,8 @@ X(Maximum, maxInterStageShaderComponents, 60, 60) \ X(Maximum, maxInterStageShaderVariables, 16, 16) \ X(Maximum, maxColorAttachments, 8, 8) \ - X(Maximum, maxColorAttachmentBytesPerSample, 32, 32) + X(Maximum, maxColorAttachmentBytesPerSample, 32, 32) \ + X(Maximum, maxFragmentCombinedOutputResources, 8, 8) // clang-format on #define LIMITS_EACH_GROUP(X) \ diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp index e47a8d7a43..6f0915a381 100644 --- a/src/dawn/native/RenderPipeline.cpp +++ b/src/dawn/native/RenderPipeline.cpp @@ -364,6 +364,36 @@ MaybeError ValidateFragmentState(DeviceBase* device, const EntryPointMetadata& fragmentMetadata = descriptor->module->GetEntryPoint(descriptor->entryPoint); + // Iterates through the bindings on the fragment state to count the number of storage buffer and + // storage textures. + uint32_t maxFragmentCombinedOutputResources = + device->GetLimits().v1.maxFragmentCombinedOutputResources; + uint32_t fragmentCombinedOutputResources = 0; + for (uint32_t i = 0; i < descriptor->targetCount; i++) { + if (descriptor->targets[i].format != wgpu::TextureFormat::Undefined) { + fragmentCombinedOutputResources += 1; + } + } + for (BindGroupIndex group(0); group < fragmentMetadata.bindings.size(); ++group) { + for (const auto& [_, shaderBinding] : fragmentMetadata.bindings[group]) { + switch (shaderBinding.bindingType) { + case BindingInfoType::Buffer: + if (shaderBinding.buffer.type == wgpu::BufferBindingType::Storage) { + fragmentCombinedOutputResources += 1; + } + break; + case BindingInfoType::StorageTexture: + fragmentCombinedOutputResources += 1; + break; + default: + break; + } + } + } + DAWN_INVALID_IF(fragmentCombinedOutputResources > maxFragmentCombinedOutputResources, + "Number of fragment output resources (%u) exceeds the maximum (%u).", + fragmentCombinedOutputResources, maxFragmentCombinedOutputResources); + if (fragmentMetadata.usesFragDepth) { DAWN_INVALID_IF( depthStencil == nullptr, diff --git a/src/dawn/tests/end2end/MaxLimitTests.cpp b/src/dawn/tests/end2end/MaxLimitTests.cpp index 679dd93bc6..204f262f9e 100644 --- a/src/dawn/tests/end2end/MaxLimitTests.cpp +++ b/src/dawn/tests/end2end/MaxLimitTests.cpp @@ -548,7 +548,7 @@ TEST_P(MaxLimitTests, MaxBufferSizes) { EXPECT_LE(baseLimits.maxStorageBufferBindingSize, baseLimits.maxBufferSize); EXPECT_LE(baseLimits.maxUniformBufferBindingSize, baseLimits.maxBufferSize); - // Base limits eith tiering. + // Base limits with tiering. GetAdapter().SetUseTieredLimits(true); wgpu::Limits tieredLimits = GetAdapterLimits().limits; EXPECT_LE(tieredLimits.maxStorageBufferBindingSize, tieredLimits.maxBufferSize); @@ -558,6 +558,21 @@ TEST_P(MaxLimitTests, MaxBufferSizes) { GetAdapter().SetUseTieredLimits(false); } +// Verifies that supported fragment combined output resource limits meet base requirements. +TEST_P(MaxLimitTests, MaxFragmentCombinedOutputResources) { + // Base limits without tiering. + wgpu::Limits baseLimits = GetAdapterLimits().limits; + EXPECT_LE(baseLimits.maxColorAttachments, baseLimits.maxFragmentCombinedOutputResources); + + // Base limits with tiering. + GetAdapter().SetUseTieredLimits(true); + wgpu::Limits tieredLimits = GetAdapterLimits().limits; + EXPECT_LE(tieredLimits.maxColorAttachments, tieredLimits.maxFragmentCombinedOutputResources); + + // Unset tiered limit usage to avoid affecting other tests. + GetAdapter().SetUseTieredLimits(false); +} + DAWN_INSTANTIATE_TEST(MaxLimitTests, D3D12Backend(), MetalBackend(), diff --git a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp index 2aadaf71c3..8468b063ff 100644 --- a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -1420,6 +1420,118 @@ TEST_F(RenderPipelineValidationTest, BindingsFromCorrectEntryPoint) { ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor)); } +// Test that total fragment output resource validations must be less than limit. +TEST_F(RenderPipelineValidationTest, MaxFragmentCombinedOutputResources) { + static constexpr wgpu::TextureFormat kFormat = wgpu::TextureFormat::R8Unorm; + wgpu::ColorTargetState kEmptyColorTargetState = {}; + kEmptyColorTargetState.format = wgpu::TextureFormat::Undefined; + wgpu::ColorTargetState kColorTargetState = {}; + kColorTargetState.format = kFormat; + + // Creates a shader with the given number of output resources. + auto CreateShader = [&](uint32_t numBuffers, uint32_t numTextures) -> wgpu::ShaderModule { + // Header to declare storage buffer struct. + static constexpr std::string_view kHeader = "struct Buf { data : array }\n"; + std::ostringstream bufferBindings; + std::ostringstream bufferOutputs; + for (uint32_t i = 0; i < numBuffers; i++) { + bufferBindings << "@group(0) @binding(" << i << ") var b" << i + << ": Buf;\n"; + bufferOutputs << " b" << i << ".data[i] = i;\n"; + } + + std::ostringstream textureBindings; + std::ostringstream textureOutputs; + for (uint32_t i = 0; i < numTextures; i++) { + textureBindings << "@group(1) @binding(" << i << ") var t" << i + << ": texture_storage_1d;\n"; + textureOutputs << " textureStore(t" << i << ", i, vec4u(i));\n"; + } + + std::ostringstream targetBindings; + std::ostringstream targetOutputs; + for (size_t i = 0; i < kMaxColorAttachments; i++) { + targetBindings << "@location(" << i << ") o" << i << " : vec4f, "; + targetOutputs << "vec4f(1), "; + } + + std::ostringstream fsShader; + fsShader << kHeader; + fsShader << bufferBindings.str(); + fsShader << textureBindings.str(); + fsShader << "struct Outputs { " << targetBindings.str() << "}\n"; + fsShader << "@fragment fn main(@builtin(sample_index) i : u32) -> Outputs {\n"; + fsShader << bufferOutputs.str(); + fsShader << textureOutputs.str(); + fsShader << " return Outputs(" << targetOutputs.str() << ");\n"; + fsShader << "}"; + return utils::CreateShaderModule(device, fsShader.str().c_str()); + }; + + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = utils::CreateShaderModule(device, R"( + @vertex fn main() -> @builtin(position) vec4f { + return vec4f(0.0, 0.0, 0.0, 1.0); + })"); + descriptor.vertex.entryPoint = "main"; + descriptor.cFragment.targetCount = kMaxColorAttachments; + descriptor.cFragment.entryPoint = "main"; + + // Runs test using the given parameters. + auto DoTest = [&](uint32_t numBuffers, uint32_t numTextures, + const std::vector& attachmentIndices, bool useDepthStencil, + bool shouldError) { + descriptor.cFragment.module = CreateShader(numBuffers, numTextures); + descriptor.cTargets.fill(kEmptyColorTargetState); + for (const uint32_t attachmentIndex : attachmentIndices) { + descriptor.cTargets[attachmentIndex] = kColorTargetState; + } + + if (useDepthStencil) { + descriptor.EnableDepthStencil(); + } else { + descriptor.DisableDepthStencil(); + } + + if (shouldError) { + ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor)); + } else { + device.CreateRenderPipeline(&descriptor); + } + }; + + // All following tests assume we are using the default limits for the following: + // - maxStorageBuffersPerShaderStage = 8 + // - maxStorageTexturesPerShaderStage = 4 + // - maxColorAttachments = 8 + // - maxFragmentCombinedOutputResources = 8 + // Note we use the defaults for the validation tests because otherwise it is hard to verify that + // we are hitting the maxFragmentCombinedOutputResources limit validations versus potentially + // hitting other validation errors from the other limits. + + // One of each resource with and without depth-stencil should pass. (Control) + DoTest(1, 1, {0}, false, false); + DoTest(1, 1, {0}, true, false); + + // Max number of any single resource within limits should pass. Note we turn on depth stencil in + // some of these cases because empty attachments are not allowed. + DoTest(8, 0, {}, true, false); + DoTest(4, 4, {}, true, false); + DoTest(0, 4, {0, 1, 2, 3}, false, false); + DoTest(0, 0, {0, 1, 2, 3, 4, 5, 6, 7}, false, false); + DoTest(0, 0, {0, 1, 2, 3, 4, 5, 6, 7}, true, false); + + // Max number of resources with different combinations should also pass. + DoTest(3, 2, {0, 3, 5}, false, false); + DoTest(2, 3, {2, 4, 6}, true, false); + DoTest(3, 3, {1, 7}, true, false); + + // Max number of resources + 1 should fail. + DoTest(3, 3, {0, 3, 5}, false, true); + DoTest(3, 3, {2, 4, 6}, true, true); + DoTest(3, 3, {1, 5, 7}, true, true); +} + // Tests validation for per-pixel accounting for render targets. The tests currently assume that the // default maxColorAttachmentBytesPerSample limit of 32 is used. TEST_P(DeprecationTests, RenderPipelineColorAttachmentBytesPerSample) { @@ -1504,7 +1616,9 @@ TEST_P(DeprecationTests, RenderPipelineColorAttachmentBytesPerSample) { @vertex fn main() -> @builtin(position) vec4f { return vec4f(0.0, 0.0, 0.0, 1.0); })"); + descriptor.vertex.entryPoint = "main"; descriptor.cFragment.module = CreateShader(testCase.formats); + descriptor.cFragment.entryPoint = "main"; descriptor.cFragment.targetCount = testCase.formats.size(); for (size_t i = 0; i < testCase.formats.size(); i++) { descriptor.cTargets[i].format = testCase.formats.at(i); diff --git a/src/dawn/utils/ComboRenderPipelineDescriptor.cpp b/src/dawn/utils/ComboRenderPipelineDescriptor.cpp index 78afd80965..5f20cc0456 100644 --- a/src/dawn/utils/ComboRenderPipelineDescriptor.cpp +++ b/src/dawn/utils/ComboRenderPipelineDescriptor.cpp @@ -142,4 +142,8 @@ wgpu::DepthStencilState* ComboRenderPipelineDescriptor::EnableDepthStencil( return &cDepthStencil; } +void ComboRenderPipelineDescriptor::DisableDepthStencil() { + this->depthStencil = nullptr; +} + } // namespace utils diff --git a/src/dawn/utils/ComboRenderPipelineDescriptor.h b/src/dawn/utils/ComboRenderPipelineDescriptor.h index b1e5c2a660..70a2b9ca63 100644 --- a/src/dawn/utils/ComboRenderPipelineDescriptor.h +++ b/src/dawn/utils/ComboRenderPipelineDescriptor.h @@ -48,6 +48,7 @@ class ComboRenderPipelineDescriptor : public wgpu::RenderPipelineDescriptor { wgpu::DepthStencilState* EnableDepthStencil( wgpu::TextureFormat format = wgpu::TextureFormat::Depth24PlusStencil8); + void DisableDepthStencil(); std::array cBuffers; std::array cAttributes; diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt index 83a419b195..bc49f660ca 100644 --- a/webgpu-cts/expectations.txt +++ b/webgpu-cts/expectations.txt @@ -484,6 +484,11 @@ crbug.com/1410936 [ monterey ] webgpu:web_platform,canvas,configure:viewFormats: crbug.com/1410936 [ monterey ] webgpu:web_platform,canvas,configure:viewFormats:canvasType="onscreen";format="rgba8unorm";viewFormatFeature="_undef_" [ Failure ] crbug.com/1410936 [ monterey ] webgpu:web_platform,canvas,getCurrentTexture:* [ Failure ] +################################################################################ +# New maxFragmentCombinedOutputResources limit breaking dependent limit tests. +################################################################################ +crbug.com/dawn/1665 webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createPipeline,at_over:limitTest="atMaximum" [ Failure ] + ################################################################################ # untriaged failures ################################################################################ @@ -507,8 +512,6 @@ crbug.com/dawn/0000 [ dawn-no-backend-validation nvidia-0x2184 ubuntu ] webgpu:a crbug.com/dawn/0000 [ dawn-no-backend-validation nvidia-0x2184 ubuntu ] webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createBindGroupLayout,at_over:limitTest="underDefault" [ RetryOnFailure ] crbug.com/dawn/0000 [ dawn-backend-validation nvidia-0x2184 ubuntu ] webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createPipeline,at_over:limitTest="atDefault" [ RetryOnFailure ] crbug.com/dawn/0000 [ dawn-no-backend-validation nvidia-0x2184 ubuntu ] webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createPipeline,at_over:limitTest="atDefault" [ RetryOnFailure ] -crbug.com/dawn/0000 [ dawn-backend-validation nvidia-0x2184 ubuntu ] webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createPipeline,at_over:limitTest="atMaximum" [ RetryOnFailure ] -crbug.com/dawn/0000 [ dawn-no-backend-validation nvidia-0x2184 ubuntu ] webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createPipeline,at_over:limitTest="atMaximum" [ RetryOnFailure ] crbug.com/dawn/0000 [ dawn-backend-validation nvidia-0x2184 ubuntu ] webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createPipeline,at_over:limitTest="underDefault" [ RetryOnFailure ] crbug.com/dawn/0000 [ dawn-no-backend-validation nvidia-0x2184 ubuntu ] webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createPipelineLayout,at_over:limitTest="atDefault" [ RetryOnFailure ] crbug.com/dawn/0000 [ dawn-backend-validation nvidia-0x2184 ubuntu ] webgpu:api,validation,capability_checks,limits,maxStorageTexturesPerShaderStage:createPipelineLayout,at_over:limitTest="atMaximum" [ RetryOnFailure ]