From 61306b882debd853b65999d564878eaa3a4c8a8a Mon Sep 17 00:00:00 2001 From: James Price Date: Mon, 8 Feb 2021 15:08:49 +0000 Subject: [PATCH] [wgsl-reader][wgsl-writer] Fix implicit handle storage class for textures Unwrap the type before checking if it is a handle type, to account for access decorations. Bug: tint:332 Change-Id: I8af9749fec1e2f5dbd7c3bec0b73e506ae111a28 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40540 Commit-Queue: dan sinclair Auto-Submit: James Price Reviewed-by: dan sinclair --- src/reader/wgsl/parser_impl.cc | 2 +- src/reader/wgsl/parser_impl_error_msg_test.cc | 4 ++-- .../wgsl/parser_impl_global_variable_decl_test.cc | 4 ++-- src/writer/wgsl/generator_impl.cc | 2 +- .../generator_impl_variable_decl_statement_test.cc | 11 +++++++---- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 3bbf1110f7..63f15c6218 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -493,7 +493,7 @@ Maybe ParserImpl::variable_decl() { if (decl.errored) return Failure::kErrored; - if (decl->type->is_handle()) { + if (decl->type->UnwrapAll()->is_handle()) { // handle types implicitly have the `UniformConstant` storage class. // TODO(jrprice): Produce an error if an explicit storage class is provided. sc = ast::StorageClass::kUniformConstant; diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc index 1a3020ea72..eec3330f15 100644 --- a/src/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/reader/wgsl/parser_impl_error_msg_test.cc @@ -1095,9 +1095,9 @@ TEST_F(ParserImplErrorTest, DISABLED_GlobalDeclSamplerExplicitStorageClass) { TEST_F(ParserImplErrorTest, DISABLED_GlobalDeclTextureExplicitStorageClass) { // TODO(jrprice): Enable this once downstream users have caught up. EXPECT( - "var x : texture_1d;", + "var x : [[access(read)]] texture_1d;", "test.wgsl:1:5 error: texture variables must not have a storage class\n" - "var x : texture_1d;\n" + "var x : [[access(read)]] texture_1d;\n" " ^^^^^^^\n"); } diff --git a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc index d23c642662..941d043efb 100644 --- a/src/reader/wgsl/parser_impl_global_variable_decl_test.cc +++ b/src/reader/wgsl/parser_impl_global_variable_decl_test.cc @@ -191,7 +191,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_SamplerImplicitStorageClass) { } TEST_F(ParserImplTest, GlobalVariableDecl_TextureImplicitStorageClass) { - auto p = parser("var s : texture_1d;"); + auto p = parser("var s : [[access(read)]] texture_1d;"); auto decos = p->decoration_list(); EXPECT_FALSE(decos.errored); EXPECT_FALSE(decos.matched); @@ -202,7 +202,7 @@ TEST_F(ParserImplTest, GlobalVariableDecl_TextureImplicitStorageClass) { ASSERT_NE(e.value, nullptr); EXPECT_EQ(e->symbol(), p->builder().Symbols().Get("s")); - EXPECT_TRUE(e->type()->Is()); + EXPECT_TRUE(e->type()->UnwrapAll()->Is()); EXPECT_EQ(e->declared_storage_class(), ast::StorageClass::kUniformConstant); } diff --git a/src/writer/wgsl/generator_impl.cc b/src/writer/wgsl/generator_impl.cc index a031363cd1..4e08026cbc 100644 --- a/src/writer/wgsl/generator_impl.cc +++ b/src/writer/wgsl/generator_impl.cc @@ -598,7 +598,7 @@ bool GeneratorImpl::EmitVariable(ast::Variable* var) { out_ << "var"; if (sem->StorageClass() != ast::StorageClass::kNone && sem->StorageClass() != ast::StorageClass::kFunction && - !var->type()->is_handle()) { + !var->type()->UnwrapAll()->is_handle()) { out_ << "<" << sem->StorageClass() << ">"; } } diff --git a/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc b/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc index 633bff94c0..32908241de 100644 --- a/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc +++ b/src/writer/wgsl/generator_impl_variable_decl_statement_test.cc @@ -19,6 +19,7 @@ #include "src/ast/identifier_expression.h" #include "src/ast/variable.h" #include "src/ast/variable_decl_statement.h" +#include "src/type/access_control_type.h" #include "src/type/f32_type.h" #include "src/type/sampled_texture_type.h" #include "src/writer/wgsl/generator_impl.h" @@ -92,9 +93,11 @@ TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Sampler) { } TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Texture) { - auto* var = Global( - "t", ast::StorageClass::kUniformConstant, - create(type::TextureDimension::k1d, ty.f32())); + auto* st = + create(type::TextureDimension::k1d, ty.f32()); + auto* var = + Global("t", ast::StorageClass::kUniformConstant, + create(ast::AccessControl::kReadOnly, st)); auto* stmt = create(var); @@ -103,7 +106,7 @@ TEST_F(WgslGeneratorImplTest, Emit_VariableDeclStatement_Texture) { gen.increment_indent(); ASSERT_TRUE(gen.EmitStatement(stmt)) << gen.error(); - EXPECT_EQ(gen.result(), " var t : texture_1d;\n"); + EXPECT_EQ(gen.result(), " var t : [[access(read)]]\ntexture_1d;\n"); } } // namespace