tint/resolver: Move texture validation to the right place Validating sampled and multisampled texture types does not belong in Validator::Variable(). Change-Id: Ie0f2502508c28af6fb6d3f4d7803171d946c511b Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/93783 Reviewed-by: Dan Sinclair <dsinclair@chromium.org> Commit-Queue: Ben Clayton <bclayton@chromium.org>
diff --git a/src/tint/resolver/resolver.cc b/src/tint/resolver/resolver.cc index 3438a13..72ff8bc 100644 --- a/src/tint/resolver/resolver.cc +++ b/src/tint/resolver/resolver.cc
@@ -240,13 +240,21 @@ [&](const ast::Sampler* t) { return builder_->create<sem::Sampler>(t->kind); }, [&](const ast::SampledTexture* t) -> sem::SampledTexture* { if (auto* el = Type(t->type)) { - return builder_->create<sem::SampledTexture>(t->dim, el); + auto* sem = builder_->create<sem::SampledTexture>(t->dim, el); + if (!validator_.SampledTexture(sem, t->source)) { + return nullptr; + } + return sem; } return nullptr; }, [&](const ast::MultisampledTexture* t) -> sem::MultisampledTexture* { if (auto* el = Type(t->type)) { - return builder_->create<sem::MultisampledTexture>(t->dim, el); + auto* sem = builder_->create<sem::MultisampledTexture>(t->dim, el); + if (!validator_.MultisampledTexture(sem, t->source)) { + return nullptr; + } + return sem; } return nullptr; },
diff --git a/src/tint/resolver/type_validation_test.cc b/src/tint/resolver/type_validation_test.cc index 8f7ef1a..ec2f8f5 100644 --- a/src/tint/resolver/type_validation_test.cc +++ b/src/tint/resolver/type_validation_test.cc
@@ -767,7 +767,7 @@ using MultisampledTextureDimensionTest = ResolverTestWithParam<DimensionParams>; TEST_P(MultisampledTextureDimensionTest, All) { auto& params = GetParam(); - Global(Source{{12, 34}}, "a", ty.multisampled_texture(params.dim, ty.i32()), + Global("a", ty.multisampled_texture(Source{{12, 34}}, params.dim, ty.i32()), ast::StorageClass::kNone, nullptr, ast::AttributeList{GroupAndBinding(0, 0)}); if (params.is_valid) { @@ -818,17 +818,16 @@ using SampledTextureTypeTest = ResolverTestWithParam<TypeParams>; TEST_P(SampledTextureTypeTest, All) { auto& params = GetParam(); - Global(Source{{12, 34}}, "a", - ty.sampled_texture(ast::TextureDimension::k2d, params.type_func(*this)), - ast::StorageClass::kNone, nullptr, ast::AttributeList{GroupAndBinding(0, 0)}); + Global( + "a", + ty.sampled_texture(Source{{12, 34}}, ast::TextureDimension::k2d, params.type_func(*this)), + ast::StorageClass::kNone, nullptr, ast::AttributeList{GroupAndBinding(0, 0)}); if (params.is_valid) { EXPECT_TRUE(r()->Resolve()) << r()->error(); } else { EXPECT_FALSE(r()->Resolve()); - EXPECT_EQ(r()->error(), - "12:34 error: texture_2d<type>: type must be f32, " - "i32 or u32"); + EXPECT_EQ(r()->error(), "12:34 error: texture_2d<type>: type must be f32, i32 or u32"); } } INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest, @@ -838,8 +837,9 @@ using MultisampledTextureTypeTest = ResolverTestWithParam<TypeParams>; TEST_P(MultisampledTextureTypeTest, All) { auto& params = GetParam(); - Global(Source{{12, 34}}, "a", - ty.multisampled_texture(ast::TextureDimension::k2d, params.type_func(*this)), + Global("a", + ty.multisampled_texture(Source{{12, 34}}, ast::TextureDimension::k2d, + params.type_func(*this)), ast::StorageClass::kNone, nullptr, ast::AttributeList{GroupAndBinding(0, 0)}); if (params.is_valid) { @@ -847,8 +847,7 @@ } else { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: texture_multisampled_2d<type>: type must be f32, " - "i32 or u32"); + "12:34 error: texture_multisampled_2d<type>: type must be f32, i32 or u32"); } } INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest, @@ -887,8 +886,7 @@ } else { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: cube dimensions for storage textures are not " - "supported"); + "12:34 error: cube dimensions for storage textures are not supported"); } } INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest, @@ -948,9 +946,8 @@ } else { EXPECT_FALSE(r()->Resolve()); EXPECT_EQ(r()->error(), - "12:34 error: image format must be one of the texel formats " - "specified for storage textues in " - "https://gpuweb.github.io/gpuweb/wgsl/#texel-formats"); + "12:34 error: image format must be one of the texel formats specified for " + "storage textues in https://gpuweb.github.io/gpuweb/wgsl/#texel-formats"); } } INSTANTIATE_TEST_SUITE_P(ResolverTypeValidationTest,
diff --git a/src/tint/resolver/validator.cc b/src/tint/resolver/validator.cc index 7562b79..10e76ca 100644 --- a/src/tint/resolver/validator.cc +++ b/src/tint/resolver/validator.cc
@@ -284,6 +284,29 @@ return true; } +bool Validator::SampledTexture(const sem::SampledTexture* t, const Source& source) const { + if (!t->type()->UnwrapRef()->is_numeric_scalar()) { + AddError("texture_2d<type>: type must be f32, i32 or u32", source); + return false; + } + + return true; +} + +bool Validator::MultisampledTexture(const sem::MultisampledTexture* t, const Source& source) const { + if (t->dim() != ast::TextureDimension::k2d) { + AddError("only 2d multisampled textures are supported", source); + return false; + } + + if (!t->type()->UnwrapRef()->is_numeric_scalar()) { + AddError("texture_multisampled_2d<type>: type must be f32, i32 or u32", source); + return false; + } + + return true; +} + bool Validator::Materialize(const sem::Materialize* m) const { auto* from = m->Expr()->Type(); auto* to = m->Type(); @@ -685,25 +708,6 @@ return false; } - if (auto* r = storage_ty->As<sem::SampledTexture>()) { - if (!r->type()->UnwrapRef()->is_numeric_scalar()) { - AddError("texture_2d<type>: type must be f32, i32 or u32", decl->source); - return false; - } - } - - if (auto* r = storage_ty->As<sem::MultisampledTexture>()) { - if (r->dim() != ast::TextureDimension::k2d) { - AddError("only 2d multisampled textures are supported", decl->source); - return false; - } - - if (!r->type()->UnwrapRef()->is_numeric_scalar()) { - AddError("texture_multisampled_2d<type>: type must be f32, i32 or u32", decl->source); - return false; - } - } - if (v->Is<sem::LocalVariable>() && as_var && IsValidationEnabled(decl->attributes, ast::DisabledValidation::kIgnoreStorageClass)) { if (!v->Type()->UnwrapRef()->IsConstructible()) {
diff --git a/src/tint/resolver/validator.h b/src/tint/resolver/validator.h index 223f3cf..238682f 100644 --- a/src/tint/resolver/validator.h +++ b/src/tint/resolver/validator.h
@@ -321,6 +321,18 @@ /// @returns true on success, false otherwise bool StorageTexture(const ast::StorageTexture* t) const; + /// Validates a sampled texture + /// @param t the texture to validate + /// @param source the source of the texture + /// @returns true on success, false otherwise + bool SampledTexture(const sem::SampledTexture* t, const Source& source) const; + + /// Validates a multisampled texture + /// @param t the texture to validate + /// @param source the source of the texture + /// @returns true on success, false otherwise + bool MultisampledTexture(const sem::MultisampledTexture* t, const Source& source) const; + /// Validates a structure /// @param str the structure to validate /// @param stage the current pipeline stage