Page MenuHome

Make the operators that change keyframe data update the motion paths
Changes PlannedPublic

Authored by Octave C (toctave) on Sep 26 2020, 8:43 PM.
Tokens
"Love" token, awarded by zNight."Love" token, awarded by dinhdong."Love" token, awarded by erickblender."Like" token, awarded by EAW."Love" token, awarded by Juangra_Membata."Love" token, awarded by Stan1."Love" token, awarded by looch.

Details

Summary

Problem

Right now, when auto-keying is on and and one transforms an object in the 3d view, its motion paths get updated. This makes it easy to iterate over an animation and make sure the object's trajectory looks right.

However, when the animation data is updated in any other way (e.g. through the graph editor, timeline, or action editor), the motion paths don't get updated right away, requiring an extra manual refresh on each incremental change.

Proposed solution

This patch makes sure that motion paths get updated whenever keyframe data changes, allowing for easier incremental changes. For now, the active object's motion paths are recomputed on every edit that triggers an "ND_KEYFRAME_PROP" notifier.

Before

After

Performance is fine now, there is no noticeable slow down with the Rain scene. Earlier it was slow because I was updating every point on the motion path as the mouse was moving, but now (as of diff 29494) only a single point is moved interactively and the whole path gets updated once the transform is confirmed, which matches what happens with viewport updates.

Here is a stress test, with a dozen motion paths having each 100 frames. I'm not an animator, so please excuse the wonkiness of it :)

I've also tried moving around the keyframes in the timeline and dope sheet, and there isn't a noticeable slow down there either, which makes sense because the code that runs is almost the same each time (tiny bit of setup + a call to ED_objects_and_pose_recalculate_paths). For very large updates, say, selecting every single keyframe of every single bone and changing their handle type at once, there is a lag of about half a second (on a Ryzen 3600 with a debug build), which in my opinion is reasonable considering that this is an unusual scenario.

Diff Detail

Repository
rB Blender
Branch
motion_paths_refresh (branched from master)
Build Status
Buildable 10420
Build 10420: arc lint + arc unit

Event Timeline

Octave C (toctave) requested review of this revision.Sep 26 2020, 8:43 PM
Octave C (toctave) created this revision.

Yes!, thank you this is a much much needed update!
Any builds i can test?

This revision is now accepted and ready to land.Sep 30 2020, 5:30 PM

I'm sorry, what do you mean "builds you could test" ? I'm new to blender development so I don't know the ins and outs yet :/

I was mean a blender version for windows where this patch is working so i can try it out :)

Oh, well I added those changes on top of the current state of the master branch, so you could try doing the same. Does that answer your question?

I tested it and it seems like you just implemented the object's motion paths but you didn't cover bones motion check the GIF below.

Yes! I noticed that as well and I'm adding those changes as we speak

  • Update motion paths for bones when animation data is changed outside the 3d view
  • formatting changes
This revision now requires review to proceed.Oct 1 2020, 1:11 PM
Sybren A. Stüvel (sybren) requested changes to this revision.EditedOct 1 2020, 1:19 PM

Thanks for the patch, I can certainly see its use.

I think the most important question is: what is the performance impact of these changes?
Have you tested it with an animated scene like https://cloud.blender.org/p/characters/5f1ede754347a0fc05ba21a0 ?

Please also take a look at Ingredients of a Patch for some guidelines on how to improve the patch description.

source/blender/editors/include/ED_object.h
65–70

This seems to be reformatting only. Please only include functional changes in this patch.

Formatting fixes and other non-functional improvements are welcome too, but they shouldn't be intermixed with functional changes.

source/blender/editors/space_action/action_edit.c
1302

Comments should be a full English sentence, so start with a capital and end with a period. This wasn't always the style rule, but it is for new/updated comments.

1302–1309

This code is now copy-pasted a few times. It should be its own function, which can then be called from various places.

This revision now requires changes to proceed.Oct 1 2020, 1:19 PM
source/blender/editors/include/ED_object.h
65–70

This is actually a functional change, I added the "struct" at the beginning of the line because it my code wouldn't compile otherwise. I suppose this is due to the placement of the typedef for Object.

Octave C (toctave) edited the summary of this revision. (Show Details)Oct 1 2020, 5:24 PM
Octave C (toctave) edited the summary of this revision. (Show Details)Oct 1 2020, 7:54 PM
Octave C (toctave) edited the summary of this revision. (Show Details)
Octave C (toctave) edited the summary of this revision. (Show Details)
Octave C (toctave) edited the summary of this revision. (Show Details)Oct 1 2020, 7:57 PM
Octave C (toctave) marked an inline comment as done.
  • Apply single frame updates to motion paths when transform is pending
  • Refactor motion paths update in a common function
Octave C (toctave) marked 2 inline comments as done.Oct 2 2020, 9:10 PM
Octave C (toctave) added a comment.EditedOct 7 2020, 8:59 PM

Clarification : performance is fine now, there is no noticeable slow down with the Rain scene. Earlier it was slow because I was updating every point on the motion path as the mouse was moving, but now (as of diff 29494) only a single point is moved interactively and the whole path gets updated once the transform is confirmed, which matches what happens with viewport updates.

Here is a stress test, with a dozen motion paths having each 100 frames. I'm not an animator, so please excuse the wonkiness of it :)

I've also tried moving around the keyframes in the timeline and dope sheet, and there isn't a noticeable slow down there either, which makes sense because the code that runs is almost the same each time (tiny bit of setup + a call to ED_objects_and_pose_recalculate_paths). For very large updates, say, selecting every single keyframe of every single bone and changing their handle type at once, there is a lag of about half a second (on a Ryzen 3600 with a debug build), which in my opinion is reasonable considering that this is an unusual scenario.

Octave C (toctave) edited the summary of this revision. (Show Details)Oct 7 2020, 10:23 PM
Octave C (toctave) edited the summary of this revision. (Show Details)

Thanks for the clarifications.
@Sergey Sharybin (sergey) could you maybe also take a look at this patch? You have some experience with motion paths I think ;-)

I have a question regarding functionality, a lot of the times you put a motion path on an object or bone that isnt the one you're animating but it is affected: example i put a motion path on the nose to track the tip of the nose but really i'm working tweaking the head. Will this make that be updated too ? Because as it is now it only works if is a direct relationship, a lot of the times you need to use an empty to put the motion path in the right place so to update a the empty's path you end up having to quit pose mode, select the empty go into its properties and hit update... which you only do 2-3 times and then give up.
Will this fix that type of update too?

I have a question regarding functionality, a lot of the times you put a motion path on an object or bone that isnt the one you're animating but it is affected: example i put a motion path on the nose to track the tip of the nose but really i'm working tweaking the head. Will this make that be updated too ? Because as it is now it only works if is a direct relationship, a lot of the times you need to use an empty to put the motion path in the right place so to update a the empty's path you end up having to quit pose mode, select the empty go into its properties and hit update... which you only do 2-3 times and then give up.
Will this fix that type of update too?

The answer is yes, this patch does work like that. Check the GIF

@Sybren A. Stüvel (sybren), I don't think it is the motion paths themselves to be looked into, but more like a system integration type of a thing. One thing to keep in mind is that calculation of motion path could be very resource-involved (both CPU time and memory).

I do see some code which loops over all objects and updates their motion paths. This should not be happening. If the intent is to update motion path of objects when changing their dependency then proper solution is to query the dependency graph for dependencies.

But even if such dependencies query is implemented, I am skeptical about such change until someone from the animation team tests this in a real animation shot (where it is rarely a single character without any other animation involved).

Is there a build available to test it?

@Luciano Muñoz Sessarego (looch) I just added test builds, look for arcpatch-D9019 for your platform on https://builder.blender.org/download/branches/

@Luciano Muñoz Sessarego (looch) I just added test builds, look for arcpatch-D9019 for your platform on https://builder.blender.org/download/branches/

Yo Octave! are still around? can you update the patch to latest master?

Octave C (toctave) planned changes to this revision.EditedJan 3 2021, 12:13 PM

Hi,

As it is this change cannot be merged into master, because of performance issues. We had looked at it together with @Luciano Muñoz Sessarego (looch), and indeed it gets very slow on mid-range hardware. As suggested by @Sergey Sharybin (sergey), it'd need finer grain queries to the dependency graph in order to function properly, if that is even attainable : motion path evaluation can trigger an arbitrarily complex chain of computations, which needs to be evaluated for a range of frames.

I have left it at that because I did not have a good enough understanding of the depsgraph to implement a smarter version. Of course, anyone is more than welcome to take it from there.

Hi,

As it is this change cannot be merged into master, because of performance issues. We had looked at it together with @Luciano Muñoz Sessarego (looch), and indeed it gets very slow on mid-range hardware. As suggested by @Sergey Sharybin (sergey), it'd need finer grain queries to the dependency graph in order to function properly, if that is even attainable : motion path evaluation can trigger an arbitrarily complex chain of computations, which needs to be evaluated for a range of frames.

I have left it at that because I did not have a good enough understanding of the depsgraph to implement a smarter version. Of course, anyone is more than welcome to take it from there.

Hi,
I understand, however what I was asking was to update the patch to the master if you would like to, I did not ask to merge to master. I would love to have it in Blender 2.92, since the last time this patch was updated there is some changes in the code and the patch cannot be applied anymore.

source/blender/editors/space_graph/graph_edit.c:

The fact is people use motion path with different workflow and different rig, motion path rely heavily on the length and the amount of path in the viewport. In my workflow I didn't notice a big slow down using this patch in contrary the patch sped my workflow.
I just wanted to give my feedback on this.

@Wala Zulu (Animation) and rigging module owners A note for reflection how can we decide about speed of feature when we don't have a .blend file that we should compare with? This feature is a must in animation and yet we are letting down someone who had put his time and energy on this feature. :(