From 75162ab8c23937fce64e7b2674cc706a008ded96 Mon Sep 17 00:00:00 2001 From: Joseph Eagar Date: Tue, 31 May 2022 15:46:09 -0700 Subject: 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. --- source/blender/blenkernel/BKE_attribute.h | 11 +++++-- source/blender/blenkernel/intern/attribute.c | 30 ++++++++++++++++++ .../editors/geometry/geometry_attributes.cc | 36 ++++++++++++++++++++++ source/blender/editors/include/ED_geometry.h | 12 +++++++- source/blender/editors/sculpt_paint/sculpt_undo.c | 19 ++++++++++++ 5 files changed, 105 insertions(+), 3 deletions(-) (limited to 'source/blender') diff --git a/source/blender/blenkernel/BKE_attribute.h b/source/blender/blenkernel/BKE_attribute.h index f3a29736bc8..c7ea12a7065 100644 --- a/source/blender/blenkernel/BKE_attribute.h +++ b/source/blender/blenkernel/BKE_attribute.h @@ -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); diff --git a/source/blender/blenkernel/intern/attribute.c b/source/blender/blenkernel/intern/attribute.c index 0cb0704ff80..870920bdf76 100644 --- a/source/blender/blenkernel/intern/attribute.c +++ b/source/blender/blenkernel/intern/attribute.c @@ -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; } diff --git a/source/blender/editors/geometry/geometry_attributes.cc b/source/blender/editors/geometry/geometry_attributes.cc index 05f9e19da71..17784a50bca 100644 --- a/source/blender/editors/geometry/geometry_attributes.cc +++ b/source/blender/editors/geometry/geometry_attributes.cc @@ -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; +} diff --git a/source/blender/editors/include/ED_geometry.h b/source/blender/editors/include/ED_geometry.h index 74ff968828c..46e6904523a 100644 --- a/source/blender/editors/include/ED_geometry.h +++ b/source/blender/editors/include/ED_geometry.h @@ -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 diff --git a/source/blender/editors/sculpt_paint/sculpt_undo.c b/source/blender/editors/sculpt_paint/sculpt_undo.c index 5867dc558de..54678c21cb8 100644 --- a/source/blender/editors/sculpt_paint/sculpt_undo.c +++ b/source/blender/editors/sculpt_paint/sculpt_undo.c @@ -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; -- cgit v1.2.3 From 6cee4049143abf692af6ffb78c109fb0760fe67d Mon Sep 17 00:00:00 2001 From: Joseph Eagar Date: Tue, 31 May 2022 16:32:42 -0700 Subject: GPU subdiv: Fix edit mode vertex color not being uploaded properly Also cleaned up the code a tad bit. Note that I found two more bugs: * GPU subdivision attribute interpolation is producing visual artifacts. * "Show on cage" mode for subdivision surface just shows black colors. --- .../mesh_extractors/extract_mesh_vbo_vcol.cc | 29 ++++++++++++---------- 1 file changed, 16 insertions(+), 13 deletions(-) (limited to 'source/blender') diff --git a/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc b/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc index fa5bf35198b..717d81d6b52 100644 --- a/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc +++ b/source/blender/draw/intern/mesh_extractors/extract_mesh_vbo_vcol.cc @@ -111,7 +111,7 @@ static GPUVertFormat *get_coarse_vcol_format() { static GPUVertFormat format = {0}; if (format.attr_len == 0) { - GPU_vertformat_attr_add(&format, "cCol", GPU_COMP_F32, 4, GPU_FETCH_FLOAT); + GPU_vertformat_attr_add(&format, "cCol", GPU_COMP_U16, 4, GPU_FETCH_INT_TO_FLOAT_UNIT); GPU_vertformat_alias_add(&format, "c"); GPU_vertformat_alias_add(&format, "ac"); } @@ -245,6 +245,7 @@ static void extract_vcol_init_subdiv(const DRWSubdivCache *subdiv_cache, &coarse_mesh->vdata; const CustomData *cd_ldata = extract_bmesh ? &coarse_mesh->edit_mesh->bm->ldata : &coarse_mesh->ldata; + const int totloop = extract_bmesh ? coarse_mesh->edit_mesh->bm->totloop : coarse_mesh->totloop; Mesh me_query = blender::dna::shallow_copy(*coarse_mesh); BKE_id_attribute_copy_domains_temp( @@ -263,7 +264,7 @@ static void extract_vcol_init_subdiv(const DRWSubdivCache *subdiv_cache, /* Dynamic as we upload and interpolate layers one at a time. */ GPU_vertbuf_init_with_format_ex(src_data, get_coarse_vcol_format(), GPU_USAGE_DYNAMIC); - GPU_vertbuf_data_alloc(src_data, coarse_mesh->totloop); + GPU_vertbuf_data_alloc(src_data, totloop); gpuMeshVcol *mesh_vcol = (gpuMeshVcol *)GPU_vertbuf_get_data(src_data); @@ -279,8 +280,6 @@ static void extract_vcol_init_subdiv(const DRWSubdivCache *subdiv_cache, const int dst_offset = (int)subdiv_cache->num_subdiv_loops * 2 * pack_layer_index++; const CustomData *cdata = ref.domain == ATTR_DOMAIN_POINT ? cd_vdata : cd_ldata; - const MLoop *ml = coarse_mesh->mloop; - int layer_i = CustomData_get_named_layer_index(cdata, ref.layer->type, ref.layer->name); if (layer_i == -1) { @@ -289,15 +288,6 @@ static void extract_vcol_init_subdiv(const DRWSubdivCache *subdiv_cache, } gpuMeshVcol *vcol = mesh_vcol; - MLoopCol *mcol = nullptr; - MPropCol *pcol = nullptr; - - if (ref.layer->type == CD_PROP_COLOR) { - pcol = static_cast(cdata->layers[layer_i].data); - } - else { - mcol = static_cast(cdata->layers[layer_i].data); - } const bool is_vert = ref.domain == ATTR_DOMAIN_POINT; @@ -331,10 +321,23 @@ static void extract_vcol_init_subdiv(const DRWSubdivCache *subdiv_cache, vcol->b = unit_float_to_ushort_clamp(pcol2->color[2]); vcol->a = unit_float_to_ushort_clamp(pcol2->color[3]); } + + vcol++; } while ((l_iter = l_iter->next) != f->l_first); } } else { + const MLoop *ml = coarse_mesh->mloop; + MLoopCol *mcol = nullptr; + MPropCol *pcol = nullptr; + + if (ref.layer->type == CD_PROP_COLOR) { + pcol = static_cast(cdata->layers[layer_i].data); + } + else { + mcol = static_cast(cdata->layers[layer_i].data); + } + for (int ml_index = 0; ml_index < coarse_mesh->totloop; ml_index++, vcol++, ml++) { int idx = is_vert ? ml->v : ml_index; -- cgit v1.2.3