Differential D4229
Bugfix T60185: loops in graph produced unorderly clean-up Authored by Jose C. Rubio (jrubio) on Jan 20 2019, 7:03 PM.
Details
Bug link: https://developer.blender.org/T60185 When having loops in a kinematic chain and also a IK constrain, the solve_cycle function seemed to remove graph relations necessary to ensure the cleanup operation was running at the very end. Due to his Blender was crashing when some operations (the bone constraints) accessed a pointer that was already freed.
Diff Detail
Event TimelineComment Actions This is effectively marking almost all of the relations as non-killable. This might be ok if those are done within a single bone channel only, but this definitely shouldn't be applied to inter-bone dependencies (i.e. from parent to solver). That will never be a reliable solution, and will fail sooner or later. The proper solution here i guess is to add dependency from every bone node to its cleanup. We can also tweak cycles solver, so it prefers to kill relation between different ID's first, if that's no possible then relation between different components. ANd that is still not possible, whatever relation. P.S. If we are ending up marking majority of relaitons as GODMODE we are doing something wrong..
Comment Actions Yeah I was not really sure this was the right approach. I'll add relations bone-cleanup as you suggest. I guess tweaking the cycles solver wouldn't be a solution in the long term anyway as long as we allow arbitrary armature graphs. Comment Actions This approach includes additional relations to avoid the unorderly cleanup rather than having unkillable relations. I had to add extra relations in bones as well as in constraints. Comment Actions The patch (updated on - P934) seems to work (no crashes). 'OBArmature.Mario.POSE_IK_SOLVER()' depends on 'OBArmature.Mario.Spine.Bendy.BONE_READY()' through 'IK Chain Parent' 'OBArmature.Mario.Spine.Bendy.BONE_READY()' depends on 'OBArmature.Mario.Spine.Bendy.BONE_CONSTRAINTS()' through 'Constraints -> Ready' 'OBArmature.Mario.Spine.Bendy.BONE_CONSTRAINTS()' depends on 'OBArmature.Mario.Spine.Tail.BONE_READY()' through 'Damped Track' 'OBArmature.Mario.Spine.Tail.BONE_READY()' depends on 'OBArmature.Mario.Spine.Tail.BONE_CONSTRAINTS()' through 'Constraints -> Ready' 'OBArmature.Mario.Spine.Tail.BONE_CONSTRAINTS()' depends on 'OBArmature.Mario.Spine.Tail.BONE_POSE_PARENT()' through 'Pose -> Constraints Stack' 'OBArmature.Mario.Spine.Tail.BONE_POSE_PARENT()' depends on 'OBArmature.Mario.Spine.Base.BONE_DONE()' through 'Parent Bone -> Child Bone' 'OBArmature.Mario.Spine.Base.BONE_DONE()' depends on 'OBArmature.Mario.POSE_IK_SOLVER()' through 'IK Chain Result' Comment Actions @Dalai Felinto (dfelinto), this change has nothing to do with solving cycles, it just makes blender to not crash when there is a cycle. Comment Actions @Brecht Van Lommel (brecht), do you feel strong here? Can't think of a better solution than to introduce more relations. Comment Actions I don't have a better local solution. There may be a better overall design to handle this, but don't have immediate ideas for that. Comment Actions The only alternative that comes to mind would be to improve the solve_cycles function so that the resulting graphs are ensured to perform orderly cleanup. But adding extra relations seems like a more clean/maintainable solution. Comment Actions Needed to apply manually, and forgot to mention the revision in the comment. Thanks for the patch! Closing, as applied. | ||||||||||