Fix T90723: Quick Favorites and Shortcuts not working well with many properties (VSE, NLA, bones, ...)
AbandonedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Aug 23 2021, 8:36 AM.

Details

Reviewers

Campbell Barton (campbellbarton)
Richard Antalik (ISS)

Maniphest Tasks

T90723: VSE: Quick Favorites not working well with strip properties (only working on the original strip, not when other strip is selected)

Summary

Since there was no context member for active VSE sequence and sequences
are all part of the scene, blender was always acting on the full RNA
path (meaning only the active sequence at the time of adding it to Quick
Favorites [or shortcut assignment] was being acted upon -- which is inconsistent to e.g. 3d objects where this always acts on the currently active object).

Now add an "active_sequence_strip" context member and use this in functions for Quick Favorites [and shortcut handling]. wm_context_member_from_ptr() needed a tweak and Sequences still need a bit of extra massage on the data paths in places that use this, but the added functionality seems worth it.

Diff Detail

Repository

rB Blender

Branch

arcpatch-D12297 (branched from master)

Build Status

Buildable 16662
Build 16662: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Aug 23 2021, 8:36 AM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B16555: Diff 40958.Aug 23 2021, 8:36 AM

Philipp Oeser (lichtwerk) added projects: User Interface, VFX & Video, Video Sequencer.Aug 23 2021, 8:37 AM

Talked in chat, while this fix is OK, I think it would be better to expose the active_sequence_strip in the context, as it seems likely we will run into this limitation again in the future.

Philipp Oeser (lichtwerk) planned changes to this revision.Aug 23 2021, 10:07 AM

add "active_sequence_strip" context member
use context member in both Quick favorites and shortcut assignement (which now works as well on active strips)
as it is now, active sequences still need a bit of extra path massage

Harbormaster completed remote builds in B16660: Diff 41172.Aug 27 2021, 5:16 PM

Philipp Oeser (lichtwerk) retitled this revision from Fix T90723: VSE: Quick Favorites not working well with strip properties to Fix T90723: VSE: Quick Favorites and Shortcuts not working well with strip properties.Aug 27 2021, 5:26 PM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)

remove unused include

Harbormaster completed remote builds in B16662: Diff 41174.Aug 27 2021, 5:28 PM

Campbell Barton (campbellbarton) requested changes to this revision.Aug 28 2021, 11:53 AM

Campbell Barton (campbellbarton) added inline comments.

source/blender/editors/interface/interface_context_menu.c
77–83	This block isn't needed once `wm_context_member_from_ptr` handles the active sequence.
352–358	This block isn't needed once `wm_context_member_from_ptr` handles the active sequence.
427–433	This block isn't needed once `wm_context_member_from_ptr` handles the active sequence.
source/blender/editors/screen/screen_user_menu.c
267	No need to move `PointerRNA ptr;` declaration up.
source/blender/windowmanager/intern/wm_operators.c
549–555	This block should be handled in `wm_context_member_from_ptr` (all other type spesific checks are done there).

This revision now requires changes to proceed.Aug 28 2021, 11:53 AM

Philipp Oeser (lichtwerk) added inline comments.Aug 28 2021, 2:41 PM

source/blender/windowmanager/intern/wm_operators.c
549–555	Not sure how that would work (might have to have another look on Monday)? But iirc, none of the cases in `wm_context_member_from_ptr` really handle a case where we bend from ID (here `scene`) to something like `sequence` and take care of cleaning up the prop path? we need to go from `context.scene.sequence_editor.sequences_all['foo'].someprop` to `context.active_sequence_strip..someprop` Without extra path massage, we might be able to look up the context member correctly, but end up with something like `context.active_sequence_strip.sequence_editor.sequences_all['foo'].someprop` Writing this from mobile, will check again on Monday.

note this also fails for many other types: bones, NLA strips, ... (Copy to Selected works for these though).
So the "path-bending" we need here is somewhat taken care of in UI_context_copy_to_selected_list -- this can possibly be unified to a certain degree.