diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp index 64d8f4123e2..71e7d3f167b 100644 --- a/src/dawn/native/RenderPipeline.cpp +++ b/src/dawn/native/RenderPipeline.cpp @@ -731,6 +731,16 @@ ResultOrError ValidateFragmentState(DeviceBase* device, } if (device->IsCompatibilityMode()) { + DAWN_INVALID_IF( + fragmentMetadata.usesSampleMaskOutput, + "sample_mask is not supported in compatibility mode in the fragment stage (%s, %s)", + descriptor->module, &entryPoint); + + DAWN_INVALID_IF( + fragmentMetadata.usesSampleIndex, + "sample_index is not supported in compatibility mode in the fragment stage (%s, %s)", + descriptor->module, &entryPoint); + // Check that all the color target states match. ColorAttachmentIndex firstColorTargetIndex{}; const ColorTargetState* firstColorTargetState = nullptr; diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp index 69e3a64c055..4996e109398 100644 --- a/src/dawn/native/ShaderModule.cpp +++ b/src/dawn/native/ShaderModule.cpp @@ -776,6 +776,7 @@ ResultOrError> ReflectEntryPointUsingTint( totalInterStageShaderComponents += 1; } metadata->usesSampleMaskOutput = entryPoint.output_sample_mask_used; + metadata->usesSampleIndex = entryPoint.sample_index_used; if (entryPoint.sample_index_used) { totalInterStageShaderComponents += 1; } diff --git a/src/dawn/native/ShaderModule.h b/src/dawn/native/ShaderModule.h index 6dff0121efa..f62158f21ff 100644 --- a/src/dawn/native/ShaderModule.h +++ b/src/dawn/native/ShaderModule.h @@ -277,6 +277,7 @@ struct EntryPointMetadata { bool usesInstanceIndex = false; bool usesNumWorkgroups = false; bool usesSampleMaskOutput = false; + bool usesSampleIndex = false; bool usesVertexIndex = false; }; diff --git a/src/dawn/tests/unittests/validation/CompatValidationTests.cpp b/src/dawn/tests/unittests/validation/CompatValidationTests.cpp index 9280c2b1fac..ac6ed9ebcd2 100644 --- a/src/dawn/tests/unittests/validation/CompatValidationTests.cpp +++ b/src/dawn/tests/unittests/validation/CompatValidationTests.cpp @@ -213,24 +213,100 @@ TEST_F(CompatValidationTest, CanNotCreatePipelineWithNonZeroDepthBiasClamp) { ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&testDescriptor)); } -TEST_F(CompatValidationTest, CanNotUseSampleMask) { - auto wgsl = R"( +TEST_F(CompatValidationTest, CanNotUseFragmentShaderWithSampleMask) { + wgpu::ShaderModule moduleSampleMaskOutput = utils::CreateShaderModule(device, R"( + @vertex fn vs() -> @builtin(position) vec4f { + return vec4f(1); + } struct Output { @builtin(sample_mask) mask_out: u32, @location(0) color : vec4f, } - )"; - ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, wgsl), // - testing::HasSubstr("sample_mask")); + @fragment fn fsWithoutSampleMaskUsage() -> @location(0) vec4f { + return vec4f(1.0, 1.0, 1.0, 1.0); + } + @fragment fn fsWithSampleMaskUsage() -> 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 = vec4f(1.0, 1.0, 1.0, 1.0); + return o; + } + )"); + + // Check we can use a fragment shader that doesn't use sample_mask from + // the same module as one that does. + { + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = moduleSampleMaskOutput; + descriptor.cFragment.module = moduleSampleMaskOutput; + descriptor.cFragment.entryPoint = "fsWithoutSampleMaskUsage"; + descriptor.multisample.count = 4; + descriptor.multisample.alphaToCoverageEnabled = false; + + device.CreateRenderPipeline(&descriptor); + } + + // Check we can not use a fragment shader that uses sample_mask. + { + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = moduleSampleMaskOutput; + descriptor.cFragment.module = moduleSampleMaskOutput; + descriptor.cFragment.entryPoint = "fsWithSampleMaskUsage"; + descriptor.multisample.count = 4; + descriptor.multisample.alphaToCoverageEnabled = false; + + ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor), + testing::HasSubstr("sample_mask")); + } } -TEST_F(CompatValidationTest, CanNotUseSampleIndex) { - auto wgsl = R"( - @fragment fn fsWithSampleIndexUsage(@builtin(sample_index) sNdx: u32) { +TEST_F(CompatValidationTest, CanNotUseFragmentShaderWithSampleIndex) { + wgpu::ShaderModule moduleSampleMaskOutput = utils::CreateShaderModule(device, R"( + @vertex fn vs() -> @builtin(position) vec4f { + return vec4f(1); } - )"; - ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, wgsl), - testing::HasSubstr("sample_index")); + struct Output { + @location(0) color : vec4f, + } + @fragment fn fsWithoutSampleIndexUsage() -> @location(0) vec4f { + return vec4f(1.0, 1.0, 1.0, 1.0); + } + @fragment fn fsWithSampleIndexUsage(@builtin(sample_index) sNdx: u32) -> Output { + var o: Output; + _ = sNdx; + o.color = vec4f(1.0, 1.0, 1.0, 1.0); + return o; + } + )"); + + // Check we can use a fragment shader that doesn't use sample_index from + // the same module as one that does. + { + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = moduleSampleMaskOutput; + descriptor.vertex.entryPoint = "vs"; + descriptor.cFragment.module = moduleSampleMaskOutput; + descriptor.cFragment.entryPoint = "fsWithoutSampleIndexUsage"; + descriptor.multisample.count = 4; + descriptor.multisample.alphaToCoverageEnabled = false; + + device.CreateRenderPipeline(&descriptor); + } + + // Check we can not use a fragment shader that uses sample_index. + { + utils::ComboRenderPipelineDescriptor descriptor; + descriptor.vertex.module = moduleSampleMaskOutput; + descriptor.vertex.entryPoint = "vs"; + descriptor.cFragment.module = moduleSampleMaskOutput; + descriptor.cFragment.entryPoint = "fsWithSampleIndexUsage"; + descriptor.multisample.count = 4; + descriptor.multisample.alphaToCoverageEnabled = false; + + ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor), + testing::HasSubstr("sample_index")); + } } TEST_F(CompatValidationTest, CanNotUseShaderWithUnsupportedInterpolateTypeOrSampling) { diff --git a/src/tint/lang/wgsl/resolver/compatibility_mode_test.cc b/src/tint/lang/wgsl/resolver/compatibility_mode_test.cc index 1fa085af3cd..c9803346d57 100644 --- a/src/tint/lang/wgsl/resolver/compatibility_mode_test.cc +++ b/src/tint/lang/wgsl/resolver/compatibility_mode_test.cc @@ -95,140 +95,6 @@ INSTANTIATE_TEST_SUITE_P(ResolverCompatibilityModeTest, core::TexelFormat::kRg32Sint, core::TexelFormat::kRg32Uint))); -TEST_F(ResolverCompatibilityModeTest, SampleMask_Parameter) { - // @fragment - // fn main(@builtin(sample_mask) mask : u32) { - // } - - Func("main", - Vector{Param( - "mask", ty.i32(), - Vector{ - create(Source{{12, 34}}, core::BuiltinValue::kSampleMask), - })}, - ty.void_(), Empty, - Vector{ - Stage(ast::PipelineStage::kFragment), - }, - Vector{ - Builtin(core::BuiltinValue::kPosition), - }); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: use of '@builtin(sample_mask)' is not allowed in compatibility mode)"); -} - -TEST_F(ResolverCompatibilityModeTest, SampleMask_ReturnValue) { - // @fragment - // fn main() -> @builtin(sample_mask) u32 { - // return 0; - // } - - Func("main", Empty, ty.u32(), - Vector{ - Return(0_u), - }, - Vector{ - Stage(ast::PipelineStage::kFragment), - }, - Vector{ - create(Source{{12, 34}}, core::BuiltinValue::kSampleMask), - }); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: use of '@builtin(sample_mask)' is not allowed in compatibility mode)"); -} - -TEST_F(ResolverCompatibilityModeTest, SampleMask_StructMember) { - // struct S { - // @builtin(sample_mask) mask : u32, - // } - - Structure("S", Vector{ - Member("mask", ty.u32(), - Vector{ - create(Source{{12, 34}}, - core::BuiltinValue::kSampleMask), - }), - }); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: use of '@builtin(sample_mask)' is not allowed in compatibility mode)"); -} - -TEST_F(ResolverCompatibilityModeTest, SampleIndex_Parameter) { - // @fragment - // fn main(@builtin(sample_index) mask : u32) { - // } - - Func("main", - Vector{Param( - "mask", ty.i32(), - Vector{ - create(Source{{12, 34}}, core::BuiltinValue::kSampleIndex), - })}, - ty.void_(), Empty, - Vector{ - Stage(ast::PipelineStage::kFragment), - }, - Vector{ - Builtin(core::BuiltinValue::kPosition), - }); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: use of '@builtin(sample_index)' is not allowed in compatibility mode)"); -} - -TEST_F(ResolverCompatibilityModeTest, SampleIndex_ReturnValue) { - // @fragment - // fn main() -> @builtin(sample_index) u32 { - // return 0; - // } - - Func("main", Empty, ty.u32(), - Vector{ - Return(0_u), - }, - Vector{ - Stage(ast::PipelineStage::kFragment), - }, - Vector{ - create(Source{{12, 34}}, core::BuiltinValue::kSampleIndex), - }); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: use of '@builtin(sample_index)' is not allowed in compatibility mode)"); -} - -TEST_F(ResolverCompatibilityModeTest, SampleIndex_StructMember) { - // struct S { - // @builtin(sample_index) mask : u32, - // } - - Structure("S", Vector{ - Member("mask", ty.u32(), - Vector{ - create(Source{{12, 34}}, - core::BuiltinValue::kSampleIndex), - }), - }); - - EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ( - r()->error(), - R"(12:34 error: use of '@builtin(sample_index)' is not allowed in compatibility mode)"); -} - TEST_F(ResolverCompatibilityModeTest, LinearInterpolation_Parameter) { // @fragment // fn main(@location(1) @interpolate(linear) value : f32) { diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc index 5745ee682d7..4159ea6c149 100644 --- a/src/tint/lang/wgsl/resolver/validator.cc +++ b/src/tint/lang/wgsl/resolver/validator.cc @@ -1067,12 +1067,6 @@ bool Validator::BuiltinAttribute(const ast::BuiltinAttribute* attr, } break; case core::BuiltinValue::kSampleMask: - if (mode_ == wgsl::ValidationMode::kCompat) { - AddError(attr->source) << "use of " << style::Attribute("@builtin") - << style::Code("(", style::Enum(builtin), ")") - << " is not allowed in compatibility mode"; - return false; - } if (stage != ast::PipelineStage::kNone && !(stage == ast::PipelineStage::kFragment)) { is_stage_mismatch = true; } @@ -1082,12 +1076,6 @@ bool Validator::BuiltinAttribute(const ast::BuiltinAttribute* attr, } break; case core::BuiltinValue::kSampleIndex: - if (mode_ == wgsl::ValidationMode::kCompat) { - AddError(attr->source) << "use of " << style::Attribute("@builtin") - << style::Code("(", style::Enum(builtin), ")") - << " is not allowed in compatibility mode"; - return false; - } if (stage != ast::PipelineStage::kNone && !(stage == ast::PipelineStage::kFragment && is_input)) { is_stage_mismatch = true;