From cb7e5d4252b63a177271835fbcb1306f11756262 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 3 Jun 2021 08:44:14 +0000 Subject: [PATCH] reader/wgsl: Support access on storage texture Add a deprecated warning for the old syntax. Bug: tint:846 Change-Id: Id46af28054b32c2ba6f1b96556da023f46a6b87b Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/53048 Kokoro: Kokoro Commit-Queue: Ben Clayton Reviewed-by: James Price --- src/reader/wgsl/parser_impl.cc | 45 +++++++++++-- src/reader/wgsl/parser_impl_error_msg_test.cc | 8 +-- .../parser_impl_texture_sampler_types_test.cc | 64 +++++++++++++++++-- 3 files changed, 102 insertions(+), 15 deletions(-) diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index af0b930455..224d2280ff 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -556,7 +556,8 @@ Maybe ParserImpl::variable_decl() { // | depth_texture_type // | sampled_texture_type LESS_THAN type_decl GREATER_THAN // | multisampled_texture_type LESS_THAN type_decl GREATER_THAN -// | storage_texture_type LESS_THAN image_storage_type GREATER_THAN +// | storage_texture_type LESS_THAN image_storage_type +// COMMA access GREATER_THAN Maybe ParserImpl::texture_sampler_types() { auto type = sampler_type(); if (type.matched) @@ -598,15 +599,47 @@ Maybe ParserImpl::texture_sampler_types() { auto storage = storage_texture_type(); if (storage.matched) { const char* use = "storage texture type"; + using StorageTextureInfo = + std::pair; + auto params = expect_lt_gt_block(use, [&]() -> Expect { + auto format = expect_image_storage_type(use); + if (format.errored) { + return Failure::kErrored; + } - auto format = - expect_lt_gt_block(use, [&] { return expect_image_storage_type(use); }); + if (!match(Token::Type::kComma)) { + deprecated( + peek().source(), + "access control is expected as last parameter of storage textures"); + return std::make_pair(format.value, + tint::ast::AccessControl::kReadWrite); + } - if (format.errored) + auto access = expect_access_type(); + if (access.errored) { + return Failure::kErrored; + } + + return std::make_pair(format.value, access.value); + }); + + if (params.errored) { return Failure::kErrored; + } - return builder_.ty.storage_texture(source_range, storage.value, - format.value); + ast::Type* ty = + builder_.ty.storage_texture(source_range, storage.value, params->first); + + if (params->second != tint::ast::AccessControl::kReadWrite) { + // TODO(crbug.com/tint/846): The ast::AccessControl decoration is + // deprecated, but while we're migrating existing WGSL over to the new + // style of having the access part of the storage texture, we need to + // support both old and new styles. For now, have the new syntax emulate + // the old style AST. + ty = builder_.ty.access(params->second, ty); + } + + return ty; } return Failure::kNoMatch; diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc index bfefc25f0b..dfb4c7f85f 100644 --- a/src/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/reader/wgsl/parser_impl_error_msg_test.cc @@ -555,10 +555,10 @@ TEST_F(ParserImplErrorTest, GlobalDeclStorageTextureMissingLessThan) { } TEST_F(ParserImplErrorTest, GlobalDeclStorageTextureMissingGreaterThan) { - EXPECT("var x : [[access(read)]] texture_storage_2d' for storage texture type\n" - "var x : [[access(read)]] texture_storage_2d' for storage texture type\n" + "var x : [[access(read)]] texture_storage_2derror(), "1:28: expected '>' for multisampled texture type"); } -TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Readonly1dR8Unorm) { +// TODO(crbug.com/tint/846): Remove +TEST_F(ParserImplTest, + TextureSamplerTypes_StorageTexture_Readonly1dR8Unorm_DEPRECATED) { auto p = parser("texture_storage_1d"); auto t = p->texture_sampler_types(); ASSERT_FALSE(p->has_error()) << p->error(); @@ -220,7 +222,29 @@ TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Readonly1dR8Unorm) { EXPECT_EQ(t.value->source().range, (Source::Range{{1u, 1u}, {1u, 28u}})); } -TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Writeonly2dR16Float) { +TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Readonly1dR8Unorm) { + auto p = parser("texture_storage_1d"); + auto a = p->texture_sampler_types(); + ASSERT_FALSE(p->has_error()) << p->error(); + EXPECT_TRUE(a.matched); + EXPECT_FALSE(a.errored); + ASSERT_NE(a.value, nullptr); + + ASSERT_TRUE(a->Is()); + EXPECT_TRUE(a->As()->IsReadOnly()); + + auto* t = a->As()->type(); + ASSERT_TRUE(t->Is()); + ASSERT_TRUE(t->Is()); + EXPECT_EQ(t->As()->image_format(), + ast::ImageFormat::kR8Unorm); + EXPECT_EQ(t->As()->dim(), ast::TextureDimension::k1d); + EXPECT_EQ(t->source().range, (Source::Range{{1u, 1u}, {1u, 34u}})); +} + +// TODO(crbug.com/tint/846): Remove +TEST_F(ParserImplTest, + TextureSamplerTypes_StorageTexture_Writeonly2dR16Float_DEPRECATED) { auto p = parser("texture_storage_2d"); auto t = p->texture_sampler_types(); ASSERT_FALSE(p->has_error()) << p->error(); @@ -236,8 +260,28 @@ TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Writeonly2dR16Float) { EXPECT_EQ(t.value->source().range, (Source::Range{{1u, 1u}, {1u, 29u}})); } +TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_Writeonly2dR16Float) { + auto p = parser("texture_storage_2d"); + auto a = p->texture_sampler_types(); + ASSERT_FALSE(p->has_error()) << p->error(); + EXPECT_TRUE(a.matched); + EXPECT_FALSE(a.errored); + ASSERT_NE(a.value, nullptr); + + ASSERT_TRUE(a->Is()); + EXPECT_TRUE(a->As()->IsWriteOnly()); + + auto* t = a->As()->type(); + ASSERT_TRUE(t->Is()); + ASSERT_TRUE(t->Is()); + EXPECT_EQ(t->As()->image_format(), + ast::ImageFormat::kR16Float); + EXPECT_EQ(t->As()->dim(), ast::TextureDimension::k2d); + EXPECT_EQ(t->source().range, (Source::Range{{1u, 1u}, {1u, 36u}})); +} + TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_InvalidType) { - auto p = parser("texture_storage_1d"); + auto p = parser("texture_storage_1d"); auto t = p->texture_sampler_types(); EXPECT_EQ(t.value, nullptr); EXPECT_FALSE(t.matched); @@ -245,6 +289,15 @@ TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_InvalidType) { EXPECT_EQ(p->error(), "1:20: invalid format for storage texture type"); } +TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_InvalidAccess) { + auto p = parser("texture_storage_1d"); + auto t = p->texture_sampler_types(); + EXPECT_EQ(t.value, nullptr); + EXPECT_FALSE(t.matched); + EXPECT_TRUE(t.errored); + EXPECT_EQ(p->error(), "1:30: invalid value for access decoration"); +} + TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_MissingType) { auto p = parser("texture_storage_1d<>"); auto t = p->texture_sampler_types(); @@ -264,13 +317,14 @@ TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_MissingLessThan) { } TEST_F(ParserImplTest, TextureSamplerTypes_StorageTexture_MissingGreaterThan) { - auto p = parser("texture_storage_1dtexture_sampler_types(); EXPECT_EQ(t.value, nullptr); EXPECT_FALSE(t.matched); EXPECT_TRUE(t.errored); - EXPECT_EQ(p->error(), "1:27: expected '>' for storage texture type"); + EXPECT_EQ(p->error(), "1:33: expected '>' for storage texture type"); } + } // namespace } // namespace wgsl } // namespace reader