VSE: Animation operators missing cache invalidation #94542

New Issue

Denis · 2022-01-01T09:39:30+01:00

Denis commented

2022-01-01 09:39:30 +01:00

System Information
Operating system: Linux Mint 19.1
Graphics card: GTX 980

Blender Version
Broken: blender-3.1.0-alpha+master.c34ea3323a3a-linux.x86_64

Short description of error
Remove created action from NLA editor, this will still animate VSE elements
Some weird VSE action cache?

Exact steps for others to reproduce the error

Add any image to VSE strip
Set Rotation=0 at first frame and Rotation=220 for some another frame. It's rotating now, right?
Go to NLA tab -> push down Scene Action and remove it. Thus, we don't have any actions in our project but the image is still rotating. Why?
After rebooting blender app the magick will gone. Is this a bug?

This has quite broad reach as all of these (if not more) dont invalidate the cache:

clearing keyframes
deleting actions
removing drivers
removing dopsheet channels
removing graph editor channels
removing nla tracks
removing nla strips
removing fcurve modifiers

**System Information** Operating system: Linux Mint 19.1 Graphics card: GTX 980 **Blender Version** Broken: blender-3.1.0-alpha+master.c34ea3323a3a-linux.x86_64 **Short description of error** Remove created action from NLA editor, this will still animate VSE elements Some weird VSE action cache? **Exact steps for others to reproduce the error** - Add any image to VSE strip - Set Rotation=0 at first frame and Rotation=220 for some another frame. It's rotating now, right? - Go to NLA tab -> push down Scene Action and remove it. Thus, we don't have any actions in our project but the image is still rotating. Why? After rebooting blender app the magick will gone. Is this a bug? This has quite broad reach as all of these (if not more) dont invalidate the cache: - clearing keyframes - deleting actions - removing drivers - removing dopsheet channels - removing graph editor channels - removing nla tracks - removing nla strips - removing fcurve modifiers

Denis commented

2022-01-01 09:39:30 +01:00

Added subscriber: @GMXArtist

Pratik Borhade commented

2022-01-01 10:57:48 +01:00

Added subscriber: @PratikPB2123

Pratik Borhade commented

2022-01-01 11:10:01 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Pratik Borhade changed title from ~~Weird VSE SceneAction cache bug. It's animating even if you don't have any action in your project. Resetting blender fixes the issue.~~ to VSE: Removing action still animate elements

2022-01-01 11:17:22 +01:00

Philipp Oeser commented

2022-01-04 15:57:44 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2022-01-04 15:57:44 +01:00

There is just no cache invalidation in regards to animation in many places.
Only doing direct RNA property manipulation calls this afaics (so e.g. button tweaking).

This has quite broad reach as all of these (if not more) dont invalidate the cache:

clearing keyframes
deleting actions
removing drivers
removing dopsheet channels
removing graph editor channels
removing nla tracks
removing nla strips
removing fcurve modifiers

Interestingly, removing a keyframe (in the dopesheet or graph editor) will invalidate the cache correctly.
But this is because delete_graph_keys does ANIM_animdata_update / ANIM_list_elem_update (and that in turn touches the RNA updates, see RNA_property_update_main there... if, and only if ANIM_animdata_update is called with ANIM_UPDATE_DEPS which is included in ANIM_UPDATE_DEFAULT which delete_graph_keys uses)

There is #93151 (VSE: Operators missing cache invalidation), but that report is for operators, so I guess it is fine to have this report separate

There is just no cache invalidation in regards to animation in many places. Only doing direct RNA property manipulation calls this afaics (so e.g. button tweaking). This has quite broad reach as all of these (if not more) dont invalidate the cache: - clearing keyframes - deleting actions - removing drivers - removing dopsheet channels - removing graph editor channels - removing nla tracks - removing nla strips - removing fcurve modifiers Interestingly, removing a keyframe (in the dopesheet or graph editor) **will invalidate the cache correctly**. But this is because `delete_graph_keys` does `ANIM_animdata_update` / `ANIM_list_elem_update` (and that in turn touches the RNA updates, see `RNA_property_update_main` there... if, and only if `ANIM_animdata_update` is called with `ANIM_UPDATE_DEPS` which is included in `ANIM_UPDATE_DEFAULT` which `delete_graph_keys` uses) There is #93151 (VSE: Operators missing cache invalidation), but that report is for operators, so I guess it is fine to have this report separate

Philipp Oeser changed title from ~~VSE: Removing action still animate elements~~ to VSE: Animation operators missing cache invalidation

2022-01-04 15:58:17 +01:00

Philipp Oeser commented

2022-01-04 16:34:05 +01:00

Added subscriber: @dr.sybren

Philipp Oeser commented

2022-01-04 16:34:05 +01:00

@dr.sybren: I wonder if it did harm if all of these "removing" animation operators should touch the RNA callbacks (using methods described above -- ANIM_animdata_update)?
After all, most of the time the RNA callbacks are implemented "most complete" (as in: do as many updates as necessary).
This does not seem performance critical?
Or should this be done in the dependency graph? (but would not know how to distinguish regular animation on e.g framechange vs. such operations as these "removing" operators...)

@dr.sybren: I wonder if it did harm if all of these "removing" animation operators should touch the RNA callbacks (using methods described above -- `ANIM_animdata_update`)? After all, most of the time the RNA callbacks are implemented "most complete" (as in: do as many updates as necessary). This does not seem performance critical? Or should this be done in the dependency graph? (but would not know how to distinguish regular animation on e.g framechange vs. such operations as these "removing" operators...)

Philipp Oeser commented

2022-01-04 16:39:53 +01:00

In #94542#1282739, @lichtwerk wrote:
@dr.sybren: I wonder if it did harm if all of these "removing" animation operators should touch the RNA callbacks (using methods described above -- ANIM_animdata_update)?

even outside the context of the VSE maybe?

> In #94542#1282739, @lichtwerk wrote: > @dr.sybren: I wonder if it did harm if all of these "removing" animation operators should touch the RNA callbacks (using methods described above -- `ANIM_animdata_update`)? even outside the context of the VSE maybe?

Richard Antalik commented

2022-01-04 16:53:29 +01:00

Added subscriber: @iss

Richard Antalik commented

2022-01-04 16:53:29 +01:00

In #94542#1282744, @lichtwerk wrote:

In #94542#1282739, @lichtwerk wrote:
@dr.sybren: I wonder if it did harm if all of these "removing" animation operators should touch the RNA callbacks (using methods described above -- ANIM_animdata_update)?

even outside the context of the VSE maybe?

Yes this is issue even outside of VSE, #90041 is related task I have created for that. In VSE this is bit more visible due to caching.

> In #94542#1282744, @lichtwerk wrote: >> In #94542#1282739, @lichtwerk wrote: >> @dr.sybren: I wonder if it did harm if all of these "removing" animation operators should touch the RNA callbacks (using methods described above -- `ANIM_animdata_update`)? > even outside the context of the VSE maybe? Yes this is issue even outside of VSE, #90041 is related task I have created for that. In VSE this is bit more visible due to caching.

Sybren A. Stüvel commented

2022-01-13 11:20:56 +01:00

In #94542#1282733, @lichtwerk wrote:
This has quite broad reach as all of these (if not more) dont invalidate the cache:

[snipped list of operations]

[...]

There is #93151 (VSE: Operators missing cache invalidation), but that report is for operators, so I guess it is fine to have this report separate

I don't understand this reasoning. If that report is for operators, and this one is separate, it means this one is not for the behaviour of operators. But you describe a list of operations that don't work well.

> In #94542#1282733, @lichtwerk wrote: > This has quite broad reach as all of these (if not more) dont invalidate the cache: > - [snipped list of operations] > > [...] > > There is #93151 (VSE: Operators missing cache invalidation), but that report is for operators, so I guess it is fine to have this report separate I don't understand this reasoning. If that report is for operators, and this one is separate, it means this one is not for the behaviour of operators. But you describe a list of operations that don't work well.

Philipp Oeser commented

2022-01-13 12:57:24 +01:00

In #94542#1287442, @dr.sybren wrote:

In #94542#1282733, @lichtwerk wrote:
This has quite broad reach as all of these (if not more) dont invalidate the cache:

[snipped list of operations]

[...]

There is #93151 (VSE: Operators missing cache invalidation), but that report is for operators, so I guess it is fine to have this report separate

I don't understand this reasoning. If that report is for operators, and this one is separate, it means this one is not for the behaviour of operators. But you describe a list of operations that don't work well.

That wording is bad indeed, please bare with me on my second try:
#93151 is for operators specific to the VSE, so it would be a bit more localized adding specific cache invalidation in those operators.
For animation operators, it would add quite a bit of overhead to check which kind of fcurve and owner are affected (and add conditional cache invalidation then).
So I think the solution to the two reports are indeed separate: in one case (the VSE operators case), we could add cache invalidation "by hand", in the other case (the animation operators case), we could take the shortcut of touching RNA updates (which are pretty much ensured to do the right thing - cache invalidation in this case).

Makes sense?

> In #94542#1287442, @dr.sybren wrote: >> In #94542#1282733, @lichtwerk wrote: >> This has quite broad reach as all of these (if not more) dont invalidate the cache: >> - [snipped list of operations] >> >> [...] >> >> There is #93151 (VSE: Operators missing cache invalidation), but that report is for operators, so I guess it is fine to have this report separate > > I don't understand this reasoning. If that report is for operators, and this one is separate, it means this one is not for the behaviour of operators. But you describe a list of operations that don't work well. That wording is bad indeed, please bare with me on my second try: #93151 is for operators specific to the VSE, so it would be a bit more localized adding specific cache invalidation in those operators. For animation operators, it would add quite a bit of overhead to check which kind of fcurve and owner are affected (and add conditional cache invalidation then). So I think the solution to the two reports are indeed separate: in one case (the VSE operators case), we could add cache invalidation "by hand", in the other case (the animation operators case), we could take the shortcut of touching RNA updates (which are pretty much ensured to do the right thing - cache invalidation in this case). Makes sense?

Philipp Oeser commented

2022-01-13 13:00:09 +01:00

It also seems inconsistent that when removing keyframes (single or multiple - but not all), RNA updates fire, but on the other hand if you remove all keyframes or whole actions (or any of the operations mentioned in the "snippet list") RNA updates on the affected properties dont fire.

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

VSE: Animation operators missing cache invalidation #94542