From 90e509d86da3bab52bd281349c76edd9241d854b Mon Sep 17 00:00:00 2001 From: Martijn Versteegh Date: Sat, 17 Jun 2023 15:47:23 +0200 Subject: [PATCH 1/4] Make ComponentAttributeAccessor handle active/default_color_attribute When Geometry Nodes created or deleted attributes it didn't handle the active/default_color_attribute strings on Mesh. This lead to Meshes with color attributes but without active/default_color_attribute set, which is a regression from previous blender versions where newly created color attributes are always set to active/default. --- .../intern/attribute_access_intern.hh | 34 ++++++++++++++++-- .../intern/geometry_component_curves.cc | 4 ++- .../intern/geometry_component_instances.cc | 2 +- .../intern/geometry_component_mesh.cc | 35 ++++++++++++++++++- .../intern/geometry_component_pointcloud.cc | 3 +- 5 files changed, 72 insertions(+), 6 deletions(-) diff --git a/source/blender/blenkernel/intern/attribute_access_intern.hh b/source/blender/blenkernel/intern/attribute_access_intern.hh index e685272de6b..faf59b98e08 100644 --- a/source/blender/blenkernel/intern/attribute_access_intern.hh +++ b/source/blender/blenkernel/intern/attribute_access_intern.hh @@ -206,6 +206,11 @@ class BuiltinCustomDataLayerProvider final : public BuiltinAttributeProvider { bool layer_exists(const CustomData &custom_data) const; }; +using AttributeAddTrigger = void (*)(void *owner, + AttributeIDRef const &attribute_id, + eCustomDataType type); +using AttributeRemoveTrigger = void (*)(void *owner, AttributeIDRef const &attribute_id); + /** * This is a container for multiple attribute providers that are used by one geometry component * type (e.g. there is a set of attribute providers for mesh components). @@ -229,10 +234,17 @@ class ComponentAttributeProviders { */ VectorSet supported_domains_; + AttributeAddTrigger add_trigger_; + AttributeRemoveTrigger remove_trigger_; + public: ComponentAttributeProviders(Span builtin_attribute_providers, - Span dynamic_attribute_providers) - : dynamic_attribute_providers_(dynamic_attribute_providers) + Span dynamic_attribute_providers, + AttributeAddTrigger add_trigger, + AttributeRemoveTrigger remove_trigger) + : dynamic_attribute_providers_(dynamic_attribute_providers), + add_trigger_(add_trigger), + remove_trigger_(remove_trigger) { for (const BuiltinAttributeProvider *provider : builtin_attribute_providers) { /* Use #add_new to make sure that no two builtin attributes have the same name. */ @@ -258,6 +270,22 @@ class ComponentAttributeProviders { { return supported_domains_; } + + void call_add_trigger(void *owner, + AttributeIDRef const &attribute_id, + eCustomDataType type) const + { + if (add_trigger_) { + add_trigger_(owner, attribute_id, type); + } + } + + void call_remove_trigger(void *owner, AttributeIDRef const &attribute_id) const + { + if (remove_trigger_) { + remove_trigger_(owner, attribute_id); + } + } }; namespace attribute_accessor_functions { @@ -403,6 +431,7 @@ inline bool remove(void *owner, const AttributeIDRef &attribute_id) } for (const DynamicAttributesProvider *provider : providers.dynamic_attribute_providers()) { if (provider->try_delete(owner, attribute_id)) { + providers.call_remove_trigger(owner, attribute_id); return true; } } @@ -435,6 +464,7 @@ inline bool add(void *owner, } for (const DynamicAttributesProvider *provider : providers.dynamic_attribute_providers()) { if (provider->try_create(owner, attribute_id, domain, data_type, initializer)) { + providers.call_add_trigger(owner, attribute_id, data_type); return true; } } diff --git a/source/blender/blenkernel/intern/geometry_component_curves.cc b/source/blender/blenkernel/intern/geometry_component_curves.cc index e20c130e9de..9ef87a6ea0f 100644 --- a/source/blender/blenkernel/intern/geometry_component_curves.cc +++ b/source/blender/blenkernel/intern/geometry_component_curves.cc @@ -560,7 +560,9 @@ static ComponentAttributeProviders create_attribute_providers_for_curve() &curve_type, &resolution, &cyclic}, - {&curve_custom_data, &point_custom_data}); + {&curve_custom_data, &point_custom_data}, + nullptr, + nullptr); } /** \} */ diff --git a/source/blender/blenkernel/intern/geometry_component_instances.cc b/source/blender/blenkernel/intern/geometry_component_instances.cc index 3af5d4696d9..f0129aeff87 100644 --- a/source/blender/blenkernel/intern/geometry_component_instances.cc +++ b/source/blender/blenkernel/intern/geometry_component_instances.cc @@ -207,7 +207,7 @@ static ComponentAttributeProviders create_attribute_providers_for_instances() static CustomDataAttributeProvider instance_custom_data(ATTR_DOMAIN_INSTANCE, instance_custom_data_access); - return ComponentAttributeProviders({&position, &id}, {&instance_custom_data}); + return ComponentAttributeProviders({&position, &id}, {&instance_custom_data}, nullptr, nullptr); } static AttributeAccessorFunctions get_instances_accessor_functions() diff --git a/source/blender/blenkernel/intern/geometry_component_mesh.cc b/source/blender/blenkernel/intern/geometry_component_mesh.cc index 3a2cbe755cf..afe188e258e 100644 --- a/source/blender/blenkernel/intern/geometry_component_mesh.cc +++ b/source/blender/blenkernel/intern/geometry_component_mesh.cc @@ -1082,6 +1082,37 @@ class VertexGroupsAttributeProvider final : public DynamicAttributesProvider { */ static ComponentAttributeProviders create_attribute_providers_for_mesh() { + static AttributeAddTrigger mesh_add_trigger = + [](void *owner, AttributeIDRef const &attribute_id, eCustomDataType type) -> void { + Mesh *mesh = static_cast(owner); + if ((type == CD_PROP_COLOR || type == CD_PROP_BYTE_COLOR) && + !attribute_id.is_anonymous()) { + if (mesh->active_color_attribute == nullptr) { + mesh->active_color_attribute = static_cast( + BLI_strdupn(attribute_id.name().data(), attribute_id.name().size())); + } + if (mesh->default_color_attribute == nullptr) { + mesh->default_color_attribute = static_cast( + BLI_strdupn(attribute_id.name().data(), attribute_id.name().size())); + } + } + }; + + static AttributeRemoveTrigger mesh_remove_trigger = + [](void *owner, AttributeIDRef const &attribute_id) -> void { + Mesh *mesh = static_cast(owner); + if (mesh->active_color_attribute && + !strncmp(mesh->active_color_attribute, attribute_id.name().data(), attribute_id.name().size())) + { + MEM_SAFE_FREE(mesh->active_color_attribute); + } + if (mesh->default_color_attribute && + !strncmp(mesh->default_color_attribute, attribute_id.name().data(), attribute_id.name().size())) + { + MEM_SAFE_FREE(mesh->default_color_attribute); + } + }; + #define MAKE_MUTABLE_CUSTOM_DATA_GETTER(NAME) \ [](void *owner) -> CustomData * { \ Mesh *mesh = static_cast(owner); \ @@ -1239,7 +1270,9 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh() &vertex_groups, &point_custom_data, &edge_custom_data, - &face_custom_data}); + &face_custom_data}, + mesh_add_trigger, + mesh_remove_trigger); } static AttributeAccessorFunctions get_mesh_accessor_functions() diff --git a/source/blender/blenkernel/intern/geometry_component_pointcloud.cc b/source/blender/blenkernel/intern/geometry_component_pointcloud.cc index f9261fdcfce..a54f93bae16 100644 --- a/source/blender/blenkernel/intern/geometry_component_pointcloud.cc +++ b/source/blender/blenkernel/intern/geometry_component_pointcloud.cc @@ -160,7 +160,8 @@ static ComponentAttributeProviders create_attribute_providers_for_point_cloud() point_access, nullptr); static CustomDataAttributeProvider point_custom_data(ATTR_DOMAIN_POINT, point_access); - return ComponentAttributeProviders({&position, &radius, &id}, {&point_custom_data}); + return ComponentAttributeProviders( + {&position, &radius, &id}, {&point_custom_data}, nullptr, nullptr); } static AttributeAccessorFunctions get_pointcloud_accessor_functions() -- 2.30.2 From 75191ba54d9880cf1b1d3a5c617119dab5bdfca6 Mon Sep 17 00:00:00 2001 From: Martijn Versteegh Date: Sat, 17 Jun 2023 18:11:16 +0200 Subject: [PATCH 2/4] Cleanup: make format --- .../blenkernel/intern/geometry_component_mesh.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/source/blender/blenkernel/intern/geometry_component_mesh.cc b/source/blender/blenkernel/intern/geometry_component_mesh.cc index afe188e258e..c8726e14a17 100644 --- a/source/blender/blenkernel/intern/geometry_component_mesh.cc +++ b/source/blender/blenkernel/intern/geometry_component_mesh.cc @@ -1085,8 +1085,7 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh() static AttributeAddTrigger mesh_add_trigger = [](void *owner, AttributeIDRef const &attribute_id, eCustomDataType type) -> void { Mesh *mesh = static_cast(owner); - if ((type == CD_PROP_COLOR || type == CD_PROP_BYTE_COLOR) && - !attribute_id.is_anonymous()) { + if ((type == CD_PROP_COLOR || type == CD_PROP_BYTE_COLOR) && !attribute_id.is_anonymous()) { if (mesh->active_color_attribute == nullptr) { mesh->active_color_attribute = static_cast( BLI_strdupn(attribute_id.name().data(), attribute_id.name().size())); @@ -1101,13 +1100,15 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh() static AttributeRemoveTrigger mesh_remove_trigger = [](void *owner, AttributeIDRef const &attribute_id) -> void { Mesh *mesh = static_cast(owner); - if (mesh->active_color_attribute && - !strncmp(mesh->active_color_attribute, attribute_id.name().data(), attribute_id.name().size())) + if (mesh->active_color_attribute && !strncmp(mesh->active_color_attribute, + attribute_id.name().data(), + attribute_id.name().size())) { MEM_SAFE_FREE(mesh->active_color_attribute); } - if (mesh->default_color_attribute && - !strncmp(mesh->default_color_attribute, attribute_id.name().data(), attribute_id.name().size())) + if (mesh->default_color_attribute && !strncmp(mesh->default_color_attribute, + attribute_id.name().data(), + attribute_id.name().size())) { MEM_SAFE_FREE(mesh->default_color_attribute); } -- 2.30.2 From 23356efa984e39c82e534e31c9383bcab5b8ee25 Mon Sep 17 00:00:00 2001 From: Martijn Versteegh Date: Sat, 17 Jun 2023 21:03:11 +0200 Subject: [PATCH 3/4] -Trigger -> Handler -call remove handler before removing the layer --- .../intern/attribute_access_intern.hh | 32 +++++++++---------- .../intern/geometry_component_mesh.cc | 8 ++--- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/source/blender/blenkernel/intern/attribute_access_intern.hh b/source/blender/blenkernel/intern/attribute_access_intern.hh index faf59b98e08..b6934792a2e 100644 --- a/source/blender/blenkernel/intern/attribute_access_intern.hh +++ b/source/blender/blenkernel/intern/attribute_access_intern.hh @@ -206,10 +206,10 @@ class BuiltinCustomDataLayerProvider final : public BuiltinAttributeProvider { bool layer_exists(const CustomData &custom_data) const; }; -using AttributeAddTrigger = void (*)(void *owner, +using AttributeAddHandler = void (*)(void *owner, AttributeIDRef const &attribute_id, eCustomDataType type); -using AttributeRemoveTrigger = void (*)(void *owner, AttributeIDRef const &attribute_id); +using AttributeRemoveHandler = void (*)(void *owner, AttributeIDRef const &attribute_id); /** * This is a container for multiple attribute providers that are used by one geometry component @@ -234,17 +234,17 @@ class ComponentAttributeProviders { */ VectorSet supported_domains_; - AttributeAddTrigger add_trigger_; - AttributeRemoveTrigger remove_trigger_; + AttributeAddHandler add_handler_; + AttributeRemoveHandler remove_handler_; public: ComponentAttributeProviders(Span builtin_attribute_providers, Span dynamic_attribute_providers, - AttributeAddTrigger add_trigger, - AttributeRemoveTrigger remove_trigger) + AttributeAddHandler add_handler, + AttributeRemoveHandler remove_handler) : dynamic_attribute_providers_(dynamic_attribute_providers), - add_trigger_(add_trigger), - remove_trigger_(remove_trigger) + add_handler_(add_handler), + remove_handler_(remove_handler) { for (const BuiltinAttributeProvider *provider : builtin_attribute_providers) { /* Use #add_new to make sure that no two builtin attributes have the same name. */ @@ -271,19 +271,19 @@ class ComponentAttributeProviders { return supported_domains_; } - void call_add_trigger(void *owner, + void call_add_handler(void *owner, AttributeIDRef const &attribute_id, eCustomDataType type) const { - if (add_trigger_) { - add_trigger_(owner, attribute_id, type); + if (add_handler_) { + add_handler_(owner, attribute_id, type); } } - void call_remove_trigger(void *owner, AttributeIDRef const &attribute_id) const + void call_remove_handler(void *owner, AttributeIDRef const &attribute_id) const { - if (remove_trigger_) { - remove_trigger_(owner, attribute_id); + if (remove_handler_) { + remove_handler_(owner, attribute_id); } } }; @@ -429,9 +429,9 @@ inline bool remove(void *owner, const AttributeIDRef &attribute_id) return provider->try_delete(owner); } } + providers.call_remove_handler(owner, attribute_id); for (const DynamicAttributesProvider *provider : providers.dynamic_attribute_providers()) { if (provider->try_delete(owner, attribute_id)) { - providers.call_remove_trigger(owner, attribute_id); return true; } } @@ -464,7 +464,7 @@ inline bool add(void *owner, } for (const DynamicAttributesProvider *provider : providers.dynamic_attribute_providers()) { if (provider->try_create(owner, attribute_id, domain, data_type, initializer)) { - providers.call_add_trigger(owner, attribute_id, data_type); + providers.call_add_handler(owner, attribute_id, data_type); return true; } } diff --git a/source/blender/blenkernel/intern/geometry_component_mesh.cc b/source/blender/blenkernel/intern/geometry_component_mesh.cc index c8726e14a17..f0bdcbe8c36 100644 --- a/source/blender/blenkernel/intern/geometry_component_mesh.cc +++ b/source/blender/blenkernel/intern/geometry_component_mesh.cc @@ -1082,7 +1082,7 @@ class VertexGroupsAttributeProvider final : public DynamicAttributesProvider { */ static ComponentAttributeProviders create_attribute_providers_for_mesh() { - static AttributeAddTrigger mesh_add_trigger = + static AttributeAddHandler mesh_add_handler = [](void *owner, AttributeIDRef const &attribute_id, eCustomDataType type) -> void { Mesh *mesh = static_cast(owner); if ((type == CD_PROP_COLOR || type == CD_PROP_BYTE_COLOR) && !attribute_id.is_anonymous()) { @@ -1097,7 +1097,7 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh() } }; - static AttributeRemoveTrigger mesh_remove_trigger = + static AttributeRemoveHandler mesh_remove_handler = [](void *owner, AttributeIDRef const &attribute_id) -> void { Mesh *mesh = static_cast(owner); if (mesh->active_color_attribute && !strncmp(mesh->active_color_attribute, @@ -1272,8 +1272,8 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh() &point_custom_data, &edge_custom_data, &face_custom_data}, - mesh_add_trigger, - mesh_remove_trigger); + mesh_add_handler, + mesh_remove_handler); } static AttributeAccessorFunctions get_mesh_accessor_functions() -- 2.30.2 From 1e69a6ac4bf33bad06c5dac9b8c580b2cc6a1434 Mon Sep 17 00:00:00 2001 From: Martijn Versteegh Date: Sat, 17 Jun 2023 21:23:12 +0200 Subject: [PATCH 4/4] Don't bother checking anonymous attributes for being active/default_color_attribute --- source/blender/blenkernel/intern/geometry_fields.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/blender/blenkernel/intern/geometry_fields.cc b/source/blender/blenkernel/intern/geometry_fields.cc index 9dc9ce7f565..9c2d0756c3c 100644 --- a/source/blender/blenkernel/intern/geometry_fields.cc +++ b/source/blender/blenkernel/intern/geometry_fields.cc @@ -538,8 +538,9 @@ bool try_capture_field_on_geometry(GeometryComponent &component, return true; } } - - attributes.remove(attribute_id); + if (!attribute_id.is_anonymous()) { + attributes.remove(attribute_id); + } if (attributes.add(attribute_id, domain, data_type, bke::AttributeInitMoveArray(buffer))) { return true; } -- 2.30.2