Page MenuHome

Non-animated property, stuck in previously keyframed values
Closed, ResolvedPublicBUG

Description

Blender Version
Broken: 5984d541b45 (blender2.8 HEAD)
Worked: Not sure.

Short description of error
Object with animation data keeps going back to previously existing keyframed values for channels with no animation on it.

Exact steps for others to reproduce the error

  1. Keyframe the initial cube LocScale (I > LocScale).
  2. Mouse hover scale and "Delete Keyframes".
  3. Change Scale X: to 4.
  4. Playback.

Scale is set back to the previously keyframed value (1.0).

Event Timeline

Dalai Felinto (dfelinto) lowered the priority of this task from 90 to High.
Dalai Felinto (dfelinto) created this task.

@Sergey Sharybin (sergey) a blind guess here, but I suspect this may have to do with the hacks in depsgraph to handle un-keyframed values.

@Dalai Felinto (dfelinto) ./blender --debug-depsgraph-tag, see that no tags are done when you delete keyframe.
This is nothing to do with "hacks", depsgraph has nothing to do with that. In fact, those hacks you're referring to are gone for months.

The actual issue is the fiasco of the optimization idea which was trying to deal with avoiding too many relations update when inserting keyframes. The correct logic should be:

  • Initial insertion of keyframe should update relation, since that is what introduces ANIMATION component of the cube.
  • Adding/removing keyframes should tag action for recalc. (ID_RECALC_ANIMATION should work. If not, ID_RECALC_COPY_ON_WRITE but with some TODO around id). This is what always should be happening, otherwise copied object will not be aware of changes in the interface.
  • Adding/removing drivers should be tagging ID itself, since it is ID who has its drivers (and not an action). Additionally, editing drivers should tag relations for update. Those are changing in 98% of edits.

Anyone should be able to fix such issues. It is just wrong if it's me who is the only one who fixes the issues all over the blender.
Otherwise you'd have to wait for until i'm done with another report which is triaged HIGH and is way more tricky.

I was reporting this while using Blender, so didn't went into debugging steps.

Anyone should be able to fix such issues. It is just wrong if it's me who is the only one who fixes the issues all over the blender.

You are right. I should probably have left the report as medium or normal but unassigned until had the time to see how who this issue belong to.

Anyone should be able to fix such issues. It is just wrong if it's me who is the only one who fixes the issues all over the blender.

Right again, and I will look at it later this week then. Either way it is tagged as high, so other devs are always welcome to pick it before I get the time to do it myself.

I was reporting this while using Blender, so didn't went into debugging steps.

Well, i did that for you ;) Didn't take much time. So no worries here. Finding a place where it is to be corrected in keyframing.c (i would imagine) will take more time :/

Yes, the ones in T60086 are pretty much the same issue (incorrect update tagging). IIRC, the logic for deciding when to tag is a bit borked right now - COW tagging only happens along with relations rebuild/update, and only when F-Curves get added/deleted, or something like that.

T61169 is something different (i.e. when multiple values are edited, but only one property gets keyframed, all other modified but unkeyed lose their unkeyed values)

Here is what is appears to be a correct behavior to me

1diff --git a/source/blender/editors/animation/keyframing.c b/source/blender/editors/animation/keyframing.c
2index 5e3880e2253..1922b876d1e 100644
3--- a/source/blender/editors/animation/keyframing.c
4+++ b/source/blender/editors/animation/keyframing.c
5@@ -1365,6 +1365,20 @@ static bool delete_keyframe_fcurve(AnimData *adt, FCurve *fcu, float cfra)
6 return false;
7 }
8
9+static void deg_tag_after_keyframe_delete(Main *bmain, ID *id, AnimData *adt)
10+{
11+ if (adt->action == NULL) {
12+ /* In the case last f-curve wes removed need to inform dependency graph
13+ * about relations update, since it needs to get rid of animation operation
14+ * for this datablock. */
15+ DEG_id_tag_update_ex(bmain, id, ID_RECALC_COPY_ON_WRITE);
16+ DEG_relations_tag_update(bmain);
17+ }
18+ else {
19+ DEG_id_tag_update_ex(bmain, &adt->action->id, ID_RECALC_COPY_ON_WRITE);
20+ }
21+}
22+
23 short delete_keyframe(Main *bmain, ReportList *reports, ID *id, bAction *act,
24 const char group[], const char rna_path[], int array_index, float cfra,
25 eInsertKeyFlags UNUSED(flag))
26@@ -1441,12 +1455,8 @@ short delete_keyframe(Main *bmain, ReportList *reports, ID *id, bAction *act,
27 ret += delete_keyframe_fcurve(adt, fcu, cfra);
28
29 }
30- /* In the case last f-curve wes removed need to inform dependency graph
31- * about relations update, since it needs to get rid of animation operation
32- * for this datablock. */
33- if (ret && adt->action == NULL) {
34- DEG_id_tag_update_ex(bmain, id, ID_RECALC_COPY_ON_WRITE);
35- DEG_relations_tag_update(bmain);
36+ if (ret) {
37+ deg_tag_after_keyframe_delete(bmain, id, adt);
38 }
39 /* return success/failure */
40 return ret;
41@@ -1537,12 +1547,8 @@ static short clear_keyframe(Main *bmain, ReportList *reports, ID *id, bAction *a
42 /* return success */
43 ret++;
44 }
45- /* In the case last f-curve wes removed need to inform dependency graph
46- * about relations update, since it needs to get rid of animation operation
47- * for this datablock. */
48- if (ret && adt->action == NULL) {
49- DEG_id_tag_update_ex(bmain, id, ID_RECALC_COPY_ON_WRITE);
50- DEG_relations_tag_update(bmain);
51+ if (ret) {
52+ deg_tag_after_keyframe_delete(bmain, id, adt);
53 }
54 /* return success/failure */
55 return ret;