From 1653668355b2e0c50f47d131c6cb97d83a1f3fb8 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 2 Mar 2021 11:53:42 +0000 Subject: [PATCH] Remove deprecated transform APIs Change-Id: I330b52ed6b485690a64c74fe34bebfe02fb52598 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42265 Commit-Queue: Ben Clayton Reviewed-by: dan sinclair --- src/transform/first_index_offset.cc | 67 +++++++++++----------------- src/transform/first_index_offset.h | 57 +++++++++-------------- src/transform/transform.h | 4 -- src/transform/vertex_pulling.cc | 28 +----------- src/transform/vertex_pulling.h | 23 ---------- src/transform/vertex_pulling_test.cc | 13 ------ 6 files changed, 48 insertions(+), 144 deletions(-) diff --git a/src/transform/first_index_offset.cc b/src/transform/first_index_offset.cc index 1690d4a8ed..55d0f27f5d 100644 --- a/src/transform/first_index_offset.cc +++ b/src/transform/first_index_offset.cc @@ -115,15 +115,17 @@ Transform::Output FirstIndexOffset::Run(const Program* in) { } } + State state{&out, binding_, group_}; + Symbol vertex_index_sym; Symbol instance_index_sym; // Lazily construct the UniformBuffer on first call to // maybe_create_buffer_var() ast::Variable* buffer_var = nullptr; - auto maybe_create_buffer_var = [&](ProgramBuilder* dst) { + auto maybe_create_buffer_var = [&]() { if (buffer_var == nullptr) { - buffer_var = AddUniformBuffer(dst); + buffer_var = state.AddUniformBuffer(); } }; @@ -138,13 +140,13 @@ Transform::Output FirstIndexOffset::Run(const Program* in) { ast::Builtin blt_type = blt_dec->value(); if (blt_type == ast::Builtin::kVertexIndex) { vertex_index_sym = var->symbol(); - has_vertex_index_ = true; + state.has_vertex_index = true; return clone_variable_with_new_name( &ctx, var, kIndexOffsetPrefix + in->Symbols().NameFor(var->symbol())); } else if (blt_type == ast::Builtin::kInstanceIndex) { instance_index_sym = var->symbol(); - has_instance_index_ = true; + state.has_instance_index = true; return clone_variable_with_new_name( &ctx, var, kIndexOffsetPrefix + in->Symbols().NameFor(var->symbol())); @@ -157,7 +159,7 @@ Transform::Output FirstIndexOffset::Run(const Program* in) { // which determines the original builtin variable names, // but this should be fine, as variables are cloned first. [&](ast::Function* func) -> ast::Function* { - maybe_create_buffer_var(ctx.dst); + maybe_create_buffer_var(); if (buffer_var == nullptr) { return nullptr; // no transform need, just clone func } @@ -165,66 +167,48 @@ Transform::Output FirstIndexOffset::Run(const Program* in) { ast::StatementList statements; for (const auto& data : func_sem->LocalReferencedBuiltinVariables()) { if (data.second->value() == ast::Builtin::kVertexIndex) { - statements.emplace_back( - CreateFirstIndexOffset(in->Symbols().NameFor(vertex_index_sym), - kFirstVertexName, buffer_var, ctx.dst)); + statements.emplace_back(state.CreateFirstIndexOffset( + in->Symbols().NameFor(vertex_index_sym), kFirstVertexName, + buffer_var)); } else if (data.second->value() == ast::Builtin::kInstanceIndex) { - statements.emplace_back(CreateFirstIndexOffset( + statements.emplace_back(state.CreateFirstIndexOffset( in->Symbols().NameFor(instance_index_sym), kFirstInstanceName, - buffer_var, ctx.dst)); + buffer_var)); } } return CloneWithStatementsAtStart(&ctx, func, statements); }); ctx.Clone(); - return Output( - Program(std::move(out)), - std::make_unique(has_vertex_index_, has_instance_index_, - vertex_index_offset_, instance_index_offset_)); + return Output(Program(std::move(out)), + std::make_unique( + state.has_vertex_index, state.has_instance_index, + state.vertex_index_offset, state.instance_index_offset)); } -bool FirstIndexOffset::HasVertexIndex() { - return has_vertex_index_; -} - -bool FirstIndexOffset::HasInstanceIndex() { - return has_instance_index_; -} - -uint32_t FirstIndexOffset::GetFirstVertexOffset() { - assert(has_vertex_index_); - return vertex_index_offset_; -} - -uint32_t FirstIndexOffset::GetFirstInstanceOffset() { - assert(has_instance_index_); - return instance_index_offset_; -} - -ast::Variable* FirstIndexOffset::AddUniformBuffer(ProgramBuilder* dst) { +ast::Variable* FirstIndexOffset::State::AddUniformBuffer() { auto* u32_type = dst->create(); ast::StructMemberList members; uint32_t offset = 0; - if (has_vertex_index_) { + if (has_vertex_index) { ast::StructMemberDecorationList member_dec; member_dec.push_back( dst->create(Source{}, offset)); members.push_back(dst->create( Source{}, dst->Symbols().Register(kFirstVertexName), u32_type, std::move(member_dec))); - vertex_index_offset_ = offset; + vertex_index_offset = offset; offset += 4; } - if (has_instance_index_) { + if (has_instance_index) { ast::StructMemberDecorationList member_dec; member_dec.push_back( dst->create(Source{}, offset)); members.push_back(dst->create( Source{}, dst->Symbols().Register(kFirstInstanceName), u32_type, std::move(member_dec))); - instance_index_offset_ = offset; + instance_index_offset = offset; offset += 4; } @@ -243,8 +227,8 @@ ast::Variable* FirstIndexOffset::AddUniformBuffer(ProgramBuilder* dst) { false, // is_const nullptr, // constructor ast::VariableDecorationList{ - dst->create(Source{}, binding_), - dst->create(Source{}, group_), + dst->create(Source{}, binding), + dst->create(Source{}, group), }); dst->AST().AddGlobalVariable(idx_var); @@ -254,11 +238,10 @@ ast::Variable* FirstIndexOffset::AddUniformBuffer(ProgramBuilder* dst) { return idx_var; } -ast::VariableDeclStatement* FirstIndexOffset::CreateFirstIndexOffset( +ast::VariableDeclStatement* FirstIndexOffset::State::CreateFirstIndexOffset( const std::string& original_name, const std::string& field_name, - ast::Variable* buffer_var, - ProgramBuilder* dst) { + ast::Variable* buffer_var) { auto* buffer = dst->create(Source{}, buffer_var->symbol()); diff --git a/src/transform/first_index_offset.h b/src/transform/first_index_offset.h index ab642c4e99..290e078659 100644 --- a/src/transform/first_index_offset.h +++ b/src/transform/first_index_offset.h @@ -101,45 +101,32 @@ class FirstIndexOffset : public Transform { /// @returns the transformation result Output Run(const Program* program) override; - /// [DEPRECATED] - Use Data - /// @returns whether shader uses vertex_index - bool HasVertexIndex(); - - /// [DEPRECATED] - Use Data - /// @returns whether shader uses instance_index - bool HasInstanceIndex(); - - /// [DEPRECATED] - Use Data - /// @returns offset of firstVertex into constant buffer - uint32_t GetFirstVertexOffset(); - - /// [DEPRECATED] - Use Data - /// @returns offset of firstInstance into constant buffer - uint32_t GetFirstInstanceOffset(); - private: - /// Adds uniform buffer with firstVertex/Instance to the program builder - /// @returns variable of new uniform buffer - ast::Variable* AddUniformBuffer(ProgramBuilder* builder); - /// Adds constant with modified original_name builtin to func - /// @param original_name the name of the original builtin used in function - /// @param field_name name of field in firstVertex/Instance buffer - /// @param buffer_var variable of firstVertex/Instance buffer - /// @param builder the target to contain the new ast nodes - ast::VariableDeclStatement* CreateFirstIndexOffset( - const std::string& original_name, - const std::string& field_name, - ast::Variable* buffer_var, - ProgramBuilder* builder); + struct State { + /// Adds uniform buffer with firstVertex/Instance to the program builder + /// @returns variable of new uniform buffer + ast::Variable* AddUniformBuffer(); + /// Adds constant with modified original_name builtin to func + /// @param original_name the name of the original builtin used in function + /// @param field_name name of field in firstVertex/Instance buffer + /// @param buffer_var variable of firstVertex/Instance buffer + ast::VariableDeclStatement* CreateFirstIndexOffset( + const std::string& original_name, + const std::string& field_name, + ast::Variable* buffer_var); + + ProgramBuilder* const dst; + uint32_t const binding; + uint32_t const group; + + bool has_vertex_index = false; + bool has_instance_index = false; + uint32_t vertex_index_offset = 0; + uint32_t instance_index_offset = 0; + }; uint32_t binding_; uint32_t group_; - - bool has_vertex_index_ = false; - bool has_instance_index_ = false; - - uint32_t vertex_index_offset_ = 0; - uint32_t instance_index_offset_ = 0; }; } // namespace transform } // namespace tint diff --git a/src/transform/transform.h b/src/transform/transform.h index c598b83651..bf996e605a 100644 --- a/src/transform/transform.h +++ b/src/transform/transform.h @@ -136,10 +136,6 @@ class Transform { /// Extra output generated by the transforms. DataMap data; - - /// Diagnostics raised while running the Transform. - /// [DEPRECATED] Use `program.Diagnostics()` - diag::List diagnostics; }; /// Runs the transform on `program`, returning the transformation result. diff --git a/src/transform/vertex_pulling.cc b/src/transform/vertex_pulling.cc index 3a7adb4e59..3f7f13865a 100644 --- a/src/transform/vertex_pulling.cc +++ b/src/transform/vertex_pulling.cc @@ -56,39 +56,13 @@ static const char kDefaultInstanceIndexName[] = "_tint_pulling_instance_index"; } // namespace -VertexPulling::VertexPulling() = default; - -VertexPulling::VertexPulling(const Config& config) - : cfg(config), vertex_state_set(true) {} +VertexPulling::VertexPulling(const Config& config) : cfg(config) {} VertexPulling::~VertexPulling() = default; -void VertexPulling::SetVertexState(const VertexStateDescriptor& vertex_state) { - cfg.vertex_state = vertex_state; - vertex_state_set = true; -} - -void VertexPulling::SetEntryPoint(std::string entry_point) { - cfg.entry_point_name = std::move(entry_point); -} - -void VertexPulling::SetPullingBufferBindingGroup(uint32_t number) { - cfg.pulling_group = number; -} - -void VertexPulling::SetPullingBufferBindingSet(uint32_t number) { - cfg.pulling_group = number; -} - Transform::Output VertexPulling::Run(const Program* in) { ProgramBuilder out; - // Check SetVertexState was called - if (!vertex_state_set) { - out.Diagnostics().add_error("SetVertexState not called"); - return Output(Program(std::move(out))); - } - // Find entry point auto* func = in->AST().Functions().Find( in->Symbols().Get(cfg.entry_point_name), ast::PipelineStage::kVertex); diff --git a/src/transform/vertex_pulling.h b/src/transform/vertex_pulling.h index a9bdb52f78..ac29cf7a84 100644 --- a/src/transform/vertex_pulling.h +++ b/src/transform/vertex_pulling.h @@ -158,9 +158,6 @@ class VertexPulling : public Transform { uint32_t pulling_group = 4u; }; - /// Constructor - VertexPulling(); - /// Constructor /// @param config the configuration options for the transform explicit VertexPulling(const Config& config); @@ -168,25 +165,6 @@ class VertexPulling : public Transform { /// Destructor ~VertexPulling() override; - /// Sets the vertex state descriptor, containing info about attributes - /// [DEPRECATED] Use the VertexPulling(const Config&) - /// @param vertex_state the vertex state descriptor - void SetVertexState(const VertexStateDescriptor& vertex_state); - /// Sets the entry point to add assignments into - /// [DEPRECATED] Use the VertexPulling(const Config&) - /// @param entry_point the vertex stage entry point - void SetEntryPoint(std::string entry_point); - /// Sets the "set" we will put all our vertex buffers into (as storage - /// buffers) - /// [DEPRECATED] Use the VertexPulling(const Config&) - /// @param number the set number we will use - void SetPullingBufferBindingSet(uint32_t number); - /// Sets the "group" we will put all our vertex buffers into (as storage - /// buffers) - /// [DEPRECATED] Use the VertexPulling(const Config&) - /// @param number the group number we will use - void SetPullingBufferBindingGroup(uint32_t number); - /// Runs the transform on `program`, returning the transformation result. /// @param program the source program to transform /// @returns the transformation result @@ -194,7 +172,6 @@ class VertexPulling : public Transform { private: Config cfg; - bool vertex_state_set = false; struct State { State(CloneContext& ctx, const Config& c); diff --git a/src/transform/vertex_pulling_test.cc b/src/transform/vertex_pulling_test.cc index cab1d4dca8..6c80e16f95 100644 --- a/src/transform/vertex_pulling_test.cc +++ b/src/transform/vertex_pulling_test.cc @@ -24,19 +24,6 @@ namespace { using VertexPullingTest = TransformTest; -TEST_F(VertexPullingTest, Error_NoVertexState) { - auto* src = R"( -[[stage(vertex)]] -fn main() -> void {} -)"; - - auto* expect = "error: SetVertexState not called"; - - auto got = Transform(src); - - EXPECT_EQ(expect, str(got)); -} - TEST_F(VertexPullingTest, Error_NoEntryPoint) { auto* src = "";