Tint: Add validation on the shader stage with `clip_distances` This patch adds the validations on the shader stages with `clip_distances` among the shader stage inputs or outputs. According to the latest WGSL SPEC, `clip_distances` can only be used as a part of vertex shader outputs. Bug: chromium:358408571 Test: tint_unittests Change-Id: Idea8dfb7e7601468415c817a6959ac7aed8200d2 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/202499 Reviewed-by: dan sinclair <dsinclair@chromium.org> Reviewed-by: James Price <jrprice@google.com> Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
diff --git a/src/tint/lang/wgsl/resolver/clip_distances_extension_test.cc b/src/tint/lang/wgsl/resolver/clip_distances_extension_test.cc index 0014a36..357c07c 100644 --- a/src/tint/lang/wgsl/resolver/clip_distances_extension_test.cc +++ b/src/tint/lang/wgsl/resolver/clip_distances_extension_test.cc
@@ -32,6 +32,7 @@ namespace tint::resolver { using namespace tint::core::fluent_types; // NOLINT +using namespace tint::core::number_suffixes; // NOLINT namespace { @@ -262,5 +263,189 @@ R"(error: store type of '@builtin(clip_distances)' must be 'array<f32, N>' (N <= 8))"); } +// Using a clip_distances builtin attribute in fragment shader inputs should fail. +TEST_F(ResolverClipDistancesExtensionTest, UseClipDistancesInFragmentShaderInputFail) { + Enable(wgsl::Extension::kClipDistances); + + auto* s = Structure( + "MyInputs", Vector{Member("clipDistances", ty.array<f32, kMaxClipDistancesSize / 2>(), + Vector{ + Builtin(Source{{56, 78}}, core::BuiltinValue::kClipDistances), + })}); + + Func("fragmentShader", + Vector{ + Param("arg", ty.Of(s)), + }, + ty.f32(), + Vector{ + Return(1_f), + }, + Vector{ + Stage(ast::PipelineStage::kFragment), + }, + Vector{ + Location(0_a), + }); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(56:78 error: '@builtin(clip_distances)' cannot be used for fragment shader input +note: while analyzing entry point 'fragmentShader')"); +} + +// Using a clip_distances builtin attribute in fragment shader outputs should fail. +TEST_F(ResolverClipDistancesExtensionTest, UseClipDistancesInFragmentShaderOutputFail) { + Enable(wgsl::Extension::kClipDistances); + + auto* s = + Structure("MyOutputs", + Vector{Member("clipDistances", ty.array<f32, kMaxClipDistancesSize / 2>(), + Vector{ + Builtin(Source{{12, 34}}, core::BuiltinValue::kClipDistances), + })}); + + Func("fragmentShader", tint::Empty, ty.Of(s), + Vector{ + Return(Call(ty.Of(s))), + }, + Vector{ + Stage(ast::PipelineStage::kFragment), + }); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(12:34 error: '@builtin(clip_distances)' cannot be used for fragment shader output +note: while analyzing entry point 'fragmentShader')"); +} + +// Using a clip_distances builtin attribute in compute shader inputs should fail. +TEST_F(ResolverClipDistancesExtensionTest, UseClipDistancesInComputeShaderInputFail) { + Enable(wgsl::Extension::kClipDistances); + + auto* s = Structure( + "MyInputs", Vector{Member("clipDistances", ty.array<f32, kMaxClipDistancesSize / 2>(), + Vector{ + Builtin(Source{{12, 34}}, core::BuiltinValue::kClipDistances), + })}); + + Func("computeShader", + Vector{ + Param("arg", ty.Of(s)), + }, + ty.void_(), tint::Empty, + Vector{ + Stage(ast::PipelineStage::kCompute), + }, + tint::Empty); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(12:34 error: '@builtin(clip_distances)' cannot be used for compute shader input +note: while analyzing entry point 'computeShader')"); +} + +// Using a clip_distances builtin attribute in compute shader outputs should fail. +TEST_F(ResolverClipDistancesExtensionTest, UseClipDistancesInComputeShaderOutputFail) { + Enable(wgsl::Extension::kClipDistances); + + auto* s = + Structure("MyOutputs", + Vector{Member("clipDistances", ty.array<f32, kMaxClipDistancesSize / 2>(), + Vector{ + Builtin(Source{{12, 34}}, core::BuiltinValue::kClipDistances), + })}); + + Func("computeShader", tint::Empty, ty.Of(s), + Vector{ + Return(Call(ty.Of(s))), + }, + Vector{ + Stage(ast::PipelineStage::kCompute), + }); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(12:34 error: '@builtin(clip_distances)' cannot be used for compute shader output +note: while analyzing entry point 'computeShader')"); +} + +// Using a clip_distances builtin attribute in vertex shader inputs should fail. +TEST_F(ResolverClipDistancesExtensionTest, UseClipDistancesInVertexShaderInputFail) { + Enable(wgsl::Extension::kClipDistances); + + auto* s = Structure( + "MyInputs", Vector{Member("clipDistances", ty.array<f32, kMaxClipDistancesSize / 2>(), + Vector{ + Builtin(Source{{12, 34}}, core::BuiltinValue::kClipDistances), + })}); + + Func("vertexShader", + Vector{ + Param("arg", ty.Of(s)), + }, + ty.vec4<f32>(), + Vector{ + Return(Call(ty.vec4<f32>())), + }, + Vector{ + Stage(ast::PipelineStage::kVertex), + }, + Vector{ + Builtin(core::BuiltinValue::kPosition), + }); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(12:34 error: '@builtin(clip_distances)' cannot be used for vertex shader input +note: while analyzing entry point 'vertexShader')"); +} + +// Using a clip_distances builtin attribute in vertex shader outputs should success. +TEST_F(ResolverClipDistancesExtensionTest, UseClipDistancesInVertexShaderOutputSuccess) { + Enable(wgsl::Extension::kClipDistances); + + auto* s = Structure( + "MyOutputs", + Vector{Member("clipDistances", ty.array<f32, kMaxClipDistancesSize / 2>(), + Vector{ + Builtin(Source{{12, 34}}, core::BuiltinValue::kClipDistances), + }), + Member("position", ty.vec4<f32>(), Vector{Builtin(core::BuiltinValue::kPosition)})}); + + Func("vertexShader", tint::Empty, ty.Of(s), + Vector{ + Return(Call(ty.Of(s))), + }, + Vector{ + Stage(ast::PipelineStage::kVertex), + }); + EXPECT_TRUE(r()->Resolve()); +} + +// Declaring a clip_distances builtin attribute more than once should fail. +TEST_F(ResolverClipDistancesExtensionTest, DuplicateClipDistancesDeclaration) { + Enable(wgsl::Extension::kClipDistances); + + auto* s = Structure( + "MyOutputs", + Vector{Member("clipDistances1", ty.array<f32, kMaxClipDistancesSize / 2>(), + Vector{ + Builtin(Source{{12, 34}}, core::BuiltinValue::kClipDistances), + }), + Member("clipDistances2", ty.array<f32, kMaxClipDistancesSize / 2>(), + Vector{ + Builtin(Source{{12, 34}}, core::BuiltinValue::kClipDistances), + }), + Member("position", ty.vec4<f32>(), Vector{Builtin(core::BuiltinValue::kPosition)})}); + + Func("vertexShader", tint::Empty, ty.Of(s), + Vector{ + Return(Call(ty.Of(s))), + }, + Vector{ + Stage(ast::PipelineStage::kVertex), + }); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), + R"(error: '@builtin(clip_distances)' appears multiple times as pipeline output +note: while analyzing entry point 'vertexShader')"); +} + } // namespace } // namespace tint::resolver
diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc index d780b9e..a873f92 100644 --- a/src/tint/lang/wgsl/resolver/validator.cc +++ b/src/tint/lang/wgsl/resolver/validator.cc
@@ -1120,7 +1120,6 @@ } break; case core::BuiltinValue::kClipDistances: { - // TODO(chromium:358408571): Add more validations on `clip_distances`. if (!enabled_extensions_.Contains(wgsl::Extension::kClipDistances)) { AddError(attr->source) << "use of " << style::Attribute("@builtin") @@ -1138,7 +1137,10 @@ << style::Type("array<f32, N>") << " (N <= " << kMaxClipDistancesSize << ")"; return false; } - + if (stage != ast::PipelineStage::kNone && + !(stage == ast::PipelineStage::kVertex && !is_input)) { + is_stage_mismatch = true; + } break; } default: