Fix #113665: Sculpt mode does not unshare color attribute data

When retrieving a color attribute to paint it, the layer data needs to
be unshared so that it doesn't modify data from separate meshes.
In the past I think we've unknowingly avoided this problem by porting
code to the new attribute API. I've been refactoring `PBVH` and
`SculptSession` to make that possible (removing the long-lived
layer pointers in the two structs). But that wouldn't fit as a bug fix.
In the meantime, make sure the color attribute data is un-shared
and separate "for read" and "for write" versions of the function.

Pull Request: https://projects.blender.org/blender/blender/pulls/114119
This commit is contained in:
Hans Goudey 2023-10-24 20:19:14 +02:00 committed by Hans Goudey
parent 7dcd777379
commit 692292536f
6 changed files with 47 additions and 19 deletions

View File

@ -77,10 +77,15 @@ struct CustomDataLayer *BKE_id_attribute_find(const struct ID *id,
eCustomDataType type,
eAttrDomain domain);
struct CustomDataLayer *BKE_id_attribute_search(struct ID *id,
const char *name,
eCustomDataMask type,
eAttrDomainMask domain_mask);
const struct CustomDataLayer *BKE_id_attribute_search(const struct ID *id,
const char *name,
eCustomDataMask type,
eAttrDomainMask domain_mask);
struct CustomDataLayer *BKE_id_attribute_search_for_write(struct ID *id,
const char *name,
eCustomDataMask type,
eAttrDomainMask domain_mask);
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);

View File

@ -147,7 +147,7 @@ static bool bke_id_attribute_rename_if_exists(ID *id,
const char *new_name,
ReportList *reports)
{
CustomDataLayer *layer = BKE_id_attribute_search(
CustomDataLayer *layer = BKE_id_attribute_search_for_write(
id, old_name, CD_MASK_PROP_ALL, ATTR_DOMAIN_MASK_ALL);
if (layer == nullptr) {
return false;
@ -183,7 +183,7 @@ bool BKE_id_attribute_rename(ID *id,
}
}
CustomDataLayer *layer = BKE_id_attribute_search(
CustomDataLayer *layer = BKE_id_attribute_search_for_write(
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");
@ -372,7 +372,7 @@ CustomDataLayer *BKE_id_attribute_duplicate(ID *id, const char *name, ReportList
BKE_uv_map_pin_name_get(uniquename, buffer_dst));
}
return BKE_id_attribute_search(id, uniquename, CD_MASK_PROP_ALL, ATTR_DOMAIN_MASK_ALL);
return BKE_id_attribute_search_for_write(id, uniquename, CD_MASK_PROP_ALL, ATTR_DOMAIN_MASK_ALL);
}
static int color_name_to_index(ID *id, const char *name)
@ -529,10 +529,10 @@ CustomDataLayer *BKE_id_attribute_find(const ID *id,
return nullptr;
}
CustomDataLayer *BKE_id_attribute_search(ID *id,
const char *name,
const eCustomDataMask type_mask,
const eAttrDomainMask domain_mask)
const CustomDataLayer *BKE_id_attribute_search(const ID *id,
const char *name,
const eCustomDataMask type_mask,
const eAttrDomainMask domain_mask)
{
if (!name) {
return nullptr;
@ -563,6 +563,28 @@ CustomDataLayer *BKE_id_attribute_search(ID *id,
return nullptr;
}
CustomDataLayer *BKE_id_attribute_search_for_write(ID *id,
const char *name,
const eCustomDataMask type_mask,
const eAttrDomainMask domain_mask)
{
/* Reuse the implementation of the const version.
* Implicit sharing for the layer's data is handled below. */
CustomDataLayer *layer = const_cast<CustomDataLayer *>(
BKE_id_attribute_search(id, name, type_mask, domain_mask));
if (!layer) {
return nullptr;
}
DomainInfo info[ATTR_DOMAIN_NUM];
get_domains(id, info);
const eAttrDomain domain = BKE_id_attribute_domain(id, layer);
CustomData_ensure_data_is_mutable(layer, info[domain].length);
return layer;
}
int BKE_id_attributes_length(const ID *id, eAttrDomainMask domain_mask, eCustomDataMask mask)
{
DomainInfo info[ATTR_DOMAIN_NUM];

View File

@ -1444,7 +1444,7 @@ static void pbvh_update_BB_redraw(PBVH *pbvh, Span<PBVHNode *> nodes, int flag)
bool BKE_pbvh_get_color_layer(Mesh *me, CustomDataLayer **r_layer, eAttrDomain *r_domain)
{
*r_layer = BKE_id_attribute_search(
*r_layer = BKE_id_attribute_search_for_write(
&me->id, me->active_color_attribute, CD_MASK_COLOR_ALL, ATTR_DOMAIN_MASK_COLOR);
*r_domain = *r_layer ? BKE_id_attribute_domain(&me->id, *r_layer) : ATTR_DOMAIN_POINT;
return *r_layer != nullptr;

View File

@ -1835,7 +1835,7 @@ static void sculpt_undo_set_active_layer(bContext *C, SculptAttrRef *attr)
* domain and just unconvert it.
*/
if (!layer) {
layer = BKE_id_attribute_search(&me->id, attr->name, CD_MASK_PROP_ALL, ATTR_DOMAIN_MASK_ALL);
layer = BKE_id_attribute_search_for_write(&me->id, attr->name, CD_MASK_PROP_ALL, ATTR_DOMAIN_MASK_ALL);
if (layer) {
if (ED_geometry_attribute_convert(
me, attr->name, eCustomDataType(attr->type), attr->domain, nullptr))

View File

@ -570,10 +570,11 @@ static void rna_AttributeGroup_update_active(Main *bmain, Scene *scene, PointerR
static PointerRNA rna_AttributeGroup_active_color_get(PointerRNA *ptr)
{
ID *id = ptr->owner_id;
CustomDataLayer *layer = BKE_id_attribute_search(ptr->owner_id,
BKE_id_attributes_active_color_name(id),
CD_MASK_COLOR_ALL,
ATTR_DOMAIN_MASK_COLOR);
CustomDataLayer *layer = BKE_id_attribute_search_for_write(
ptr->owner_id,
BKE_id_attributes_active_color_name(id),
CD_MASK_COLOR_ALL,
ATTR_DOMAIN_MASK_COLOR);
PointerRNA attribute_ptr = RNA_pointer_create(id, &RNA_Attribute, layer);
return attribute_ptr;

View File

@ -1107,7 +1107,7 @@ DEFINE_CUSTOMDATA_LAYER_COLLECTION(vertex_color, ldata, CD_PROP_BYTE_COLOR)
static PointerRNA rna_Mesh_vertex_color_active_get(PointerRNA *ptr)
{
Mesh *mesh = (Mesh *)ptr->data;
CustomDataLayer *layer = BKE_id_attribute_search(
CustomDataLayer *layer = BKE_id_attribute_search_for_write(
&mesh->id, mesh->active_color_attribute, CD_MASK_PROP_BYTE_COLOR, ATTR_DOMAIN_MASK_CORNER);
return rna_pointer_inherit_refine(ptr, &RNA_MeshLoopColorLayer, layer);
}
@ -1129,7 +1129,7 @@ static void rna_Mesh_vertex_color_active_set(PointerRNA *ptr,
static int rna_Mesh_vertex_color_active_index_get(PointerRNA *ptr)
{
Mesh *mesh = (Mesh *)ptr->data;
CustomDataLayer *layer = BKE_id_attribute_search(
const CustomDataLayer *layer = BKE_id_attribute_search(
&mesh->id, mesh->active_color_attribute, CD_MASK_PROP_BYTE_COLOR, ATTR_DOMAIN_MASK_CORNER);
if (!layer) {
return 0;