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 <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>

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

src/transform/binding_remapper_test.cc

@@ -44,7 +44,7 @@ fn f() {
   DataMap data;
   data.Add<BindingRemapper::Remappings>(BindingRemapper::BindingPoints{},
                                         BindingRemapper::AccessControls{});
-  auto got = Run<BindingRemapper>(src, std::move(data));
+  auto got = Run<BindingRemapper>(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<BindingRemapper>(src, std::move(data));
+  auto got = Run<BindingRemapper>(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<BindingRemapper>(src, std::move(data));
+  auto got = Run<BindingRemapper>(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<BindingRemapper>(src, std::move(data));
+  auto got = Run<BindingRemapper>(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<BindingRemapper>(src, std::move(data));
+  auto got = Run<BindingRemapper>(src, data);
 
   EXPECT_EQ(expect, str(got));
 }

src/transform/test_helper.h

@@ -41,7 +41,7 @@ class TransformTestBase : public BASE {
   Transform::Output Run(
       std::string in,
       std::vector<std::unique_ptr<transform::Transform>> transforms,
-      DataMap data = {}) {
+      const DataMap& data = {}) {
     auto file = std::make_unique<Source::File>("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::Transform> transform,
-                        DataMap data = {}) {
+                        const DataMap& data = {}) {
     std::vector<std::unique_ptr<transform::Transform>> 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 <typename TRANSFORM>
-  Transform::Output Run(std::string in, DataMap data = {}) {
+  Transform::Output Run(std::string in, const DataMap& data = {}) {
-    return Run(std::move(in), std::make_unique<TRANSFORM>(), std::move(data));
+    return Run(std::move(in), std::make_unique<TRANSFORM>(), data);
   }
 
   /// @param output the output of the transform

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(

src/transform/transform.h

@@ -36,6 +36,10 @@ class Data : public Castable<Data> {
 
   /// 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.

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<Config>()) {
+    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 {

src/transform/vertex_pulling.h

@@ -132,7 +132,7 @@ using VertexStateDescriptor = std::vector<VertexBufferLayoutDescriptor>;
 class VertexPulling : public Transform {
  public:
   /// Configuration options for the transform
-  struct Config {
+  struct Config : public Castable<Config, Data> {
     /// 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);

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;
+  DataMap data;
-
+  data.Add<VertexPulling::Config>();
-  auto transform = std::make_unique<VertexPulling>(cfg);
+  auto got = Run<VertexPulling>(src, data);
-
-  auto got = Run(src, std::move(transform));
 
   EXPECT_EQ(expect, str(got));
 }
@@ -49,9 +47,9 @@ fn main() {}
   VertexPulling::Config cfg;
   cfg.entry_point_name = "_";
 
-  auto transform = std::make_unique<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(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<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(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<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(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<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(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<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(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<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(src, data);
 
   EXPECT_EQ(expect, str(got));
 }
@@ -288,9 +286,9 @@ fn main() {
   }};
   cfg.entry_point_name = "main";
 
-  auto transform = std::make_unique<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(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<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(src, data);
 
   EXPECT_EQ(expect, str(got));
 }
@@ -396,9 +394,9 @@ fn main() {
   }};
   cfg.entry_point_name = "main";
 
-  auto transform = std::make_unique<VertexPulling>(cfg);
+  DataMap data;
-
+  data.Add<VertexPulling::Config>(cfg);
-  auto got = Run(src, std::move(transform));
+  auto got = Run<VertexPulling>(src, data);
 
   EXPECT_EQ(expect, str(got));
 }