VSE, marker area is invisibly blocking the strips #57976

New Issue

Hjalti Hjálmarsson · 2018-11-21T15:16:09+01:00

2018-11-21 15:16:09 +01:00

System Information
Xbuntu / AMD

Blender Version
e85aa8b

Short description of error
When working in the VSE, if you create a marker there's an invisible "marker area" that's created a the bottom but you can still see the strips underneath even though you can't interact with them in this area. The marker area would really need to be its own thing and push the rest of the VSE upwards so those two areas are not in visual conflict.

Note: Pablo (Vazques) asked me to report this as a bug.

Exact steps for others to reproduce the error
New file
Go to the VSE
Add a strip on layer 0 (bottom layer)
Create a marker
Try to interact with the strip with the cursor at the bottom of the VSE

**System Information** Xbuntu / AMD **Blender Version** e85aa8b **Short description of error** When working in the VSE, if you create a marker there's an invisible "marker area" that's created a the bottom but you can still see the strips underneath even though you can't interact with them in this area. The marker area would really need to be its own thing and push the rest of the VSE upwards so those two areas are not in visual conflict. Note: Pablo (Vazques) asked me to report this as a bug. **Exact steps for others to reproduce the error** New file Go to the VSE Add a strip on layer 0 (bottom layer) Create a marker Try to interact with the strip with the cursor at the bottom of the VSE

Hjalti Hjálmarsson commented

2018-11-21 15:16:09 +01:00

Added subscriber: @Hjalti

blender-admin commented

2018-11-21 15:16:09 +01:00

#90099 was marked as duplicate of this issue

blender-admin commented

2018-11-21 15:16:09 +01:00

#82345 was marked as duplicate of this issue

blender-admin commented

2018-11-21 15:16:09 +01:00

#73772 was marked as duplicate of this issue

blender-admin commented

2018-11-21 15:16:09 +01:00

#65061 was marked as duplicate of this issue

Philipp Oeser commented

2018-11-22 12:43:22 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2018-11-22 12:43:22 +01:00

Wouldnt it be enough to restrict the horizontal clickable "hotspot" to some extend around the actual marker position? (as of now it seems to total region width is occupied for that -- indicated by a slight transparent overlay [which goes away if you have no markers at all]).
(Just saying, because pushing the whole area upwards would mean wasting quite some vertical space, no?)

Wouldnt it be enough to restrict the horizontal clickable "hotspot" to some extend around the actual marker position? (as of now it seems to total region width is occupied for that -- indicated by a slight transparent overlay [which goes away if you have no markers at all]). (Just saying, because pushing the whole area upwards would mean wasting quite some vertical space, no?)

Brecht Van Lommel commented

2018-11-22 13:08:36 +01:00

Added subscriber: @pablovazquez

Brecht Van Lommel commented

2018-11-22 13:11:27 +01:00

Added subscriber: @brecht

Brecht Van Lommel commented

2018-11-22 13:11:27 +01:00

Markers were designed to work this way, overlapping the contents of the editor also in the dopesheet for example.

This could be improved, if there is a design for it perhaps it can be added to the UI paper cuts. But I'm not sure I would consider this a bug.

Markers were designed to work this way, overlapping the contents of the editor also in the dopesheet for example. This could be improved, if there is a design for it perhaps it can be added to the UI paper cuts. But I'm not sure I would consider this a bug.

Richard Antalik commented

2019-02-20 01:53:36 +01:00

Added subscriber: @iss

Richard Antalik commented

2019-02-20 01:53:36 +01:00

Problem is, that blocked area is absolute, regardless of timeline zoom and it can block lower channels., as you can not pan below zero.

Simplest thing would be turn markers upside down. so they are selected on the upper part of timeline.
Should I write patch for this?

Problem is, that blocked area is absolute, regardless of timeline zoom and it can block lower channels., as you can not pan below zero. Simplest thing would be turn markers upside down. so they are selected on the upper part of timeline. Should I write patch for this?

Eitan Traurig commented

2020-01-02 19:10:26 +01:00

Added subscriber: @EitanSomething

Eitan Traurig commented

2020-01-02 19:10:26 +01:00

I am unable to reproduce can you provide a simple .blend file.

Richard Antalik commented

2020-01-02 23:29:44 +01:00

The issue is demonstrated in this image

The issue is demonstrated in this image ![screen.png](https://archive.blender.org/developer/F8257065/screen.png)

Eitan Traurig commented

2020-01-15 00:49:29 +01:00

In #57976#623927, @iss wrote:
Problem is, that blocked area is absolute, regardless of timeline zoom and it can block lower channels., as you can not pan below zero.

Simplest thing would be turn markers upside down. so they are selected on the upper part of timeline.
Should I write patch for this?

Putting the marker on the top won't help because it would then block the sequences on the top.

> In #57976#623927, @iss wrote: > Problem is, that blocked area is absolute, regardless of timeline zoom and it can block lower channels., as you can not pan below zero. > > Simplest thing would be turn markers upside down. so they are selected on the upper part of timeline. > Should I write patch for this? Putting the marker on the top won't help because it would then block the sequences on the top.

Eitan Traurig self-assigned this 2020-01-15 01:20:18 +01:00

Eitan Traurig commented

2020-01-15 01:26:49 +01:00

I try to get this fixed as quickly as possible in order to get it into 2.82

Richard Antalik commented

2020-01-15 11:34:11 +01:00

Added subscriber: @WilliamReynish

Richard Antalik commented

2020-01-15 11:34:11 +01:00

In #57976#849113, @EitanSomething wrote:

In #57976#623927, @iss wrote:
Problem is, that blocked area is absolute, regardless of timeline zoom and it can block lower channels., as you can not pan below zero.

Simplest thing would be turn markers upside down. so they are selected on the upper part of timeline.
Should I write patch for this?

Putting the marker on the top won't help because it would then block the sequences on the top.

You can pan, to put upper sequences lower in view.
It would be good idea to discuss this with @WilliamReynish first. I will seek approval of UI team anyway.

If not changing placement you can offset drawing bottom channel or make markers area auto-hide.

> In #57976#849113, @EitanSomething wrote: >> In #57976#623927, @iss wrote: >> Problem is, that blocked area is absolute, regardless of timeline zoom and it can block lower channels., as you can not pan below zero. >> >> Simplest thing would be turn markers upside down. so they are selected on the upper part of timeline. >> Should I write patch for this? > > Putting the marker on the top won't help because it would then block the sequences on the top. You can pan, to put upper sequences lower in view. It would be good idea to discuss this with @WilliamReynish first. I will seek approval of UI team anyway. If not changing placement you can offset drawing bottom channel or make markers area auto-hide.

William Reynish commented

2020-01-15 11:38:39 +01:00

I think the best solution is to offset the strips so the bottom ones aren't always covered. Or, make it possible to scroll past the bottom layer.

Julian Eisel commented

2020-01-15 15:53:40 +01:00

Added subscriber: @JulianEisel

Julian Eisel commented

2020-01-15 15:53:40 +01:00

I don't think we should move the marker region to the top. It would make things inconsistent, but it's also a workaround exposed as feature.

To fix this, we should progressively push in a margin at the bottom, so that the strip always stays in view. Like demonstrated here (the video player controls overlap the important bit, move the mouse away to make them disappear):
recording.mp4

This is the hack I used to get this behavior:
P1215: Proof of concept hack for #57976

diff --git a/source/blender/editors/space_sequencer/sequencer_draw.c b/source/blender/editors/space_sequencer/sequencer_draw.c
index 333a51e2eac..d558db45103 100644
--- a/source/blender/editors/space_sequencer/sequencer_draw.c
+++ b/source/blender/editors/space_sequencer/sequencer_draw.c
@@ -2003,6 +2003,12 @@ static void draw_cache_view(const bContext *C)
   GPU_blend(false);
 }
 
+static bool sequencer_has_markers(const bContext *C, SpaceSeq *sseq)
+{
+  const ListBase *markers = ED_context_get_markers(C);
+  return (sseq->flag & SEQ_SHOW_MARKERS && markers && !BLI_listbase_is_empty(markers));
+}
+
 /* Draw Timeline/Strip Editor Mode for Sequencer */
 void draw_timeline_seq(const bContext *C, ARegion *ar)
 {
@@ -2010,9 +2016,13 @@ void draw_timeline_seq(const bContext *C, ARegion *ar)
   Editing *ed = BKE_sequencer_editing_get(scene, false);
   SpaceSeq *sseq = CTX_wm_space_seq(C);
   View2D *v2d = &ar->v2d;
+  const bool has_markers = sequencer_has_markers(C, sseq);
+
   View2DScrollers *scrollers;
   short cfra_flag = 0;
   float col[3];
+  /* View space */
+  float pad_bottom_view = 0.0f;
 
   seq_prefetch_wm_notify(C, scene);
 
@@ -2026,6 +2036,20 @@ void draw_timeline_seq(const bContext *C, ARegion *ar)
   }
   GPU_clear(GPU_COLOR_BIT);
 
+  if (has_markers) {
+    float yscale;
+
+    UI_view2d_scale_get(v2d, NULL, &yscale);
+
+    /* Effectively, what this does is: If the yscale is smaller than the marker region, add the
+     * difference as bottom offset so that no channel gets stuck behind the marker region. */
+    if (yscale < UI_MARKER_MARGIN_Y) {
+      pad_bottom_view = ((UI_MARKER_MARGIN_Y + 2) / yscale) - 1.0f;
+    }
+  }
+  /* Scale up current rect at the bottom, making space for markers if needed. */
+  v2d->cur.ymin -= pad_bottom_view;
+
   UI_view2d_view_ortho(v2d);
 
   /* calculate extents of sequencer strips/data
@@ -2122,4 +2146,6 @@ void draw_timeline_seq(const bContext *C, ARegion *ar)
     BLI_rcti_init(&rect, 0, 15 * UI_DPI_FAC, 15 * UI_DPI_FAC, ar->winy - UI_TIME_SCRUB_MARGIN_Y);
     UI_view2d_draw_scale_y__block(ar, v2d, &rect, TH_SCROLL_TEXT);
   }
+
+  v2d->cur.ymin += pad_bottom_view;
 }

Note however that this only covers drawing. Input handling (e.g. selecting strips) does not get the offset applied and is therefore notably off.
What we'd have to do is consistently apply an offset, wherever coordinates are accessed - in both, drawing and handling. I'd suggest adding a runtime struct to SpaceSeq so that the offset can be accessed like SpaceSeq.runtime.offset_bottom. It would be set/updated on drawing.
Not the easiest task, although most of it is probably "monkey work".

For reference: I did something similar for the file browser file list header (the bar saying "Name", "Date Modified" and "Size"): It is part of the main region, but the file list and the vertical scrollbar get offset to make space for it. See SpaceFile.layout.offset_top.

Ideally these offsets would be supported by View2D, but that is another can of worms.

I don't think we should move the marker region to the top. It would make things inconsistent, but it's also a workaround exposed as feature. To fix this, we should progressively push in a margin at the bottom, so that the strip always stays in view. Like demonstrated here (the video player controls overlap the important bit, move the mouse away to make them disappear): [recording.mp4](https://archive.blender.org/developer/F8279782/recording.mp4) This is the hack I used to get this behavior: [P1215: Proof of concept hack for #57976](https://archive.blender.org/developer/P1215.txt) ``` diff --git a/source/blender/editors/space_sequencer/sequencer_draw.c b/source/blender/editors/space_sequencer/sequencer_draw.c index 333a51e2eac..d558db45103 100644 --- a/source/blender/editors/space_sequencer/sequencer_draw.c +++ b/source/blender/editors/space_sequencer/sequencer_draw.c @@ -2003,6 +2003,12 @@ static void draw_cache_view(const bContext *C) GPU_blend(false); } +static bool sequencer_has_markers(const bContext *C, SpaceSeq *sseq) +{ + const ListBase *markers = ED_context_get_markers(C); + return (sseq->flag & SEQ_SHOW_MARKERS && markers && !BLI_listbase_is_empty(markers)); +} + /* Draw Timeline/Strip Editor Mode for Sequencer */ void draw_timeline_seq(const bContext *C, ARegion *ar) { @@ -2010,9 +2016,13 @@ void draw_timeline_seq(const bContext *C, ARegion *ar) Editing *ed = BKE_sequencer_editing_get(scene, false); SpaceSeq *sseq = CTX_wm_space_seq(C); View2D *v2d = &ar->v2d; + const bool has_markers = sequencer_has_markers(C, sseq); + View2DScrollers *scrollers; short cfra_flag = 0; float col[3]; + /* View space */ + float pad_bottom_view = 0.0f; seq_prefetch_wm_notify(C, scene); @@ -2026,6 +2036,20 @@ void draw_timeline_seq(const bContext *C, ARegion *ar) } GPU_clear(GPU_COLOR_BIT); + if (has_markers) { + float yscale; + + UI_view2d_scale_get(v2d, NULL, &yscale); + + /* Effectively, what this does is: If the yscale is smaller than the marker region, add the + * difference as bottom offset so that no channel gets stuck behind the marker region. */ + if (yscale < UI_MARKER_MARGIN_Y) { + pad_bottom_view = ((UI_MARKER_MARGIN_Y + 2) / yscale) - 1.0f; + } + } + /* Scale up current rect at the bottom, making space for markers if needed. */ + v2d->cur.ymin -= pad_bottom_view; + UI_view2d_view_ortho(v2d); /* calculate extents of sequencer strips/data @@ -2122,4 +2146,6 @@ void draw_timeline_seq(const bContext *C, ARegion *ar) BLI_rcti_init(&rect, 0, 15 * UI_DPI_FAC, 15 * UI_DPI_FAC, ar->winy - UI_TIME_SCRUB_MARGIN_Y); UI_view2d_draw_scale_y__block(ar, v2d, &rect, TH_SCROLL_TEXT); } + + v2d->cur.ymin += pad_bottom_view; } ``` Note however that this only covers drawing. Input handling (e.g. selecting strips) does not get the offset applied and is therefore notably off. What we'd have to do is consistently apply an offset, wherever coordinates are accessed - in both, drawing and handling. I'd suggest adding a runtime struct to `SpaceSeq` so that the offset can be accessed like `SpaceSeq.runtime.offset_bottom`. It would be set/updated on drawing. Not the easiest task, although most of it is probably "monkey work". For reference: I did something similar for the file browser file list header (the bar saying "Name", "Date Modified" and "Size"): It is part of the main region, but the file list and the vertical scrollbar get offset to make space for it. See `SpaceFile.layout.offset_top`. Ideally these offsets would be supported by `View2D`, but that is another can of worms.

Richard Antalik commented

2020-01-15 16:56:10 +01:00

I agree with adding offset in bottom part.

Thanks for suggestion to add offset in sseq.

I agree with adding offset in bottom part. Thanks for suggestion to add offset in sseq.

Richard Antalik commented

2020-02-02 23:44:02 +01:00

Added subscribers: @WayneDixon, @JacquesLucke