From 934041e8d6fd55411139a0eeb7401e3e8dd80b96 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Wed, 19 Jun 2024 12:16:24 +0200 Subject: [PATCH] BLO: support overaligned types in SDNA This fixes #121695. `float4x4` matrices are generally expected to be 16 byte aligned. Currently, there is no mechanism (afaik) that allows allocating these overaligned types when loading files from disk. This patch adds an array with alignment information for each type in `SDNA`. Currently, the alignment is just `__STDCPP_DEFAULT_NEW_ALIGNMENT__` for all types and is manually set for the `mat4x4f` DNA type. The .blend file format is not changed at all. The alignment information is purely runtime data. In the future it would probably be good to generalize this a bit more instead of hardcoding the alignment for `mat4x4f`, but would make it unnecessarily complex for now because this is intended for the release branch. Pull Request: https://projects.blender.org/blender/blender/pulls/123271 --- .../blender/blenkernel/intern/customdata.cc | 2 ++ source/blender/blenloader/intern/readfile.cc | 3 ++- source/blender/makesdna/DNA_genfile.h | 5 ++++ source/blender/makesdna/DNA_sdna_types.h | 7 ++++++ source/blender/makesdna/DNA_vec_types.h | 5 ++++ source/blender/makesdna/intern/dna_genfile.cc | 24 ++++++++++++++++++- 6 files changed, 44 insertions(+), 2 deletions(-) diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc index 55898872224..fcee9ab4c71 100644 --- a/source/blender/blenkernel/intern/customdata.cc +++ b/source/blender/blenkernel/intern/customdata.cc @@ -2155,6 +2155,8 @@ static const LayerTypeInfo LAYERTYPEINFO[CD_NUMTYPES] = { layerDefault_propquaternion}, }; +static_assert(sizeof(mat4x4f) == sizeof(blender::float4x4)); + static const char *LAYERTYPENAMES[CD_NUMTYPES] = { /* 0-4 */ "CDMVert", "CDMSticky", diff --git a/source/blender/blenloader/intern/readfile.cc b/source/blender/blenloader/intern/readfile.cc index 2c461775696..164948e44f7 100644 --- a/source/blender/blenloader/intern/readfile.cc +++ b/source/blender/blenloader/intern/readfile.cc @@ -1825,7 +1825,8 @@ static void *read_struct(FileData *fd, BHead *bh, const char *blockname) } else { /* SDNA_CMP_EQUAL */ - temp = MEM_mallocN(bh->len, blockname); + const int alignment = DNA_struct_alignment(fd->filesdna, bh->SDNAnr); + temp = MEM_mallocN_aligned(bh->len, alignment, blockname); #ifdef USE_BHEAD_READ_ON_DEMAND if (BHEADN_FROM_BHEAD(bh)->has_data) { memcpy(temp, (bh + 1), bh->len); diff --git a/source/blender/makesdna/DNA_genfile.h b/source/blender/makesdna/DNA_genfile.h index c277fdd1ad5..374f3dafbc7 100644 --- a/source/blender/makesdna/DNA_genfile.h +++ b/source/blender/makesdna/DNA_genfile.h @@ -190,6 +190,11 @@ int DNA_struct_member_size(const struct SDNA *sdna, short type, short name); */ int DNA_elem_type_size(eSDNA_Type elem_nr); +/** + * Get the alignment that should be used when allocating memory for this type. + */ +int DNA_struct_alignment(const struct SDNA *sdna, int struct_nr); + /** * Rename a struct */ diff --git a/source/blender/makesdna/DNA_sdna_types.h b/source/blender/makesdna/DNA_sdna_types.h index 4dc216204e5..ad2ff9e5049 100644 --- a/source/blender/makesdna/DNA_sdna_types.h +++ b/source/blender/makesdna/DNA_sdna_types.h @@ -80,6 +80,13 @@ typedef struct SDNA { /** A version of #SDNA.structs_map that uses #SDNA.alias.types for its keys. */ struct GHash *structs_map; } alias; + + /** + * Alignment used when allocating pointers to this type. The actual minimum alignment of the + * type may be lower in some cases. For example, the pointer alignment of a single char is at + * least 8 bytes, but the alignment of the type itself is 1. + */ + int *types_alignment; } SDNA; # diff --git a/source/blender/makesdna/DNA_vec_types.h b/source/blender/makesdna/DNA_vec_types.h index ba98dadf16b..d3023d5c432 100644 --- a/source/blender/makesdna/DNA_vec_types.h +++ b/source/blender/makesdna/DNA_vec_types.h @@ -50,6 +50,11 @@ typedef struct vec4f { float x, y, z, w; } vec4f; +/** + * This type generally shouldn't be used. It only exists for cases where a DNA type that + * corresponds to `blender:float4x4` is required. Note that `float4x4` is 16 byte aligned, but we + * can't enforce that this struct yet. + */ typedef struct mat4x4f { float value[4][4]; } mat4x4f; diff --git a/source/blender/makesdna/intern/dna_genfile.cc b/source/blender/makesdna/intern/dna_genfile.cc index 885c882ba4b..5ddcdb7c571 100644 --- a/source/blender/makesdna/intern/dna_genfile.cc +++ b/source/blender/makesdna/intern/dna_genfile.cc @@ -21,6 +21,7 @@ #include "MEM_guardedalloc.h" /* for MEM_freeN MEM_mallocN MEM_callocN */ #include "BLI_endian_switch.h" +#include "BLI_math_matrix_types.hh" #include "BLI_memarena.h" #include "BLI_string.h" #include "BLI_utildefines.h" @@ -132,6 +133,7 @@ void DNA_sdna_free(SDNA *sdna) MEM_freeN((void *)sdna->names_array_len); MEM_freeN((void *)sdna->types); MEM_freeN(sdna->structs); + MEM_freeN(sdna->types_alignment); #ifdef WITH_DNA_GHASH if (sdna->structs_map) { @@ -522,6 +524,19 @@ static bool init_structDNA(SDNA *sdna, bool do_endian_swap, const char **r_error sdna->names_array_len = names_array_len; } + sdna->types_alignment = static_cast( + MEM_malloc_arrayN(sdna->types_len, sizeof(int), __func__)); + for (int i = 0; i < sdna->types_len; i++) { + sdna->types_alignment[i] = int(__STDCPP_DEFAULT_NEW_ALIGNMENT__); + } + { + uint dummy_index = 0; + /* TODO: This should be generalized at some point. We should be able to specify overaligned + * types directly in the DNA struct definitions. */ + sdna->types_alignment[DNA_struct_find_without_alias_ex(sdna, "mat4x4f", &dummy_index)] = + alignof(blender::float4x4); + } + return true; } @@ -1265,7 +1280,9 @@ void *DNA_struct_reconstruct(const DNA_ReconstructInfo *reconstruct_info, const SDNA_Struct *new_struct = newsdna->structs[new_struct_nr]; const int new_block_size = newsdna->types_size[new_struct->type]; - char *new_blocks = static_cast(MEM_callocN(blocks * new_block_size, "reconstruct")); + const int alignment = DNA_struct_alignment(newsdna, new_struct_nr); + char *new_blocks = static_cast( + MEM_calloc_arrayN_aligned(new_block_size, blocks, alignment, "reconstruct")); reconstruct_structs(reconstruct_info, blocks, old_struct_nr, @@ -1697,6 +1714,11 @@ int DNA_elem_type_size(const eSDNA_Type elem_nr) return 8; } +int DNA_struct_alignment(const SDNA *sdna, const int struct_nr) +{ + return sdna->types_alignment[struct_nr]; +} + /* -------------------------------------------------------------------- */ /** \name Version Patch DNA * \{ */