[gnome-shell] [StScrollBar] avoid queueing a relayout during allocation

[Owen Taylor <otaylor src gnome org>]

commit 524e2ca8e232f462597ccd7be58a20224e7a1166
Author: Owen W. Taylor <otaylor fishsoup net>
Date:   Fri Mar 5 16:17:07 2010 -0500

    [StScrollBar] avoid queueing a relayout during allocation
    
    When an StScrollView is allocated, allocating the child would
    cause the adjustment values to change, which would result in
    the scrollbars queueing a relayout, which isn't allowed during
    allocation.
    
    To avoid this, instead of queueing a relayout when the adjustment
    changes:
    
     - When we have a valid allocation already, just go ahead
       and reallocate the children.
     - Otherwise do nothing immediately and wait until we get allocated
    
    Because the 'needs_allocation' flag in ClutterActor isn't exposed,
    this requires some slightly ugly code to shadow that state locally.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=611944

 src/st/st-scroll-bar.c |  119 ++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 101 insertions(+), 18 deletions(-)
---
diff --git a/src/st/st-scroll-bar.c b/src/st/st-scroll-bar.c
index 2698397..b649eb8 100644
--- a/src/st/st-scroll-bar.c
+++ b/src/st/st-scroll-bar.c
@@ -77,7 +77,15 @@ struct _StScrollBarPrivate
 
   ClutterAnimation *paging_animation;
 
-  gboolean          vertical;
+  guint             vertical : 1;
+
+  /* We want to keep track of whether we have a currently valid
+   * allocation or not. This isn't exported from ClutterActor
+   * so we need to shadow the computations and track it ourselves.
+   *
+   * http://bugzilla.openedhand.com/show_bug.cgi?id=2024
+   */
+  guint             needs_allocation : 1;
 };
 
 enum
@@ -273,19 +281,37 @@ st_scroll_bar_unmap (ClutterActor *actor)
 }
 
 static void
-st_scroll_bar_allocate (ClutterActor          *actor,
-                        const ClutterActorBox *box,
-                        ClutterAllocationFlags flags)
+st_scroll_bar_parent_set (ClutterActor  *actor,
+                          ClutterActor   *old_parent)
+{
+  StScrollBarPrivate *priv = ST_SCROLL_BAR (actor)->priv;
+
+  priv->needs_allocation = TRUE;
+
+  if (CLUTTER_ACTOR_CLASS (st_scroll_bar_parent_class)->parent_set)
+    CLUTTER_ACTOR_CLASS (st_scroll_bar_parent_class)->parent_set (actor, old_parent);
+}
+
+static void
+st_scroll_bar_queue_relayout (ClutterActor *actor)
 {
   StScrollBarPrivate *priv = ST_SCROLL_BAR (actor)->priv;
-  StThemeNode *theme_node = st_widget_get_theme_node (ST_WIDGET (actor));
+
+  priv->needs_allocation = TRUE;
+
+  CLUTTER_ACTOR_CLASS (st_scroll_bar_parent_class)->queue_relayout (actor);
+}
+
+static void
+scroll_bar_allocate_children (StScrollBar           *bar,
+                              const ClutterActorBox *box,
+                              ClutterAllocationFlags flags)
+{
+  StScrollBarPrivate *priv = bar->priv;
+  StThemeNode *theme_node = st_widget_get_theme_node (ST_WIDGET (bar));
   ClutterActorBox content_box, bw_box, fw_box, trough_box;
   gfloat bw_stepper_size, fw_stepper_size, min_size, natural_size;
 
-  /* Chain up */
-  CLUTTER_ACTOR_CLASS (st_scroll_bar_parent_class)->
-  allocate (actor, box, flags);
-
   st_theme_node_get_content_box (theme_node, box, &content_box);
 
   if (priv->vertical)
@@ -361,7 +387,6 @@ st_scroll_bar_allocate (ClutterActor          *actor,
 
   if (priv->adjustment)
     {
-      StThemeNode *theme_node = st_widget_get_theme_node (ST_WIDGET (actor));
       float handle_size, position, avail_size, stepper_size;
       gdouble value, lower, upper, page_size, increment, min_size, max_size;
       ClutterActorBox handle_box = { 0, };
@@ -430,6 +455,45 @@ st_scroll_bar_allocate (ClutterActor          *actor,
 }
 
 static void
+st_scroll_bar_allocate (ClutterActor          *actor,
+                        const ClutterActorBox *box,
+                        ClutterAllocationFlags flags)
+{
+  StScrollBar *bar = ST_SCROLL_BAR (actor);
+  StScrollBarPrivate *priv = bar->priv;
+  priv->needs_allocation = FALSE;
+
+  /* Chain up */
+  CLUTTER_ACTOR_CLASS (st_scroll_bar_parent_class)->allocate (actor, box, flags);
+
+  scroll_bar_allocate_children (bar, box, flags);
+}
+
+static void
+scroll_bar_update_positions (StScrollBar *bar)
+{
+  StScrollBarPrivate *priv = bar->priv;
+  ClutterActorBox box;
+
+  /* Due to a change in the adjustments, we need to reposition our
+   * children; since adjustments changes can come from allocation
+   * changes in the scrolled area, we can't just queue a new relayout -
+   * we may already be in a relayout cycle. On the other hand, if
+   * a relayout is already queued, we can't just go ahead and allocate
+   * our children, since we don't have a valid allocation, and calling
+   * clutter_actor_get_allocation_box() will trigger an immediate
+   * stage relayout. So what we do is go ahead and immediately
+   * allocate our children if we already have a valid allocation, and
+   * otherwise just wait for the queued relayout.
+   */
+  if (priv->needs_allocation)
+    return;
+
+  clutter_actor_get_allocation_box (CLUTTER_ACTOR (bar), &box);
+  scroll_bar_allocate_children (bar, &box, CLUTTER_ALLOCATION_NONE);
+}
+
+static void
 st_scroll_bar_style_changed (StWidget *widget)
 {
   StScrollBarPrivate *priv = ST_SCROLL_BAR (widget)->priv;
@@ -532,6 +596,8 @@ st_scroll_bar_class_init (StScrollBarClass *klass)
   object_class->dispose      = st_scroll_bar_dispose;
   object_class->constructor  = st_scroll_bar_constructor;
 
+  actor_class->parent_set     = st_scroll_bar_parent_set;
+  actor_class->queue_relayout = st_scroll_bar_queue_relayout;
   actor_class->allocate       = st_scroll_bar_allocate;
   actor_class->paint          = st_scroll_bar_paint;
   actor_class->pick           = st_scroll_bar_pick;
@@ -1041,6 +1107,8 @@ st_scroll_bar_init (StScrollBar *self)
 
   g_signal_connect (self, "notify::reactive",
                     G_CALLBACK (st_scroll_bar_notify_reactive), NULL);
+
+  self->priv->needs_allocation = TRUE;
 }
 
 StWidget *
@@ -1051,6 +1119,21 @@ st_scroll_bar_new (StAdjustment *adjustment)
                        NULL);
 }
 
+static void
+on_notify_value (GObject     *object,
+                 GParamSpec  *pspec,
+                 StScrollBar *bar)
+{
+  scroll_bar_update_positions (bar);
+}
+
+static void
+on_changed (StAdjustment *adjustment,
+            StScrollBar  *bar)
+{
+  scroll_bar_update_positions (bar);
+}
+
 void
 st_scroll_bar_set_adjustment (StScrollBar  *bar,
                               StAdjustment *adjustment)
@@ -1063,10 +1146,10 @@ st_scroll_bar_set_adjustment (StScrollBar  *bar,
   if (priv->adjustment)
     {
       g_signal_handlers_disconnect_by_func (priv->adjustment,
-                                            clutter_actor_queue_relayout,
+                                            on_notify_value,
                                             bar);
       g_signal_handlers_disconnect_by_func (priv->adjustment,
-                                            clutter_actor_queue_relayout,
+                                            on_changed,
                                             bar);
       g_object_unref (priv->adjustment);
       priv->adjustment = NULL;
@@ -1076,12 +1159,12 @@ st_scroll_bar_set_adjustment (StScrollBar  *bar,
     {
       priv->adjustment = g_object_ref (adjustment);
 
-      g_signal_connect_swapped (priv->adjustment, "notify::value",
-                                G_CALLBACK (clutter_actor_queue_relayout),
-                                bar);
-      g_signal_connect_swapped (priv->adjustment, "changed",
-                                G_CALLBACK (clutter_actor_queue_relayout),
-                                bar);
+      g_signal_connect (priv->adjustment, "notify::value",
+                        G_CALLBACK (on_notify_value),
+                        bar);
+      g_signal_connect (priv->adjustment, "changed",
+                        G_CALLBACK (on_changed),
+                        bar);
 
       clutter_actor_queue_relayout (CLUTTER_ACTOR (bar));
     }