From c988a048024e2b29a456c37221f6eb8366f05872 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Fri, 21 Feb 2025 18:48:54 +0100 Subject: [PATCH] Fix #134120: Crash evaluating rigid body in animation render This shared state between original data and depsgraphs was added in 98a0bcd4252e952fa5438e9d1b69b0204f8a8746. Other physics systems also share the pointcache, but not the simulation state to this extent, which leads to this kind of crash. The mutex lock is not a great solution, you don't really want both render and viewport to be filling the same cache in parallel. However this kind of problem also exists in other physics systems, and solving that is certainly beyond the scope of 4.4, and probably needs to wait for a bigger physics rewrite. In general the recommendation is to bake everything before rendering. Pull Request: https://projects.blender.org/blender/blender/pulls/134779 --- source/blender/blenkernel/BKE_rigidbody.h | 7 ++ source/blender/blenkernel/intern/rigidbody.cc | 95 ++++++++++++------- source/blender/blenkernel/intern/scene.cc | 7 +- .../editors/physics/rigidbody_world.cc | 5 +- source/blender/makesdna/DNA_rigidbody_types.h | 6 +- .../blender/makesrna/intern/rna_rigidbody.cc | 16 ++-- 6 files changed, 87 insertions(+), 49 deletions(-) diff --git a/source/blender/blenkernel/BKE_rigidbody.h b/source/blender/blenkernel/BKE_rigidbody.h index e6a60a5a125..402337b7b4c 100644 --- a/source/blender/blenkernel/BKE_rigidbody.h +++ b/source/blender/blenkernel/BKE_rigidbody.h @@ -17,6 +17,7 @@ extern "C" { struct RigidBodyOb; struct RigidBodyWorld; +struct rbDynamicsWorld; struct Collection; struct Depsgraph; @@ -114,6 +115,12 @@ void BKE_rigidbody_main_collection_object_add(struct Main *bmain, struct RigidBodyWorld *BKE_rigidbody_world_copy(struct RigidBodyWorld *rbw, int flag); void BKE_rigidbody_world_groups_relink(struct RigidBodyWorld *rbw); +/** + * Runtime data. + */ +void BKE_rigidbody_world_init_runtime(struct RigidBodyWorld *rbw); +struct rbDynamicsWorld *BKE_rigidbody_world_physics(struct RigidBodyWorld *rbw); + /** * 'validate' (i.e. make new or replace old) Physics-Engine objects. */ diff --git a/source/blender/blenkernel/intern/rigidbody.cc b/source/blender/blenkernel/intern/rigidbody.cc index c81e5219736..149e8a81493 100644 --- a/source/blender/blenkernel/intern/rigidbody.cc +++ b/source/blender/blenkernel/intern/rigidbody.cc @@ -12,6 +12,7 @@ #include #include #include +#include #include "CLG_log.h" @@ -69,9 +70,24 @@ struct rbRigidBody; /* Freeing Methods --------------------- */ +struct RigidBodyWorld_Runtime { +#ifdef WITH_BULLET + rbDynamicsWorld *physics_world = nullptr; +#endif + std::mutex mutex; + + ~RigidBodyWorld_Runtime() + { +#ifdef WITH_BULLET + if (physics_world) { + RB_dworld_delete(physics_world); + } +#endif + } +}; + #ifdef WITH_BULLET static void rigidbody_update_ob_array(RigidBodyWorld *rbw); - #else static void RB_dworld_remove_constraint(void * /*world*/, void * /*con*/) {} static void RB_dworld_remove_body(void * /*world*/, void * /*body*/) {} @@ -82,6 +98,18 @@ static void RB_constraint_delete(void * /*con*/) {} #endif +void BKE_rigidbody_world_init_runtime(RigidBodyWorld *rbw) +{ + if (rbw->shared) { + rbw->shared->runtime = MEM_new(__func__); + } +} + +rbDynamicsWorld *BKE_rigidbody_world_physics(RigidBodyWorld *rbw) +{ + return (rbw->shared) ? rbw->shared->runtime->physics_world : nullptr; +} + void BKE_rigidbody_free_world(Scene *scene) { bool is_orig = (scene->id.tag & ID_TAG_COPIED_ON_EVAL) == 0; @@ -93,7 +121,7 @@ void BKE_rigidbody_free_world(Scene *scene) return; } - if (is_orig && rbw->shared->physics_world) { + if (is_orig && rbw->shared->runtime->physics_world) { /* Free physics references, * we assume that all physics objects in will have been added to the world. */ if (rbw->constraints) { @@ -101,7 +129,7 @@ void BKE_rigidbody_free_world(Scene *scene) if (object->rigidbody_constraint) { RigidBodyCon *rbc = object->rigidbody_constraint; if (rbc->physics_constraint) { - RB_dworld_remove_constraint(static_cast(rbw->shared->physics_world), + RB_dworld_remove_constraint(rbw->shared->runtime->physics_world, static_cast(rbc->physics_constraint)); } } @@ -115,8 +143,6 @@ void BKE_rigidbody_free_world(Scene *scene) } FOREACH_COLLECTION_OBJECT_RECURSIVE_END; } - /* free dynamics world */ - RB_dworld_delete(static_cast(rbw->shared->physics_world)); } if (rbw->objects) { free(rbw->objects); @@ -127,6 +153,7 @@ void BKE_rigidbody_free_world(Scene *scene) BKE_ptcache_free_list(&(rbw->shared->ptcaches)); rbw->shared->pointcache = nullptr; + MEM_delete(rbw->shared->runtime); MEM_freeN(rbw->shared); } @@ -152,11 +179,11 @@ void BKE_rigidbody_free_object(Object *ob, RigidBodyWorld *rbw) /* free physics references */ if (is_orig) { if (rbo->shared->physics_object) { - if (rbw != nullptr && rbw->shared->physics_world != nullptr) { + if (rbw != nullptr && rbw->shared->runtime->physics_world != nullptr) { /* We can only remove the body from the world if the world is known. * The world is generally only unknown if it's an evaluated copy of * an object that's being freed, in which case this code isn't run anyway. */ - RB_dworld_remove_body(static_cast(rbw->shared->physics_world), + RB_dworld_remove_body(rbw->shared->runtime->physics_world, static_cast(rbo->shared->physics_object)); } else { @@ -166,8 +193,8 @@ void BKE_rigidbody_free_object(Object *ob, RigidBodyWorld *rbw) scene = static_cast(scene->id.next)) { RigidBodyWorld *scene_rbw = scene->rigidbody_world; - if (scene_rbw != nullptr && scene_rbw->shared->physics_world != nullptr) { - RB_dworld_remove_body(static_cast(scene_rbw->shared->physics_world), + if (scene_rbw != nullptr && scene_rbw->shared->runtime->physics_world != nullptr) { + RB_dworld_remove_body(scene_rbw->shared->runtime->physics_world, static_cast(rbo->shared->physics_object)); } } @@ -695,7 +722,7 @@ static void rigidbody_validate_sim_object(RigidBodyWorld *rbw, Object *ob, bool if (rbo->shared->physics_object && !rebuild) { /* Don't remove body on rebuild as it has already been removed when deleting and rebuilding the * world. */ - RB_dworld_remove_body(static_cast(rbw->shared->physics_world), + RB_dworld_remove_body(rbw->shared->runtime->physics_world, static_cast(rbo->shared->physics_object)); } if (!rbo->shared->physics_object || rebuild) { @@ -747,8 +774,8 @@ static void rigidbody_validate_sim_object(RigidBodyWorld *rbw, Object *ob, bool rbo->flag & RBO_FLAG_KINEMATIC || rbo->flag & RBO_FLAG_DISABLED); } - if (rbw && rbw->shared->physics_world && rbo->shared->physics_object) { - RB_dworld_add_body(static_cast(rbw->shared->physics_world), + if (rbw && rbw->shared->runtime->physics_world && rbo->shared->physics_object) { + RB_dworld_add_body(rbw->shared->runtime->physics_world, static_cast(rbo->shared->physics_object), rbo->col_groups); } @@ -908,7 +935,7 @@ static void rigidbody_validate_sim_constraint(RigidBodyWorld *rbw, Object *ob, b if (ELEM(nullptr, rbc->ob1, rbc->ob1->rigidbody_object, rbc->ob2, rbc->ob2->rigidbody_object)) { if (rbc->physics_constraint) { - RB_dworld_remove_constraint(static_cast(rbw->shared->physics_world), + RB_dworld_remove_constraint(rbw->shared->runtime->physics_world, static_cast(rbc->physics_constraint)); RB_constraint_delete(static_cast(rbc->physics_constraint)); rbc->physics_constraint = nullptr; @@ -917,7 +944,7 @@ static void rigidbody_validate_sim_constraint(RigidBodyWorld *rbw, Object *ob, b } if (rbc->physics_constraint && rebuild == false) { - RB_dworld_remove_constraint(static_cast(rbw->shared->physics_world), + RB_dworld_remove_constraint(rbw->shared->runtime->physics_world, static_cast(rbc->physics_constraint)); } if (rbc->physics_constraint == nullptr || rebuild) { @@ -1070,8 +1097,8 @@ static void rigidbody_validate_sim_constraint(RigidBodyWorld *rbw, Object *ob, b } } - if (rbw && rbw->shared->physics_world && rbc->physics_constraint) { - RB_dworld_add_constraint(static_cast(rbw->shared->physics_world), + if (rbw && rbw->shared->runtime->physics_world && rbc->physics_constraint) { + RB_dworld_add_constraint(rbw->shared->runtime->physics_world, static_cast(rbc->physics_constraint), rbc->flag & RBC_FLAG_DISABLE_COLLISIONS); } @@ -1087,16 +1114,15 @@ void BKE_rigidbody_validate_sim_world(Scene *scene, RigidBodyWorld *rbw, bool re } /* create new sim world */ - if (rebuild || rbw->shared->physics_world == nullptr) { - if (rbw->shared->physics_world) { - RB_dworld_delete(static_cast(rbw->shared->physics_world)); + if (rebuild || rbw->shared->runtime->physics_world == nullptr) { + if (rbw->shared->runtime->physics_world) { + RB_dworld_delete(rbw->shared->runtime->physics_world); } - rbw->shared->physics_world = RB_dworld_new(scene->physics_settings.gravity); + rbw->shared->runtime->physics_world = RB_dworld_new(scene->physics_settings.gravity); } - RB_dworld_set_solver_iterations(static_cast(rbw->shared->physics_world), - rbw->num_solver_iterations); - RB_dworld_set_split_impulse(static_cast(rbw->shared->physics_world), + RB_dworld_set_solver_iterations(rbw->shared->runtime->physics_world, rbw->num_solver_iterations); + RB_dworld_set_split_impulse(rbw->shared->runtime->physics_world, rbw->flag & RBW_FLAG_USE_SPLIT_IMPULSE); } @@ -1136,6 +1162,8 @@ RigidBodyWorld *BKE_rigidbody_create_world(Scene *scene) rbw->shared->pointcache = BKE_ptcache_add(&(rbw->shared->ptcaches)); rbw->shared->pointcache->step = 1; + BKE_rigidbody_world_init_runtime(rbw); + /* return this sim world */ return rbw; } @@ -1162,6 +1190,7 @@ RigidBodyWorld *BKE_rigidbody_world_copy(RigidBodyWorld *rbw, const int flag) MEM_callocN(sizeof(*rbw_copy->shared), "RigidBodyWorld_Shared")); BKE_ptcache_copy_list(&rbw_copy->shared->ptcaches, &rbw->shared->ptcaches, LIB_ID_COPY_CACHES); rbw_copy->shared->pointcache = static_cast(rbw_copy->shared->ptcaches.first); + BKE_rigidbody_world_init_runtime(rbw_copy); } rbw_copy->objects = nullptr; @@ -1561,8 +1590,8 @@ void BKE_rigidbody_remove_constraint(Main *bmain, Scene *scene, Object *ob, cons } /* remove from rigidbody world, free object won't do this */ - if (rbw->shared->physics_world && rbc->physics_constraint) { - RB_dworld_remove_constraint(static_cast(rbw->shared->physics_world), + if (rbw->shared->runtime->physics_world && rbc->physics_constraint) { + RB_dworld_remove_constraint(rbw->shared->runtime->physics_world, static_cast(rbc->physics_constraint)); } } @@ -1632,7 +1661,7 @@ static void rigidbody_update_sim_world(Scene *scene, RigidBodyWorld *rbw) } /* update gravity, since this RNA setting is not part of RigidBody settings */ - RB_dworld_set_gravity(static_cast(rbw->shared->physics_world), adj_gravity); + RB_dworld_set_gravity(rbw->shared->runtime->physics_world, adj_gravity); /* update object array in case there are changes */ rigidbody_update_ob_array(rbw); @@ -1711,7 +1740,7 @@ static void rigidbody_update_simulation(Depsgraph *depsgraph, /* update world */ /* Note physics_world can get nullptr when undoing the deletion of the last object in it (see * #70667). */ - if (rebuild || rbw->shared->physics_world == nullptr) { + if (rebuild || rbw->shared->runtime->physics_world == nullptr) { BKE_rigidbody_validate_sim_world(scene, rbw, rebuild); /* We have rebuilt the world so we need to make sure the rest is rebuilt as well. */ rebuild = true; @@ -1729,7 +1758,7 @@ static void rigidbody_update_simulation(Depsgraph *depsgraph, FOREACH_COLLECTION_OBJECT_RECURSIVE_BEGIN (rbw->constraints, ob) { RigidBodyCon *rbc = ob->rigidbody_constraint; if (rbc && rbc->physics_constraint) { - RB_dworld_remove_constraint(static_cast(rbw->shared->physics_world), + RB_dworld_remove_constraint(rbw->shared->runtime->physics_world, static_cast(rbc->physics_constraint)); RB_constraint_delete(static_cast(rbc->physics_constraint)); rbc->physics_constraint = nullptr; @@ -2146,6 +2175,9 @@ void BKE_rigidbody_rebuild_world(Depsgraph *depsgraph, Scene *scene, float ctime PTCacheID pid; int startframe, endframe; + /* Avoid multiple depsgraph evaluations accessing the same shared data. */ + std::unique_lock lock(rbw->shared->runtime->mutex); + BKE_ptcache_id_from_rigidbody(&pid, nullptr, rbw); BKE_ptcache_id_time(&pid, scene, ctime, &startframe, &endframe, nullptr); cache = rbw->shared->pointcache; @@ -2164,7 +2196,7 @@ void BKE_rigidbody_rebuild_world(Depsgraph *depsgraph, Scene *scene, float ctime } FOREACH_COLLECTION_OBJECT_RECURSIVE_END; - if (rbw->shared->physics_world == nullptr || rbw->numbodies != n) { + if (rbw->shared->runtime->physics_world == nullptr || rbw->numbodies != n) { cache->flag |= PTCACHE_OUTDATED; } @@ -2198,7 +2230,7 @@ void BKE_rigidbody_do_simulation(Depsgraph *depsgraph, Scene *scene, float ctime ctime = std::min(ctime, endframe); /* don't try to run the simulation if we don't have a world yet but allow reading baked cache */ - if (rbw->shared->physics_world == nullptr && !(cache->flag & PTCACHE_BAKED)) { + if (rbw->shared->runtime->physics_world == nullptr && !(cache->flag & PTCACHE_BAKED)) { return; } if (rbw->objects == nullptr) { @@ -2245,8 +2277,7 @@ void BKE_rigidbody_do_simulation(Depsgraph *depsgraph, Scene *scene, float ctime for (int i = 0; i < rbw->substeps_per_frame; i++) { rigidbody_update_external_forces(depsgraph, scene, rbw); rigidbody_update_kinematic_obj_substep(&kinematic_substep_targets, cur_interp_val); - RB_dworld_step_simulation( - static_cast(rbw->shared->physics_world), substep, 0, substep); + RB_dworld_step_simulation(rbw->shared->runtime->physics_world, substep, 0, substep); cur_interp_val += interp_step; } rigidbody_free_substep_data(&kinematic_substep_targets); diff --git a/source/blender/blenkernel/intern/scene.cc b/source/blender/blenkernel/intern/scene.cc index f08897cd09d..ee0e72e0b5d 100644 --- a/source/blender/blenkernel/intern/scene.cc +++ b/source/blender/blenkernel/intern/scene.cc @@ -1449,11 +1449,6 @@ static void scene_blend_read_data(BlendDataReader *reader, ID *id) } } else { - /* must nullify the reference to physics sim object, since it no-longer exist - * (and will need to be recalculated) - */ - rbw->shared->physics_world = nullptr; - /* link caches */ BKE_ptcache_blend_read_data(reader, &rbw->shared->ptcaches, &rbw->shared->pointcache, false); @@ -1462,6 +1457,8 @@ static void scene_blend_read_data(BlendDataReader *reader, ID *id) rbw->ltime = float(rbw->shared->pointcache->startframe); } } + + BKE_rigidbody_world_init_runtime(rbw); rbw->objects = nullptr; rbw->numbodies = 0; diff --git a/source/blender/editors/physics/rigidbody_world.cc b/source/blender/editors/physics/rigidbody_world.cc index 8987ced177a..e846677a9dc 100644 --- a/source/blender/editors/physics/rigidbody_world.cc +++ b/source/blender/editors/physics/rigidbody_world.cc @@ -140,7 +140,8 @@ static int rigidbody_world_export_exec(bContext *C, wmOperator *op) BKE_report(op->reports, RPT_ERROR, "No Rigid Body World to export"); return OPERATOR_CANCELLED; } - if (rbw->shared->physics_world == nullptr) { + rbDynamicsWorld *physics_world = BKE_rigidbody_world_physics(rbw); + if (physics_world == nullptr) { BKE_report( op->reports, RPT_ERROR, "Rigid Body World has no associated physics data to export"); return OPERATOR_CANCELLED; @@ -148,7 +149,7 @@ static int rigidbody_world_export_exec(bContext *C, wmOperator *op) RNA_string_get(op->ptr, "filepath", filepath); #ifdef WITH_BULLET - RB_dworld_export(static_cast(rbw->shared->physics_world), filepath); + RB_dworld_export(physics_world, filepath); #endif return OPERATOR_FINISHED; } diff --git a/source/blender/makesdna/DNA_rigidbody_types.h b/source/blender/makesdna/DNA_rigidbody_types.h index 8010e40e777..056970958c2 100644 --- a/source/blender/makesdna/DNA_rigidbody_types.h +++ b/source/blender/makesdna/DNA_rigidbody_types.h @@ -15,6 +15,7 @@ struct Collection; struct EffectorWeights; +struct RigidBodyWorld_Runtime; /* ******************************** */ /* RigidBody World */ @@ -25,9 +26,8 @@ typedef struct RigidBodyWorld_Shared { struct PointCache *pointcache; struct ListBase ptcaches; - /* References to Physics Sim objects. Exist at runtime only ---------------------- */ - /** Physics sim world (i.e. #btDiscreteDynamicsWorld). */ - void *physics_world; + /* Runtime data. */ + struct RigidBodyWorld_Runtime *runtime; } RigidBodyWorld_Shared; /* RigidBodyWorld (rbw) diff --git a/source/blender/makesrna/intern/rna_rigidbody.cc b/source/blender/makesrna/intern/rna_rigidbody.cc index 0068d81ca16..75367fe2a00 100644 --- a/source/blender/makesrna/intern/rna_rigidbody.cc +++ b/source/blender/makesrna/intern/rna_rigidbody.cc @@ -159,9 +159,9 @@ static void rna_RigidBodyWorld_num_solver_iterations_set(PointerRNA *ptr, int va rbw->num_solver_iterations = value; # ifdef WITH_BULLET - if (rbw->shared->physics_world) { - RB_dworld_set_solver_iterations(static_cast(rbw->shared->physics_world), - value); + rbDynamicsWorld *physics_world = BKE_rigidbody_world_physics(rbw); + if (physics_world) { + RB_dworld_set_solver_iterations(physics_world, value); } # endif } @@ -173,8 +173,9 @@ static void rna_RigidBodyWorld_split_impulse_set(PointerRNA *ptr, bool value) SET_FLAG_FROM_TEST(rbw->flag, value, RBW_FLAG_USE_SPLIT_IMPULSE); # ifdef WITH_BULLET - if (rbw->shared->physics_world) { - RB_dworld_set_split_impulse(static_cast(rbw->shared->physics_world), value); + rbDynamicsWorld *physics_world = BKE_rigidbody_world_physics(rbw); + if (physics_world) { + RB_dworld_set_split_impulse(physics_world, value); } # endif } @@ -826,9 +827,10 @@ static void rna_RigidBodyWorld_convex_sweep_test(RigidBodyWorld *rbw, { # ifdef WITH_BULLET RigidBodyOb *rob = object->rigidbody_object; + rbDynamicsWorld *physics_world = BKE_rigidbody_world_physics(rbw); - if (rbw->shared->physics_world != nullptr && rob->shared->physics_object != nullptr) { - RB_world_convex_sweep_test(static_cast(rbw->shared->physics_world), + if (physics_world != nullptr && rob->shared->physics_object != nullptr) { + RB_world_convex_sweep_test(physics_world, static_cast(rob->shared->physics_object), ray_start, ray_end,