From 3152d68b7006f5e5279be01badb4c494ccc9e928 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Sun, 29 May 2022 11:02:10 +0200 Subject: Cleanup: Simplify custom data file writing process Previously the function had a fair amount of ugly boilerplate to avoid allocating the temporary layers array, and then free it if necessary. `blender::Vector` solves that problem more elegantly. Passing a span, using references in a few cases, and using a switch statement also make the functions simpler. This refactoring is in preparation for D14583 and D14685. Differential Revision: https://developer.blender.org/D15011 --- source/blender/blenkernel/BKE_customdata.h | 44 ++++---- source/blender/blenkernel/intern/curves.cc | 24 ++--- source/blender/blenkernel/intern/customdata.cc | 141 ++++++++++--------------- source/blender/blenkernel/intern/mesh.cc | 52 +++------ source/blender/blenkernel/intern/pointcloud.cc | 20 ++-- source/blender/makesdna/DNA_customdata_types.h | 2 - 6 files changed, 111 insertions(+), 172 deletions(-) (limited to 'source/blender') diff --git a/source/blender/blenkernel/BKE_customdata.h b/source/blender/blenkernel/BKE_customdata.h index f05dfb164cf..64c49830dc5 100644 --- a/source/blender/blenkernel/BKE_customdata.h +++ b/source/blender/blenkernel/BKE_customdata.h @@ -10,6 +10,10 @@ #include "BLI_sys_types.h" #include "BLI_utildefines.h" +#ifdef __cplusplus +# include "BLI_span.hh" +# include "BLI_vector.hh" +#endif #include "DNA_customdata_types.h" @@ -700,39 +704,33 @@ void CustomData_data_transfer(const struct MeshPairRemap *me_remap, /* .blend file I/O */ +#ifdef __cplusplus + /** * Prepare given custom data for file writing. * - * \param data: the custom-data to tweak for .blend file writing (modified in place). - * \param r_write_layers: contains a reduced set of layers to be written to file, - * use it with #writestruct_at_address() - * (caller must free it if != \a write_layers_buff). - * - * \param write_layers_buff: An optional buffer for r_write_layers (to avoid allocating it). - * \param write_layers_size: The size of pre-allocated \a write_layer_buff. + * \param data: The custom-data to tweak for .blend file writing (modified in place). + * \param layers_to_write: A reduced set of layers to be written to file. * - * \warning After this function has ran, given custom data is no more valid from Blender POV - * (its `totlayer` is invalid). This function shall always be called with localized data - * (as it is in write_meshes()). - * - * \note `data->typemap` is not updated here, since it is always rebuilt on file read anyway. - * This means written `typemap` does not match written layers (as returned by \a r_write_layers). - * Trivial to fix is ever needed. + * \warning This function invalidates the custom data struct by changing the layer counts and the + * #layers pointer, and by invalidating the type map. It expects to work on a shallow copy of + * the struct. */ -void CustomData_blend_write_prepare(struct CustomData *data, - struct CustomDataLayer **r_write_layers, - struct CustomDataLayer *write_layers_buff, - size_t write_layers_size); +void CustomData_blend_write_prepare(CustomData &data, + blender::Vector &layers_to_write); /** - * \param layers: The layers argument assigned by #CustomData_blend_write_prepare. + * \param layers_to_write: Layers created by #CustomData_blend_write_prepare. */ -void CustomData_blend_write(struct BlendWriter *writer, - struct CustomData *data, - CustomDataLayer *layers, +void CustomData_blend_write(BlendWriter *writer, + CustomData *data, + blender::Span layers_to_write, int count, CustomDataMask cddata_mask, - struct ID *id); + ID *id); + +#endif + void CustomData_blend_read(struct BlendDataReader *reader, struct CustomData *data, int count); #ifndef NDEBUG diff --git a/source/blender/blenkernel/intern/curves.cc b/source/blender/blenkernel/intern/curves.cc index d38bc790978..ab9dd702630 100644 --- a/source/blender/blenkernel/intern/curves.cc +++ b/source/blender/blenkernel/intern/curves.cc @@ -23,6 +23,7 @@ #include "BLI_span.hh" #include "BLI_string.h" #include "BLI_utildefines.h" +#include "BLI_vector.hh" #include "BKE_anim_data.h" #include "BKE_curves.hh" @@ -48,6 +49,7 @@ using blender::IndexRange; using blender::MutableSpan; using blender::RandomNumberGenerator; using blender::Span; +using blender::Vector; static const char *ATTR_POSITION = "position"; @@ -121,12 +123,10 @@ static void curves_blend_write(BlendWriter *writer, ID *id, const void *id_addre { Curves *curves = (Curves *)id; - CustomDataLayer *players = nullptr, players_buff[CD_TEMP_CHUNK_SIZE]; - CustomDataLayer *clayers = nullptr, clayers_buff[CD_TEMP_CHUNK_SIZE]; - CustomData_blend_write_prepare( - &curves->geometry.point_data, &players, players_buff, ARRAY_SIZE(players_buff)); - CustomData_blend_write_prepare( - &curves->geometry.curve_data, &clayers, clayers_buff, ARRAY_SIZE(clayers_buff)); + Vector point_layers; + Vector curve_layers; + CustomData_blend_write_prepare(curves->geometry.point_data, point_layers); + CustomData_blend_write_prepare(curves->geometry.curve_data, curve_layers); /* Write LibData */ BLO_write_id_struct(writer, Curves, id_address, &curves->id); @@ -135,13 +135,13 @@ static void curves_blend_write(BlendWriter *writer, ID *id, const void *id_addre /* Direct data */ CustomData_blend_write(writer, &curves->geometry.point_data, - players, + point_layers, curves->geometry.point_num, CD_MASK_ALL, &curves->id); CustomData_blend_write(writer, &curves->geometry.curve_data, - clayers, + curve_layers, curves->geometry.curve_num, CD_MASK_ALL, &curves->id); @@ -152,14 +152,6 @@ static void curves_blend_write(BlendWriter *writer, ID *id, const void *id_addre if (curves->adt) { BKE_animdata_blend_write(writer, curves->adt); } - - /* Remove temporary data. */ - if (players && players != players_buff) { - MEM_freeN(players); - } - if (clayers && clayers != clayers_buff) { - MEM_freeN(clayers); - } } static void curves_blend_read_data(BlendDataReader *reader, ID *id) diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc index 27e8bf96dc6..da5c8389be2 100644 --- a/source/blender/blenkernel/intern/customdata.cc +++ b/source/blender/blenkernel/intern/customdata.cc @@ -25,6 +25,7 @@ #include "BLI_math_vector.hh" #include "BLI_mempool.h" #include "BLI_path_util.h" +#include "BLI_span.hh" #include "BLI_string.h" #include "BLI_string_utils.h" #include "BLI_utildefines.h" @@ -54,6 +55,9 @@ /* only for customdata_data_transfer_interp_normal_normals */ #include "data_transfer_intern.h" +using blender::Span; +using blender::Vector; + /* number of layers to add when growing a CustomData object */ #define CUSTOMDATA_GROW 5 @@ -4349,45 +4353,19 @@ void CustomData_file_write_info(int type, const char **r_struct_name, int *r_str *r_struct_num = typeInfo->structnum; } -void CustomData_blend_write_prepare(CustomData *data, - CustomDataLayer **r_write_layers, - CustomDataLayer *write_layers_buff, - size_t write_layers_size) +void CustomData_blend_write_prepare(CustomData &data, Vector &layers_to_write) { - CustomDataLayer *write_layers = write_layers_buff; - const size_t chunk_size = (write_layers_size > 0) ? write_layers_size : CD_TEMP_CHUNK_SIZE; - - const int totlayer = data->totlayer; - int i, j; - - for (i = 0, j = 0; i < totlayer; i++) { - CustomDataLayer *layer = &data->layers[i]; - /* Layers with this flag set are not written to file. */ - if ((layer->flag & CD_FLAG_NOCOPY) || layer->anonymous_id != nullptr) { - data->totlayer--; - // CLOG_WARN(&LOG, "skipping layer %p (%s)", layer, layer->name); + for (const CustomDataLayer &layer : Span(data.layers, data.totlayer)) { + if (layer.flag & CD_FLAG_NOCOPY) { + continue; } - else { - if (UNLIKELY((size_t)j >= write_layers_size)) { - if (write_layers == write_layers_buff) { - write_layers = (CustomDataLayer *)MEM_malloc_arrayN( - (write_layers_size + chunk_size), sizeof(*write_layers), __func__); - if (write_layers_buff) { - memcpy(write_layers, write_layers_buff, sizeof(*write_layers) * write_layers_size); - } - } - else { - write_layers = (CustomDataLayer *)MEM_reallocN( - write_layers, sizeof(*write_layers) * (write_layers_size + chunk_size)); - } - write_layers_size += chunk_size; - } - write_layers[j++] = *layer; + if (layer.anonymous_id != nullptr) { + continue; } + layers_to_write.append(layer); } - BLI_assert(j == data->totlayer); - data->maxlayer = data->totlayer; /* We only write that much of data! */ - *r_write_layers = write_layers; + data.totlayer = layers_to_write.size(); + data.maxlayer = data.totlayer; } int CustomData_sizeof(int type) @@ -5186,7 +5164,7 @@ static void write_grid_paint_mask(BlendWriter *writer, void CustomData_blend_write(BlendWriter *writer, CustomData *data, - CustomDataLayer *layers, + Span layers_to_write, int count, CustomDataMask cddata_mask, ID *id) @@ -5196,55 +5174,50 @@ void CustomData_blend_write(BlendWriter *writer, CustomData_external_write(data, id, cddata_mask, count, 0); } - BLO_write_struct_array_at_address(writer, CustomDataLayer, data->totlayer, data->layers, layers); - - for (int i = 0; i < data->totlayer; i++) { - CustomDataLayer *layer = &layers[i]; + BLO_write_struct_array_at_address( + writer, CustomDataLayer, data->totlayer, data->layers, layers_to_write.data()); - if (layer->type == CD_MDEFORMVERT) { - /* layer types that allocate own memory need special handling */ - BKE_defvert_blend_write(writer, count, static_cast(layer->data)); - } - else if (layer->type == CD_MDISPS) { - write_mdisps( - writer, count, static_cast(layer->data), layer->flag & CD_FLAG_EXTERNAL); - } - else if (layer->type == CD_PAINT_MASK) { - const float *layer_data = static_cast(layer->data); - BLO_write_raw(writer, sizeof(*layer_data) * count, layer_data); - } - else if (layer->type == CD_SCULPT_FACE_SETS) { - const float *layer_data = static_cast(layer->data); - BLO_write_raw(writer, sizeof(*layer_data) * count, layer_data); - } - else if (layer->type == CD_GRID_PAINT_MASK) { - write_grid_paint_mask(writer, count, static_cast(layer->data)); - } - else if (layer->type == CD_FACEMAP) { - const int *layer_data = static_cast(layer->data); - BLO_write_raw(writer, sizeof(*layer_data) * count, layer_data); - } - else if (layer->type == CD_PROP_BOOL) { - const bool *layer_data = static_cast(layer->data); - BLO_write_raw(writer, sizeof(*layer_data) * count, layer_data); - } - else if (layer->type == CD_CREASE) { - const float *layer_data = static_cast(layer->data); - BLO_write_raw(writer, sizeof(*layer_data) * count, layer_data); - } - else { - const char *structname; - int structnum; - CustomData_file_write_info(layer->type, &structname, &structnum); - if (structnum) { - int datasize = structnum * count; - BLO_write_struct_array_by_name(writer, structname, datasize, layer->data); - } - else if (!BLO_write_is_undo(writer)) { /* Do not warn on undo. */ - printf("%s error: layer '%s':%d - can't be written to file\n", - __func__, - structname, - layer->type); + for (const CustomDataLayer &layer : layers_to_write) { + switch (layer.type) { + case CD_MDEFORMVERT: + BKE_defvert_blend_write(writer, count, static_cast(layer.data)); + break; + case CD_MDISPS: + write_mdisps( + writer, count, static_cast(layer.data), layer.flag & CD_FLAG_EXTERNAL); + break; + case CD_PAINT_MASK: + BLO_write_raw(writer, sizeof(float) * count, static_cast(layer.data)); + break; + case CD_SCULPT_FACE_SETS: + BLO_write_raw(writer, sizeof(float) * count, static_cast(layer.data)); + break; + case CD_GRID_PAINT_MASK: + write_grid_paint_mask(writer, count, static_cast(layer.data)); + break; + case CD_FACEMAP: + BLO_write_raw(writer, sizeof(int) * count, static_cast(layer.data)); + break; + case CD_PROP_BOOL: + BLO_write_raw(writer, sizeof(bool) * count, static_cast(layer.data)); + break; + case CD_CREASE: + BLO_write_raw(writer, sizeof(float) * count, static_cast(layer.data)); + break; + default: { + const char *structname; + int structnum; + CustomData_file_write_info(layer.type, &structname, &structnum); + if (structnum) { + int datasize = structnum * count; + BLO_write_struct_array_by_name(writer, structname, datasize, layer.data); + } + else if (!BLO_write_is_undo(writer)) { /* Do not warn on undo. */ + printf("%s error: layer '%s':%d - can't be written to file\n", + __func__, + structname, + layer.type); + } } } } diff --git a/source/blender/blenkernel/intern/mesh.cc b/source/blender/blenkernel/intern/mesh.cc index 05baf156099..0becea62810 100644 --- a/source/blender/blenkernel/intern/mesh.cc +++ b/source/blender/blenkernel/intern/mesh.cc @@ -31,6 +31,7 @@ #include "BLI_string.h" #include "BLI_task.hh" #include "BLI_utildefines.h" +#include "BLI_vector.hh" #include "BLT_translation.h" @@ -60,6 +61,7 @@ #include "BLO_read_write.h" using blender::float3; +using blender::Vector; static void mesh_clear_geometry(Mesh *mesh); static void mesh_tessface_clear_intern(Mesh *mesh, int free_customdata); @@ -208,46 +210,40 @@ static void mesh_blend_write(BlendWriter *writer, ID *id, const void *id_address Mesh *mesh = (Mesh *)id; const bool is_undo = BLO_write_is_undo(writer); - CustomDataLayer *vlayers = nullptr, vlayers_buff[CD_TEMP_CHUNK_SIZE]; - CustomDataLayer *elayers = nullptr, elayers_buff[CD_TEMP_CHUNK_SIZE]; - CustomDataLayer *flayers = nullptr, flayers_buff[CD_TEMP_CHUNK_SIZE]; - CustomDataLayer *llayers = nullptr, llayers_buff[CD_TEMP_CHUNK_SIZE]; - CustomDataLayer *players = nullptr, players_buff[CD_TEMP_CHUNK_SIZE]; + Vector vert_layers; + Vector edge_layers; + Vector loop_layers; + Vector poly_layers; /* cache only - don't write */ mesh->mface = nullptr; mesh->totface = 0; memset(&mesh->fdata, 0, sizeof(mesh->fdata)); mesh->runtime = blender::dna::shallow_zero_initialize(); - flayers = flayers_buff; /* Do not store actual geometry data in case this is a library override ID. */ if (ID_IS_OVERRIDE_LIBRARY(mesh) && !is_undo) { mesh->mvert = nullptr; mesh->totvert = 0; memset(&mesh->vdata, 0, sizeof(mesh->vdata)); - vlayers = vlayers_buff; mesh->medge = nullptr; mesh->totedge = 0; memset(&mesh->edata, 0, sizeof(mesh->edata)); - elayers = elayers_buff; mesh->mloop = nullptr; mesh->totloop = 0; memset(&mesh->ldata, 0, sizeof(mesh->ldata)); - llayers = llayers_buff; mesh->mpoly = nullptr; mesh->totpoly = 0; memset(&mesh->pdata, 0, sizeof(mesh->pdata)); - players = players_buff; } else { - CustomData_blend_write_prepare(&mesh->vdata, &vlayers, vlayers_buff, ARRAY_SIZE(vlayers_buff)); - CustomData_blend_write_prepare(&mesh->edata, &elayers, elayers_buff, ARRAY_SIZE(elayers_buff)); - CustomData_blend_write_prepare(&mesh->ldata, &llayers, llayers_buff, ARRAY_SIZE(llayers_buff)); - CustomData_blend_write_prepare(&mesh->pdata, &players, players_buff, ARRAY_SIZE(players_buff)); + CustomData_blend_write_prepare(mesh->vdata, vert_layers); + CustomData_blend_write_prepare(mesh->edata, edge_layers); + CustomData_blend_write_prepare(mesh->ldata, loop_layers); + CustomData_blend_write_prepare(mesh->pdata, poly_layers); } BLO_write_id_struct(writer, Mesh, id_address, &mesh->id); @@ -264,33 +260,15 @@ static void mesh_blend_write(BlendWriter *writer, ID *id, const void *id_address BLO_write_raw(writer, sizeof(MSelect) * mesh->totselect, mesh->mselect); CustomData_blend_write( - writer, &mesh->vdata, vlayers, mesh->totvert, CD_MASK_MESH.vmask, &mesh->id); + writer, &mesh->vdata, vert_layers, mesh->totvert, CD_MASK_MESH.vmask, &mesh->id); CustomData_blend_write( - writer, &mesh->edata, elayers, mesh->totedge, CD_MASK_MESH.emask, &mesh->id); + writer, &mesh->edata, edge_layers, mesh->totedge, CD_MASK_MESH.emask, &mesh->id); /* fdata is really a dummy - written so slots align */ + CustomData_blend_write(writer, &mesh->fdata, {}, mesh->totface, CD_MASK_MESH.fmask, &mesh->id); CustomData_blend_write( - writer, &mesh->fdata, flayers, mesh->totface, CD_MASK_MESH.fmask, &mesh->id); + writer, &mesh->ldata, loop_layers, mesh->totloop, CD_MASK_MESH.lmask, &mesh->id); CustomData_blend_write( - writer, &mesh->ldata, llayers, mesh->totloop, CD_MASK_MESH.lmask, &mesh->id); - CustomData_blend_write( - writer, &mesh->pdata, players, mesh->totpoly, CD_MASK_MESH.pmask, &mesh->id); - - /* Free temporary data */ - - /* Free custom-data layers, when not assigned a buffer value. */ -#define CD_LAYERS_FREE(id) \ - if (id && id != id##_buff) { \ - MEM_freeN(id); \ - } \ - ((void)0) - - CD_LAYERS_FREE(vlayers); - CD_LAYERS_FREE(elayers); - // CD_LAYER_FREE(flayers); /* Never allocated. */ - CD_LAYERS_FREE(llayers); - CD_LAYERS_FREE(players); - -#undef CD_LAYERS_FREE + writer, &mesh->pdata, poly_layers, mesh->totpoly, CD_MASK_MESH.pmask, &mesh->id); } static void mesh_blend_read_data(BlendDataReader *reader, ID *id) diff --git a/source/blender/blenkernel/intern/pointcloud.cc b/source/blender/blenkernel/intern/pointcloud.cc index 3ee46fc4f15..9720c61e3b9 100644 --- a/source/blender/blenkernel/intern/pointcloud.cc +++ b/source/blender/blenkernel/intern/pointcloud.cc @@ -20,6 +20,7 @@ #include "BLI_string.h" #include "BLI_task.hh" #include "BLI_utildefines.h" +#include "BLI_vector.hh" #include "BKE_anim_data.h" #include "BKE_customdata.h" @@ -44,6 +45,7 @@ using blender::float3; using blender::IndexRange; using blender::Span; +using blender::Vector; /* PointCloud datablock */ @@ -107,27 +109,25 @@ static void pointcloud_blend_write(BlendWriter *writer, ID *id, const void *id_a { PointCloud *pointcloud = (PointCloud *)id; - CustomDataLayer *players = nullptr, players_buff[CD_TEMP_CHUNK_SIZE]; - CustomData_blend_write_prepare( - &pointcloud->pdata, &players, players_buff, ARRAY_SIZE(players_buff)); + Vector point_layers; + CustomData_blend_write_prepare(pointcloud->pdata, point_layers); /* Write LibData */ BLO_write_id_struct(writer, PointCloud, id_address, &pointcloud->id); BKE_id_blend_write(writer, &pointcloud->id); /* Direct data */ - CustomData_blend_write( - writer, &pointcloud->pdata, players, pointcloud->totpoint, CD_MASK_ALL, &pointcloud->id); + CustomData_blend_write(writer, + &pointcloud->pdata, + point_layers, + pointcloud->totpoint, + CD_MASK_ALL, + &pointcloud->id); BLO_write_pointer_array(writer, pointcloud->totcol, pointcloud->mat); if (pointcloud->adt) { BKE_animdata_blend_write(writer, pointcloud->adt); } - - /* Remove temporary data. */ - if (players && players != players_buff) { - MEM_freeN(players); - } } static void pointcloud_blend_read_data(BlendDataReader *reader, ID *id) diff --git a/source/blender/makesdna/DNA_customdata_types.h b/source/blender/makesdna/DNA_customdata_types.h index ef937fb139b..fe06e97946c 100644 --- a/source/blender/makesdna/DNA_customdata_types.h +++ b/source/blender/makesdna/DNA_customdata_types.h @@ -260,8 +260,6 @@ enum { #define DYNTOPO_NODE_NONE -1 -#define CD_TEMP_CHUNK_SIZE 128 - #ifdef __cplusplus } #endif -- cgit v1.2.3