Attributes: Use names instead of layers for some functions

This mirrors the C++ attribute API better, separates the implementation
of attributes from CustomData slightly, and makes functions simpler,
clearer, and safer.

Also fix an issue with removing an attribute caused by 97712b018df71c
meant the first attribute with the given type was removed instead of
the attribute with the given name.

10 files changed, 76 insertions, 72 deletions

diff --git a/source/blender/blenkernel/BKE_attribute.h b/source/blender/blenkernel/BKE_attribute.h
index 65d93089002..52a3e01026c 100644
--- a/source/blender/blenkernel/BKE_attribute.h
+++ b/source/blender/blenkernel/BKE_attribute.h
@@ -58,9 +58,7 @@ bool BKE_attribute_allow_procedural_access(const char *attribute_name);
  */
 struct CustomDataLayer *BKE_id_attribute_new(
     struct ID *id, const char *name, int type, eAttrDomain domain, struct ReportList *reports);
-bool BKE_id_attribute_remove(struct ID *id,
-                             struct CustomDataLayer *layer,
-                             struct ReportList *reports);
+bool BKE_id_attribute_remove(struct ID *id, const char *name, struct ReportList *reports);
 
 struct CustomDataLayer *BKE_id_attribute_find(const struct ID *id,
                                               const char *name,
@@ -74,9 +72,9 @@ struct CustomDataLayer *BKE_id_attribute_search(const struct ID *id,
 
 eAttrDomain BKE_id_attribute_domain(const struct ID *id, const struct CustomDataLayer *layer);
 int BKE_id_attribute_data_length(struct ID *id, struct CustomDataLayer *layer);
-bool BKE_id_attribute_required(const struct ID *id, struct CustomDataLayer *layer);
+bool BKE_id_attribute_required(const struct ID *id, const char *name);
 bool BKE_id_attribute_rename(struct ID *id,
-                             struct CustomDataLayer *layer,
+                             const char *old_name,
                              const char *new_name,
                              struct ReportList *reports);
 
diff --git a/source/blender/blenkernel/BKE_curves.h b/source/blender/blenkernel/BKE_curves.h
index f86df8b7434..c3302c6d2aa 100644
--- a/source/blender/blenkernel/BKE_curves.h
+++ b/source/blender/blenkernel/BKE_curves.h
@@ -25,7 +25,7 @@ void *BKE_curves_add(struct Main *bmain, const char *name);
 
 struct BoundBox *BKE_curves_boundbox_get(struct Object *ob);
 
-bool BKE_curves_customdata_required(const struct Curves *curves, struct CustomDataLayer *layer);
+bool BKE_curves_customdata_required(const struct Curves *curves, const char *name);
 
 /* Depsgraph */
 
diff --git a/source/blender/blenkernel/BKE_pointcloud.h b/source/blender/blenkernel/BKE_pointcloud.h
index 50537d982ef..6dbba11a56d 100644
--- a/source/blender/blenkernel/BKE_pointcloud.h
+++ b/source/blender/blenkernel/BKE_pointcloud.h
@@ -30,8 +30,7 @@ struct BoundBox *BKE_pointcloud_boundbox_get(struct Object *ob);
 bool BKE_pointcloud_minmax(const struct PointCloud *pointcloud, float r_min[3], float r_max[3]);
 
 void BKE_pointcloud_update_customdata_pointers(struct PointCloud *pointcloud);
-bool BKE_pointcloud_customdata_required(const struct PointCloud *pointcloud,
-                                        struct CustomDataLayer *layer);
+bool BKE_pointcloud_customdata_required(const struct PointCloud *pointcloud, const char *name);
 
 /* Dependency Graph */
 
diff --git a/source/blender/blenkernel/intern/attribute.cc b/source/blender/blenkernel/intern/attribute.cc
index 509161386cd..eac42d19b52 100644
--- a/source/blender/blenkernel/intern/attribute.cc
+++ b/source/blender/blenkernel/intern/attribute.cc
@@ -89,22 +89,6 @@ static void get_domains(const ID *id, DomainInfo info[ATTR_DOMAIN_NUM])
   }
 }
 
-static CustomData *attribute_customdata_find(ID *id, CustomDataLayer *layer)
-{
-  DomainInfo info[ATTR_DOMAIN_NUM];
-  get_domains(id, info);
-
-  for (const int domain : IndexRange(ATTR_DOMAIN_NUM)) {
-    CustomData *customdata = info[domain].customdata;
-    if (customdata &&
-        ARRAY_HAS_ITEM((CustomDataLayer *)layer, customdata->layers, customdata->totlayer)) {
-      return customdata;
-    }
-  }
-
-  return nullptr;
-}
-
 bool BKE_id_attributes_supported(const ID *id)
 {
   DomainInfo info[ATTR_DOMAIN_NUM];
@@ -123,17 +107,18 @@ bool BKE_attribute_allow_procedural_access(const char *attribute_name)
 }
 
 bool BKE_id_attribute_rename(ID *id,
-                             CustomDataLayer *layer,
+                             const char *old_name,
                              const char *new_name,
                              ReportList *reports)
 {
-  if (BKE_id_attribute_required(id, layer)) {
+  if (BKE_id_attribute_required(id, old_name)) {
     BLI_assert_msg(0, "Required attribute name is not editable");
     return false;
   }
 
-  CustomData *customdata = attribute_customdata_find(id, layer);
-  if (customdata == nullptr) {
+  CustomDataLayer *layer = BKE_id_attribute_search(
+      id, old_name, CD_MASK_PROP_ALL, ATTR_DOMAIN_MASK_ALL);
+  if (layer == nullptr) {
     BKE_report(reports, RPT_ERROR, "Attribute is not part of this geometry");
     return false;
   }
@@ -223,44 +208,41 @@ CustomDataLayer *BKE_id_attribute_new(
   return (index == -1) ? nullptr : &(customdata->layers[index]);
 }
 
-bool BKE_id_attribute_remove(ID *id, CustomDataLayer *layer, ReportList *reports)
+bool BKE_id_attribute_remove(ID *id, const char *name, ReportList *reports)
 {
-  CustomData *customdata = attribute_customdata_find(id, layer);
-  const int index = (customdata) ?
-                        CustomData_get_named_layer_index(customdata, layer->type, layer->name) :
-                        -1;
-
-  if (index == -1) {
-    BKE_report(reports, RPT_ERROR, "Attribute is not part of this geometry");
-    return false;
-  }
-
-  if (BKE_id_attribute_required(id, layer)) {
+  if (BKE_id_attribute_required(id, name)) {
     BKE_report(reports, RPT_ERROR, "Attribute is required and can't be removed");
     return false;
   }
 
+  DomainInfo info[ATTR_DOMAIN_NUM];
+  get_domains(id, info);
+
   switch (GS(id->name)) {
     case ID_ME: {
-      Mesh *me = (Mesh *)id;
-      BMEditMesh *em = me->edit_mesh;
-      if (em != nullptr) {
-        BM_data_layer_free(em->bm, customdata, layer->type);
-      }
-      else {
-        const int length = BKE_id_attribute_data_length(id, layer);
-        CustomData_free_layer(customdata, layer->type, length, index);
+      Mesh *mesh = reinterpret_cast<Mesh *>(id);
+      if (BMEditMesh *em = mesh->edit_mesh) {
+        for (const int domain : IndexRange(ATTR_DOMAIN_NUM)) {
+          if (CustomData *data = info[domain].customdata) {
+            if (BM_data_layer_free_named(em->bm, data, name)) {
+              return true;
+            }
+          }
+        }
+        return false;
       }
-      break;
-    }
-    default: {
-      const int length = BKE_id_attribute_data_length(id, layer);
-      CustomData_free_layer(customdata, layer->type, length, index);
-      break;
+      ATTR_FALLTHROUGH;
     }
+    default:
+      for (const int domain : IndexRange(ATTR_DOMAIN_NUM)) {
+        if (CustomData *data = info[domain].customdata) {
+          if (CustomData_free_layer_named(data, name, info[domain].length)) {
+            return true;
+          }
+        }
+      }
+      return false;
   }
-
-  return true;
 }
 
 CustomDataLayer *BKE_id_attribute_find(const ID *id,
@@ -382,14 +364,14 @@ int BKE_id_attribute_data_length(ID *id, CustomDataLayer *layer)
   return 0;
 }
 
-bool BKE_id_attribute_required(const ID *id, CustomDataLayer *layer)
+bool BKE_id_attribute_required(const ID *id, const char *name)
 {
   switch (GS(id->name)) {
     case ID_PT: {
-      return BKE_pointcloud_customdata_required((const PointCloud *)id, layer);
+      return BKE_pointcloud_customdata_required((const PointCloud *)id, name);
     }
     case ID_CV: {
-      return BKE_curves_customdata_required((const Curves *)id, layer);
+      return BKE_curves_customdata_required((const Curves *)id, name);
     }
     default:
       return false;
@@ -555,9 +537,9 @@ int BKE_id_attribute_to_index(const ID *id,
       continue;
     }
 
-    CustomData *cdata = info[domains[i]].customdata;
+    const CustomData *cdata = info[domains[i]].customdata;
     for (int j = 0; j < cdata->totlayer; j++) {
-      CustomDataLayer *layer_iter = cdata->layers + j;
+      const CustomDataLayer *layer_iter = cdata->layers + j;
 
       if (!(CD_TYPE_AS_MASK(layer_iter->type) & layer_mask) ||
           (layer_iter->flag & CD_FLAG_TEMPORARY)) {
diff --git a/source/blender/blenkernel/intern/curves.cc b/source/blender/blenkernel/intern/curves.cc
index a348e546e46..7ad83263b73 100644
--- a/source/blender/blenkernel/intern/curves.cc
+++ b/source/blender/blenkernel/intern/curves.cc
@@ -273,9 +273,9 @@ BoundBox *BKE_curves_boundbox_get(Object *ob)
   return ob->runtime.bb;
 }
 
-bool BKE_curves_customdata_required(const Curves *UNUSED(curves), CustomDataLayer *layer)
+bool BKE_curves_customdata_required(const Curves *UNUSED(curves), const char *name)
 {
-  return layer->type == CD_PROP_FLOAT3 && STREQ(layer->name, ATTR_POSITION);
+  return STREQ(name, ATTR_POSITION);
 }
 
 Curves *BKE_curves_copy_for_eval(Curves *curves_src, bool reference)
diff --git a/source/blender/blenkernel/intern/pointcloud.cc b/source/blender/blenkernel/intern/pointcloud.cc
index 3e94a865de3..e38c20d8eb7 100644
--- a/source/blender/blenkernel/intern/pointcloud.cc
+++ b/source/blender/blenkernel/intern/pointcloud.cc
@@ -315,10 +315,9 @@ void BKE_pointcloud_update_customdata_pointers(PointCloud *pointcloud)
       CustomData_get_layer_named(&pointcloud->pdata, CD_PROP_FLOAT, POINTCLOUD_ATTR_RADIUS));
 }
 
-bool BKE_pointcloud_customdata_required(const PointCloud *UNUSED(pointcloud),
-                                        CustomDataLayer *layer)
+bool BKE_pointcloud_customdata_required(const PointCloud *UNUSED(pointcloud), const char *name)
 {
-  return layer->type == CD_PROP_FLOAT3 && STREQ(layer->name, POINTCLOUD_ATTR_POSITION);
+  return STREQ(name, POINTCLOUD_ATTR_POSITION);
 }
 
 /* Dependency Graph */
diff --git a/source/blender/bmesh/intern/bmesh_interp.c b/source/blender/bmesh/intern/bmesh_interp.c
index 2f5827bdc4c..0c3db31dd1f 100644
--- a/source/blender/bmesh/intern/bmesh_interp.c
+++ b/source/blender/bmesh/intern/bmesh_interp.c
@@ -894,6 +894,27 @@ void BM_data_layer_free(BMesh *bm, CustomData *data, int type)
   }
 }
 
+bool BM_data_layer_free_named(BMesh *bm, CustomData *data, const char *name)
+{
+  CustomData olddata = *data;
+  olddata.layers = (olddata.layers) ? MEM_dupallocN(olddata.layers) : NULL;
+
+  /* the pool is now owned by olddata and must not be shared */
+  data->pool = NULL;
+
+  const bool has_layer = CustomData_free_layer_named(data, name, 0);
+
+  if (has_layer) {
+    update_data_blocks(bm, &olddata, data);
+  }
+
+  if (olddata.layers) {
+    MEM_freeN(olddata.layers);
+  }
+
+  return has_layer;
+}
+
 void BM_data_layer_free_n(BMesh *bm, CustomData *data, int type, int n)
 {
   CustomData olddata;
diff --git a/source/blender/bmesh/intern/bmesh_interp.h b/source/blender/bmesh/intern/bmesh_interp.h
index a2f1dfc706d..2cf9dffceec 100644
--- a/source/blender/bmesh/intern/bmesh_interp.h
+++ b/source/blender/bmesh/intern/bmesh_interp.h
@@ -59,6 +59,10 @@ void BM_data_interp_face_vert_edge(
 void BM_data_layer_add(BMesh *bm, CustomData *data, int type);
 void BM_data_layer_add_named(BMesh *bm, CustomData *data, int type, const char *name);
 void BM_data_layer_free(BMesh *bm, CustomData *data, int type);
+/**
+ * Remove a named custom data layer, if it existed. Return true if the layer was removed.
+ */
+bool BM_data_layer_free_named(BMesh *bm, CustomData *data, const char *name);
 void BM_data_layer_free_n(BMesh *bm, CustomData *data, int type, int n);
 void BM_data_layer_copy(BMesh *bm, CustomData *data, int type, int src_n, int dst_n);
 
diff --git a/source/blender/editors/geometry/geometry_attributes.cc b/source/blender/editors/geometry/geometry_attributes.cc
index ae588e49565..63059a49a9e 100644
--- a/source/blender/editors/geometry/geometry_attributes.cc
+++ b/source/blender/editors/geometry/geometry_attributes.cc
@@ -182,7 +182,7 @@ static int geometry_attribute_remove_exec(bContext *C, wmOperator *op)
 
   next_color_attributes(id, layer);
 
-  if (!BKE_id_attribute_remove(id, layer, op->reports)) {
+  if (!BKE_id_attribute_remove(id, layer->name, op->reports)) {
     return OPERATOR_CANCELLED;
   }
 
@@ -468,7 +468,7 @@ static int geometry_color_attribute_remove_exec(bContext *C, wmOperator *op)
 
   next_color_attributes(id, layer);
 
-  if (!BKE_id_attribute_remove(id, layer, op->reports)) {
+  if (!BKE_id_attribute_remove(id, layer->name, op->reports)) {
     return OPERATOR_CANCELLED;
   }
 
diff --git a/source/blender/makesrna/intern/rna_attribute.c b/source/blender/makesrna/intern/rna_attribute.c
index 2670e75d057..5e17f22ecf5 100644
--- a/source/blender/makesrna/intern/rna_attribute.c
+++ b/source/blender/makesrna/intern/rna_attribute.c
@@ -163,13 +163,14 @@ static StructRNA *rna_Attribute_refine(PointerRNA *ptr)
 
 static void rna_Attribute_name_set(PointerRNA *ptr, const char *value)
 {
-  BKE_id_attribute_rename(ptr->owner_id, ptr->data, value, NULL);
+  const CustomDataLayer *layer = (const CustomDataLayer *)ptr->data;
+  BKE_id_attribute_rename(ptr->owner_id, layer->name, value, NULL);
 }
 
 static int rna_Attribute_name_editable(PointerRNA *ptr, const char **r_info)
 {
   CustomDataLayer *layer = ptr->data;
-  if (BKE_id_attribute_required(ptr->owner_id, layer)) {
+  if (BKE_id_attribute_required(ptr->owner_id, layer->name)) {
     *r_info = N_("Cannot modify name of required geometry attribute");
     return false;
   }
@@ -358,8 +359,8 @@ static PointerRNA rna_AttributeGroup_new(
 
 static void rna_AttributeGroup_remove(ID *id, ReportList *reports, PointerRNA *attribute_ptr)
 {
-  CustomDataLayer *layer = (CustomDataLayer *)attribute_ptr->data;
-  BKE_id_attribute_remove(id, layer, reports);
+  const CustomDataLayer *layer = (const CustomDataLayer *)attribute_ptr->data;
+  BKE_id_attribute_remove(id, layer->name, reports);
   RNA_POINTER_INVALIDATE(attribute_ptr);
 
   DEG_id_tag_update(id, ID_RECALC_GEOMETRY);