From 16899329729f578e955befbbb129a310e9c8da16 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Thu, 8 Feb 2024 12:46:21 +0100 Subject: [PATCH] Fix #117572: Top vertical scrollbar tool not working properly Remove ugly/fragile special case in `view2d_masks()` that would clamp the scrollbar-masks by the scrubbing UI. It's now possible to pass custom scrollbar-masks to `View2D` via `UI_view2d_scrollers_draw()`. So use this instead, making region code control its own masks, rather than relying on special case handling in generic `View2D` code. Also update comment in `scroller_activate_init()` to make the implicit relationship explicit. Alternative to, and based on the investigation in !117685. --- source/blender/editors/animation/time_scrub_ui.cc | 7 +++++++ source/blender/editors/include/ED_time_scrub_ui.hh | 8 ++++++++ source/blender/editors/interface/view2d.cc | 7 ------- source/blender/editors/interface/view2d_ops.cc | 12 +++++++++--- source/blender/editors/space_clip/space_clip.cc | 3 ++- source/blender/editors/space_graph/space_graph.cc | 3 ++- .../space_sequencer/sequencer_timeline_draw.cc | 3 ++- 7 files changed, 30 insertions(+), 13 deletions(-) diff --git a/source/blender/editors/animation/time_scrub_ui.cc b/source/blender/editors/animation/time_scrub_ui.cc index 1718bea9646..bffcd738167 100644 --- a/source/blender/editors/animation/time_scrub_ui.cc +++ b/source/blender/editors/animation/time_scrub_ui.cc @@ -183,6 +183,13 @@ void ED_time_scrub_draw(const ARegion *region, GPU_matrix_pop_projection(); } +rcti ED_time_scrub_clamp_scroller_mask(const rcti &scroller_mask) +{ + rcti clamped_mask = scroller_mask; + clamped_mask.ymax -= UI_TIME_SCRUB_MARGIN_Y; + return clamped_mask; +} + bool ED_time_scrub_event_in_region(const ARegion *region, const wmEvent *event) { rcti rect = region->winrct; diff --git a/source/blender/editors/include/ED_time_scrub_ui.hh b/source/blender/editors/include/ED_time_scrub_ui.hh index 70a11255bb4..925fa16e37e 100644 --- a/source/blender/editors/include/ED_time_scrub_ui.hh +++ b/source/blender/editors/include/ED_time_scrub_ui.hh @@ -21,6 +21,14 @@ void ED_time_scrub_draw(const ARegion *region, const Scene *scene, bool display_seconds, bool discrete_frames); +/** + * Scroll-bars shouldn't overlap the time scrub UI. So this returns a mask adjusted to exclude it, + * which can be passed to #UI_view2d_scrollers_draw(). + * + * \param scroller_mask: Typically #View2D.mask (or something smaller, if further parts have been + * masked out already). + */ +rcti ED_time_scrub_clamp_scroller_mask(const rcti &scroller_mask); bool ED_time_scrub_event_in_region(const ARegion *region, const wmEvent *event); diff --git a/source/blender/editors/interface/view2d.cc b/source/blender/editors/interface/view2d.cc index e8425ffff08..569ef81c7d2 100644 --- a/source/blender/editors/interface/view2d.cc +++ b/source/blender/editors/interface/view2d.cc @@ -181,13 +181,6 @@ static void view2d_masks(View2D *v2d, const rcti *mask_scroll) v2d->vert.xmin = v2d->vert.xmax - scroll_width; } - /* Currently, all regions that have vertical scale handles, - * also have the scrubbing area at the top. - * So the scroll-bar has to move down a bit. */ - if (scroll & V2D_SCROLL_VERTICAL_HANDLES) { - v2d->vert.ymax -= UI_TIME_SCRUB_MARGIN_Y; - } - /* horizontal scroller */ if (scroll & V2D_SCROLL_BOTTOM) { /* on bottom edge of region */ diff --git a/source/blender/editors/interface/view2d_ops.cc b/source/blender/editors/interface/view2d_ops.cc index 24117643cbb..f63e8c480ac 100644 --- a/source/blender/editors/interface/view2d_ops.cc +++ b/source/blender/editors/interface/view2d_ops.cc @@ -1908,11 +1908,17 @@ static void scroller_activate_init(bContext *C, * - zooming must be allowed on this axis, otherwise, default to pan */ View2DScrollers scrollers; - /* Some Editors like the File-browser or Spreadsheet already set up custom masks for scroll-bars - * (they don't cover the whole region width or height), these need to be considered, otherwise - * coords for `mouse_in_scroller_handle` later are not compatible. */ + /* Reconstruct the custom scroller mask passed to #UI_view2d_scrollers_draw(). + * + * Some editors like the File Browser, Spreadsheet or scrubbing UI already set up custom masks + * for scroll-bars (they don't cover the whole region width or height), these need to be + * considered, otherwise coords for `mouse_in_scroller_handle` later are not compatible. This + * should be a reliable way to do it. Otherwise the custom scroller mask could also be stored in + * #View2D. + */ rcti scroller_mask = v2d->hor; BLI_rcti_union(&scroller_mask, &v2d->vert); + view2d_scrollers_calc(v2d, &scroller_mask, &scrollers); /* Use a union of 'cur' & 'tot' in case the current view is far outside 'tot'. In this cases diff --git a/source/blender/editors/space_clip/space_clip.cc b/source/blender/editors/space_clip/space_clip.cc index 9ab14e5f39a..08ddd3c89d2 100644 --- a/source/blender/editors/space_clip/space_clip.cc +++ b/source/blender/editors/space_clip/space_clip.cc @@ -874,7 +874,8 @@ static void graph_region_draw(const bContext *C, ARegion *region) ED_time_scrub_draw_current_frame(region, scene, sc->flag & SC_SHOW_SECONDS); /* scrollers */ - UI_view2d_scrollers_draw(v2d, nullptr); + const rcti scroller_mask = ED_time_scrub_clamp_scroller_mask(v2d->mask); + UI_view2d_scrollers_draw(v2d, &scroller_mask); /* scale indicators */ { diff --git a/source/blender/editors/space_graph/space_graph.cc b/source/blender/editors/space_graph/space_graph.cc index 83291b61817..e7196c14588 100644 --- a/source/blender/editors/space_graph/space_graph.cc +++ b/source/blender/editors/space_graph/space_graph.cc @@ -333,8 +333,9 @@ static void graph_main_region_draw_overlay(const bContext *C, ARegion *region) } /* scrollers */ + const rcti scroller_mask = ED_time_scrub_clamp_scroller_mask(v2d->mask); /* FIXME: args for scrollers depend on the type of data being shown. */ - UI_view2d_scrollers_draw(v2d, nullptr); + UI_view2d_scrollers_draw(v2d, &scroller_mask); /* scale numbers */ { diff --git a/source/blender/editors/space_sequencer/sequencer_timeline_draw.cc b/source/blender/editors/space_sequencer/sequencer_timeline_draw.cc index b8b9e78b6b6..1455d0b7e4d 100644 --- a/source/blender/editors/space_sequencer/sequencer_timeline_draw.cc +++ b/source/blender/editors/space_sequencer/sequencer_timeline_draw.cc @@ -1925,5 +1925,6 @@ void draw_timeline_seq_display(const bContext *C, ARegion *region) const ListBase *seqbase = SEQ_active_seqbase_get(SEQ_editing_get(scene)); SEQ_timeline_boundbox(scene, seqbase, &v2d->tot); - UI_view2d_scrollers_draw(v2d, nullptr); + const rcti scroller_mask = ED_time_scrub_clamp_scroller_mask(v2d->mask); + UI_view2d_scrollers_draw(v2d, &scroller_mask); } -- 2.30.2