Fix T79872: VSE - splitting strip shows the channel number when unused
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Aug 19 2020, 1:40 PM.

Details

Reviewers

Richard Antalik (ISS)
Julian Eisel (Severin)
Hans Goudey (HooglyBoogly)

Maniphest Tasks

T79872: VSE - Strip menu - Split - adjust last operator displays wrong channel number

Commits

rBe74ba9e09e74: Fix T79872: VSE - splitting strip shows the channel number when unused

Summary

This operator is dependent on mouse position (if the Use Cursor Position
option is used). The Channel property is irrelevant/unused in this case.
So it is not optimal displaying this property when calling this from the
menu (or even using the shortcut with default settings).

Now use a custom UI in the Adjust Last Operation panel in this case.
The properties are now drawn in relation to another then (Channel
underneath Use Cursor Position) next to some other minor layout
improvements.

Thx @Hans Goudey (HooglyBoogly) for feedback (also providing UI code)

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Aug 19 2020, 1:40 PM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B9636: Diff 27905.Aug 19 2020, 1:40 PM

Philipp Oeser (lichtwerk) added projects: User Interface, VFX & Video, Video Sequencer.Aug 19 2020, 1:41 PM

I guess this is OK solution. I am not aware of how ot->ui should be used though. By looking quickly at some code it looks like the "same thing", but dynamic?

This revision is now accepted and ready to land.Aug 20 2020, 1:32 AM

This is completely personal, but I tend to prefer a custom drawing callback in this situation. It's about the same amount of code, it's more explicit, and (obviously) possible to customize.
Small downside is you now have to add new properties to the layout when you add them, small upside is avoiding a bunch of string comparisons when drawing the layout.

Here's an alternate solution with that method:

diff --git a/source/blender/editors/space_sequencer/sequencer_edit.c b/source/blender/editors/space_sequencer/sequencer_edit.c
index 78ca2832c55..8d288e11987 100644
--- a/source/blender/editors/space_sequencer/sequencer_edit.c
+++ b/source/blender/editors/space_sequencer/sequencer_edit.c
@@ -58,6 +58,7 @@
 #include "ED_sequencer.h"
 
 #include "UI_interface.h"
+#include "UI_resources.h"
 #include "UI_view2d.h"
 
 #include "DEG_depsgraph.h"
@@ -2376,6 +2377,28 @@ static int sequencer_split_invoke(bContext *C, wmOperator *op, const wmEvent *ev
   return sequencer_split_exec(C, op);
 }
 
+static void sequencer_split_ui(bContext *UNUSED(C), wmOperator *op)
+{
+  uiLayout *layout = op->layout;
+  uiLayoutSetPropSep(layout, true);
+  uiLayoutSetPropDecorate(layout, false);
+
+  PointerRNA ptr;
+  RNA_pointer_create(NULL, op->type->srna, op->properties, &ptr);
+
+  uiLayout *row = uiLayoutRow(layout, false);
+  uiItemR(row, &ptr, "type", UI_ITEM_R_EXPAND, NULL, ICON_NONE);
+  uiItemR(layout, &ptr, "frame", 0, NULL, ICON_NONE);
+  uiItemR(layout, &ptr, "side", 0, NULL, ICON_NONE);
+
+  uiItemS(layout);
+
+  uiItemR(layout, &ptr, "use_cursor_position", 0, NULL, ICON_NONE);
+  if (RNA_boolean_get(&ptr, "use_cursor_position")) {
+    uiItemR(layout, &ptr, "channel", 0, NULL, ICON_NONE);
+  }
+}
+
 void SEQUENCER_OT_split(struct wmOperatorType *ot)
 {
   /* Identifiers. */
@@ -2387,6 +2410,7 @@ void SEQUENCER_OT_split(struct wmOperatorType *ot)
   ot->invoke = sequencer_split_invoke;
   ot->exec = sequencer_split_exec;
   ot->poll = sequencer_edit_poll;
+  ot->ui = sequencer_split_ui;
 
   /* Flags. */
   ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;