[Bf-blender-cvs] [a7c65ef4cbb] master: Fix T96498: Modifiers affect multiple curve objects

[Hans Goudey]

Commit: a7c65ef4cbbd71972170cc8406a85b2332d304f2
Author: Hans Goudey
Date:   Fri Apr 22 10:26:08 2022 -0500
Branches: master
https://developer.blender.org/rBa7c65ef4cbbd71972170cc8406a85b2332d304f2

Fix T96498: Modifiers affect multiple curve objects

The original mistake I made in b9febb54a492ac6c938 was thinking
that the input curve object data to `BKE_displist_make_curveTypes`
was already copied from the original. I think I misread some of its
`ID` flags. This commit places the result of curves evaluation in a
duplicated curve instead, and copies the edit mode pointers
necessary for drawing overlays. `Curve` needs to know not to
free those pointers.

I still don't have a full understanding of why some of the tactics I've
used work and others don't. I've probably tried around 8 different
solutions at this point, and this is the best I came up with.

The dependency graph seems to have some handling of edit mode
pointers that make the edit mode overlays work if the evaluated
result is only an empty curve created by the evaluated geometry set.
This doesn't work with the current method and I need to set the
edit mode pointers at the end of evaluation explicitly.

We're constrained by the confusing duality of the old curves system
combined with the new design using the evaluated geometry set.
Older areas of Blender expect the evaluated `Curve` to be a copy
of the original, even if it was replaced by some arbitrary evaluated mesh.

Differential Revision: https://developer.blender.org/D14561

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

M	source/blender/blenkernel/intern/curve.cc
M	source/blender/blenkernel/intern/displist.cc
M	source/blender/blenkernel/intern/object.cc
M	source/blender/makesdna/DNA_curve_types.h

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

diff --git a/source/blender/blenkernel/intern/curve.cc b/source/blender/blenkernel/intern/curve.cc
index bad70d4ccc6..4338883853d 100644
--- a/source/blender/blenkernel/intern/curve.cc
+++ b/source/blender/blenkernel/intern/curve.cc
@@ -113,9 +113,12 @@ static void curve_free_data(ID *id)
   BKE_curve_batch_cache_free(curve);
 
   BKE_nurbList_free(&curve->nurb);
-  BKE_curve_editfont_free(curve);
 
-  BKE_curve_editNurb_free(curve);
+  if (!curve->edit_data_from_original) {
+    BKE_curve_editfont_free(curve);
+
+    BKE_curve_editNurb_free(curve);
+  }
 
   BKE_curveprofile_free(curve->bevel_profile);
 
diff --git a/source/blender/blenkernel/intern/displist.cc b/source/blender/blenkernel/intern/displist.cc
index 5caed6d6219..63aa03483b2 100644
--- a/source/blender/blenkernel/intern/displist.cc
+++ b/source/blender/blenkernel/intern/displist.cc
@@ -1466,10 +1466,12 @@ void BKE_displist_make_curveTypes(Depsgraph *depsgraph,
                                   const bool for_render)
 {
   BLI_assert(ELEM(ob->type, OB_SURF, OB_CURVES_LEGACY, OB_FONT));
-  Curve &cow_curve = *(Curve *)ob->data;
 
   BKE_object_free_derived_caches(ob);
-  cow_curve.curve_eval = nullptr;
+
+  /* It's important to retrieve this after calling #BKE_object_free_derived_caches,
+   * which may reset the object data pointer in some cases. */
+  const Curve &original_curve = *static_cast<const Curve *>(ob->data);
 
   ob->runtime.curve_cache = MEM_cnew<CurveCache>(__func__);
   ListBase *dispbase = &ob->runtime.curve_cache->disp;
@@ -1482,14 +1484,27 @@ void BKE_displist_make_curveTypes(Depsgraph *depsgraph,
     GeometrySet geometry = evaluate_curve_type_object(depsgraph, scene, ob, for_render, dispbase);
 
     if (geometry.has_curves()) {
-      /* Assign the evaluated curve to the object's "data_eval". In addition to the curve_eval
-       * added to the curve here, it will also contain a copy of the original curve's data. This is
-       * essential, because it maintains the expected behavior for evaluated curve data from before
-       * the CurveEval data type was introduced, when an evaluated object's curve data was just a
-       * copy of the original curve and everything else ended up in #CurveCache. */
-      CurveComponent &curve_component = geometry.get_component_for_write<CurveComponent>();
-      cow_curve.curve_eval = curve_component.get_for_read();
-      BKE_object_eval_assign_data(ob, &cow_curve.id, false);
+      /* Create a copy of the original curve and add necessary pointers to evaluated and edit mode
+       * data. This is needed for a few reasons:
+       * - Existing code from before curve evaluation was changed to use #GeometrySet expected to
+       *   have a copy of the original curve data. (Any evaluated data was placed in
+       *   #Object.runtime.curve_cache).
+       * - The result of modifier evaluation is not a #Curve data-block but a #Curves data-block,
+       *   which can support constructive modifiers and geometry nodes.
+       * - The dependency graph has handling of edit mode pointers (see #update_edit_mode_pointers)
+       *   but it doesn't seem to work in this case.
+       *
+       * Since the the plan is to replace this legacy curve object with the curves data-block
+       * (see T95355), this somewhat hacky inefficient solution is relatively temporary.
+       */
+      Curve &cow_curve = *reinterpret_cast<Curve *>(
+          BKE_id_copy_ex(nullptr, &original_curve.id, nullptr, LIB_ID_COPY_LOCALIZE));
+      cow_curve.curve_eval = geometry.get_curves_for_read();
+      /* Copy edit mode pointers necessary for drawing to the duplicated curve. */
+      cow_curve.editnurb = original_curve.editnurb;
+      cow_curve.editfont = original_curve.editfont;
+      cow_curve.edit_data_from_original = true;
+      BKE_object_eval_assign_data(ob, &cow_curve.id, true);
     }
 
     ob->runtime.geometry_set_eval = new GeometrySet(std::move(geometry));
diff --git a/source/blender/blenkernel/intern/object.cc b/source/blender/blenkernel/intern/object.cc
index bafdbb6bb7e..948064ad170 100644
--- a/source/blender/blenkernel/intern/object.cc
+++ b/source/blender/blenkernel/intern/object.cc
@@ -1789,6 +1789,7 @@ void BKE_object_free_derived_caches(Object *ob)
         BKE_mesh_eval_delete((Mesh *)data_eval);
       }
       else {
+        BKE_libblock_free_data(data_eval, false);
         BKE_libblock_free_datablock(data_eval, 0);
         MEM_freeN(data_eval);
       }
diff --git a/source/blender/makesdna/DNA_curve_types.h b/source/blender/makesdna/DNA_curve_types.h
index 305b913b19e..6e18d442ee2 100644
--- a/source/blender/makesdna/DNA_curve_types.h
+++ b/source/blender/makesdna/DNA_curve_types.h
@@ -298,6 +298,13 @@ typedef struct Curve {
    * original object data.
    */
   const struct Curves *curve_eval;
+  /**
+   * If non-zero, the #editfont and #editnurb pointers are not owned by this #Curve. That means
+   * this curve is a container for the result of object geometry evaluation. This only works
+   * because evaluated object data never outlives original data.
+   */
+  char edit_data_from_original;
+  char _pad3[7];
 
   void *batch_cache;
 } Curve;