From 4a63612cd2afd94ee57c9082145671531e66a37a Mon Sep 17 00:00:00 2001 From: shrekshao Date: Thu, 16 Jun 2022 00:11:20 +0000 Subject: [PATCH] Fix alphaToCoverageEnabled and sample_mask output validation Bug: tint:506 Change-Id: Ie2cf5b7f446d89c92491068b56027b13807cb807 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93700 Kokoro: Kokoro Reviewed-by: Austin Eng Commit-Queue: Shrek Shao --- src/dawn/native/RenderPipeline.cpp | 11 +++- src/dawn/native/ShaderModule.cpp | 1 + src/dawn/native/ShaderModule.h | 2 + .../RenderPipelineValidationTests.cpp | 50 +++++++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp index 1830b10ca8..364c645b38 100644 --- a/src/dawn/native/RenderPipeline.cpp +++ b/src/dawn/native/RenderPipeline.cpp @@ -326,7 +326,8 @@ MaybeError ValidateColorTargetState( MaybeError ValidateFragmentState(DeviceBase* device, const FragmentState* descriptor, - const PipelineLayoutBase* layout) { + const PipelineLayoutBase* layout, + bool alphaToCoverageEnabled) { DAWN_INVALID_IF(descriptor->nextInChain != nullptr, "nextInChain must be nullptr."); DAWN_TRY_CONTEXT(ValidateProgrammableStage(device, descriptor->module, descriptor->entryPoint, @@ -357,6 +358,11 @@ MaybeError ValidateFragmentState(DeviceBase* device, } } + DAWN_INVALID_IF(fragmentMetadata.usesSampleMaskOutput && alphaToCoverageEnabled, + "alphaToCoverageEnabled is true when the sample_mask builtin is a " + "pipeline output of fragment stage of %s.", + descriptor->module); + return {}; } @@ -447,7 +453,8 @@ MaybeError ValidateRenderPipelineDescriptor(DeviceBase* device, "validating multisample state."); if (descriptor->fragment != nullptr) { - DAWN_TRY_CONTEXT(ValidateFragmentState(device, descriptor->fragment, descriptor->layout), + DAWN_TRY_CONTEXT(ValidateFragmentState(device, descriptor->fragment, descriptor->layout, + descriptor->multisample.alphaToCoverageEnabled), "validating fragment state."); DAWN_INVALID_IF(descriptor->fragment->targetCount == 0 && !descriptor->depthStencil, diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp index a1e96ac269..206869f407 100644 --- a/src/dawn/native/ShaderModule.cpp +++ b/src/dawn/native/ShaderModule.cpp @@ -773,6 +773,7 @@ ResultOrError> ReflectEntryPointUsingTint( if (entryPoint.input_sample_mask_used) { totalInterStageShaderComponents += 1; } + metadata->usesSampleMaskOutput = entryPoint.output_sample_mask_used; if (entryPoint.sample_index_used) { totalInterStageShaderComponents += 1; } diff --git a/src/dawn/native/ShaderModule.h b/src/dawn/native/ShaderModule.h index 1df775cfba..a491a2141b 100644 --- a/src/dawn/native/ShaderModule.h +++ b/src/dawn/native/ShaderModule.h @@ -249,6 +249,8 @@ struct EntryPointMetadata { std::unordered_set initializedOverridableConstants; bool usesNumWorkgroups = false; + // Used at render pipeline validation. + bool usesSampleMaskOutput = false; }; class ShaderModuleBase : public ApiObjectBase, public CachedObject { diff --git a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp index bc47381c34..4e265a2797 100644 --- a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -768,6 +768,56 @@ TEST_F(RenderPipelineValidationTest, AlphaToCoverageAndSampleCount) { } } +// Tests if the sample_mask builtin is a pipeline output of fragment shader, +// then alphaToCoverageEnabled must be false +TEST_F(RenderPipelineValidationTest, AlphaToCoverageAndSampleMaskOutput) { + wgpu::ShaderModule fsModuleSampleMaskOutput = utils::CreateShaderModule(device, R"( + struct Output { + @builtin(sample_mask) mask_out: u32, + @location(0) color : vec4, + } + @fragment fn main() -> Output { + var o: Output; + // We need to make sure this sample_mask isn't optimized out even its value equals "no op". + o.mask_out = 0xFFFFFFFFu; + o.color = vec4(1.0, 1.0, 1.0, 1.0); + return o; + } + )"); + + { + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vsModule; + descriptor.cFragment.module = fsModuleSampleMaskOutput; + descriptor.multisample.count = 4; + descriptor.multisample.alphaToCoverageEnabled = false; + + device.CreateRenderPipeline(&descriptor); + } + + { + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vsModule; + descriptor.cFragment.module = fsModuleSampleMaskOutput; + descriptor.multisample.count = 4; + descriptor.multisample.alphaToCoverageEnabled = true; + + ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor)); + } + + { + // Control cases: when fragment has no sample_mask output, it's good to have + // alphaToCoverageEnabled enabled + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = vsModule; + descriptor.cFragment.module = fsModule; + descriptor.multisample.count = 4; + descriptor.multisample.alphaToCoverageEnabled = true; + + device.CreateRenderPipeline(&descriptor); + } +} + // Tests that the texture component type in shader must match the bind group layout. TEST_F(RenderPipelineValidationTest, TextureComponentTypeCompatibility) { constexpr uint32_t kNumTextureComponentType = 3u;