Sculpt: New attribute API

New unified attribute API for sculpt code.

= Basic Design =

The sculpt attribute API can create temporary or permanent attributes (only supported in `PBVH_FACES` mode).  Attributes are created via `BKE_sculpt_attribute_ensure.`

Attributes can be explicit CustomData attributes or simple array-based pseudo-attributes (this is useful for PBVH_GRIDS and PBVH_BMESH).

== `SculptAttributePointers` ==

There is a structure in `SculptSession` for convenience attribute pointers, `ss->attrs`.  Standard attributes should assign these; the attribute API will automatically clear them when the associated attributes are released.  For example, the automasking code stores its factor attribute layer in `ss->attrs.automasking_factor`.

== Naming ==

Temporary attributes should use the SCULPT_ATTRIBUTE_NAME macro for naming, it takes an entry in `SculptAttributePointers` and builds a layer name.

== `SculptAttribute` ==

Attributes are referenced by a special `SculptAttribute` structure, which holds
all the info needed to look up elements of an attribute at run time.

All of these structures live in a preallocated flat array in `SculptSession`, `ss->temp_attributes`.  This is extremely important.  Since any change to the `CustomData` layout can in principle invalidate every extant `SculptAttribute`, having them all in one block of memory whose location doesn't change allows us to update them transparently.

This makes for much simpler code and eliminates bugs.  To see why this is tricky to get right, imagine we want to create three attributes in PBVH_BMESH mode and we provide our own `SculptAttribute` structs for the API to fill in.  Each new layer will invalidate the `CustomData` block offsets in the prior one, leading to memory corruption.

Reviewed by: Brecht Van Lommel
Differential Revision: https://developer.blender.org/D15496
Ref D15496

7 files changed, 72 insertions, 95 deletions

diff --git a/source/blender/editors/sculpt_paint/sculpt.c b/source/blender/editors/sculpt_paint/sculpt.c
index 1cb466db304..53f65960ede 100644
--- a/source/blender/editors/sculpt_paint/sculpt.c
+++ b/source/blender/editors/sculpt_paint/sculpt.c
@@ -194,9 +194,10 @@ void SCULPT_vertex_normal_get(SculptSession *ss, PBVHVertRef vertex, float no[3]
 
 const float *SCULPT_vertex_persistent_co_get(SculptSession *ss, PBVHVertRef vertex)
 {
-  if (ss->persistent_base) {
-    return ss->persistent_base[BKE_pbvh_vertex_to_index(ss->pbvh, vertex)].co;
+  if (ss->attrs.persistent_co) {
+    return (const float *)SCULPT_vertex_attr_get(vertex, ss->attrs.persistent_co);
   }
+
   return SCULPT_vertex_co_get(ss, vertex);
 }
 
@@ -240,8 +241,8 @@ void SCULPT_vertex_limit_surface_get(SculptSession *ss, PBVHVertRef vertex, floa
 
 void SCULPT_vertex_persistent_normal_get(SculptSession *ss, PBVHVertRef vertex, float no[3])
 {
-  if (ss->persistent_base) {
-    copy_v3_v3(no, ss->persistent_base[vertex.i].no);
+  if (ss->attrs.persistent_no) {
+    copy_v3_v3(no, (float *)SCULPT_vertex_attr_get(vertex, ss->attrs.persistent_no));
     return;
   }
   SCULPT_vertex_normal_get(ss, vertex, no);
@@ -5322,6 +5323,8 @@ void SCULPT_flush_update_done(const bContext *C, Object *ob, SculptUpdateType up
     BKE_pbvh_update_vertex_data(ss->pbvh, PBVH_UpdateColor);
   }
 
+  BKE_sculpt_attributes_destroy_temporary_stroke(ob);
+
   if (BKE_pbvh_type(ss->pbvh) == PBVH_BMESH) {
     BKE_pbvh_bmesh_after_stroke(ss->pbvh);
   }
diff --git a/source/blender/editors/sculpt_paint/sculpt_automasking.cc b/source/blender/editors/sculpt_paint/sculpt_automasking.cc
index a9fe8cc4b2f..34d7a459c1b 100644
--- a/source/blender/editors/sculpt_paint/sculpt_automasking.cc
+++ b/source/blender/editors/sculpt_paint/sculpt_automasking.cc
@@ -122,13 +122,11 @@ float SCULPT_automasking_factor_get(AutomaskingCache *automasking,
     return 1.0f;
   }
 
-  int index = BKE_pbvh_vertex_to_index(ss->pbvh, vert);
-
   /* If the cache is initialized with valid info, use the cache. This is used when the
    * automasking information can't be computed in real time per vertex and needs to be
    * initialized for the whole mesh when the stroke starts. */
-  if (automasking->factor) {
-    return automasking->factor[index];
+  if (ss->attrs.automasking_factor) {
+    return *(float *)SCULPT_vertex_attr_get(vert, ss->attrs.automasking_factor);
   }
 
   if (automasking->settings.flags & BRUSH_AUTOMASKING_FACE_SETS) {
@@ -158,7 +156,6 @@ void SCULPT_automasking_cache_free(AutomaskingCache *automasking)
     return;
   }
 
-  MEM_SAFE_FREE(automasking->factor);
   MEM_SAFE_FREE(automasking);
 }
 
@@ -176,7 +173,6 @@ static bool sculpt_automasking_is_constrained_by_radius(Brush *br)
 }
 
 struct AutomaskFloodFillData {
-  float *automask_factor;
   float radius;
   bool use_radius;
   float location[3];
@@ -190,29 +186,29 @@ static bool automask_floodfill_cb(SculptSession *ss,
                                   void *userdata)
 {
   AutomaskFloodFillData *data = (AutomaskFloodFillData *)userdata;
-  int from_v_i = BKE_pbvh_vertex_to_index(ss->pbvh, from_v);
-  int to_v_i = BKE_pbvh_vertex_to_index(ss->pbvh, to_v);
 
-  data->automask_factor[to_v_i] = 1.0f;
-  data->automask_factor[from_v_i] = 1.0f;
+  *(float *)SCULPT_vertex_attr_get(to_v, ss->attrs.automasking_factor) = 1.0f;
+  *(float *)SCULPT_vertex_attr_get(from_v, ss->attrs.automasking_factor) = 1.0f;
   return (!data->use_radius ||
           SCULPT_is_vertex_inside_brush_radius_symm(
               SCULPT_vertex_co_get(ss, to_v), data->location, data->radius, data->symm));
 }
 
-static float *SCULPT_topology_automasking_init(Sculpt *sd, Object *ob, float *automask_factor)
+static void SCULPT_topology_automasking_init(Sculpt *sd, Object *ob)
 {
   SculptSession *ss = ob->sculpt;
   Brush *brush = BKE_paint_brush(&sd->paint);
 
   if (BKE_pbvh_type(ss->pbvh) == PBVH_FACES && !ss->pmap) {
     BLI_assert_msg(0, "Topology masking: pmap missing");
-    return nullptr;
+    return;
   }
 
   const int totvert = SCULPT_vertex_count_get(ss);
   for (int i : IndexRange(totvert)) {
-    automask_factor[i] = 0.0f;
+    PBVHVertRef vertex = BKE_pbvh_index_to_vertex(ss->pbvh, i);
+
+    (*(float *)SCULPT_vertex_attr_get(vertex, ss->attrs.automasking_factor)) = 0.0f;
   }
 
   /* Flood fill automask to connected vertices. Limited to vertices inside
@@ -222,9 +218,8 @@ static float *SCULPT_topology_automasking_init(Sculpt *sd, Object *ob, float *au
   const float radius = ss->cache ? ss->cache->radius : FLT_MAX;
   SCULPT_floodfill_add_active(sd, ob, ss, &flood, radius);
 
-  AutomaskFloodFillData fdata = {nullptr};
+  AutomaskFloodFillData fdata = {0};
 
-  fdata.automask_factor = automask_factor;
   fdata.radius = radius;
   fdata.use_radius = ss->cache && sculpt_automasking_is_constrained_by_radius(brush);
   fdata.symm = SCULPT_mesh_symmetry_xyz_get(ob);
@@ -232,22 +227,20 @@ static float *SCULPT_topology_automasking_init(Sculpt *sd, Object *ob, float *au
   copy_v3_v3(fdata.location, SCULPT_active_vertex_co_get(ss));
   SCULPT_floodfill_execute(ss, &flood, automask_floodfill_cb, &fdata);
   SCULPT_floodfill_free(&flood);
-
-  return automask_factor;
 }
 
-static float *sculpt_face_sets_automasking_init(Sculpt *sd, Object *ob, float *automask_factor)
+static void sculpt_face_sets_automasking_init(Sculpt *sd, Object *ob)
 {
   SculptSession *ss = ob->sculpt;
   Brush *brush = BKE_paint_brush(&sd->paint);
 
   if (!SCULPT_is_automasking_enabled(sd, ss, brush)) {
-    return nullptr;
+    return;
   }
 
   if (BKE_pbvh_type(ss->pbvh) == PBVH_FACES && !ss->pmap) {
     BLI_assert_msg(0, "Face Sets automasking: pmap missing");
-    return nullptr;
+    return;
   }
 
   int tot_vert = SCULPT_vertex_count_get(ss);
@@ -256,25 +249,22 @@ static float *sculpt_face_sets_automasking_init(Sculpt *sd, Object *ob, float *a
     PBVHVertRef vertex = BKE_pbvh_index_to_vertex(ss->pbvh, i);
 
     if (!SCULPT_vertex_has_face_set(ss, vertex, active_face_set)) {
-      automask_factor[i] *= 0.0f;
+      *(float *)SCULPT_vertex_attr_get(vertex, ss->attrs.automasking_factor) = 0.0f;
     }
   }
-
-  return automask_factor;
 }
 
 #define EDGE_DISTANCE_INF -1
 
-float *SCULPT_boundary_automasking_init(Object *ob,
-                                        eBoundaryAutomaskMode mode,
-                                        int propagation_steps,
-                                        float *automask_factor)
+void SCULPT_boundary_automasking_init(Object *ob,
+                                      eBoundaryAutomaskMode mode,
+                                      int propagation_steps)
 {
   SculptSession *ss = ob->sculpt;
 
   if (!ss->pmap) {
     BLI_assert_msg(0, "Boundary Edges masking: pmap missing");
-    return nullptr;
+    return;
   }
 
   const int totvert = SCULPT_vertex_count_get(ss);
@@ -316,16 +306,19 @@ float *SCULPT_boundary_automasking_init(Object *ob,
   }
 
   for (int i : IndexRange(totvert)) {
+    PBVHVertRef vertex = BKE_pbvh_index_to_vertex(ss->pbvh, i);
+
     if (edge_distance[i] == EDGE_DISTANCE_INF) {
       continue;
     }
     const float p = 1.0f - ((float)edge_distance[i] / (float)propagation_steps);
     const float edge_boundary_automask = pow2f(p);
-    automask_factor[i] *= (1.0f - edge_boundary_automask);
+
+    *(float *)SCULPT_vertex_attr_get(
+        vertex, ss->attrs.automasking_factor) *= (1.0f - edge_boundary_automask);
   }
 
   MEM_SAFE_FREE(edge_distance);
-  return automask_factor;
 }
 
 static void SCULPT_automasking_cache_settings_update(AutomaskingCache *automasking,
@@ -355,9 +348,16 @@ AutomaskingCache *SCULPT_automasking_cache_init(Sculpt *sd, Brush *brush, Object
     return automasking;
   }
 
-  automasking->factor = (float *)MEM_malloc_arrayN(totvert, sizeof(float), "automask_factor");
+  SculptAttributeParams params = {0};
+  params.stroke_only = true;
+
+  ss->attrs.automasking_factor = BKE_sculpt_attribute_ensure(
+      ob, ATTR_DOMAIN_POINT, CD_PROP_FLOAT, SCULPT_ATTRIBUTE_NAME(automasking_factor), &params);
+
   for (int i : IndexRange(totvert)) {
-    automasking->factor[i] = 1.0f;
+    PBVHVertRef vertex = BKE_pbvh_index_to_vertex(ss->pbvh, i);
+
+    (*(float *)SCULPT_vertex_attr_get(vertex, ss->attrs.automasking_factor)) = 0.0f;
   }
 
   const int boundary_propagation_steps = brush ?
@@ -366,22 +366,21 @@ AutomaskingCache *SCULPT_automasking_cache_init(Sculpt *sd, Brush *brush, Object
 
   if (SCULPT_is_automasking_mode_enabled(sd, brush, BRUSH_AUTOMASKING_TOPOLOGY)) {
     SCULPT_vertex_random_access_ensure(ss);
-    SCULPT_topology_automasking_init(sd, ob, automasking->factor);
+    SCULPT_topology_automasking_init(sd, ob);
   }
   if (SCULPT_is_automasking_mode_enabled(sd, brush, BRUSH_AUTOMASKING_FACE_SETS)) {
     SCULPT_vertex_random_access_ensure(ss);
-    sculpt_face_sets_automasking_init(sd, ob, automasking->factor);
+    sculpt_face_sets_automasking_init(sd, ob);
   }
 
   if (SCULPT_is_automasking_mode_enabled(sd, brush, BRUSH_AUTOMASKING_BOUNDARY_EDGES)) {
     SCULPT_vertex_random_access_ensure(ss);
-    SCULPT_boundary_automasking_init(
-        ob, AUTOMASK_INIT_BOUNDARY_EDGES, boundary_propagation_steps, automasking->factor);
+    SCULPT_boundary_automasking_init(ob, AUTOMASK_INIT_BOUNDARY_EDGES, boundary_propagation_steps);
   }
   if (SCULPT_is_automasking_mode_enabled(sd, brush, BRUSH_AUTOMASKING_BOUNDARY_FACE_SETS)) {
     SCULPT_vertex_random_access_ensure(ss);
     SCULPT_boundary_automasking_init(
-        ob, AUTOMASK_INIT_BOUNDARY_FACE_SETS, boundary_propagation_steps, automasking->factor);
+        ob, AUTOMASK_INIT_BOUNDARY_FACE_SETS, boundary_propagation_steps);
   }
 
   return automasking;
diff --git a/source/blender/editors/sculpt_paint/sculpt_brush_types.c b/source/blender/editors/sculpt_paint/sculpt_brush_types.c
index 245cbe0f54e..e849cc9e5f4 100644
--- a/source/blender/editors/sculpt_paint/sculpt_brush_types.c
+++ b/source/blender/editors/sculpt_paint/sculpt_brush_types.c
@@ -1472,7 +1472,7 @@ static void do_layer_brush_task_cb_ex(void *__restrict userdata,
   Sculpt *sd = data->sd;
   const Brush *brush = data->brush;
 
-  const bool use_persistent_base = ss->persistent_base && brush->flag & BRUSH_PERSISTENT;
+  const bool use_persistent_base = !ss->bm && ss->attrs.persistent_co && brush->flag & BRUSH_PERSISTENT;
 
   PBVHVertexIter vd;
   SculptOrigVertData orig_data;
@@ -1503,7 +1503,7 @@ static void do_layer_brush_task_cb_ex(void *__restrict userdata,
     const int vi = vd.index;
     float *disp_factor;
     if (use_persistent_base) {
-      disp_factor = &ss->persistent_base[vi].disp;
+      disp_factor = (float *)SCULPT_vertex_attr_get(vd.vertex, ss->attrs.persistent_disp);
     }
     else {
       disp_factor = &ss->cache->layer_displacement_factor[vi];
diff --git a/source/blender/editors/sculpt_paint/sculpt_dyntopo.c b/source/blender/editors/sculpt_paint/sculpt_dyntopo.c
index 46674c5d239..1126d34bcee 100644
--- a/source/blender/editors/sculpt_paint/sculpt_dyntopo.c
+++ b/source/blender/editors/sculpt_paint/sculpt_dyntopo.c
@@ -88,45 +88,6 @@ void SCULPT_pbvh_clear(Object *ob)
   DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY);
 }
 
-void SCULPT_dyntopo_node_layers_add(SculptSession *ss)
-{
-  int cd_node_layer_index;
-
-  char node_vertex_id[] = "_dyntopo_vnode_id";
-  char node_face_id[] = "_dyntopo_fnode_id";
-
-  cd_node_layer_index = CustomData_get_named_layer_index(
-      &ss->bm->vdata, CD_PROP_INT32, node_vertex_id);
-
-  if (cd_node_layer_index == -1) {
-    BM_data_layer_add_named(ss->bm, &ss->bm->vdata, CD_PROP_INT32, node_vertex_id);
-    cd_node_layer_index = CustomData_get_named_layer_index(
-        &ss->bm->vdata, CD_PROP_INT32, node_vertex_id);
-  }
-
-  ss->cd_vert_node_offset = CustomData_get_n_offset(
-      &ss->bm->vdata,
-      CD_PROP_INT32,
-      cd_node_layer_index - CustomData_get_layer_index(&ss->bm->vdata, CD_PROP_INT32));
-
-  ss->bm->vdata.layers[cd_node_layer_index].flag |= CD_FLAG_TEMPORARY;
-
-  cd_node_layer_index = CustomData_get_named_layer_index(
-      &ss->bm->pdata, CD_PROP_INT32, node_face_id);
-  if (cd_node_layer_index == -1) {
-    BM_data_layer_add_named(ss->bm, &ss->bm->pdata, CD_PROP_INT32, node_face_id);
-    cd_node_layer_index = CustomData_get_named_layer_index(
-        &ss->bm->pdata, CD_PROP_INT32, node_face_id);
-  }
-
-  ss->cd_face_node_offset = CustomData_get_n_offset(
-      &ss->bm->pdata,
-      CD_PROP_INT32,
-      cd_node_layer_index - CustomData_get_layer_index(&ss->bm->pdata, CD_PROP_INT32));
-
-  ss->bm->pdata.layers[cd_node_layer_index].flag |= CD_FLAG_TEMPORARY;
-}
-
 void SCULPT_dynamic_topology_enable_ex(Main *bmain, Depsgraph *depsgraph, Scene *scene, Object *ob)
 {
   SculptSession *ss = ob->sculpt;
@@ -156,8 +117,9 @@ void SCULPT_dynamic_topology_enable_ex(Main *bmain, Depsgraph *depsgraph, Scene
                          .active_shapekey = ob->shapenr,
                      }));
   SCULPT_dynamic_topology_triangulate(ss->bm);
+
   BM_data_layer_add(ss->bm, &ss->bm->vdata, CD_PAINT_MASK);
-  SCULPT_dyntopo_node_layers_add(ss);
+  
   /* Make sure the data for existing faces are initialized. */
   if (me->totpoly != ss->bm->totface) {
     BM_mesh_normals_update(ss->bm);
@@ -185,6 +147,9 @@ static void SCULPT_dynamic_topology_disable_ex(
   SculptSession *ss = ob->sculpt;
   Mesh *me = ob->data;
 
+  BKE_sculpt_attribute_destroy(ob, ss->attrs.dyntopo_node_id_vertex);
+  BKE_sculpt_attribute_destroy(ob, ss->attrs.dyntopo_node_id_face);
+
   SCULPT_pbvh_clear(ob);
 
   if (unode) {
diff --git a/source/blender/editors/sculpt_paint/sculpt_intern.h b/source/blender/editors/sculpt_paint/sculpt_intern.h
index 681c90bafb6..cdfa9c2586f 100644
--- a/source/blender/editors/sculpt_paint/sculpt_intern.h
+++ b/source/blender/editors/sculpt_paint/sculpt_intern.h
@@ -402,9 +402,6 @@ typedef struct AutomaskingSettings {
 
 typedef struct AutomaskingCache {
   AutomaskingSettings settings;
-  /* Precomputed auto-mask factor indexed by vertex, owned by the auto-masking system and
-   * initialized in #SCULPT_automasking_cache_init when needed. */
-  float *factor;
 } AutomaskingCache;
 
 typedef struct FilterCache {
@@ -1273,7 +1270,6 @@ void sculpt_dynamic_topology_disable_with_undo(struct Main *bmain,
 bool SCULPT_stroke_is_dynamic_topology(const SculptSession *ss, const Brush *brush);
 
 void SCULPT_dynamic_topology_triangulate(struct BMesh *bm);
-void SCULPT_dyntopo_node_layers_add(struct SculptSession *ss);
 
 enum eDynTopoWarnFlag SCULPT_dynamic_topology_check(Scene *scene, Object *ob);
 
@@ -1849,3 +1845,8 @@ BLI_INLINE bool SCULPT_tool_is_face_sets(int tool)
 #ifdef __cplusplus
 }
 #endif
+
+/* Make SCULPT_ alias to a few blenkernel sculpt methods. */
+
+#define SCULPT_vertex_attr_get BKE_sculpt_vertex_attr_get
+#define SCULPT_face_attr_get BKE_sculpt_face_attr_get
diff --git a/source/blender/editors/sculpt_paint/sculpt_ops.c b/source/blender/editors/sculpt_paint/sculpt_ops.c
index cfd02438188..39c77129f0e 100644
--- a/source/blender/editors/sculpt_paint/sculpt_ops.c
+++ b/source/blender/editors/sculpt_paint/sculpt_ops.c
@@ -117,24 +117,33 @@ static int sculpt_set_persistent_base_exec(bContext *C, wmOperator *UNUSED(op))
   Object *ob = CTX_data_active_object(C);
   SculptSession *ss = ob->sculpt;
 
-  if (!ss) {
+  /* Do not allow in DynTopo just yet. */
+  if (!ss || (ss && ss->bm)) {
     return OPERATOR_FINISHED;
   }
   SCULPT_vertex_random_access_ensure(ss);
   BKE_sculpt_update_object_for_edit(depsgraph, ob, false, false, false);
 
-  MEM_SAFE_FREE(ss->persistent_base);
+  SculptAttributeParams params = {0};
+  params.permanent = true;
+
+  ss->attrs.persistent_co = BKE_sculpt_attribute_ensure(
+      ob, ATTR_DOMAIN_POINT, CD_PROP_FLOAT3, SCULPT_ATTRIBUTE_NAME(persistent_co), &params);
+  ss->attrs.persistent_no = BKE_sculpt_attribute_ensure(
+      ob, ATTR_DOMAIN_POINT, CD_PROP_FLOAT3, SCULPT_ATTRIBUTE_NAME(persistent_no), &params);
+  ss->attrs.persistent_disp = BKE_sculpt_attribute_ensure(
+      ob, ATTR_DOMAIN_POINT, CD_PROP_FLOAT, SCULPT_ATTRIBUTE_NAME(persistent_disp), &params);
 
   const int totvert = SCULPT_vertex_count_get(ss);
-  ss->persistent_base = MEM_mallocN(sizeof(SculptPersistentBase) * totvert,
-                                    "layer persistent base");
 
   for (int i = 0; i < totvert; i++) {
     PBVHVertRef vertex = BKE_pbvh_index_to_vertex(ss->pbvh, i);
 
-    copy_v3_v3(ss->persistent_base[i].co, SCULPT_vertex_co_get(ss, vertex));
-    SCULPT_vertex_normal_get(ss, vertex, ss->persistent_base[i].no);
-    ss->persistent_base[i].disp = 0.0f;
+    copy_v3_v3((float *)SCULPT_vertex_attr_get(vertex, ss->attrs.persistent_co),
+               SCULPT_vertex_co_get(ss, vertex));
+    SCULPT_vertex_normal_get(
+        ss, vertex, (float *)SCULPT_vertex_attr_get(vertex, ss->attrs.persistent_no));
+    (*(float *)SCULPT_vertex_attr_get(vertex, ss->attrs.persistent_disp)) = 0.0f;
   }
 
   return OPERATOR_FINISHED;
diff --git a/source/blender/editors/sculpt_paint/sculpt_undo.c b/source/blender/editors/sculpt_paint/sculpt_undo.c
index 9bf63ad1468..d37bf9b052d 100644
--- a/source/blender/editors/sculpt_paint/sculpt_undo.c
+++ b/source/blender/editors/sculpt_paint/sculpt_undo.c
@@ -547,7 +547,7 @@ static void sculpt_undo_bmesh_enable(Object *ob, SculptUndoNode *unode)
                               .use_toolflags = false,
                           }));
   BM_data_layer_add(ss->bm, &ss->bm->vdata, CD_PAINT_MASK);
-  SCULPT_dyntopo_node_layers_add(ss);
+  
   me->flag |= ME_SCULPT_DYNAMIC_TOPOLOGY;
 
   /* Restore the BMLog using saved entries. */