Compensate for scenes-within-scenes when evaluating FCurves so that sequence strips in the VSE work correctly.
Needs ReviewPublic
Actions

Authored by Olly Funkster (Funkster) on Oct 18 2016, 3:10 PM.

Details

Reviewers

Sergey Sharybin (sergey)

Maniphest Tasks

T49658: Evaluation / animation of f-curves does not correct for a scene's position within another scene

Summary

References T49658

In order to evaluate FCurves at the correct frame when using sequence strips within other scenes in the VSE, recursively search for a link between the scene being rendered and the scene in which the FCurve was created, and calculate a frame offset between the two points.

Diff Detail

Repository: rB Blender

Event Timeline

Olly Funkster (Funkster) updated this revision to Diff 7640.Oct 18 2016, 3:10 PM

Olly Funkster (Funkster) retitled this revision from to Compensate for scenes-within-scenes when evaluating FCurves so that sequence strips in the VSE work correctly..

Olly Funkster (Funkster) updated this object.

Olly Funkster (Funkster) set the repository for this revision to rB Blender.

Olly Funkster (Funkster) added a project: Video Sequencer.

Olly Funkster (Funkster) added a task: T49658: Evaluation / animation of f-curves does not correct for a scene's position within another scene.

Olly Funkster (Funkster) added a subscriber: Olly Funkster (Funkster).

Olly Funkster (Funkster) mentioned this in T49658: Evaluation / animation of f-curves does not correct for a scene's position within another scene.Oct 27 2016, 10:51 AM

Bastien Montagne (mont29) added a reviewer: Sergey Sharybin (sergey).Oct 27 2016, 12:47 PM

I found a crash in the form of an infinite recursion loop in calculate_evaluation_frame_offset_recursive while looking at a (somewhat dodgy) file in T50112. Fixed by making sure that we don't recurse if the scene referenced by the current sequence in the loop is the parent scene.

I approve this patch :)
The changes are basically in anim_sys.c, which does not change and complicates much of the rest of the Blender code.
(Maybe just a little more Blender style would be nice)

Yes, sorry about the coding style stuff... force of habit from the style used at work and in all my home projects! I just can't stop my fingers from doing C++ style comments. If people are generally happy with the functionality I'll reformat it (and make sure the patch still works against an up-to-date master).

I took a better look at the patch, and there are some things that worry me.
The animsys_evaluate_fcurves function is used indirectly elsewhere in Blender, and referencing the pointer as a Scene does not seem very safe.
Also making a recursive function to look for scenes among all the strips does not seem at all efficient.
This is a clever solution, but it looks like a hack :\
The problem is that the f-curves are evaluated with the frame of the current scene and it seems that to solve we need a big refactor

Alberto Mardegan (mardy) added a subscriber: Alberto Mardegan (mardy).Mar 12 2017, 3:12 PM

Thanks for working on this!

One question: this seems to assume that a scene is used only once; but what if the same scene is used as a sequence at two different times in the same scene?

@Germano Cavalcante (mano-wii) sorry for the delay in replying, have been AFK on holiday. I agree that the recursive search every frame is not efficient. I ended up doing it this way in order to avoid changing any structs / file data, to make sure it was compatible with as much as possible. It would be much better to record the parent / child's pointers into the scene / strip struct at the point of adding the scene, and save this into the file somehow, rather than working it out from scratch every time.

An inbetween approach might be to modify the structs to have the pointers we need, but not save it to the file and work it out just once upon file loading / first evaluation of fcurve, so that it's not being searched for very often.

@Alberto Mardegan (mardy) I don't think it works for multiple instances of the sub-scene.