From a59a992daaa001100b07ca10ae12ec1edba499d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Tue, 1 Aug 2023 20:35:28 +0200 Subject: [PATCH] st/scroll-view: Allocate the scrollbars 0-size instead of not painting them StScrollView applies the policy of whether to show or hide the scroll bar, and with the AUTOMATIC policy the scroll bar should be hidden as soon as the content of the scroll view is small enough to fit without scrolling. Now we only know about the final size of the content when we're inside st_scroll_view_allocate(), so that's where we can decide whether the scroll bar should be visible or not. Clutter really doesn't like calling clutter_actor_show/hide() in the middle of an allocation cycle though, so what we do instead is saving the state into priv->vscrollbar_visible, and then just not painting the scroll bar based on that in a paint() vfunc override. This approach is not great for several reasons, it means we also have to override pick() and finally it means the paint volume of the scroll bar is incorrect. While the greatest solution to this would be to just hide/show the scroll bar inside the allocate() function as it is possible in gtk, we have an established pattern for this kind of case too: We usually allocate a 0-sized rect for the thing we want to hide, so let's do that instead. A nice side effect is that we can conveniently drop another paint() and pick() vfunc override. Part-of: --- src/st/st-scroll-view.c | 89 ++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 55 deletions(-) diff --git a/src/st/st-scroll-view.c b/src/st/st-scroll-view.c index c95e12a82..f278568dc 100644 --- a/src/st/st-scroll-view.c +++ b/src/st/st-scroll-view.c @@ -268,41 +268,6 @@ st_scroll_view_dispose (GObject *object) G_OBJECT_CLASS (st_scroll_view_parent_class)->dispose (object); } -static void -st_scroll_view_paint (ClutterActor *actor, - ClutterPaintContext *paint_context) -{ - StScrollViewPrivate *priv = - st_scroll_view_get_instance_private (ST_SCROLL_VIEW (actor)); - - st_widget_paint_background (ST_WIDGET (actor), paint_context); - - if (priv->child) - clutter_actor_paint (priv->child, paint_context); - if (priv->hscrollbar_visible) - clutter_actor_paint (priv->hscroll, paint_context); - if (priv->vscrollbar_visible) - clutter_actor_paint (priv->vscroll, paint_context); -} - -static void -st_scroll_view_pick (ClutterActor *actor, - ClutterPickContext *pick_context) -{ - StScrollViewPrivate *priv = - st_scroll_view_get_instance_private (ST_SCROLL_VIEW (actor)); - - /* Chain up so we get a bounding box pained (if we are reactive) */ - CLUTTER_ACTOR_CLASS (st_scroll_view_parent_class)->pick (actor, pick_context); - - if (priv->child) - clutter_actor_pick (priv->child, pick_context); - if (priv->hscrollbar_visible) - clutter_actor_pick (priv->hscroll, pick_context); - if (priv->vscrollbar_visible) - clutter_actor_pick (priv->vscroll, pick_context); -} - static gboolean st_scroll_view_get_paint_volume (ClutterActor *actor, ClutterPaintVolume *volume) @@ -624,36 +589,52 @@ st_scroll_view_allocate (ClutterActor *actor, */ /* Vertical scrollbar */ - if (clutter_actor_get_text_direction (actor) == CLUTTER_TEXT_DIRECTION_RTL) + if (vscrollbar_visible) { - child_box.x1 = content_box.x1; - child_box.x2 = content_box.x1 + sb_width; + if (clutter_actor_get_text_direction (actor) == CLUTTER_TEXT_DIRECTION_RTL) + { + child_box.x1 = content_box.x1; + child_box.x2 = content_box.x1 + sb_width; + } + else + { + child_box.x1 = content_box.x2 - sb_width; + child_box.x2 = content_box.x2; + } + child_box.y1 = content_box.y1; + child_box.y2 = content_box.y2 - (hscrollbar_visible ? sb_height : 0); + + clutter_actor_allocate (priv->vscroll, &child_box); } else { - child_box.x1 = content_box.x2 - sb_width; - child_box.x2 = content_box.x2; + ClutterActorBox empty_box = { 0, }; + clutter_actor_allocate (priv->vscroll, &empty_box); } - child_box.y1 = content_box.y1; - child_box.y2 = content_box.y2 - (hscrollbar_visible ? sb_height : 0); - - clutter_actor_allocate (priv->vscroll, &child_box); /* Horizontal scrollbar */ - if (clutter_actor_get_text_direction (actor) == CLUTTER_TEXT_DIRECTION_RTL) + if (hscrollbar_visible) { - child_box.x1 = content_box.x1 + (vscrollbar_visible ? sb_width : 0); - child_box.x2 = content_box.x2; + if (clutter_actor_get_text_direction (actor) == CLUTTER_TEXT_DIRECTION_RTL) + { + child_box.x1 = content_box.x1 + (vscrollbar_visible ? sb_width : 0); + child_box.x2 = content_box.x2; + } + else + { + child_box.x1 = content_box.x1; + child_box.x2 = content_box.x2 - (vscrollbar_visible ? sb_width : 0); + } + child_box.y1 = content_box.y2 - sb_height; + child_box.y2 = content_box.y2; + + clutter_actor_allocate (priv->hscroll, &child_box); } else { - child_box.x1 = content_box.x1; - child_box.x2 = content_box.x2 - (vscrollbar_visible ? sb_width : 0); + ClutterActorBox empty_box = { 0, }; + clutter_actor_allocate (priv->hscroll, &empty_box); } - child_box.y1 = content_box.y2 - sb_height; - child_box.y2 = content_box.y2; - - clutter_actor_allocate (priv->hscroll, &child_box); /* In case the scrollbar policy is NEVER or EXTERNAL or scrollbars * should be overlaid, we don't trim the content box allocation by @@ -868,8 +849,6 @@ st_scroll_view_class_init (StScrollViewClass *klass) object_class->set_property = st_scroll_view_set_property; object_class->dispose = st_scroll_view_dispose; - actor_class->paint = st_scroll_view_paint; - actor_class->pick = st_scroll_view_pick; actor_class->get_paint_volume = st_scroll_view_get_paint_volume; actor_class->get_preferred_width = st_scroll_view_get_preferred_width; actor_class->get_preferred_height = st_scroll_view_get_preferred_height;