Fix T76734: Changing Envelope FModifier controlpoints missing update
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on May 14 2020, 2:01 PM.

Details

Reviewers

Sybren A. Stüvel (sybren)
Sebastian Parborg (zeddb)
Julian Eisel (Severin)

Maniphest Tasks

T76734: Changing Envelope FModifier controlpoints missing update

Commits

rBe5ace51295b9: Fix T76734: Changing Envelope FModifier controlpoints missing update

Summary

Issue is that update functions defined in
rna_def_fmodifier_envelope_ctrl (namely rna_FModifier_update) are
actually never called.

This is because UI code for FCurve modifiers often does not use RNA
buttons but uses custom update functions (or non at all). For example,
rB9a88bd55903a did this for the generators, envelope control points did
not have this at all.

This is now changed to use RNA buttons for the envelope control points,
this could done for other non-RNA buttons as well to get rid of
validate_fmodifier_cb.

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.May 14 2020, 2:01 PM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B8045: Diff 24761.May 14 2020, 2:02 PM

Will also subscribe @Julian Eisel (Severin) here [since it touches UI realm]

The real issue seems to be that update functions defined in rna_def_fmodifier_envelope_ctrl are actually never called.
we have a bunch of rna_FModifier_update defined there -- if that would be called I assume we would be good...

Yes. These are called when you change these properties from Python. This code immediately has the intended effect:

mod = D.objects['Cube'].animation_data.nla_tracks['NlaTrack'].strips['CubeAction.001'].modifiers[0]
mod.control_points[0].max = 4

So it's indeed the issue that the UI code doesn't trigger the RNA update callbacks.
I'm not familiar with the C UI code (except that I'm a bit scared of it). I suppose it's it meant to use RNA access like the Python UI code?

Sadly I don't really remember why I fixed it like this before.
I think I must have asked Sergey or Brecht about this at the office.
The pull request is kinda empty of discussion so... https://developer.blender.org/D5101

But for me it is fine if you just piggy back ontop of the older solution.

The buttons are not hooked up with RNA at all, they just directly get the DNA float value as pointer. I don't see why this would be done, it's probably just because some dev happened to do it this way in a galaxy far far away. Git blame says this was already the case since at least 2009. I think it was common to do UIs like this back then, because RNA wasn't fully integrated yet.

These functions should probably use uiDefButR() to make use of RNA. The update callbacks will be called then too.

In D7732#186809, @Julian Eisel (Severin) wrote:

The buttons are not hooked up with RNA at all, they just directly get the DNA float value as pointer. I don't see why this would be done, it's probably just because some dev happened to do it this way in a galaxy far far away. Git blame says this was already the case since at least 2009. I think it was common to do UIs like this back then, because RNA wasn't fully integrated yet.

These functions should probably use uiDefButR() to make use of RNA. The update callbacks will be called then too.

Thx for the input, will check in a bit.
If that works out, we might be able to remove that workaround in more places...