Fix T97408: Temporary fix for attribute convert undo

Sculpt undo now detects if an attribute layer has
changed type/domain and unconverts it back.  This
is a temporary workaround to a more fundamental
bug in the undo system.

Memfile undo assumes it can always rebuild the
application state from the prior undo step,
which isn't true with incremental undo systems.

The correct fix is to push an extra undo step prior
to running an operator if an incremental undo system
is active and the operator is using memfile undo.
This commit is contained in:
Joseph Eagar 2022-05-31 15:46:09 -07:00
parent 511a08585d
commit 75162ab8c2
Notes: blender-bot 2023-02-14 07:08:26 +01:00
Referenced by issue #98528, Build error macOS intel: (error: cannot increment expression of enum type 'AttributeDomain')
Referenced by issue #97408, Converting the color attribute causes undo to fail
5 changed files with 105 additions and 3 deletions

View File

@ -43,6 +43,8 @@ typedef enum AttributeDomainMask {
ATTR_DOMAIN_MASK_ALL = (1 << 5) - 1
} AttributeDomainMask;
#define ATTR_DOMAIN_AS_MASK(domain) ((AttributeDomainMask)((1 << (int)(domain))))
/* All domains that support color attributes. */
#define ATTR_DOMAIN_MASK_COLOR \
((AttributeDomainMask)((ATTR_DOMAIN_MASK_POINT | ATTR_DOMAIN_MASK_CORNER)))
@ -62,8 +64,13 @@ bool BKE_id_attribute_remove(struct ID *id,
struct CustomDataLayer *BKE_id_attribute_find(const struct ID *id,
const char *name,
int type,
AttributeDomain domain);
const int type,
const AttributeDomain domain);
struct CustomDataLayer *BKE_id_attribute_search(const struct ID *id,
const char *name,
const CustomDataMask type,
const AttributeDomainMask domain_mask);
AttributeDomain 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

@ -274,6 +274,35 @@ CustomDataLayer *BKE_id_attribute_find(const ID *id,
return NULL;
}
CustomDataLayer *BKE_id_attribute_search(const ID *id,
const char *name,
const CustomDataMask type_mask,
const AttributeDomainMask domain_mask)
{
DomainInfo info[ATTR_DOMAIN_NUM];
get_domains(id, info);
for (AttributeDomain domain = ATTR_DOMAIN_POINT; domain < ATTR_DOMAIN_NUM; domain++) {
if (!(domain_mask & ATTR_DOMAIN_AS_MASK(domain))) {
continue;
}
CustomData *customdata = info[domain].customdata;
if (customdata == NULL) {
return NULL;
}
for (int i = 0; i < customdata->totlayer; i++) {
CustomDataLayer *layer = &customdata->layers[i];
if ((CD_TYPE_AS_MASK(layer->type) & type_mask) && STREQ(layer->name, name)) {
return layer;
}
}
}
return NULL;
}
int BKE_id_attributes_length(const ID *id, AttributeDomainMask domain_mask, CustomDataMask mask)
{
DomainInfo info[ATTR_DOMAIN_NUM];
@ -641,6 +670,7 @@ CustomDataLayer *BKE_id_attributes_color_find(const ID *id, const char *name)
if (layer == NULL) {
layer = BKE_id_attribute_find(id, name, CD_PROP_BYTE_COLOR, ATTR_DOMAIN_CORNER);
}
return layer;
}

View File

@ -33,6 +33,7 @@
#include "UI_interface.h"
#include "UI_resources.h"
#include "ED_geometry.h"
#include "ED_object.h"
#include "geometry_intern.hh"
@ -580,3 +581,38 @@ void GEOMETRY_OT_attribute_convert(wmOperatorType *ot)
}
} // namespace blender::ed::geometry
using blender::CPPType;
using blender::GVArray;
bool ED_geometry_attribute_convert(Mesh *mesh,
const char *layer_name,
CustomDataType old_type,
AttributeDomain old_domain,
CustomDataType new_type,
AttributeDomain new_domain)
{
CustomDataLayer *layer = BKE_id_attribute_find(&mesh->id, layer_name, old_type, old_domain);
const std::string name = layer->name;
if (!layer) {
return false;
}
MeshComponent mesh_component;
mesh_component.replace(mesh, GeometryOwnershipType::Editable);
GVArray src_varray = mesh_component.attribute_get_for_read(name, new_domain, new_type);
const CPPType &cpp_type = src_varray.type();
void *new_data = MEM_malloc_arrayN(src_varray.size(), cpp_type.size(), __func__);
src_varray.materialize_to_uninitialized(new_data);
mesh_component.attribute_try_delete(name);
mesh_component.attribute_try_create(name, new_domain, new_type, AttributeInitMove(new_data));
int *active_index = BKE_id_attributes_active_index_p(&mesh->id);
if (*active_index > 0) {
*active_index -= 1;
}
return true;
}

View File

@ -7,12 +7,22 @@
#pragma once
#include "BKE_attribute.h"
#include "DNA_customdata_types.h"
#ifdef __cplusplus
extern "C" {
#endif
void ED_operatortypes_geometry(void);
struct Mesh;
void ED_operatortypes_geometry(void);
bool ED_geometry_attribute_convert(struct Mesh *mesh,
const char *layer_name,
CustomDataType old_type,
AttributeDomain old_domain,
CustomDataType new_type,
AttributeDomain new_domain);
#ifdef __cplusplus
}
#endif

View File

@ -50,6 +50,7 @@
#include "WM_api.h"
#include "WM_types.h"
#include "ED_geometry.h"
#include "ED_object.h"
#include "ED_sculpt.h"
#include "ED_undo.h"
@ -1565,6 +1566,24 @@ static void sculpt_undo_set_active_layer(struct bContext *C, SculptAttrRef *attr
CustomDataLayer *layer;
layer = BKE_id_attribute_find(&me->id, attr->name, attr->type, attr->domain);
/* Temporary fix for T97408. This is a fundamental
* bug in the undo stack; the operator code needs to push
* an extra undo step before running an operator if a
* non-memfile undo system is active.
*
* For now, detect if the layer does exist but with a different
* domain and just unconvert it.
*/
if (!layer) {
layer = BKE_id_attribute_search(&me->id, attr->name, CD_MASK_PROP_ALL, ATTR_DOMAIN_MASK_ALL);
AttributeDomain domain = layer ? BKE_id_attribute_domain(&me->id, layer) : ATTR_DOMAIN_NUM;
if (layer && ED_geometry_attribute_convert(
me, attr->name, layer->type, domain, attr->type, attr->domain)) {
layer = BKE_id_attribute_find(&me->id, attr->name, attr->type, attr->domain);
}
}
if (!layer) {
/* Memfile undo killed the layer; re-create it. */
CustomData *cdata = attr->domain == ATTR_DOMAIN_POINT ? &me->vdata : &me->ldata;