From 12ac6e5d80d8db6b9d0cbc2f6023e60664fabf1b Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 13 Apr 2021 18:46:47 +0000 Subject: [PATCH] transform::VertexPulling - use DataMap for inputs Migrate this transform to using the transform::DataMap pattern for configuration. Allows users to fully construct their transforms ahead of time, and pass in the configuration options for each run. Bug: tint:389 Change-Id: Ie4a8bf80d7b09cfe7bdd4ef01287d994b6b9eb4f Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47626 Reviewed-by: Antonio Maiorano Reviewed-by: James Price Commit-Queue: Antonio Maiorano Auto-Submit: Ben Clayton --- src/castable.h | 3 -- src/transform/binding_remapper_test.cc | 10 ++--- src/transform/test_helper.h | 10 ++--- src/transform/transform.cc | 8 +--- src/transform/transform.h | 4 ++ src/transform/vertex_pulling.cc | 18 +++++--- src/transform/vertex_pulling.h | 14 ++++-- src/transform/vertex_pulling_test.cc | 62 +++++++++++++------------- 8 files changed, 68 insertions(+), 61 deletions(-) diff --git a/src/castable.h b/src/castable.h index 34d98d50b7..460e509629 100644 --- a/src/castable.h +++ b/src/castable.h @@ -158,9 +158,6 @@ class CastableBase { /// Copy constructor CastableBase(const CastableBase&) = default; - /// Move constructor - CastableBase(CastableBase&&) = default; - virtual ~CastableBase() = default; /// @returns the TypeInfo of the object diff --git a/src/transform/binding_remapper_test.cc b/src/transform/binding_remapper_test.cc index 5e798a6ab2..2ca2fd1960 100644 --- a/src/transform/binding_remapper_test.cc +++ b/src/transform/binding_remapper_test.cc @@ -44,7 +44,7 @@ fn f() { DataMap data; data.Add(BindingRemapper::BindingPoints{}, BindingRemapper::AccessControls{}); - auto got = Run(src, std::move(data)); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -86,7 +86,7 @@ fn f() { // Keep [[group(3), binding(2)]] as is }, BindingRemapper::AccessControls{}); - auto got = Run(src, std::move(data)); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -137,7 +137,7 @@ fn f() { // Keep [[group(3), binding(2)]] as is {{4, 3}, ast::AccessControl::kReadOnly}, // Add access control }); - auto got = Run(src, std::move(data)); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -202,7 +202,7 @@ fn f() { // Keep [[group(3), binding(2)]] as is {{4, 3}, ast::AccessControl::kReadOnly}, // Add access control }); - auto got = Run(src, std::move(data)); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -249,7 +249,7 @@ fn f() { {{2, 1}, ast::AccessControl::kWriteOnly}, {{3, 2}, ast::AccessControl::kWriteOnly}, }); - auto got = Run(src, std::move(data)); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } diff --git a/src/transform/test_helper.h b/src/transform/test_helper.h index 4dc38d95fe..52f3a77aeb 100644 --- a/src/transform/test_helper.h +++ b/src/transform/test_helper.h @@ -41,7 +41,7 @@ class TransformTestBase : public BASE { Transform::Output Run( std::string in, std::vector> transforms, - DataMap data = {}) { + const DataMap& data = {}) { auto file = std::make_unique("test", in); auto program = reader::wgsl::Parse(file.get()); @@ -67,10 +67,10 @@ class TransformTestBase : public BASE { /// @return the transformed output Transform::Output Run(std::string in, std::unique_ptr transform, - DataMap data = {}) { + const DataMap& data = {}) { std::vector> transforms; transforms.emplace_back(std::move(transform)); - return Run(std::move(in), std::move(transforms), std::move(data)); + return Run(std::move(in), std::move(transforms), data); } /// Transforms and returns the WGSL source `in`, transformed using @@ -79,8 +79,8 @@ class TransformTestBase : public BASE { /// @param data the optional DataMap to pass to Transform::Run() /// @return the transformed output template - Transform::Output Run(std::string in, DataMap data = {}) { - return Run(std::move(in), std::make_unique(), std::move(data)); + Transform::Output Run(std::string in, const DataMap& data = {}) { + return Run(std::move(in), std::make_unique(), data); } /// @param output the output of the transform diff --git a/src/transform/transform.cc b/src/transform/transform.cc index 20cfb83d37..da3a5d0a90 100644 --- a/src/transform/transform.cc +++ b/src/transform/transform.cc @@ -24,23 +24,17 @@ namespace tint { namespace transform { Data::Data() = default; - Data::Data(const Data&) = default; - Data::~Data() = default; +Data& Data::operator=(const Data&) = default; DataMap::DataMap() = default; - DataMap::DataMap(DataMap&&) = default; - DataMap::~DataMap() = default; Transform::Output::Output() = default; - Transform::Output::Output(Program&& p) : program(std::move(p)) {} - Transform::Transform() = default; - Transform::~Transform() = default; ast::Function* Transform::CloneWithStatementsAtStart( diff --git a/src/transform/transform.h b/src/transform/transform.h index e87d2c5049..2c96b27db9 100644 --- a/src/transform/transform.h +++ b/src/transform/transform.h @@ -36,6 +36,10 @@ class Data : public Castable { /// Destructor ~Data() override; + + /// Assignment operator + /// @returns this Data + Data& operator=(const Data&); }; /// DataMap is a map of Data unique pointers keyed by the Data's ClassID. diff --git a/src/transform/vertex_pulling.cc b/src/transform/vertex_pulling.cc index 9abcbd9c27..951f4fd934 100644 --- a/src/transform/vertex_pulling.cc +++ b/src/transform/vertex_pulling.cc @@ -23,6 +23,8 @@ #include "src/program_builder.h" #include "src/semantic/variable.h" +TINT_INSTANTIATE_TYPEINFO(tint::transform::VertexPulling::Config); + namespace tint { namespace transform { namespace { @@ -36,13 +38,19 @@ static const char kDefaultInstanceIndexName[] = "_tint_pulling_instance_index"; } // namespace -VertexPulling::VertexPulling(const Config& config) : cfg(config) {} +VertexPulling::VertexPulling() = default; +VertexPulling::VertexPulling(const Config& config) : cfg_(config) {} VertexPulling::~VertexPulling() = default; -Transform::Output VertexPulling::Run(const Program* in, const DataMap&) { +Transform::Output VertexPulling::Run(const Program* in, const DataMap& data) { ProgramBuilder out; + auto cfg = cfg_; + if (auto* cfg_data = data.Get()) { + cfg = *cfg_data; + } + // Find entry point auto* func = in->AST().Functions().Find( in->Symbols().Get(cfg.entry_point_name), ast::PipelineStage::kVertex); @@ -80,16 +88,14 @@ Transform::Output VertexPulling::Run(const Program* in, const DataMap&) { } VertexPulling::Config::Config() = default; - VertexPulling::Config::Config(const Config&) = default; - VertexPulling::Config::~Config() = default; +VertexPulling::Config& VertexPulling::Config::operator=(const Config&) = + default; VertexPulling::State::State(CloneContext& context, const Config& c) : ctx(context), cfg(c) {} - VertexPulling::State::State(const State&) = default; - VertexPulling::State::~State() = default; std::string VertexPulling::State::GetVertexBufferName(uint32_t index) const { diff --git a/src/transform/vertex_pulling.h b/src/transform/vertex_pulling.h index 05b3a839db..8be1812bf5 100644 --- a/src/transform/vertex_pulling.h +++ b/src/transform/vertex_pulling.h @@ -132,7 +132,7 @@ using VertexStateDescriptor = std::vector; class VertexPulling : public Transform { public: /// Configuration options for the transform - struct Config { + struct Config : public Castable { /// Constructor Config(); @@ -140,7 +140,11 @@ class VertexPulling : public Transform { Config(const Config&); /// Destructor - ~Config(); + ~Config() override; + + /// Assignment operator + /// @returns this Config + Config& operator=(const Config&); /// The entry point to add assignments into std::string entry_point_name; @@ -154,6 +158,10 @@ class VertexPulling : public Transform { }; /// Constructor + VertexPulling(); + + /// Constructor + /// [DEPRECATED] - pass Config as part of the `data` to Run() /// @param config the configuration options for the transform explicit VertexPulling(const Config& config); @@ -167,7 +175,7 @@ class VertexPulling : public Transform { Output Run(const Program* program, const DataMap& data = {}) override; private: - Config cfg; + Config cfg_; 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 aea3da9414..e1222e6f1f 100644 --- a/src/transform/vertex_pulling_test.cc +++ b/src/transform/vertex_pulling_test.cc @@ -29,11 +29,9 @@ TEST_F(VertexPullingTest, Error_NoEntryPoint) { auto* expect = "error: Vertex stage entry point not found"; - VertexPulling::Config cfg; - - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -49,9 +47,9 @@ fn main() {} VertexPulling::Config cfg; cfg.entry_point_name = "_"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -67,9 +65,9 @@ fn main() {} VertexPulling::Config cfg; cfg.entry_point_name = "main"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -97,9 +95,9 @@ fn main() { VertexPulling::Config cfg; cfg.entry_point_name = "main"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -139,9 +137,9 @@ fn main() { {{4, InputStepMode::kVertex, {{VertexFormat::kF32, 0, 0}}}}}; cfg.entry_point_name = "main"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -181,9 +179,9 @@ fn main() { {{4, InputStepMode::kInstance, {{VertexFormat::kF32, 0, 0}}}}}; cfg.entry_point_name = "main"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -224,9 +222,9 @@ fn main() { cfg.pulling_group = 5; cfg.entry_point_name = "main"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -288,9 +286,9 @@ fn main() { }}; cfg.entry_point_name = "main"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -337,9 +335,9 @@ fn main() { {{VertexFormat::kF32, 0, 0}, {VertexFormat::kVec4F32, 0, 1}}}}}; cfg.entry_point_name = "main"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); } @@ -396,9 +394,9 @@ fn main() { }}; cfg.entry_point_name = "main"; - auto transform = std::make_unique(cfg); - - auto got = Run(src, std::move(transform)); + DataMap data; + data.Add(cfg); + auto got = Run(src, data); EXPECT_EQ(expect, str(got)); }