[Bf-blender-cvs] [eb547412264] temp-geometry-nodes-fields--fields: Don't destruct reused inputs after a function call

[Hans Goudey]

Commit: eb547412264d36debb0c33ea789a10f418f2d8ae
Author: Hans Goudey
Date:   Sat Aug 28 18:10:38 2021 -0500
Branches: temp-geometry-nodes-fields--fields
https://developer.blender.org/rBeb547412264d36debb0c33ea789a10f418f2d8ae

Don't destruct reused inputs after a function call

===================================================================

M	source/blender/functions/FN_field.hh
M	source/blender/functions/intern/field.cc
M	source/blender/functions/tests/FN_field_test.cc

===================================================================

diff --git a/source/blender/functions/FN_field.hh b/source/blender/functions/FN_field.hh
index 65220b6a4a8..f0e2fdb7a8f 100644
--- a/source/blender/functions/FN_field.hh
+++ b/source/blender/functions/FN_field.hh
@@ -66,19 +66,13 @@ class Field {
 
   std::shared_ptr<FieldInput> input_;
 
-  StringRef name_;
-
  public:
-  Field(const fn::CPPType &type,
-        std::shared_ptr<FieldFunction> function,
-        const int output_index,
-        StringRef name = "")
-      : type_(&type), function_(function), output_index_(output_index), name_(name)
+  Field(const fn::CPPType &type, std::shared_ptr<FieldFunction> function, const int output_index)
+      : type_(&type), function_(function), output_index_(output_index)
   {
   }
 
-  Field(const fn::CPPType &type, std::shared_ptr<FieldInput> input, StringRef name = "")
-      : type_(&type), input_(input), name_(name)
+  Field(const fn::CPPType &type, std::shared_ptr<FieldInput> input) : type_(&type), input_(input)
   {
   }
 
@@ -116,11 +110,6 @@ class Field {
     BLI_assert(input_ == nullptr);
     return output_index_;
   }
-
-  blender::StringRef name() const
-  {
-    return name_;
-  }
 };
 
 /**
@@ -156,8 +145,21 @@ class FieldFunction {
 };
 
 class FieldInput {
+
+ protected:
+  StringRef name_;
+
  public:
+  FieldInput(StringRef name = "") : name_(name)
+  {
+  }
+
   virtual GVArrayPtr retrieve_data(IndexMask mask) const = 0;
+
+  blender::StringRef name() const
+  {
+    return name_;
+  }
 };
 
 /**
diff --git a/source/blender/functions/intern/field.cc b/source/blender/functions/intern/field.cc
index 19c43111be8..b323ff7e8fa 100644
--- a/source/blender/functions/intern/field.cc
+++ b/source/blender/functions/intern/field.cc
@@ -16,6 +16,7 @@
 
 #include "BLI_map.hh"
 #include "BLI_set.hh"
+#include "BLI_vector_set.hh"
 
 #include "FN_field.hh"
 
@@ -56,8 +57,9 @@ static void add_field_variables_recursive(const Field &field,
   if (field.is_input()) {
     const FieldInput &input = field.input();
     if (!variable_map.contains(&input)) {
-      variable_map.add(&input,
-                       {&builder.add_input_parameter(MFDataType::ForSingle(field.type()))});
+      MFVariable &variable = builder.add_input_parameter(MFDataType::ForSingle(field.type()),
+                                                         input.name());
+      variable_map.add(&input, {&variable});
     }
   }
   else {
@@ -66,17 +68,18 @@ static void add_field_variables_recursive(const Field &field,
       add_field_variables_recursive(input_field, builder, variable_map); /* TODO: Use stack. */
     }
 
-    /* Add the immediate inputs to this field, which were added earlier in the
-     * recursive call. This will be skipped for functions with no inputs. */
+    /* Add the immediate inputs to this field, which were added earlier in the recursive call.  */
     Vector<MFVariable *> inputs;
+    VectorSet<MFVariable *> unique_inputs;
     for (const Field &input_field : function.inputs()) {
       MFVariable &input = get_field_variable(input_field, variable_map);
+      unique_inputs.add(&input);
       inputs.append(&input);
     }
 
     Vector<MFVariable *> outputs = builder.add_call(function.multi_function(), inputs);
 
-    builder.add_destruct(inputs);
+    builder.add_destruct(unique_inputs);
 
     variable_map.add(&function, std::move(outputs));
   }
@@ -121,7 +124,7 @@ static void gather_inputs_recursive(const Field &field,
     if (!computed_inputs.contains(variable)) {
       GVArrayPtr data = input.retrieve_data(mask);
       computed_inputs.add_new(variable);
-      params.add_readonly_single_input(*data, field.name());
+      params.add_readonly_single_input(*data, input.name());
       r_inputs.append(std::move(data));
     }
   }
diff --git a/source/blender/functions/tests/FN_field_test.cc b/source/blender/functions/tests/FN_field_test.cc
index b6b9e84f2f1..c0d7d4b2a82 100644
--- a/source/blender/functions/tests/FN_field_test.cc
+++ b/source/blender/functions/tests/FN_field_test.cc
@@ -26,6 +26,7 @@ TEST(field, ConstantFunction)
 }
 
 class IndexFieldInput final : public FieldInput {
+  StringRef name_ = "Index";
   GVArrayPtr retrieve_data(IndexMask mask) const final
   {
     auto index_func = [](int i) { return i; };
@@ -37,7 +38,7 @@ class IndexFieldInput final : public FieldInput {
 
 TEST(field, VArrayInput)
 {
-  Field index_field = Field(CPPType::get<int>(), std::make_shared<IndexFieldInput>(), "Index");
+  Field index_field = Field(CPPType::get<int>(), std::make_shared<IndexFieldInput>());
 
   Array<int> result_1(4);
   GMutableSpan result_generic_1(result_1.as_mutable_span());
@@ -60,8 +61,8 @@ TEST(field, VArrayInput)
 TEST(field, VArrayInputMultipleOutputs)
 {
   std::shared_ptr<FieldInput> index_input = std::make_shared<IndexFieldInput>();
-  Field field_1 = Field(CPPType::get<int>(), index_input, "Index");
-  Field field_2 = Field(CPPType::get<int>(), index_input, "Index");
+  Field field_1 = Field(CPPType::get<int>(), index_input);
+  Field field_2 = Field(CPPType::get<int>(), index_input);
 
   Array<int> result_1(10);
   Array<int> result_2(10);
@@ -81,7 +82,7 @@ TEST(field, VArrayInputMultipleOutputs)
 
 TEST(field, InputAndFunction)
 {
-  Field index_field = Field(CPPType::get<int>(), std::make_shared<IndexFieldInput>(), "Index");
+  Field index_field = Field(CPPType::get<int>(), std::make_shared<IndexFieldInput>());
 
   Field output_field = Field(CPPType::get<int>(),
                              std::make_shared<FieldFunction>(
@@ -90,10 +91,6 @@ TEST(field, InputAndFunction)
                                                {index_field, index_field})),
                              0);
 
-  std::shared_ptr<FieldInput> index_input = std::make_shared<IndexFieldInput>();
-  Field field_1 = Field(CPPType::get<int>(), index_input);
-  Field field_2 = Field(CPPType::get<int>(), index_input);
-
   Array<int> result(10);
   GMutableSpan result_generic(result.as_mutable_span());
   evaluate_fields({output_field}, {2, 4, 6, 8}, {result_generic});