Fix: Crash when parenting mesh to armature with automatic weights

There probably are more cases where crash will happen as it is
rooting into the issue with BKE_object_workob_calc_parent() which
is used in multiple places.

The issue is caused by the access to a runtime field of workob
outside of the BKE_object_workob_calc_parent(): the runtime field
is stack-allocated in the function, and can not be accessed outside
of the function.

The easiest way to reproduce is to use ASAN, and parent mesh to an
armature with automatic weights. Although, on macOS ASAN did not
report issues, so setting workob->runtime to nullptr at the end of
of the BKE_object_workob_calc_parent() was the easiest.

The solution is simple: make the function to return the matrix,
and take care of the working object inside of it, so all tricky
parts are hidden from the API.

The patch is targeting the main branch, as in 4.1 it is not
required to do such change because all uses of the function only
access object_to_world, which is stored in the object in 4.1.

A double-check in the what_does_obaction() might be needed as it
follows the similar pattern, but it does not seem that runtime
field of the workob is accessed in usages of the what_does_obaction().

Pull Request: https://projects.blender.org/blender/blender/pulls/118847
This commit is contained in:
Sergey Sharybin
2024-02-28 15:06:02 +01:00
committed by Sergey Sharybin
parent de5451b112
commit 87a98c361c
6 changed files with 36 additions and 45 deletions
+4 -1
View File
@@ -13,6 +13,7 @@
#include "BLI_bounds_types.hh"
#include "BLI_compiler_attrs.h"
#include "BLI_math_matrix_types.hh"
#include "BLI_math_vector_types.hh"
#include "BLI_sys_types.h"
#include "BLI_vector.hh"
@@ -54,8 +55,10 @@ void BKE_object_workob_clear(Object *workob);
* Otherwise, after changing ob->parent you need to call:
* - #DEG_relations_tag_update(bmain);
* - #BKE_scene_graph_update_tagged(depsgraph, bmain);
*
* \return calculated object_to_world.
*/
void BKE_object_workob_calc_parent(Depsgraph *depsgraph, Scene *scene, Object *ob, Object *workob);
blender::float4x4 BKE_object_calc_parent(Depsgraph *depsgraph, Scene *scene, Object *ob);
void BKE_object_transform_copy(Object *ob_tar, const Object *ob_src);
void BKE_object_copy_softbody(Object *ob_dst, const Object *ob_src, int flag);
+18 -15
View File
@@ -3389,27 +3389,28 @@ void BKE_object_where_is_calc(Depsgraph *depsgraph, Scene *scene, Object *ob)
object_where_is_calc_ex(depsgraph, scene, ob, ctime, nullptr, nullptr);
}
void BKE_object_workob_calc_parent(Depsgraph *depsgraph, Scene *scene, Object *ob, Object *workob)
blender::float4x4 BKE_object_calc_parent(Depsgraph *depsgraph, Scene *scene, Object *ob)
{
blender::bke::ObjectRuntime workob_runtime;
BKE_object_workob_clear(workob);
workob->runtime = &workob_runtime;
Object workob;
BKE_object_workob_clear(&workob);
workob.runtime = &workob_runtime;
unit_m4(workob->runtime->object_to_world.ptr());
unit_m4(workob->parentinv);
unit_m4(workob->constinv);
unit_m4(workob.runtime->object_to_world.ptr());
unit_m4(workob.parentinv);
unit_m4(workob.constinv);
/* Since this is used while calculating parenting,
* at this moment ob_eval->parent is still nullptr. */
workob->parent = DEG_get_evaluated_object(depsgraph, ob->parent);
workob.parent = DEG_get_evaluated_object(depsgraph, ob->parent);
workob->trackflag = ob->trackflag;
workob->upflag = ob->upflag;
workob.trackflag = ob->trackflag;
workob.upflag = ob->upflag;
workob->partype = ob->partype;
workob->par1 = ob->par1;
workob->par2 = ob->par2;
workob->par3 = ob->par3;
workob.partype = ob->partype;
workob.par1 = ob->par1;
workob.par2 = ob->par2;
workob.par3 = ob->par3;
/* The effects of constraints should NOT be included in the parent-inverse matrix. Constraints
* are supposed to be applied after the object's local loc/rot/scale. If the (inverted) effect of
@@ -3417,9 +3418,11 @@ void BKE_object_workob_calc_parent(Depsgraph *depsgraph, Scene *scene, Object *o
* object's local loc/rot/scale instead of after. For example, a "Copy Rotation" constraint would
* rotate the object's local translation as well. See #82156. */
STRNCPY(workob->parsubstr, ob->parsubstr);
STRNCPY(workob.parsubstr, ob->parsubstr);
BKE_object_where_is_calc(depsgraph, scene, workob);
BKE_object_where_is_calc(depsgraph, scene, &workob);
return workob.object_to_world();
}
void BKE_object_apply_mat4_ex(Object *ob,
@@ -83,10 +83,9 @@ static void applyarmature_fix_boneparents(const bContext *C, Scene *scene, Objec
* in this function. */
Depsgraph *depsgraph = CTX_data_depsgraph_pointer(C);
Main *bmain = CTX_data_main(C);
Object workob, *ob;
/* go through all objects in database */
for (ob = static_cast<Object *>(bmain->objects.first); ob;
for (Object *ob = static_cast<Object *>(bmain->objects.first); ob;
ob = static_cast<Object *>(ob->id.next))
{
/* if parent is bone in this armature, apply corrections */
@@ -96,8 +95,7 @@ static void applyarmature_fix_boneparents(const bContext *C, Scene *scene, Objec
*/
BKE_object_apply_mat4(ob, ob->object_to_world().ptr(), false, false);
BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob);
invert_m4_m4(ob->parentinv, workob.object_to_world().ptr());
invert_m4_m4(ob->parentinv, BKE_object_calc_parent(depsgraph, scene, ob).ptr());
}
}
}
@@ -254,7 +254,6 @@ static int vertex_parent_set_exec(bContext *C, wmOperator *op)
BKE_report(op->reports, RPT_ERROR, "Loop in parents");
}
else {
Object workob;
BKE_view_layer_synced_ensure(scene, view_layer);
ob->parent = BKE_view_layer_active_object_get(view_layer);
if (par3 != INDEX_UNSET) {
@@ -264,16 +263,14 @@ static int vertex_parent_set_exec(bContext *C, wmOperator *op)
ob->par3 = par3;
/* inverse parent matrix */
BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob);
invert_m4_m4(ob->parentinv, workob.object_to_world().ptr());
invert_m4_m4(ob->parentinv, BKE_object_calc_parent(depsgraph, scene, ob).ptr());
}
else {
ob->partype = PARVERT1;
ob->par1 = par1;
/* inverse parent matrix */
BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob);
invert_m4_m4(ob->parentinv, workob.object_to_world().ptr());
invert_m4_m4(ob->parentinv, BKE_object_calc_parent(depsgraph, scene, ob).ptr());
}
}
}
@@ -575,8 +572,6 @@ bool ED_object_parent_set(ReportList *reports,
}
}
Object workob;
/* Apply transformation of previous parenting. */
if (keep_transform) {
/* Was removed because of bug #23577, * but this can be handy in some cases too #32616, so
@@ -727,9 +722,8 @@ bool ED_object_parent_set(ReportList *reports,
}
/* get corrected inverse */
ob->partype = PAROBJECT;
BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob);
invert_m4_m4(ob->parentinv, workob.object_to_world().ptr());
invert_m4_m4(ob->parentinv, BKE_object_calc_parent(depsgraph, scene, ob).ptr());
}
else if (is_armature_parent && (ob->type == OB_GPENCIL_LEGACY) && (par->type == OB_ARMATURE)) {
if (partype == PAR_ARMATURE) {
@@ -745,9 +739,8 @@ bool ED_object_parent_set(ReportList *reports,
}
/* get corrected inverse */
ob->partype = PAROBJECT;
BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob);
invert_m4_m4(ob->parentinv, workob.object_to_world().ptr());
invert_m4_m4(ob->parentinv, BKE_object_calc_parent(depsgraph, scene, ob).ptr());
}
else if ((ob->type == OB_GPENCIL_LEGACY) && (par->type == OB_LATTICE)) {
/* Add Lattice modifier */
@@ -756,14 +749,12 @@ bool ED_object_parent_set(ReportList *reports,
}
/* get corrected inverse */
ob->partype = PAROBJECT;
BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob);
invert_m4_m4(ob->parentinv, workob.object_to_world().ptr());
invert_m4_m4(ob->parentinv, BKE_object_calc_parent(depsgraph, scene, ob).ptr());
}
else {
/* calculate inverse parent matrix */
BKE_object_workob_calc_parent(depsgraph, scene, ob, &workob);
invert_m4_m4(ob->parentinv, workob.object_to_world().ptr());
invert_m4_m4(ob->parentinv, BKE_object_calc_parent(depsgraph, scene, ob).ptr());
}
DEG_id_tag_update(&ob->id, ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY);
@@ -532,8 +532,6 @@ void OBJECT_OT_origin_clear(wmOperatorType *ot)
* should stay in the same place, e.g. for apply-size-rot or object center */
static void ignore_parent_tx(Main *bmain, Depsgraph *depsgraph, Scene *scene, Object *ob)
{
Object workob;
Scene *scene_eval = DEG_get_evaluated_scene(depsgraph);
/* a change was made, adjust the children to compensate */
@@ -541,8 +539,8 @@ static void ignore_parent_tx(Main *bmain, Depsgraph *depsgraph, Scene *scene, Ob
if (ob_child->parent == ob) {
Object *ob_child_eval = DEG_get_evaluated_object(depsgraph, ob_child);
BKE_object_apply_mat4(ob_child_eval, ob_child_eval->object_to_world().ptr(), true, false);
BKE_object_workob_calc_parent(depsgraph, scene, ob_child_eval, &workob);
invert_m4_m4(ob_child->parentinv, workob.object_to_world().ptr());
invert_m4_m4(ob_child->parentinv,
BKE_object_calc_parent(depsgraph, scene, ob_child_eval).ptr());
/* Copy result of BKE_object_apply_mat4(). */
BKE_object_transform_copy(ob_child, ob_child_eval);
/* Make sure evaluated object is in a consistent state with the original one.
@@ -687,9 +685,7 @@ static int apply_objects_internal(bContext *C,
obact = CTX_data_active_object(C);
invert_m4_m4(obact_invmat, obact->object_to_world().ptr());
Object workob;
BKE_object_workob_calc_parent(depsgraph, scene, obact, &workob);
copy_m4_m4(obact_parent, workob.object_to_world().ptr());
copy_m4_m4(obact_parent, BKE_object_calc_parent(depsgraph, scene, obact).ptr());
copy_m4_m4(obact_parentinv, obact->parentinv);
if (apply_objects_internal_need_single_user(C)) {
+3 -3
View File
@@ -209,12 +209,12 @@ void SkinInfo::link_armature(bContext *C,
bc_set_parent(ob, ob_arm, C);
}
#else
Object workob;
Depsgraph *depsgraph = CTX_data_ensure_evaluated_depsgraph(C);
ob->parent = ob_arm;
ob->partype = PAROBJECT;
BKE_object_workob_calc_parent(scene, ob, &workob);
invert_m4_m4(ob->parentinv, workob.object_to_world);
invert_m4_m4(ob->parentinv, BKE_object_calc_parent(depsgraph, scene, ob).ptr());
DEG_id_tag_update(&obn->id, ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY);
#endif