From f0f1afd25033e0d991f4575694ebbc43cd93e287 Mon Sep 17 00:00:00 2001 From: Christoph Lendenfeld Date: Thu, 18 Jan 2024 15:40:07 +0100 Subject: [PATCH] Refactor: remove driver dependency in keying code No functional changes expected. Before this PR the keyframing code was aware of driver specifics and changed the frame where a key is inserted to the current driver value. This is now changed so that the calling code needs to know where on the FCurve the key should be inserted. This removes the need for the `INSERTKEY_DRIVER` flag. Pull Request: https://projects.blender.org/blender/blender/pulls/116823 --- source/blender/animrig/ANIM_driver.hh | 25 ++++++++ source/blender/animrig/CMakeLists.txt | 2 + source/blender/animrig/intern/driver.cc | 29 +++++++++ source/blender/animrig/intern/keyframing.cc | 59 +++++-------------- .../blender/editors/animation/keyframing.cc | 9 ++- source/blender/makesdna/DNA_anim_types.h | 2 - source/blender/python/intern/bpy_rna_anim.cc | 5 +- 7 files changed, 81 insertions(+), 50 deletions(-) create mode 100644 source/blender/animrig/ANIM_driver.hh create mode 100644 source/blender/animrig/intern/driver.cc diff --git a/source/blender/animrig/ANIM_driver.hh b/source/blender/animrig/ANIM_driver.hh new file mode 100644 index 00000000000..4a2517dd357 --- /dev/null +++ b/source/blender/animrig/ANIM_driver.hh @@ -0,0 +1,25 @@ +/* SPDX-FileCopyrightText: 2024 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup animrig + * + * \brief Functions to work with drivers. + */ + +#include "RNA_types.hh" + +struct AnimationEvalContext; +struct FCurve; + +namespace blender::animrig { + +/** Evaluates the driver on the frame given in `anim_eval_context` and returns the value. Returns 0 + * if the RNA path can't be resolved. */ +float remap_driver_frame(const AnimationEvalContext *anim_eval_context, + PointerRNA *ptr, + PropertyRNA *prop, + const FCurve *fcu); + +} // namespace blender::animrig \ No newline at end of file diff --git a/source/blender/animrig/CMakeLists.txt b/source/blender/animrig/CMakeLists.txt index 46ab538dbb0..feb8df8e8b7 100644 --- a/source/blender/animrig/CMakeLists.txt +++ b/source/blender/animrig/CMakeLists.txt @@ -24,6 +24,7 @@ set(SRC intern/animdata.cc intern/bone_collections.cc intern/bonecolor.cc + intern/driver.cc intern/fcurve.cc intern/keyframing.cc intern/keyframing_auto.cc @@ -34,6 +35,7 @@ set(SRC ANIM_armature_iter.hh ANIM_bone_collections.hh ANIM_bonecolor.hh + ANIM_driver.hh ANIM_fcurve.hh ANIM_keyframing.hh ANIM_rna.hh diff --git a/source/blender/animrig/intern/driver.cc b/source/blender/animrig/intern/driver.cc new file mode 100644 index 00000000000..fc2c4d41f0d --- /dev/null +++ b/source/blender/animrig/intern/driver.cc @@ -0,0 +1,29 @@ +/* SPDX-FileCopyrightText: 2024 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup animrig + */ + +#include "ANIM_driver.hh" +#include "BKE_animsys.h" +#include "BKE_fcurve_driver.h" +#include "DNA_anim_types.h" +#include "RNA_access.hh" + +namespace blender::animrig { + +float remap_driver_frame(const AnimationEvalContext *anim_eval_context, + PointerRNA *ptr, + PropertyRNA *prop, + const FCurve *fcu) +{ + PathResolvedRNA anim_rna; + if (!RNA_path_resolved_create(ptr, prop, fcu->array_index, &anim_rna)) { + return 0.0f; + } + return evaluate_driver(&anim_rna, fcu->driver, fcu->driver, anim_eval_context); +} + +} // namespace blender::animrig \ No newline at end of file diff --git a/source/blender/animrig/intern/keyframing.cc b/source/blender/animrig/intern/keyframing.cc index 2354ecf5277..830c02da946 100644 --- a/source/blender/animrig/intern/keyframing.cc +++ b/source/blender/animrig/intern/keyframing.cc @@ -21,7 +21,6 @@ #include "BKE_anim_data.h" #include "BKE_animsys.h" #include "BKE_fcurve.h" -#include "BKE_fcurve_driver.h" #include "BKE_idtype.h" #include "BKE_lib_id.hh" #include "BKE_nla.h" @@ -279,12 +278,12 @@ static bool new_key_needed(FCurve *fcu, const float frame, const float value) return true; } -static AnimationEvalContext nla_time_remap(const AnimationEvalContext *anim_eval_context, - PointerRNA *id_ptr, - AnimData *adt, - bAction *act, - ListBase *nla_cache, - NlaKeyframingContext **r_nla_context) +static float nla_time_remap(const AnimationEvalContext *anim_eval_context, + PointerRNA *id_ptr, + AnimData *adt, + bAction *act, + ListBase *nla_cache, + NlaKeyframingContext **r_nla_context) { if (adt && adt->action == act) { *r_nla_context = BKE_animsys_get_nla_keyframing_context( @@ -292,28 +291,11 @@ static AnimationEvalContext nla_time_remap(const AnimationEvalContext *anim_eval const float remapped_frame = BKE_nla_tweakedit_remap( adt, anim_eval_context->eval_time, NLATIME_CONVERT_UNMAP); - return BKE_animsys_eval_context_construct_at(anim_eval_context, remapped_frame); + return remapped_frame; } *r_nla_context = nullptr; - return *anim_eval_context; -} - -/* Adjust frame on which to add keyframe, to make it easier to add corrective drivers. */ -static float remap_driver_frame(const AnimationEvalContext *anim_eval_context, - PointerRNA *ptr, - PropertyRNA *prop, - const FCurve *fcu) -{ - float cfra = anim_eval_context->eval_time; - PathResolvedRNA anim_rna; - if (RNA_path_resolved_create(ptr, prop, fcu->array_index, &anim_rna)) { - cfra = evaluate_driver(&anim_rna, fcu->driver, fcu->driver, anim_eval_context); - } - else { - cfra = 0.0f; - } - return cfra; + return anim_eval_context->eval_time; } /* Insert the specified keyframe value into a single F-Curve. */ @@ -418,11 +400,7 @@ bool insert_keyframe_direct(ReportList *reports, return false; } - float cfra = anim_eval_context->eval_time; - if ((flag & INSERTKEY_DRIVER) && (fcu->driver)) { - cfra = remap_driver_frame(anim_eval_context, &ptr, prop, fcu); - } - + const float cfra = anim_eval_context->eval_time; const bool success = insert_keyframe_value(fcu, cfra, current_value, keytype, flag); if (!success) { @@ -445,7 +423,7 @@ static bool insert_keyframe_fcurve_value(Main *bmain, const char group[], const char rna_path[], int array_index, - const AnimationEvalContext *anim_eval_context, + const float fcurve_frame, float curval, eBezTriple_KeyframeType keytype, eInsertKeyFlags flag) @@ -476,12 +454,7 @@ static bool insert_keyframe_fcurve_value(Main *bmain, /* Update F-Curve flags to ensure proper behavior for property type. */ update_autoflags_fcurve_direct(fcu, prop); - float cfra = anim_eval_context->eval_time; - if ((flag & INSERTKEY_DRIVER) && (fcu->driver)) { - cfra = remap_driver_frame(anim_eval_context, ptr, prop, fcu); - } - - const bool success = insert_keyframe_value(fcu, cfra, curval, keytype, flag); + const bool success = insert_keyframe_value(fcu, fcurve_frame, curval, keytype, flag); if (!success) { BKE_reportf(reports, @@ -552,7 +525,7 @@ int insert_keyframe(Main *bmain, NlaKeyframingContext *nla_context = nullptr; ListBase nla_cache = {nullptr, nullptr}; AnimData *adt = BKE_animdata_from_id(id); - const AnimationEvalContext remapped_context = nla_time_remap( + const float nla_mapped_frame = nla_time_remap( anim_eval_context, &id_ptr, adt, act, &nla_cache, &nla_context); bool force_all; @@ -587,7 +560,7 @@ int insert_keyframe(Main *bmain, group, rna_path, array_index, - &remapped_context, + nla_mapped_frame, values[array_index], keytype, flag)) @@ -615,7 +588,7 @@ int insert_keyframe(Main *bmain, group, rna_path, array_index, - &remapped_context, + nla_mapped_frame, values[array_index], keytype, flag); @@ -638,7 +611,7 @@ int insert_keyframe(Main *bmain, group, rna_path, array_index, - &remapped_context, + nla_mapped_frame, values[array_index], keytype, flag); @@ -658,7 +631,7 @@ int insert_keyframe(Main *bmain, group, rna_path, array_index, - &remapped_context, + nla_mapped_frame, values[array_index], keytype, flag); diff --git a/source/blender/editors/animation/keyframing.cc b/source/blender/editors/animation/keyframing.cc index 50e8d1ffa52..c4e0bdc3a28 100644 --- a/source/blender/editors/animation/keyframing.cc +++ b/source/blender/editors/animation/keyframing.cc @@ -47,6 +47,7 @@ #include "ANIM_animdata.hh" #include "ANIM_bone_collections.hh" +#include "ANIM_driver.hh" #include "ANIM_fcurve.hh" #include "ANIM_keyframing.hh" #include "ANIM_rna.hh" @@ -938,15 +939,19 @@ static int insert_key_button_exec(bContext *C, wmOperator *op) C, &ptr, prop, index, nullptr, nullptr, &driven, &special); if (fcu && driven) { + const float driver_frame = blender::animrig::remap_driver_frame( + &anim_eval_context, &ptr, prop, fcu); + AnimationEvalContext remapped_context = BKE_animsys_eval_context_construct( + CTX_data_depsgraph_pointer(C), driver_frame); changed = blender::animrig::insert_keyframe_direct( op->reports, ptr, prop, fcu, - &anim_eval_context, + &remapped_context, eBezTriple_KeyframeType(ts->keyframe_type), nullptr, - INSERTKEY_DRIVER); + INSERTKEY_NOFLAGS); } } else { diff --git a/source/blender/makesdna/DNA_anim_types.h b/source/blender/makesdna/DNA_anim_types.h index 0aa6271ad00..02763118d5f 100644 --- a/source/blender/makesdna/DNA_anim_types.h +++ b/source/blender/makesdna/DNA_anim_types.h @@ -1055,8 +1055,6 @@ typedef enum eInsertKeyFlags { * Used by copy/paste code. */ INSERTKEY_OVERWRITE_FULL = (1 << 7), - /** for driver FCurves, use driver's "input" value - for easier corrective driver setup */ - INSERTKEY_DRIVER = (1 << 8), /** for cyclic FCurves, adjust key timing to preserve the cycle period and flow */ INSERTKEY_CYCLE_AWARE = (1 << 9), /** don't create new F-Curves (implied by INSERTKEY_REPLACE) */ diff --git a/source/blender/python/intern/bpy_rna_anim.cc b/source/blender/python/intern/bpy_rna_anim.cc index 6a52f0bf009..0c283ed1112 100644 --- a/source/blender/python/intern/bpy_rna_anim.cc +++ b/source/blender/python/intern/bpy_rna_anim.cc @@ -341,9 +341,8 @@ PyObject *pyrna_struct_keyframe_insert(BPy_StructRNA *self, PyObject *args, PyOb * it turns out to be necessary for some reason to insert keyframes on evaluated objects, we can * revisit this and add an explicit `depsgraph` keyword argument to the function call. * - * It is unlikely that driver code (which is the reason this depsgraph pointer is obtained) will - * be executed from this function call, as this only happens when `options` has - * `INSERTKEY_DRIVER`, which is not exposed to Python. */ + * The depsgraph is only used for evaluating the NLA so this might not be needed in the future. + */ bContext *C = BPY_context_get(); Depsgraph *depsgraph = CTX_data_depsgraph_pointer(C); const AnimationEvalContext anim_eval_context = BKE_animsys_eval_context_construct(depsgraph,