From f61fbc468ab9f67ae25ac22f73f3496b7bfb105c Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Thu, 19 Jun 2025 11:54:07 -0400 Subject: [PATCH] Attributes: Remove AttributeStorage legacy compatibility option Remove the use_attribute_storage_write experimental option and always write in the new format, which is supported by 4.5. The new format is only used at runtime by point clouds currently but there is no reason for it to be an option at this point. This is a second commit repeating 84212bae4b82908a45c4 after that was reverted. Issues that came up with Grease Pencil writing have been resolved in the meantime. --- scripts/startup/bl_ui/space_userpref.py | 1 - .../BKE_attribute_storage_blend_write.hh | 10 +--- .../blenkernel/intern/attribute_storage.cc | 46 +----------------- .../blenkernel/intern/curves_geometry.cc | 5 +- .../blender/blenkernel/intern/customdata.cc | 47 +++++++++---------- .../blenkernel/intern/grease_pencil.cc | 4 +- source/blender/blenkernel/intern/mesh.cc | 35 +++++--------- .../blender/blenkernel/intern/pointcloud.cc | 2 +- source/blender/makesdna/DNA_userdef_types.h | 3 +- source/blender/makesrna/intern/rna_userdef.cc | 6 --- 10 files changed, 42 insertions(+), 117 deletions(-) diff --git a/scripts/startup/bl_ui/space_userpref.py b/scripts/startup/bl_ui/space_userpref.py index 789869f18a1..04eaa3a3680 100644 --- a/scripts/startup/bl_ui/space_userpref.py +++ b/scripts/startup/bl_ui/space_userpref.py @@ -2871,7 +2871,6 @@ class USERPREF_PT_experimental_prototypes(ExperimentalPanel, Panel): ({"property": "use_new_curves_tools"}, ("blender/blender/issues/68981", "#68981")), ({"property": "use_sculpt_texture_paint"}, ("blender/blender/issues/96225", "#96225")), ({"property": "write_legacy_blend_file_format"}, ("/blender/blender/issues/129309", "#129309")), - ({"property": "use_attribute_storage_write"}, ("/blender/blender/issues/122398", "#122398")), ), ) diff --git a/source/blender/blenkernel/BKE_attribute_storage_blend_write.hh b/source/blender/blenkernel/BKE_attribute_storage_blend_write.hh index 731aa44419c..300a648f6c5 100644 --- a/source/blender/blenkernel/BKE_attribute_storage_blend_write.hh +++ b/source/blender/blenkernel/BKE_attribute_storage_blend_write.hh @@ -6,10 +6,6 @@ #include "BKE_attribute_storage.hh" -#include "BLI_map.hh" - -#include "DNA_customdata_types.h" - namespace blender::bke { /** @@ -18,9 +14,7 @@ namespace blender::bke { * created just for the writing process. Creating them mutates the struct, which must be done * before writing the struct that embeds it. */ -void attribute_storage_blend_write_prepare( - AttributeStorage &data, - const Map *> &layers_to_write, - AttributeStorage::BlendWriteData &write_data); +void attribute_storage_blend_write_prepare(AttributeStorage &data, + AttributeStorage::BlendWriteData &write_data); } // namespace blender::bke diff --git a/source/blender/blenkernel/intern/attribute_storage.cc b/source/blender/blenkernel/intern/attribute_storage.cc index c1282ccce78..ecace71266a 100644 --- a/source/blender/blenkernel/intern/attribute_storage.cc +++ b/source/blender/blenkernel/intern/attribute_storage.cc @@ -528,52 +528,10 @@ static void write_array_data(BlendWriter &writer, } } -void attribute_storage_blend_write_prepare( - AttributeStorage &data, - const Map *> &layers_to_write, - AttributeStorage::BlendWriteData &write_data) +void attribute_storage_blend_write_prepare(AttributeStorage &data, + AttributeStorage::BlendWriteData &write_data) { - Set all_names_written; - for (Vector *const layers : layers_to_write.values()) { - for (const CustomDataLayer &layer : *layers) { - all_names_written.add(layer.name); - } - } data.foreach([&](Attribute &attr) { - if (!U.experimental.use_attribute_storage_write && !layers_to_write.is_empty()) { - /* In version 4.5, all attribute data is written in the #CustomData format (at least when the - * debug option is not enabled), so the #Attribute needs to be converted to a - * #CustomDataLayer in the proper list. This is only relevant when #AttributeStorage is - * actually used at runtime. - * - * When removing this option to always write the new format in 5.0, #BLENDER_FILE_MIN_VERSION - * must be increased. */ - if (const std::optional data_type = attr_type_to_custom_data_type(attr.data_type())) { - if (const auto *array_data = std::get_if(&attr.data())) { - CustomDataLayer layer{}; - layer.type = *data_type; - layer.data = array_data->data; - layer.sharing_info = array_data->sharing_info.get(); - - /* Because the #Attribute::name_ `std::string` has no length limit (unlike - * #CustomDataLayer::name), we have to manually make the name unique in case it exceeds - * the limit. */ - BLI_uniquename_cb( - [&](const StringRefNull name) { return all_names_written.contains(name); }, - attr.name().c_str(), - '.', - layer.name, - MAX_CUSTOMDATA_LAYER_NAME); - all_names_written.add(layer.name); - - layers_to_write.lookup(attr.domain())->append(layer); - } - } - return; - } - - /* Names within an AttributeStorage are unique. */ - all_names_written.add(attr.name()); ::Attribute attribute_dna{}; attribute_dna.name = attr.name().c_str(); attribute_dna.data_type = int16_t(attr.data_type()); diff --git a/source/blender/blenkernel/intern/curves_geometry.cc b/source/blender/blenkernel/intern/curves_geometry.cc index af91f692fa1..955377f802e 100644 --- a/source/blender/blenkernel/intern/curves_geometry.cc +++ b/source/blender/blenkernel/intern/curves_geometry.cc @@ -1926,10 +1926,7 @@ CurvesGeometry::BlendWriteData::BlendWriteData(ResourceScope &scope) void CurvesGeometry::blend_write_prepare(CurvesGeometry::BlendWriteData &write_data) { - attribute_storage_blend_write_prepare(this->attribute_storage.wrap(), - {{AttrDomain::Point, &write_data.point_layers}, - {AttrDomain::Curve, &write_data.curve_layers}}, - write_data.attribute_data); + attribute_storage_blend_write_prepare(this->attribute_storage.wrap(), write_data.attribute_data); CustomData_blend_write_prepare(this->point_data, AttrDomain::Point, this->points_num(), diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc index 29fe5df8856..30a29262784 100644 --- a/source/blender/blenkernel/intern/customdata.cc +++ b/source/blender/blenkernel/intern/customdata.cc @@ -5131,34 +5131,31 @@ void CustomData_blend_write_prepare(CustomData &data, if (attribute_name_is_anonymous(name)) { continue; } - if (U.experimental.use_attribute_storage_write) { - /* When this experimental option is turned on, we always write the data in the new - * #AttributeStorage format, even though it's not yet used at runtime. This is meant for - * testing the forward compatibility capabilities meant to be shipped in version 4.5, in - * other words, the ability to read #AttributeStorage and convert it to #CustomData. This - * block should be removed when the new format is used at runtime. */ - const eCustomDataType data_type = eCustomDataType(layer.type); - if (const std::optional type = custom_data_type_to_attr_type(data_type)) { - ::Attribute attribute_dna{}; - attribute_dna.name = layer.name; - attribute_dna.data_type = int16_t(*type); - attribute_dna.domain = int8_t(domain); - attribute_dna.storage_type = int8_t(AttrStorageType::Array); - /* Do not increase the user count; #::AttributeArray does not act as an owner of the - * attribute data, since it's only used temporarily for writing files. Changing the user - * count would be okay too, but it's unnecessary because none of this data should be - * modified while it's being written anyway. */ - auto &array_dna = write_data.scope.construct<::AttributeArray>(); - array_dna.data = layer.data; - array_dna.sharing_info = layer.sharing_info; - array_dna.size = domain_size; - attribute_dna.data = &array_dna; + /* We always write the data in the new #AttributeStorage format, even though it's not yet used + * at runtime. This block should be removed when the new format is used at runtime. */ + const eCustomDataType data_type = eCustomDataType(layer.type); + if (const std::optional type = custom_data_type_to_attr_type(data_type)) { + ::Attribute attribute_dna{}; + attribute_dna.name = layer.name; + attribute_dna.data_type = int16_t(*type); + attribute_dna.domain = int8_t(domain); + attribute_dna.storage_type = int8_t(AttrStorageType::Array); - write_data.attributes.append(attribute_dna); - continue; - } + /* Do not increase the user count; #::AttributeArray does not act as an owner of the + * attribute data, since it's only used temporarily for writing files. Changing the user + * count would be okay too, but it's unnecessary because none of this data should be + * modified while it's being written anyway. */ + auto &array_dna = write_data.scope.construct<::AttributeArray>(); + array_dna.data = layer.data; + array_dna.sharing_info = layer.sharing_info; + array_dna.size = domain_size; + attribute_dna.data = &array_dna; + + write_data.attributes.append(attribute_dna); + continue; } + layers_to_write.append(layer); } data.totlayer = layers_to_write.size(); diff --git a/source/blender/blenkernel/intern/grease_pencil.cc b/source/blender/blenkernel/intern/grease_pencil.cc index 95a25cba256..56f0607fcd0 100644 --- a/source/blender/blenkernel/intern/grease_pencil.cc +++ b/source/blender/blenkernel/intern/grease_pencil.cc @@ -272,9 +272,7 @@ static void grease_pencil_blend_write(BlendWriter *writer, ID *id, const void *i blender::Vector layers_data_layers; blender::bke::AttributeStorage::BlendWriteData attribute_data{scope}; - attribute_storage_blend_write_prepare(grease_pencil->attribute_storage.wrap(), - {{AttrDomain::Layer, &layers_data_layers}}, - attribute_data); + attribute_storage_blend_write_prepare(grease_pencil->attribute_storage.wrap(), attribute_data); CustomData_blend_write_prepare(grease_pencil->layers_data, AttrDomain::Layer, grease_pencil->layers().size(), diff --git a/source/blender/blenkernel/intern/mesh.cc b/source/blender/blenkernel/intern/mesh.cc index df97bec10a0..28d241c764d 100644 --- a/source/blender/blenkernel/intern/mesh.cc +++ b/source/blender/blenkernel/intern/mesh.cc @@ -309,26 +309,20 @@ static void mesh_blend_write(BlendWriter *writer, ID *id, const void *id_address mesh->totface_legacy = 0; mesh->fdata_legacy = CustomData{}; - if (U.experimental.use_attribute_storage_write) { - /* Convert from the format still used at runtime (flags on #CustomDataLayer) to the format - * reserved for future runtime use (names stored on #Mesh). */ - if (const char *name = CustomData_get_active_layer_name(&mesh->corner_data, CD_PROP_FLOAT2)) { - mesh->active_uv_map_attribute = const_cast( - scope.allocator().copy_string(name).c_str()); - } - else { - mesh->active_uv_map_attribute = nullptr; - } - if (const char *name = CustomData_get_render_layer_name(&mesh->corner_data, CD_PROP_FLOAT2)) { - mesh->default_uv_map_attribute = const_cast( - scope.allocator().copy_string(name).c_str()); - } - else { - mesh->default_uv_map_attribute = nullptr; - } + /* Convert from the format still used at runtime (flags on #CustomDataLayer) to the format + * reserved for future runtime use (names stored on #Mesh). */ + if (const char *name = CustomData_get_active_layer_name(&mesh->corner_data, CD_PROP_FLOAT2)) { + mesh->active_uv_map_attribute = const_cast( + scope.allocator().copy_string(name).c_str()); } else { mesh->active_uv_map_attribute = nullptr; + } + if (const char *name = CustomData_get_render_layer_name(&mesh->corner_data, CD_PROP_FLOAT2)) { + mesh->default_uv_map_attribute = const_cast( + scope.allocator().copy_string(name).c_str()); + } + else { mesh->default_uv_map_attribute = nullptr; } @@ -348,12 +342,7 @@ static void mesh_blend_write(BlendWriter *writer, ID *id, const void *id_address mesh->face_offset_indices = nullptr; } else { - attribute_storage_blend_write_prepare(mesh->attribute_storage.wrap(), - {{AttrDomain::Point, &vert_layers}, - {AttrDomain::Edge, &edge_layers}, - {AttrDomain::Face, &face_layers}, - {AttrDomain::Corner, &loop_layers}}, - attribute_data); + attribute_storage_blend_write_prepare(mesh->attribute_storage.wrap(), attribute_data); CustomData_blend_write_prepare( mesh->vert_data, AttrDomain::Point, mesh->verts_num, vert_layers, attribute_data); CustomData_blend_write_prepare( diff --git a/source/blender/blenkernel/intern/pointcloud.cc b/source/blender/blenkernel/intern/pointcloud.cc index f42bb5d6113..5af023631bf 100644 --- a/source/blender/blenkernel/intern/pointcloud.cc +++ b/source/blender/blenkernel/intern/pointcloud.cc @@ -118,7 +118,7 @@ static void pointcloud_blend_write(BlendWriter *writer, ID *id, const void *id_a ResourceScope scope; bke::AttributeStorage::BlendWriteData attribute_data{scope}; - attribute_storage_blend_write_prepare(pointcloud->attribute_storage.wrap(), {}, attribute_data); + attribute_storage_blend_write_prepare(pointcloud->attribute_storage.wrap(), attribute_data); BLI_assert(pointcloud->pdata_legacy.totlayer == 0); pointcloud->attribute_storage.dna_attributes = attribute_data.attributes.data(); pointcloud->attribute_storage.dna_attributes_num = attribute_data.attributes.size(); diff --git a/source/blender/makesdna/DNA_userdef_types.h b/source/blender/makesdna/DNA_userdef_types.h index 0fcfa4ede95..783a5b04dea 100644 --- a/source/blender/makesdna/DNA_userdef_types.h +++ b/source/blender/makesdna/DNA_userdef_types.h @@ -217,7 +217,6 @@ typedef struct UserDef_Experimental { char use_extensions_debug; char use_recompute_usercount_on_save_debug; char write_legacy_blend_file_format; - char use_attribute_storage_write; char SANITIZE_AFTER_HERE; /* The following options are automatically sanitized (set to 0) * when the release cycle is not alpha. */ @@ -228,7 +227,7 @@ typedef struct UserDef_Experimental { char use_shader_node_previews; char use_bundle_and_closure_nodes; char use_socket_structure_type; - char _pad[4]; + char _pad[5]; } UserDef_Experimental; #define USER_EXPERIMENTAL_TEST(userdef, member) \ diff --git a/source/blender/makesrna/intern/rna_userdef.cc b/source/blender/makesrna/intern/rna_userdef.cc index 84b33db3fe5..f80e5c88d01 100644 --- a/source/blender/makesrna/intern/rna_userdef.cc +++ b/source/blender/makesrna/intern/rna_userdef.cc @@ -7575,12 +7575,6 @@ static void rna_def_userdef_experimental(BlenderRNA *brna) "Recompute all ID usercounts before saving to a blendfile. Allows to " "work around invalid usercount handling in code that may lead to loss " "of data due to wrongly detected unused data-blocks"); - - prop = RNA_def_property(srna, "use_attribute_storage_write", PROP_BOOLEAN, PROP_NONE); - RNA_def_property_ui_text(prop, - "Write New Attribute Storage Format", - "Instead of writing with the older \"CustomData\" format for forward " - "compatibility, use the new \"AttributeStorage\" format"); } static void rna_def_userdef_addon_collection(BlenderRNA *brna, PropertyRNA *cprop)