Page MenuHome

FModifers RNA update: call corresponding FCurve properties RNA update as well
AbandonedPublic

Authored by Philipp Oeser (lichtwerk) on Sep 28 2020, 12:29 PM.

Details

Summary

When tweaking an FModifiers UI, updates are not happening the same way as
tweaking other UI parts in blender. General DEG tagging takes place, but
the animation system / dependency graph evaluation does not call RNA
update functions for performance reasons, usually one can expect this to
happen through the UI though.

So to make sure we have the same updates happening, we need to ensure
that the FCurve's corresponding property's RNA callbacks are executed.

  • tweaking the property in the Properties Editor would call it directly
  • tweaking keyframes in an FCurve in animation editors would call it (via ANIM_animdata_update --> ANIM_list_elem_update --> RNA_property_update_main (on fcu->rna_path)
  • adding or removing FCurve Modifiers would do it via ANIM_animdata_update as well
  • BUT: the UI for the FCurve Modifiers has its own RNA, the properties associated with the FCurve will not get their RNA update functions called (this is what we need to change I think)

What updates happen with the UI for the FCurve Modifiers?

  • the corresponding RNA update function for the FCurve Modifier is called (mostly just rna_FModifier_update), again: not the corresponding property the FCurve is coupled with
  • rna_FModifier_update only does very general update tagging (rna_tag_animation_update -- which now tags the Action ID_RECALC_ANIMATION)
  • depending on the Type of the FCurve Modifier, its UI will also jump through hoops and call 'deg_update()' [which sends NC_ANIMATION | ND_KEYFRAME | NA_EDITED notifiers, as well as tagging the Action ID_RECALC_ANIMATION -- but these are also not enough here]

So, I think what we should be after when tweaking the FModifiers UI is:

  • [1] call proper RNA updates on the underlying properties coupled with that FCurve
  • [2] clean up the whole FModifier UI
    • get rid of custom button/block callbacks and use FModifier RNA where possible
    • get rid of this 'deg_update()'

This patch implements [1].
This is a costly operation, it loops over the main database to find IDs using this particular action (but I dont see a way around this).

Note: [2] is partly done in D7997

Fixes T80121

Diff Detail

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

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Sep 28 2020, 12:29 PM

Since this touches the realm between DEG updates and UI updates [see also discussion in T80121 itself], would kindly ask @Sergey Sharybin (sergey) (and maybe @Brecht Van Lommel (brecht)) to spread wisdom.

I'm not sure this is the correct solution.

What happens here is that the evaluated value changes for some property. Editing the value directly and f-curve modifiers are some ways that can happen, but it can also happen by editing the f-curve control points or by having a driver and editing the other object.

Physics point caches are reset by the dependency graph for that reason (see DEG_UPDATE_SOURCE_USER_EDIT and ID_RECALC_POINT_CACHE), since a complex chain of dependencies can cause the reset. And so I would expect f-curve modifiers to be taken into account in the depsgraph for that.

Philipp Oeser (lichtwerk) abandoned this revision.EditedSep 28 2020, 4:37 PM

I'm not sure this is the correct solution.

What happens here is that the evaluated value changes for some property. Editing the value directly and f-curve modifiers are some ways that can happen, but it can also happen by editing the f-curve control points or by having a driver and editing the other object.

Physics point caches are reset by the dependency graph for that reason (see DEG_UPDATE_SOURCE_USER_EDIT and ID_RECALC_POINT_CACHE), since a complex chain of dependencies can cause the reset. And so I would expect f-curve modifiers to be taken into account in the depsgraph for that.

Slowly makes sense, and D9036: Fix T80121: Forcefield F-curve modifier changes don't reset cache sounds like it is the better solution.
It can still confusing though (or at least for me it still is) that editing the f-curve control points goes the extra mile of ensuring that RNA property update callbacks are executed [the action is already tagged ID_RECALC_ANIMATION there as well, so in an ideal world, this would ripple through the depsgrah and all updates should take place correctly -- even without forcing the RNA update callbacks?]
(might be interesting to check out what would go missing if that is commented out...)

In any case: thx having a look!

It can still confusing though (or at least for me it still is) that editing the f-curve control points goes the extra mile of ensuring that RNA property update callbacks are executed [the action is already tagged ID_RECALC_ANIMATION there as well, so in an ideal world, this would ripple through the depsgraph and all updates should take place correctly -- even without forcing the RNA update callbacks?]
(might be interesting to check out what would go missing if that is commented out...)

That's a good point. If it's needed for control points, probably it's also needed for f-curve modifiers. Not for physics caches, but other cases? Or maybe this code is now obsolete with the new depsgraph? This would require digging into the git history to find out.

In general I think RNA set functions should do the types of updates that are always needed along with setting the value, and update should be left for notifiers and update tagging. There may some cases where that design fails, but I don't remember specific cases.