From c196ca37407d4dacb832d4c888ac87e66d7094c5 Mon Sep 17 00:00:00 2001
From: Jacques Lucke <jacques@blender.org>
Date: Thu, 21 Apr 2022 15:29:07 +0200
Subject: Functions: fix procedure executor not writing output in correct
 buffer

The issue was that the executor would forget about the caller provided
storage if the variable is destructed.
---
 .../intern/multi_function_procedure_executor.cc    | 27 ++++++++-----------
 .../tests/FN_multi_function_procedure_test.cc      | 30 ++++++++++++++++++++++
 2 files changed, 40 insertions(+), 17 deletions(-)

(limited to 'source/blender/functions')

diff --git a/source/blender/functions/intern/multi_function_procedure_executor.cc b/source/blender/functions/intern/multi_function_procedure_executor.cc
index 7a0b74d8039..c58ca0bb8fd 100644
--- a/source/blender/functions/intern/multi_function_procedure_executor.cc
+++ b/source/blender/functions/intern/multi_function_procedure_executor.cc
@@ -681,14 +681,9 @@ class VariableState : NonCopyable, NonMovable {
     /* Sanity check to make sure that enough indices can be destructed. */
     BLI_assert(new_tot_initialized >= 0);
 
-    bool do_destruct_self = false;
-
     switch (value_->type) {
       case ValueType::GVArray: {
-        if (mask.size() == full_mask.size()) {
-          do_destruct_self = true;
-        }
-        else {
+        if (mask.size() < full_mask.size()) {
           /* Not all elements are destructed. Since we can't work on the original array, we have to
            * create a copy first. */
           this->ensure_is_mutable(full_mask, data_type, value_allocator);
@@ -701,16 +696,10 @@ class VariableState : NonCopyable, NonMovable {
       case ValueType::Span: {
         const CPPType &type = data_type.single_type();
         type.destruct_indices(this->value_as<VariableValue_Span>()->data, mask);
-        if (new_tot_initialized == 0) {
-          do_destruct_self = true;
-        }
         break;
       }
       case ValueType::GVVectorArray: {
-        if (mask.size() == full_mask.size()) {
-          do_destruct_self = true;
-        }
-        else {
+        if (mask.size() < full_mask.size()) {
           /* Not all elements are cleared. Since we can't work on the original vector array, we
            * have to create a copy first. A possible future optimization is to create the partial
            * copy directly. */
@@ -729,22 +718,26 @@ class VariableState : NonCopyable, NonMovable {
         BLI_assert(value_typed->is_initialized);
         UNUSED_VARS_NDEBUG(value_typed);
         if (mask.size() == tot_initialized_) {
-          do_destruct_self = true;
+          const CPPType &type = data_type.single_type();
+          type.destruct(value_typed->data);
+          value_typed->is_initialized = false;
         }
         break;
       }
       case ValueType::OneVector: {
         auto *value_typed = this->value_as<VariableValue_OneVector>();
-        UNUSED_VARS(value_typed);
         if (mask.size() == tot_initialized_) {
-          do_destruct_self = true;
+          value_typed->data.clear(IndexRange(1));
         }
         break;
       }
     }
 
     tot_initialized_ = new_tot_initialized;
-    return do_destruct_self;
+
+    const bool should_self_destruct = new_tot_initialized == 0 &&
+                                      caller_provided_storage_ == nullptr;
+    return should_self_destruct;
   }
 
   void indices_split(IndexMask mask, IndicesSplitVectors &r_indices)
diff --git a/source/blender/functions/tests/FN_multi_function_procedure_test.cc b/source/blender/functions/tests/FN_multi_function_procedure_test.cc
index a196d0396ec..e7cedb40f83 100644
--- a/source/blender/functions/tests/FN_multi_function_procedure_test.cc
+++ b/source/blender/functions/tests/FN_multi_function_procedure_test.cc
@@ -378,4 +378,34 @@ TEST(multi_function_procedure, BufferReuse)
   EXPECT_EQ(results[4], 53);
 }
 
+TEST(multi_function_procedure, OutputBufferReplaced)
+{
+  MFProcedure procedure;
+  MFProcedureBuilder builder{procedure};
+
+  const int output_value = 42;
+  CustomMF_GenericConstant constant_fn(CPPType::get<int>(), &output_value, false);
+  MFVariable &var_o = procedure.new_variable(MFDataType::ForSingle<int>());
+  builder.add_output_parameter(var_o);
+  builder.add_call_with_all_variables(constant_fn, {&var_o});
+  builder.add_destruct(var_o);
+  builder.add_call_with_all_variables(constant_fn, {&var_o});
+  builder.add_return();
+
+  EXPECT_TRUE(procedure.validate());
+
+  MFProcedureExecutor procedure_fn{procedure};
+
+  Array<int> output(3, 0);
+  fn::MFParamsBuilder params(procedure_fn, output.size());
+  params.add_uninitialized_single_output(output.as_mutable_span());
+
+  fn::MFContextBuilder context;
+  procedure_fn.call(IndexMask(output.size()), params, context);
+
+  EXPECT_EQ(output[0], output_value);
+  EXPECT_EQ(output[1], output_value);
+  EXPECT_EQ(output[2], output_value);
+}
+
 }  // namespace blender::fn::tests
-- 
cgit v1.2.3