From dad9f45561a1cd129d86cbdd93b883dcaef0df26 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Tue, 10 Jan 2023 17:07:30 +1100 Subject: [PATCH] Cleanup: move run-time members of SurfaceModifierData into a struct Using run-time members in the surface modifier complicated code-review and caused an unnecessary renaming in `dna_rename_defs.h`. Also rename: - `x` -> `vert_positions_prev`. - `v` -> `vert_velocities`. - `cfra` -> `cfra_prev`. --- source/blender/blenkernel/intern/effect.c | 17 ++-- .../blender/makesdna/DNA_modifier_defaults.h | 7 +- source/blender/makesdna/DNA_modifier_types.h | 18 ++-- .../blender/makesdna/intern/dna_rename_defs.h | 1 - source/blender/modifiers/intern/MOD_surface.c | 96 +++++++++---------- 5 files changed, 66 insertions(+), 73 deletions(-) diff --git a/source/blender/blenkernel/intern/effect.c b/source/blender/blenkernel/intern/effect.c index 84fda9034cb..3c8b76380ee 100644 --- a/source/blender/blenkernel/intern/effect.c +++ b/source/blender/blenkernel/intern/effect.c @@ -644,13 +644,13 @@ bool closest_point_on_surface(SurfaceModifierData *surmd, float surface_nor[3], float surface_vel[3]) { + BVHTreeFromMesh *bvhtree = surmd->runtime.bvhtree; BVHTreeNearest nearest; nearest.index = -1; nearest.dist_sq = FLT_MAX; - BLI_bvhtree_find_nearest( - surmd->bvhtree->tree, co, &nearest, surmd->bvhtree->nearest_callback, surmd->bvhtree); + BLI_bvhtree_find_nearest(bvhtree->tree, co, &nearest, bvhtree->nearest_callback, bvhtree); if (nearest.index != -1) { copy_v3_v3(surface_co, nearest.co); @@ -660,12 +660,12 @@ bool closest_point_on_surface(SurfaceModifierData *surmd, } if (surface_vel) { - const MLoop *mloop = surmd->bvhtree->loop; - const MLoopTri *lt = &surmd->bvhtree->looptri[nearest.index]; + const MLoop *mloop = bvhtree->loop; + const MLoopTri *lt = &bvhtree->looptri[nearest.index]; - copy_v3_v3(surface_vel, surmd->v[mloop[lt->tri[0]].v]); - add_v3_v3(surface_vel, surmd->v[mloop[lt->tri[1]].v]); - add_v3_v3(surface_vel, surmd->v[mloop[lt->tri[2]].v]); + copy_v3_v3(surface_vel, surmd->runtime.vert_velocities[mloop[lt->tri[0]].v]); + add_v3_v3(surface_vel, surmd->runtime.vert_velocities[mloop[lt->tri[1]].v]); + add_v3_v3(surface_vel, surmd->runtime.vert_velocities[mloop[lt->tri[2]].v]); mul_v3_fl(surface_vel, (1.0f / 3.0f)); } @@ -684,7 +684,8 @@ bool get_effector_data(EffectorCache *eff, /* In case surface object is in Edit mode when loading the .blend, * surface modifier is never executed and bvhtree never built, see T48415. */ - if (eff->pd && eff->pd->shape == PFIELD_SHAPE_SURFACE && eff->surmd && eff->surmd->bvhtree) { + if (eff->pd && eff->pd->shape == PFIELD_SHAPE_SURFACE && eff->surmd && + eff->surmd->runtime.bvhtree) { /* closest point in the object surface is an effector */ float vec[3]; diff --git a/source/blender/makesdna/DNA_modifier_defaults.h b/source/blender/makesdna/DNA_modifier_defaults.h index 92a65a50bd4..6ba973ee4ec 100644 --- a/source/blender/makesdna/DNA_modifier_defaults.h +++ b/source/blender/makesdna/DNA_modifier_defaults.h @@ -616,12 +616,7 @@ #define _DNA_DEFAULT_SurfaceModifierData \ { \ - .x = NULL, \ - .v = NULL, \ - .mesh = NULL, \ - .bvhtree = NULL, \ - .cfra = 0, \ - .verts_num = 0, \ + /* Intentionally empty (all run-time data). */ \ } #define _DNA_DEFAULT_SurfaceDeformModifierData \ diff --git a/source/blender/makesdna/DNA_modifier_types.h b/source/blender/makesdna/DNA_modifier_types.h index 170fa0b4351..aa7df999cf5 100644 --- a/source/blender/makesdna/DNA_modifier_types.h +++ b/source/blender/makesdna/DNA_modifier_types.h @@ -877,20 +877,24 @@ typedef struct CollisionModifierData { struct BVHTree *bvhtree; } CollisionModifierData; -typedef struct SurfaceModifierData { - ModifierData modifier; +typedef struct SurfaceModifierData_Runtime { - /** Old position. */ - float (*x)[3]; - /** Velocity. */ - float (*v)[3]; + float (*vert_positions_prev)[3]; + float (*vert_velocities)[3]; struct Mesh *mesh; /** Bounding volume hierarchy of the mesh faces. */ struct BVHTreeFromMesh *bvhtree; - int cfra, verts_num; + int cfra_prev, verts_num; + +} SurfaceModifierData_Runtime; + +typedef struct SurfaceModifierData { + ModifierData modifier; + + SurfaceModifierData_Runtime runtime; } SurfaceModifierData; typedef struct BooleanModifierData { diff --git a/source/blender/makesdna/intern/dna_rename_defs.h b/source/blender/makesdna/intern/dna_rename_defs.h index 6d4592bb338..dce7d353fe6 100644 --- a/source/blender/makesdna/intern/dna_rename_defs.h +++ b/source/blender/makesdna/intern/dna_rename_defs.h @@ -125,7 +125,6 @@ DNA_STRUCT_RENAME_ELEM(SpaceSeq, overlay_type, overlay_frame_type) DNA_STRUCT_RENAME_ELEM(SurfaceDeformModifierData, num_mesh_verts, mesh_verts_num) DNA_STRUCT_RENAME_ELEM(SurfaceDeformModifierData, numpoly, target_polys_num) DNA_STRUCT_RENAME_ELEM(SurfaceDeformModifierData, numverts, bind_verts_num) -DNA_STRUCT_RENAME_ELEM(SurfaceModifierData, numverts, verts_num) DNA_STRUCT_RENAME_ELEM(Text, name, filepath) DNA_STRUCT_RENAME_ELEM(ThemeSpace, scrubbing_background, time_scrub_background) DNA_STRUCT_RENAME_ELEM(ThemeSpace, show_back_grad, background_type) diff --git a/source/blender/modifiers/intern/MOD_surface.c b/source/blender/modifiers/intern/MOD_surface.c index ba63468d075..00fe739b49c 100644 --- a/source/blender/modifiers/intern/MOD_surface.c +++ b/source/blender/modifiers/intern/MOD_surface.c @@ -56,10 +56,7 @@ static void copyData(const ModifierData *md_src, ModifierData *md_dst, const int BKE_modifier_copydata_generic(md_src, md_dst, flag); - surmd_dst->bvhtree = NULL; - surmd_dst->mesh = NULL; - surmd_dst->x = NULL; - surmd_dst->v = NULL; + memset(&surmd_dst->runtime, 0, sizeof(surmd_dst->runtime)); } static void freeData(ModifierData *md) @@ -67,19 +64,19 @@ static void freeData(ModifierData *md) SurfaceModifierData *surmd = (SurfaceModifierData *)md; if (surmd) { - if (surmd->bvhtree) { - free_bvhtree_from_mesh(surmd->bvhtree); - MEM_SAFE_FREE(surmd->bvhtree); + if (surmd->runtime.bvhtree) { + free_bvhtree_from_mesh(surmd->runtime.bvhtree); + MEM_SAFE_FREE(surmd->runtime.bvhtree); } - if (surmd->mesh) { - BKE_id_free(NULL, surmd->mesh); - surmd->mesh = NULL; + if (surmd->runtime.mesh) { + BKE_id_free(NULL, surmd->runtime.mesh); + surmd->runtime.mesh = NULL; } - MEM_SAFE_FREE(surmd->x); + MEM_SAFE_FREE(surmd->runtime.vert_positions_prev); - MEM_SAFE_FREE(surmd->v); + MEM_SAFE_FREE(surmd->runtime.vert_velocities); } } @@ -98,23 +95,24 @@ static void deformVerts(ModifierData *md, const int cfra = (int)DEG_get_ctime(ctx->depsgraph); /* Free mesh and BVH cache. */ - if (surmd->bvhtree) { - free_bvhtree_from_mesh(surmd->bvhtree); - MEM_SAFE_FREE(surmd->bvhtree); + if (surmd->runtime.bvhtree) { + free_bvhtree_from_mesh(surmd->runtime.bvhtree); + MEM_SAFE_FREE(surmd->runtime.bvhtree); } - if (surmd->mesh) { - BKE_id_free(NULL, surmd->mesh); - surmd->mesh = NULL; + if (surmd->runtime.mesh) { + BKE_id_free(NULL, surmd->runtime.mesh); + surmd->runtime.mesh = NULL; } if (mesh) { /* Not possible to use get_mesh() in this case as we'll modify its vertices * and get_mesh() would return 'mesh' directly. */ - surmd->mesh = (Mesh *)BKE_id_copy_ex(NULL, (ID *)mesh, NULL, LIB_ID_COPY_LOCALIZE); + surmd->runtime.mesh = (Mesh *)BKE_id_copy_ex(NULL, (ID *)mesh, NULL, LIB_ID_COPY_LOCALIZE); } else { - surmd->mesh = MOD_deform_mesh_eval_get(ctx->object, NULL, NULL, NULL, verts_num, false); + surmd->runtime.mesh = MOD_deform_mesh_eval_get( + ctx->object, NULL, NULL, NULL, verts_num, false); } if (!ctx->object->pd) { @@ -122,61 +120,61 @@ static void deformVerts(ModifierData *md, return; } - if (surmd->mesh) { + if (surmd->runtime.mesh) { uint mesh_verts_num = 0, i = 0; int init = 0; - BKE_mesh_vert_coords_apply(surmd->mesh, vertexCos); + BKE_mesh_vert_coords_apply(surmd->runtime.mesh, vertexCos); - mesh_verts_num = surmd->mesh->totvert; + mesh_verts_num = surmd->runtime.mesh->totvert; - if (mesh_verts_num != surmd->verts_num || surmd->x == NULL || surmd->v == NULL || - cfra != surmd->cfra + 1) { - if (surmd->x) { - MEM_freeN(surmd->x); - surmd->x = NULL; - } - if (surmd->v) { - MEM_freeN(surmd->v); - surmd->v = NULL; - } + if ((mesh_verts_num != surmd->runtime.verts_num) || + (surmd->runtime.vert_positions_prev == NULL) || (surmd->runtime.vert_velocities == NULL) || + (cfra != surmd->runtime.cfra_prev + 1)) { - surmd->x = MEM_calloc_arrayN(mesh_verts_num, sizeof(float[3]), __func__); - surmd->v = MEM_calloc_arrayN(mesh_verts_num, sizeof(float[3]), __func__); + MEM_SAFE_FREE(surmd->runtime.vert_positions_prev); + MEM_SAFE_FREE(surmd->runtime.vert_velocities); - surmd->verts_num = mesh_verts_num; + surmd->runtime.vert_positions_prev = MEM_calloc_arrayN( + mesh_verts_num, sizeof(float[3]), __func__); + surmd->runtime.vert_velocities = MEM_calloc_arrayN( + mesh_verts_num, sizeof(float[3]), __func__); + + surmd->runtime.verts_num = mesh_verts_num; init = 1; } /* convert to global coordinates and calculate velocity */ - float(*positions)[3] = BKE_mesh_vert_positions_for_write(surmd->mesh); + float(*positions)[3] = BKE_mesh_vert_positions_for_write(surmd->runtime.mesh); for (i = 0; i < mesh_verts_num; i++) { float *vec = positions[i]; mul_m4_v3(ctx->object->object_to_world, vec); if (init) { - surmd->v[i][0] = surmd->v[i][1] = surmd->v[i][2] = 0.0f; + zero_v3(surmd->runtime.vert_velocities[i]); } else { - sub_v3_v3v3(surmd->v[i], vec, surmd->x[i]); + sub_v3_v3v3(surmd->runtime.vert_velocities[i], vec, surmd->runtime.vert_positions_prev[i]); } - copy_v3_v3(surmd->x[i], vec); + copy_v3_v3(surmd->runtime.vert_positions_prev[i], vec); } - surmd->cfra = cfra; + surmd->runtime.cfra_prev = cfra; - const bool has_poly = surmd->mesh->totpoly > 0; - const bool has_edge = surmd->mesh->totedge > 0; + const bool has_poly = surmd->runtime.mesh->totpoly > 0; + const bool has_edge = surmd->runtime.mesh->totedge > 0; if (has_poly || has_edge) { - surmd->bvhtree = MEM_callocN(sizeof(BVHTreeFromMesh), "BVHTreeFromMesh"); + surmd->runtime.bvhtree = MEM_callocN(sizeof(BVHTreeFromMesh), "BVHTreeFromMesh"); if (has_poly) { - BKE_bvhtree_from_mesh_get(surmd->bvhtree, surmd->mesh, BVHTREE_FROM_LOOPTRI, 2); + BKE_bvhtree_from_mesh_get( + surmd->runtime.bvhtree, surmd->runtime.mesh, BVHTREE_FROM_LOOPTRI, 2); } else if (has_edge) { - BKE_bvhtree_from_mesh_get(surmd->bvhtree, surmd->mesh, BVHTREE_FROM_EDGES, 2); + BKE_bvhtree_from_mesh_get( + surmd->runtime.bvhtree, surmd->runtime.mesh, BVHTREE_FROM_EDGES, 2); } } } @@ -202,11 +200,7 @@ static void blendRead(BlendDataReader *UNUSED(reader), ModifierData *md) { SurfaceModifierData *surmd = (SurfaceModifierData *)md; - surmd->mesh = NULL; - surmd->bvhtree = NULL; - surmd->x = NULL; - surmd->v = NULL; - surmd->verts_num = 0; + memset(&surmd->runtime, 0, sizeof(surmd->runtime)); } ModifierTypeInfo modifierType_Surface = {