[Bf-blender-cvs] [8625495b1c8] master: Functions: improve handling of unused multi-function outputs

Jacques Lucke noreply at git.blender.org
Sat Jan 14 15:43:02 CET 2023


Commit: 8625495b1c8240e21abac4d255312ddd76fd794d
Author: Jacques Lucke
Date:   Sat Jan 14 15:35:44 2023 +0100
Branches: master
https://developer.blender.org/rB8625495b1c8240e21abac4d255312ddd76fd794d

Functions: improve handling of unused multi-function outputs

Previously, `ParamsBuilder` lazily allocated an array for an
output when it was unused, but the called multi-function
wanted to access it. Now, whether the multi-function supports
an output to be unused is part of the signature. This way, the
allocation can happen earlier when the parameters are build.
The benefit is that this makes all methods of `MFParams`
thread-safe again, removing the need for a mutex.

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

M	source/blender/functions/FN_multi_function_params.hh
M	source/blender/functions/FN_multi_function_signature.hh
M	source/blender/functions/intern/multi_function.cc
M	source/blender/functions/intern/multi_function_params.cc
M	source/blender/nodes/function/nodes/node_fn_separate_color.cc
M	source/blender/nodes/geometry/nodes/node_geo_curve_sample.cc
M	source/blender/nodes/geometry/nodes/node_geo_image_texture.cc
M	source/blender/nodes/geometry/nodes/node_geo_proximity.cc
M	source/blender/nodes/geometry/nodes/node_geo_raycast.cc
M	source/blender/nodes/geometry/nodes/node_geo_sample_nearest_surface.cc
M	source/blender/nodes/geometry/nodes/node_geo_sample_uv_surface.cc
M	source/blender/nodes/shader/nodes/node_shader_sepcomb_xyz.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_brick.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_checker.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_gradient.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_magic.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_musgrave.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_noise.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_voronoi.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_wave.cc
M	source/blender/nodes/shader/nodes/node_shader_tex_white_noise.cc

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

diff --git a/source/blender/functions/FN_multi_function_params.hh b/source/blender/functions/FN_multi_function_params.hh
index 9865a62df1b..8f176b36a19 100644
--- a/source/blender/functions/FN_multi_function_params.hh
+++ b/source/blender/functions/FN_multi_function_params.hh
@@ -32,9 +32,6 @@ class ParamsBuilder {
   Vector<std::variant<GVArray, GMutableSpan, const GVVectorArray *, GVectorArray *>>
       actual_params_;
 
-  std::mutex mutex_;
-  Vector<std::pair<int, GMutableSpan>> dummy_output_spans_;
-
   friend class MFParams;
 
   ParamsBuilder(const Signature &signature, const IndexMask mask)
@@ -127,9 +124,15 @@ class ParamsBuilder {
     const ParamType &param_type = signature_->params[param_index].type;
     BLI_assert(param_type.category() == ParamCategory::SingleOutput);
     const CPPType &type = param_type.data_type().single_type();
-    /* An empty span indicates that this is ignored. */
-    const GMutableSpan dummy_span{type};
-    actual_params_.append_unchecked_as(std::in_place_type<GMutableSpan>, dummy_span);
+
+    if (bool(signature_->params[param_index].flag & ParamFlag::SupportsUnusedOutput)) {
+      /* An empty span indicates that this is ignored. */
+      const GMutableSpan dummy_span{type};
+      actual_params_.append_unchecked_as(std::in_place_type<GMutableSpan>, dummy_span);
+    }
+    else {
+      this->add_unused_output_for_unsupporting_function(type);
+    }
   }
 
   void add_vector_output(GVectorArray &vector_array, StringRef expected_name = "")
@@ -210,6 +213,8 @@ class ParamsBuilder {
   {
     return actual_params_.size();
   }
+
+  void add_unused_output_for_unsupporting_function(const CPPType &type);
 };
 
 class MFParams {
@@ -252,13 +257,11 @@ class MFParams {
   GMutableSpan uninitialized_single_output(int param_index, StringRef name = "")
   {
     this->assert_correct_param(param_index, name, ParamCategory::SingleOutput);
+    BLI_assert(
+        !bool(builder_->signature_->params[param_index].flag & ParamFlag::SupportsUnusedOutput));
     GMutableSpan span = std::get<GMutableSpan>(builder_->actual_params_[param_index]);
-    if (!span.is_empty()) {
-      return span;
-    }
-    /* The output is ignored by the caller, but the multi-function does not handle this case. So
-     * create a temporary buffer that the multi-function can write to. */
-    return this->ensure_dummy_single_output(param_index);
+    BLI_assert(span.size() >= builder_->min_array_size_);
+    return span;
   }
 
   /**
@@ -273,6 +276,8 @@ class MFParams {
   GMutableSpan uninitialized_single_output_if_required(int param_index, StringRef name = "")
   {
     this->assert_correct_param(param_index, name, ParamCategory::SingleOutput);
+    BLI_assert(
+        bool(builder_->signature_->params[param_index].flag & ParamFlag::SupportsUnusedOutput));
     return std::get<GMutableSpan>(builder_->actual_params_[param_index]);
   }
 
@@ -342,8 +347,6 @@ class MFParams {
     }
 #endif
   }
-
-  GMutableSpan ensure_dummy_single_output(int param_index);
 };
 
 }  // namespace blender::fn::multi_function
diff --git a/source/blender/functions/FN_multi_function_signature.hh b/source/blender/functions/FN_multi_function_signature.hh
index 7de7507d851..3f4aade820e 100644
--- a/source/blender/functions/FN_multi_function_signature.hh
+++ b/source/blender/functions/FN_multi_function_signature.hh
@@ -15,10 +15,22 @@
 
 namespace blender::fn::multi_function {
 
+enum class ParamFlag {
+  None = 0,
+  /**
+   * If set, the multi-function parameter can be accessed using
+   * #MFParams::uninitialized_single_output_if_required which can result in better performance
+   * because the output does not have to be computed when it is not needed.
+   */
+  SupportsUnusedOutput = 1 << 0,
+};
+ENUM_OPERATORS(ParamFlag, ParamFlag::SupportsUnusedOutput);
+
 struct Signature {
   struct ParamInfo {
     ParamType type;
     const char *name;
+    ParamFlag flag = ParamFlag::None;
   };
 
   /**
@@ -68,25 +80,27 @@ class SignatureBuilder {
 
   /* Output Parameter Types */
 
-  template<typename T> void single_output(const char *name)
+  template<typename T> void single_output(const char *name, const ParamFlag flag = ParamFlag::None)
   {
-    this->single_output(name, CPPType::get<T>());
+    this->single_output(name, CPPType::get<T>(), flag);
   }
-  void single_output(const char *name, const CPPType &type)
+  void single_output(const char *name, const CPPType &type, const ParamFlag flag = ParamFlag::None)
   {
-    this->output(name, DataType::ForSingle(type));
+    this->output(name, DataType::ForSingle(type), flag);
   }
-  template<typename T> void vector_output(const char *name)
+  template<typename T> void vector_output(const char *name, const ParamFlag flag = ParamFlag::None)
   {
-    this->vector_output(name, CPPType::get<T>());
+    this->vector_output(name, CPPType::get<T>(), flag);
   }
-  void vector_output(const char *name, const CPPType &base_type)
+  void vector_output(const char *name,
+                     const CPPType &base_type,
+                     const ParamFlag flag = ParamFlag::None)
   {
-    this->output(name, DataType::ForVector(base_type));
+    this->output(name, DataType::ForVector(base_type), flag);
   }
-  void output(const char *name, DataType data_type)
+  void output(const char *name, DataType data_type, const ParamFlag flag = ParamFlag::None)
   {
-    signature_.params.append({ParamType(ParamType::Output, data_type), name});
+    signature_.params.append({ParamType(ParamType::Output, data_type), name, flag});
   }
 
   /* Mutable Parameter Types */
diff --git a/source/blender/functions/intern/multi_function.cc b/source/blender/functions/intern/multi_function.cc
index 3d20ce4bac3..d81bb57ebed 100644
--- a/source/blender/functions/intern/multi_function.cc
+++ b/source/blender/functions/intern/multi_function.cc
@@ -108,11 +108,18 @@ void MultiFunction::call_auto(IndexMask mask, MFParams params, Context context)
           break;
         }
         case ParamCategory::SingleOutput: {
-          const GMutableSpan span = params.uninitialized_single_output_if_required(param_index);
-          if (span.is_empty()) {
-            offset_params.add_ignored_single_output();
+          if (bool(signature_ref_->params[param_index].flag & ParamFlag::SupportsUnusedOutput)) {
+            const GMutableSpan span = params.uninitialized_single_output_if_required(param_index);
+            if (span.is_empty()) {
+              offset_params.add_ignored_single_output();
+            }
+            else {
+              const GMutableSpan sliced_span = span.slice(input_slice_range);
+              offset_params.add_uninitialized_single_output(sliced_span);
+            }
           }
           else {
+            const GMutableSpan span = params.uninitialized_single_output(param_index);
             const GMutableSpan sliced_span = span.slice(input_slice_range);
             offset_params.add_uninitialized_single_output(sliced_span);
           }
diff --git a/source/blender/functions/intern/multi_function_params.cc b/source/blender/functions/intern/multi_function_params.cc
index a85a2ed539c..ce5421e4b8c 100644
--- a/source/blender/functions/intern/multi_function_params.cc
+++ b/source/blender/functions/intern/multi_function_params.cc
@@ -4,27 +4,16 @@
 
 namespace blender::fn::multi_function {
 
-GMutableSpan MFParams::ensure_dummy_single_output(int param_index)
+void ParamsBuilder::add_unused_output_for_unsupporting_function(const CPPType &type)
 {
-  /* Lock because we are actually modifying #builder_ and it may be used by multiple threads. */
-  std::lock_guard lock{builder_->mutex_};
-
-  for (const std::pair<int, GMutableSpan> &items : builder_->dummy_output_spans_) {
-    if (items.first == param_index) {
-      return items.second;
-    }
-  }
-
-  const CPPType &type = std::get_if<GMutableSpan>(&builder_->actual_params_[param_index])->type();
-  void *buffer = builder_->scope_.linear_allocator().allocate(
-      builder_->min_array_size_ * type.size(), type.alignment());
+  void *buffer = scope_.linear_allocator().allocate(type.size() * min_array_size_,
+                                                    type.alignment());
+  const GMutableSpan span{type, buffer, min_array_size_};
+  actual_params_.append_unchecked_as(std::in_place_type<GMutableSpan>, span);
   if (!type.is_trivially_destructible()) {
-    builder_->scope_.add_destruct_call(
-        [&type, buffer, mask = builder_->mask_]() { type.destruct_indices(buffer, mask); });
+    scope_.add_destruct_call(
+        [&type, buffer, mask = mask_]() { type.destruct_indices(buffer, mask); });
   }
-  const GMutableSpan span{type, buffer, builder_->min_array_size_};
-  builder_->dummy_output_spans_.append({param_index, span});
-  return span;
 }
 
 }  // namespace blender::fn::multi_function
diff --git a/source/blender/nodes/function/nodes/node_fn_separate_color.cc b/source/blender/nodes/function/nodes/node_fn_separate_color.cc
index fae7c275f5d..f2bcf41a8d5 100644
--- a/source/blender/nodes/function/nodes/node_fn_separate_color.cc
+++ b/source/blender/nodes/function/nodes/node_fn_separate_color.cc
@@ -45,10 +45,10 @@ class SeparateRGBAFunction : public mf::MultiFunction {
       mf::Signature signature;
       mf::SignatureBuilder builder{"Separate Color", signature};
       builder.single_input<ColorGeometry4f>("Color");
-      builder.single_output<float>("Red");
-      builder.single_output<float>("Green");
-      builder.single_output<float>("Blue");
-      builder.single_output<float>("Alpha");
+      builder.single_output<float>("Red", mf::ParamFlag::SupportsUnusedOutput);
+      builder.single_output<float>("Green", mf::ParamFlag::SupportsUnusedOutput);
+      builder.single_output<float>("Blue", mf::ParamFlag::SupportsUnusedOutput);
+      builder.single_output<float>("Alpha", mf::ParamFlag::SupportsUnusedOutput);
       return signature;
     }();
     this->set_signature(&signature);
@@ -107,7 +107,7 @@ class SeparateHSVAFunction : public mf::MultiFunction {
       builder.single_output<float>("Hue");
       builder.single_output<float>("Saturation");
       builder.single_output<float>("Value");
-      builder.single_output<float>("Alpha");
+      builder.sing

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list