Geometry: Use implicit sharing for deformed positions

Avoid copying the positions array into the evaluated edit hints array
that's used to support editing with deformed positions when there is
a topology-changing procedural operation. In a simple test in sculpt
mode with 706k curve points, memory usage went from 78 to 70 MB.

This adds more duplication would be ideal, mainly because retrieving
the data with write access and making implicit sharing info for arbitrary
arrays aren't abstracted by implicit sharing utilities. It may be possible
to improve both of those aspects, either now or in the future.

Pull Request: https://projects.blender.org/blender/blender/pulls/120146

source/blender/blenkernel/BKE_curves.hh

@@ -11,7 +11,7 @@
 
 #include "BLI_bounds_types.hh"
 #include "BLI_generic_virtual_array.hh"
-#include "BLI_implicit_sharing.hh"
+#include "BLI_implicit_sharing_ptr.hh"
 #include "BLI_index_mask_fwd.hh"
 #include "BLI_math_matrix_types.hh"
 #include "BLI_math_vector_types.hh"
@@ -451,7 +451,7 @@ class CurvesEditHints {
    * Evaluated positions for the points in #curves_orig. If this is empty, the positions from the
    * evaluated #Curves should be used if possible.
    */
-  std::optional<Array<float3>> positions;
+  ImplicitSharingPtrAndData positions_data;
   /**
    * Matrices which transform point movement vectors from original data to corresponding movements
    * of evaluated data.
@@ -460,6 +460,9 @@ class CurvesEditHints {
 
   CurvesEditHints(const Curves &curves_id_orig) : curves_id_orig(curves_id_orig) {}
 
+  std::optional<Span<float3>> positions() const;
+  std::optional<MutableSpan<float3>> positions_for_write();
+
   /**
    * The edit hints have to correspond to the original curves, i.e. the number of deformed points
    * is the same as the number of original points.

source/blender/blenkernel/BKE_grease_pencil.hh

@@ -14,6 +14,7 @@
 #include "BLI_array_utils.hh"
 #include "BLI_color.hh"
 #include "BLI_function_ref.hh"
+#include "BLI_implicit_sharing_ptr.hh"
 #include "BLI_map.hh"
 #include "BLI_math_matrix_types.hh"
 #include "BLI_math_vector_types.hh"
@@ -773,7 +774,11 @@ class GreasePencilRuntime {
 
 class GreasePencilDrawingEditHints {
  public:
-  std::optional<Array<float3>> positions;
+  const greasepencil::Drawing *drawing_orig;
+  ImplicitSharingPtrAndData positions_data;
+
+  std::optional<Span<float3>> positions() const;
+  std::optional<MutableSpan<float3>> positions_for_write();
 };
 
 /**

source/blender/blenkernel/intern/crazyspace.cc

@@ -599,9 +599,9 @@ GeometryDeformation get_evaluated_curves_deformation(const Object *ob_eval, cons
   if (edit_component_eval != nullptr) {
     const CurvesEditHints *edit_hints = edit_component_eval->curves_edit_hints_.get();
     if (edit_hints != nullptr && &edit_hints->curves_id_orig == &curves_id_orig) {
-      if (edit_hints->positions.has_value()) {
-        BLI_assert(edit_hints->positions->size() == points_num);
-        deformation.positions = *edit_hints->positions;
+      if (const std::optional<Span<float3>> positions = edit_hints->positions()) {
+        BLI_assert(positions->size() == points_num);
+        deformation.positions = *positions;
         uses_extra_positions = true;
       }
       if (edit_hints->deform_mats.has_value()) {
@@ -677,8 +677,8 @@ GeometryDeformation get_evaluated_grease_pencil_drawing_deformation(const Object
     BLI_assert(edit_hints->drawing_hints->size() == layers_orig.size());
     const GreasePencilDrawingEditHints &drawing_hints =
         edit_hints->drawing_hints.value()[layer_index];
-    if (drawing_hints.positions.has_value()) {
-      deformation.positions = *drawing_hints.positions;
+    if (const std::optional<Span<float3>> positions = drawing_hints.positions()) {
+      deformation.positions = *positions;
       return deformation;
     }
   }

source/blender/blenkernel/intern/curves.cc

@@ -333,8 +333,8 @@ CurvesSurfaceTransforms::CurvesSurfaceTransforms(const Object &curves_ob, const
 bool CurvesEditHints::is_valid() const
 {
   const int point_num = this->curves_id_orig.geometry.point_num;
-  if (this->positions.has_value()) {
-    if (this->positions->size() != point_num) {
+  if (this->positions().has_value()) {
+    if (this->positions()->size() != point_num) {
       return false;
     }
   }
@@ -346,6 +346,35 @@ bool CurvesEditHints::is_valid() const
   return true;
 }
 
+std::optional<Span<float3>> CurvesEditHints::positions() const
+{
+  if (!this->positions_data.has_value()) {
+    return std::nullopt;
+  }
+  const int points_num = this->curves_id_orig.geometry.wrap().points_num();
+  return Span(static_cast<const float3 *>(this->positions_data.data), points_num);
+}
+
+std::optional<MutableSpan<float3>> CurvesEditHints::positions_for_write()
+{
+  if (!this->positions_data.has_value()) {
+    return std::nullopt;
+  }
+
+  const int points_num = this->curves_id_orig.geometry.wrap().points_num();
+  ImplicitSharingPtrAndData &data = this->positions_data;
+  if (data.sharing_info->is_mutable()) {
+    data.sharing_info->tag_ensured_mutable();
+  }
+  else {
+    auto *new_sharing_info = new ImplicitSharedValue<Array<float3>>(*this->positions());
+    data.sharing_info = ImplicitSharingPtr<ImplicitSharingInfo>(new_sharing_info);
+    data.data = new_sharing_info->data.data();
+  }
+
+  return MutableSpan(const_cast<float3 *>(static_cast<const float3 *>(data.data)), points_num);
+}
+
 void curves_normals_point_domain_calc(const CurvesGeometry &curves, MutableSpan<float3> normals)
 {
   const bke::CurvesFieldContext context(curves, AttrDomain::Point);

source/blender/blenkernel/intern/geometry_component_edit_data.cc

@@ -40,25 +40,37 @@ void GeometryComponentEditData::clear()
   grease_pencil_edit_hints_.reset();
 }
 
+static ImplicitSharingPtrAndData save_shared_attribute(const GAttributeReader &attribute)
+{
+  if (attribute.sharing_info && attribute.varray.is_span()) {
+    const void *data = attribute.varray.get_internal_span().data();
+    attribute.sharing_info->add_user();
+    return {ImplicitSharingPtr(attribute.sharing_info), data};
+  }
+  auto *data = new ImplicitSharedValue<GArray<>>(attribute.varray.type(), attribute.varray.size());
+  attribute.varray.materialize(data->data.data());
+  return {ImplicitSharingPtr<ImplicitSharingInfo>(data), data->data.data()};
+}
+
 static void remember_deformed_curve_positions_if_necessary(
     const Curves *curves_id, GeometryComponentEditData &edit_component)
 {
   if (!edit_component.curves_edit_hints_) {
     return;
   }
-  if (edit_component.curves_edit_hints_->positions.has_value()) {
+  if (curves_id == nullptr) {
     return;
   }
-  if (curves_id == nullptr) {
+  CurvesEditHints &edit_hints = *edit_component.curves_edit_hints_;
+  if (edit_hints.positions().has_value()) {
     return;
   }
   const CurvesGeometry &curves = curves_id->geometry.wrap();
   const int points_num = curves.points_num();
-  if (points_num != edit_component.curves_edit_hints_->curves_id_orig.geometry.point_num) {
+  if (points_num != edit_hints.curves_id_orig.geometry.point_num) {
     return;
   }
-  edit_component.curves_edit_hints_->positions.emplace(points_num);
-  edit_component.curves_edit_hints_->positions->as_mutable_span().copy_from(curves.positions());
+  edit_hints.positions_data = save_shared_attribute(curves.attributes().lookup("position"));
 }
 
 static void remember_deformed_grease_pencil_if_necessary(const GreasePencil *grease_pencil,
@@ -91,14 +103,15 @@ static void remember_deformed_grease_pencil_if_necessary(const GreasePencil *gre
     const greasepencil::Drawing *orig_drawing = orig_grease_pencil.get_drawing_at(
         orig_layer, grease_pencil->runtime->eval_frame);
     GreasePencilDrawingEditHints &drawing_hints = all_hints[layer_index];
-
     if (!drawing || !orig_drawing) {
       continue;
     }
-    if (drawing->strokes().points_num() != orig_drawing->strokes().points_num()) {
+    drawing_hints.drawing_orig = orig_drawing;
+    const CurvesGeometry &curves = drawing->strokes();
+    if (curves.points_num() != orig_drawing->strokes().points_num()) {
       continue;
     }
-    drawing_hints.positions.emplace(drawing->strokes().positions());
+    drawing_hints.positions_data = save_shared_attribute(curves.attributes().lookup("position"));
   }
 }
 

source/blender/blenkernel/intern/grease_pencil.cc

@@ -1570,6 +1570,40 @@ void LayerGroup::update_from_dna_read()
 
 }  // namespace blender::bke::greasepencil
 
+namespace blender::bke {
+
+std::optional<Span<float3>> GreasePencilDrawingEditHints::positions() const
+{
+  if (!this->positions_data.has_value()) {
+    return std::nullopt;
+  }
+  const int points_num = this->drawing_orig->geometry.wrap().points_num();
+  return Span(static_cast<const float3 *>(this->positions_data.data), points_num);
+}
+
+std::optional<MutableSpan<float3>> GreasePencilDrawingEditHints::positions_for_write()
+{
+  if (!this->positions_data.has_value()) {
+    return std::nullopt;
+  }
+
+  const int points_num = this->drawing_orig->geometry.wrap().points_num();
+  ImplicitSharingPtrAndData &data = this->positions_data;
+  if (data.sharing_info->is_mutable()) {
+    /* If the referenced component is already mutable, return it directly. */
+    data.sharing_info->tag_ensured_mutable();
+  }
+  else {
+    auto *new_sharing_info = new ImplicitSharedValue<Array<float3>>(*this->positions());
+    data.sharing_info = ImplicitSharingPtr<ImplicitSharingInfo>(new_sharing_info);
+    data.data = new_sharing_info->data.data();
+  }
+
+  return MutableSpan(const_cast<float3 *>(static_cast<const float3 *>(data.data)), points_num);
+}
+
+}  // namespace blender::bke
+
 /* ------------------------------------------------------------------- */
 /** \name Grease Pencil kernel functions
  * \{ */

source/blender/blenlib/BLI_implicit_sharing.hh

@@ -199,6 +199,30 @@ class ImplicitSharingMixin : public ImplicitSharingInfo {
   virtual void delete_self() = 0;
 };
 
+/**
+ * Utility for creating an allocated shared resource, to be used like:
+ * `new ImplicitSharedValue<T>(args);`
+ */
+template<typename T> class ImplicitSharedValue : public ImplicitSharingInfo {
+ public:
+  T data;
+
+  template<typename... Args>
+  ImplicitSharedValue(Args &&...args) : data(std::forward<Args>(args)...)
+  {
+  }
+
+#ifdef WITH_CXX_GUARDEDALLOC
+  MEM_CXX_CLASS_ALLOC_FUNCS("ImplicitSharedValue");
+#endif
+
+ private:
+  void delete_self_with_data() override
+  {
+    delete this;
+  }
+};
+
 /**
  * Utility that contains sharing information and the data that is shared.
  */

source/blender/blenlib/BLI_implicit_sharing_ptr.hh

@@ -8,6 +8,9 @@
  * \ingroup bli
  */
 
+#include <memory>
+#include <utility>
+
 #include "BLI_implicit_sharing.hh"
 #include "BLI_struct_equality_utils.hh"
 
@@ -132,4 +135,64 @@ template<typename T> class ImplicitSharingPtr {
   }
 };
 
+/**
+ * Utility struct to allow used #ImplicitSharingPtr when it's necessary to type-erase the backing
+ * storage for user-exposed data. For example, #blender::Vector, or #std::vector might be used to
+ * store an implicitly shared array that is only accessed with #Span or #MutableSpan.
+ *
+ * This class handles RAII for the sharing info and the exposed data pointer.
+ * Retrieving the data with write access and type safety must be handled elsewhere.
+ */
+class ImplicitSharingPtrAndData {
+ public:
+  ImplicitSharingPtr<ImplicitSharingInfo> sharing_info;
+  const void *data = nullptr;
+
+  ImplicitSharingPtrAndData() = default;
+  ImplicitSharingPtrAndData(ImplicitSharingPtr<ImplicitSharingInfo> sharing_info, const void *data)
+      : sharing_info(std::move(sharing_info)), data(data)
+  {
+  }
+
+  ImplicitSharingPtrAndData(const ImplicitSharingPtrAndData &other)
+      : sharing_info(other.sharing_info), data(other.data)
+  {
+  }
+
+  ImplicitSharingPtrAndData(ImplicitSharingPtrAndData &&other)
+      : sharing_info(std::move(other.sharing_info)), data(std::exchange(other.data, nullptr))
+  {
+  }
+
+  ImplicitSharingPtrAndData &operator=(const ImplicitSharingPtrAndData &other)
+  {
+    if (this == &other) {
+      return *this;
+    }
+    std::destroy_at(this);
+    new (this) ImplicitSharingPtrAndData(other);
+    return *this;
+  }
+
+  ImplicitSharingPtrAndData &operator=(ImplicitSharingPtrAndData &&other)
+  {
+    if (this == &other) {
+      return *this;
+    }
+    std::destroy_at(this);
+    new (this) ImplicitSharingPtrAndData(std::move(other));
+    return *this;
+  }
+
+  ~ImplicitSharingPtrAndData()
+  {
+    this->data = nullptr;
+  }
+
+  bool has_value() const
+  {
+    return this->sharing_info.has_value();
+  }
+};
+
 }  // namespace blender

source/blender/geometry/intern/transform.cc

@@ -177,8 +177,8 @@ static void translate_volume(Volume &volume, const float3 translation)
 
 static void transform_curve_edit_hints(bke::CurvesEditHints &edit_hints, const float4x4 &transform)
 {
-  if (edit_hints.positions.has_value()) {
-    transform_positions(*edit_hints.positions, transform);
+  if (const std::optional<MutableSpan<float3>> positions = edit_hints.positions_for_write()) {
+    transform_positions(*positions, transform);
   }
   float3x3 deform_mat;
   copy_m3_m4(deform_mat.ptr(), transform.ptr());
@@ -197,8 +197,8 @@ static void transform_curve_edit_hints(bke::CurvesEditHints &edit_hints, const f
 
 static void translate_curve_edit_hints(bke::CurvesEditHints &edit_hints, const float3 &translation)
 {
-  if (edit_hints.positions.has_value()) {
-    translate_positions(*edit_hints.positions, translation);
+  if (const std::optional<MutableSpan<float3>> positions = edit_hints.positions_for_write()) {
+    translate_positions(*positions, translation);
   }
 }
 

source/blender/nodes/geometry/nodes/node_geo_deform_curves_on_surface.cc

@@ -318,8 +318,8 @@ static void node_geo_exec(GeoNodeExecParams params)
   MutableSpan<float3> edit_hint_positions;
   MutableSpan<float3x3> edit_hint_rotations;
   if (edit_hints != nullptr) {
-    if (edit_hints->positions.has_value()) {
-      edit_hint_positions = *edit_hints->positions;
+    if (const std::optional<MutableSpan<float3>> positions = edit_hints->positions_for_write()) {
+      edit_hint_positions = *positions;
     }
     if (!edit_hints->deform_mats.has_value()) {
       edit_hints->deform_mats.emplace(edit_hints->curves_id_orig.geometry.point_num,

source/blender/nodes/intern/geometry_nodes_log.cc

@@ -120,7 +120,7 @@ GeometryInfoLog::GeometryInfoLog(const bke::GeometrySet &geometry_set)
         {
           EditDataInfo &info = this->edit_data_info.emplace();
           info.has_deform_matrices = curve_edit_hints->deform_mats.has_value();
-          info.has_deformed_positions = curve_edit_hints->positions.has_value();
+          info.has_deformed_positions = curve_edit_hints->positions().has_value();
         }
         break;
       }