[mutter] Use "later functions" to fix priority problems with Clutter redraw



commit 3508c4aa87a782f69c0f3880c41d5da3d3c97327
Author: Owen W. Taylor <otaylor fishsoup net>
Date:   Fri Sep 25 15:47:22 2009 -0400

    Use "later functions" to fix priority problems with Clutter redraw
    
    There was a problem where if, for example, a restack was triggered
    out of a clutter event handler, then after Clutter processed the
    events, it would proceed immmediately on to repaint the stage without
    ever returning control to the GLib main loop. So even though we
    had an idle handler installed with a higher priority than the
    Clutter stage repainting the clutter stage repainting would happen
    first and we'd get a wrong frame.
    
    Fix this by introducing the idea of "later functions", which abstract
    the idea of "doing something later" away from g_idle_add() and use
    a combination of GLib idle functions and Clutter "repaint functions"
    to get our callbacks triggered at the right time, even when they
    are installed from a clutter event handler.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=596334
    
    This also resolve a FIXME where MUTTER_PRIORITY_BEFORE_REDRAW
    could starve stage repainting.

 src/core/screen-private.h |    2 +-
 src/core/screen.c         |   28 +++---
 src/core/stack-tracker.c  |   20 +++---
 src/core/util.c           |  199 +++++++++++++++++++++++++++++++++++++++++++++
 src/core/window.c         |   37 +++++----
 src/include/common.h      |    6 +-
 src/include/util.h        |   20 +++++
 7 files changed, 267 insertions(+), 45 deletions(-)
---
diff --git a/src/core/screen-private.h b/src/core/screen-private.h
index f21c8c0..d6f28ce 100644
--- a/src/core/screen-private.h
+++ b/src/core/screen-private.h
@@ -117,7 +117,7 @@ struct _MetaScreen
 
   Window wm_cm_selection_window;
 
-  guint work_area_idle;
+  guint work_area_later;
 
   int rows_of_workspaces;
   int columns_of_workspaces;
diff --git a/src/core/screen.c b/src/core/screen.c
index 5e2197e..1ed502f 100644
--- a/src/core/screen.c
+++ b/src/core/screen.c
@@ -686,7 +686,7 @@ meta_screen_new (MetaDisplay *display,
   screen->wm_cm_selection_window = meta_create_offscreen_window (xdisplay, 
                                                                  xroot, 
                                                                  NoEventMask);
-  screen->work_area_idle = 0;
+  screen->work_area_later = 0;
 
   screen->active_workspace = NULL;
   screen->workspaces = NULL;
@@ -873,8 +873,8 @@ meta_screen_free (MetaScreen *screen,
   XDestroyWindow (screen->display->xdisplay,
                   screen->wm_sn_selection_window);
   
-  if (screen->work_area_idle != 0)
-    g_source_remove (screen->work_area_idle);
+  if (screen->work_area_later != 0)
+    g_source_remove (screen->work_area_later);
 
 
   if (XGetGCValues (screen->display->xdisplay,
@@ -2291,12 +2291,12 @@ set_work_area_hint (MetaScreen *screen)
 }
 
 static gboolean
-set_work_area_idle_func (MetaScreen *screen)
+set_work_area_later_func (MetaScreen *screen)
 {
   meta_topic (META_DEBUG_WORKAREA,
-              "Running work area idle function\n");
+              "Running work area hint computation function\n");
   
-  screen->work_area_idle = 0;
+  screen->work_area_later = 0;
   
   set_work_area_hint (screen);
   
@@ -2306,16 +2306,16 @@ set_work_area_idle_func (MetaScreen *screen)
 void
 meta_screen_queue_workarea_recalc (MetaScreen *screen)
 {
-  /* Recompute work area in an idle */
-  if (screen->work_area_idle == 0)
+  /* Recompute work area later before redrawing */
+  if (screen->work_area_later == 0)
     {
       meta_topic (META_DEBUG_WORKAREA,
-                  "Adding work area hint idle function\n");
-      screen->work_area_idle =
-        g_idle_add_full (META_PRIORITY_BEFORE_REDRAW,
-                         (GSourceFunc) set_work_area_idle_func,
-                         screen,
-                         NULL);
+                  "Adding work area hint computation function\n");
+      screen->work_area_later =
+        meta_later_add (META_LATER_BEFORE_REDRAW,
+                        (GSourceFunc) set_work_area_later_func,
+                        screen,
+                        NULL);
     }
 }
 
diff --git a/src/core/stack-tracker.c b/src/core/stack-tracker.c
index 73f96c2..07517d8 100644
--- a/src/core/stack-tracker.c
+++ b/src/core/stack-tracker.c
@@ -134,7 +134,7 @@ struct _MetaStackTracker
   /* Idle function used to sync the compositor's view of the window
    * stack up with our best guess before a frame is drawn.
    */
-  guint sync_stack_idle;
+  guint sync_stack_later;
 };
 
 static void
@@ -383,8 +383,8 @@ meta_stack_tracker_new (MetaScreen *screen)
 void
 meta_stack_tracker_free (MetaStackTracker *tracker)
 {
-  if (tracker->sync_stack_idle)
-    g_source_remove (tracker->sync_stack_idle);
+  if (tracker->sync_stack_later)
+    meta_later_remove (tracker->sync_stack_later);
 
   g_array_free (tracker->server_stack, TRUE);
   if (tracker->predicted_stack)
@@ -667,10 +667,10 @@ meta_stack_tracker_sync_stack (MetaStackTracker *tracker)
   int n_windows;
   int i;
 
-  if (tracker->sync_stack_idle)
+  if (tracker->sync_stack_later)
     {
-      g_source_remove (tracker->sync_stack_idle);
-      tracker->sync_stack_idle = 0;
+      meta_later_remove (tracker->sync_stack_later);
+      tracker->sync_stack_later = 0;
     }
 
   meta_stack_tracker_get_stack (tracker, &windows, &n_windows);
@@ -696,7 +696,7 @@ meta_stack_tracker_sync_stack (MetaStackTracker *tracker)
 }
 
 static gboolean
-stack_tracker_sync_stack_idle (gpointer data)
+stack_tracker_sync_stack_later (gpointer data)
 {
   meta_stack_tracker_sync_stack (data);
 
@@ -719,10 +719,10 @@ stack_tracker_sync_stack_idle (gpointer data)
 void
 meta_stack_tracker_queue_sync_stack (MetaStackTracker *tracker)
 {
-  if (tracker->sync_stack_idle == 0)
+  if (tracker->sync_stack_later == 0)
     {
-      tracker->sync_stack_idle = g_idle_add_full (META_PRIORITY_BEFORE_REDRAW,
-                                                  stack_tracker_sync_stack_idle,
+      tracker->sync_stack_later = meta_later_add (META_LATER_BEFORE_REDRAW,
+                                                  stack_tracker_sync_stack_later,
                                                   tracker, NULL);
     }
 }
diff --git a/src/core/util.c b/src/core/util.c
index aca85f3..f83d23d 100644
--- a/src/core/util.c
+++ b/src/core/util.c
@@ -26,9 +26,12 @@
 #define _POSIX_C_SOURCE 200112L /* for fdopen() */
 
 #include <config.h>
+#include "common.h"
 #include "util.h"
 #include "main.h"
 
+#include <clutter/clutter.h> /* For clutter_threads_add_repaint_func() */
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -663,5 +666,201 @@ meta_nexus_get_type (void)
   return nexus_type;
 }
 
+/***************************************************************************
+ * Later functions: like idles but integrated with the Clutter repaint loop
+ ***************************************************************************/
+
+static guint last_later_id = 0;
+
+typedef struct
+{
+  guint id;
+  MetaLaterType when;
+  GSourceFunc func;
+  gpointer data;
+  GDestroyNotify notify;
+  int source;
+  gboolean run_once;
+} MetaLater;
+
+static GSList *laters = NULL;
+/* This is a dummy timeline used to get the Clutter master clock running */
+static ClutterTimeline *later_timeline;
+static guint later_repaint_func = 0;
+
+static void ensure_later_repaint_func (void);
+
+static void
+destroy_later (MetaLater *later)
+{
+  if (later->source)
+    g_source_remove (later->source);
+  if (later->notify)
+    later->notify (later->data);
+  g_slice_free (MetaLater, later);
+}
+
+/* Used to sort the list of laters with the highest priority
+ * functions first.
+ */
+static int
+compare_laters (gconstpointer a,
+                gconstpointer b)
+{
+  return ((const MetaLater *)a)->when - ((const MetaLater *)b)->when;
+}
+
+static gboolean
+run_repaint_laters (gpointer data)
+{
+  GSList *old_laters = laters;
+  GSList *l;
+  gboolean keep_timeline_running = FALSE;
+  laters = NULL;
+
+  for (l = old_laters; l; l = l->next)
+    {
+      MetaLater *later = l->data;
+      if (later->source == 0 ||
+          (later->when <= META_LATER_BEFORE_REDRAW && !later->run_once))
+        {
+          if (later->func (later->data))
+            {
+              if (later->source == 0)
+                keep_timeline_running = TRUE;
+              laters = g_slist_insert_sorted (laters, later, compare_laters);
+            }
+          else
+            destroy_later (later);
+        }
+      else
+        laters = g_slist_insert_sorted (laters, later, compare_laters);
+    }
+
+  if (!keep_timeline_running)
+    clutter_timeline_stop (later_timeline);
+
+  g_slist_free (old_laters);
+
+  /* Just keep the repaint func around - it's cheap if the list is empty */
+  return TRUE;
+}
+
+static void
+ensure_later_repaint_func (void)
+{
+  if (!later_timeline)
+    later_timeline = clutter_timeline_new (0);
+
+  if (later_repaint_func == 0)
+    later_repaint_func = clutter_threads_add_repaint_func (run_repaint_laters,
+                                                           NULL, NULL);
+
+  /* Make sure the repaint function gets run */
+  clutter_timeline_start (later_timeline);
+}
+
+static gboolean
+call_idle_later (gpointer data)
+{
+  MetaLater *later = data;
+
+  if (!later->func (later->data))
+    {
+      laters = g_slist_remove (laters, later);
+      later->source = 0;
+      destroy_later (later);
+      return FALSE;
+    }
+  else
+    {
+      later->run_once = TRUE;
+      return TRUE;
+    }
+}
+
+/**
+ * meta_later_add:
+ * @when:     enumeration value determining the phase at which to run the callback
+ * @func:     callback to run later
+ * @data:     data to pass to the callback
+ * @notify:   function to call to destroy @data when it is no longer in use, or %NULL
+ *
+ * Sets up a callback  to be called at some later time. @when determines the
+ * particular later occasion at which it is called. This is much like g_idle_add(),
+ * except that the functions interact properly with clutter event handling.
+ * If a "later" function is added from a clutter event handler, and is supposed
+ * to be run before the stage is redrawn, it will be run before that redraw
+ * of the stage, not the next one.
+ *
+ * Return value: an integer ID (guaranteed to be non-zero) that can be used
+ *  to cancel the callback and prevent it from being run.
+ */
+guint
+meta_later_add (MetaLaterType  when,
+                GSourceFunc    func,
+                gpointer       data,
+                GDestroyNotify notify)
+{
+  MetaLater *later = g_slice_new0 (MetaLater);
+
+  later->id = ++last_later_id;
+  later->when = when;
+  later->func = func;
+  later->data = data;
+  later->notify = notify;
+
+  laters = g_slist_insert_sorted (laters, later, compare_laters);
+
+  switch (when)
+    {
+    case META_LATER_RESIZE:
+      /* We add this one two ways - as a high-priority idle and as a
+       * repaint func. If we are in a clutter event callback, the repaint
+       * handler will get hit first, and we'll take care of this function
+       * there so it gets called before the stage is redrawn, even if
+       * we haven't gotten back to the main loop. Otherwise, the idle
+       * handler will get hit first and we want to call this function
+       * there so it will happen before GTK+ repaints.
+       */
+      later->source = g_idle_add_full (META_PRIORITY_RESIZE, call_idle_later, later, NULL);
+      ensure_later_repaint_func ();
+      break;
+    case META_LATER_BEFORE_REDRAW:
+      ensure_later_repaint_func ();
+      break;
+    case META_LATER_IDLE:
+      later->source = g_idle_add_full (G_PRIORITY_DEFAULT_IDLE, call_idle_later, later, NULL);
+      break;
+    }
+
+  return later->id;
+}
+
+/**
+ * meta_later_remove:
+ * @later_id: the integer ID returned from meta_later_add()
+ *
+ * Removes a callback added with meta_later_add()
+ */
+void
+meta_later_remove (guint later_id)
+{
+  GSList *l;
+
+  for (l = laters; l; l = l->next)
+    {
+      MetaLater *later = l->data;
+      if (later->id == later_id)
+        {
+          laters = g_slist_remove_link (laters, l);
+          /* If this was a "repaint func" later, we just let the
+           * repaint func run and get removed
+           */
+          destroy_later (later);
+        }
+    }
+}
+
 /* eof util.c */
 
diff --git a/src/core/window.c b/src/core/window.c
index 5d184c0..2e4c109 100644
--- a/src/core/window.c
+++ b/src/core/window.c
@@ -116,8 +116,9 @@ static void meta_window_apply_session_info (MetaWindow                  *window,
 static void unmaximize_window_before_freeing (MetaWindow        *window);
 static void unminimize_window_and_all_transient_parents (MetaWindow *window);
 
-/* Idle handlers for the three queues. The "data" parameter in each case
- * will be a GINT_TO_POINTER of the index into the queue arrays to use.
+/* Idle handlers for the three queues (run with meta_later_add()). The
+ * "data" parameter in each case will be a GINT_TO_POINTER of the
+ * index into the queue arrays to use.
  *
  * TODO: Possibly there is still some code duplication among these, which we
  * need to sort out at some point.
@@ -1705,7 +1706,7 @@ meta_window_calc_showing (MetaWindow  *window)
   implement_showing (window, meta_window_should_be_showing (window));
 }
 
-static guint queue_idle[NUMBER_OF_QUEUES] = {0, 0, 0};
+static guint queue_later[NUMBER_OF_QUEUES] = {0, 0, 0};
 static GSList *queue_pending[NUMBER_OF_QUEUES] = {NULL, NULL, NULL};
 
 static int
@@ -1743,7 +1744,7 @@ idle_calc_showing (gpointer data)
   copy = g_slist_copy (queue_pending[queue_index]);
   g_slist_free (queue_pending[queue_index]);
   queue_pending[queue_index] = NULL;
-  queue_idle[queue_index] = 0;
+  queue_later[queue_index] = 0;
 
   destroying_windows_disallowed += 1;
 
@@ -1901,10 +1902,10 @@ meta_window_unqueue (MetaWindow *window, guint queuebits)
            * In that case, we should kill the function that deals with
            * the queue, because there's nothing left for it to do.
            */
-          if (queue_pending[queuenum] == NULL && queue_idle[queuenum] != 0)
+          if (queue_pending[queuenum] == NULL && queue_later[queuenum] != 0)
             {
-              g_source_remove (queue_idle[queuenum]);
-              queue_idle[queuenum] = 0;
+              meta_later_remove (queue_later[queuenum]);
+              queue_later[queuenum] = 0;
             }
         }
     }
@@ -1937,14 +1938,14 @@ meta_window_queue (MetaWindow *window, guint queuebits)
            * I seem to be turning into a Perl programmer.
            */
 
-          const gint window_queue_idle_priority[NUMBER_OF_QUEUES] =
+          const MetaLaterType window_queue_later_when[NUMBER_OF_QUEUES] =
             {
-              META_PRIORITY_BEFORE_REDRAW, /* CALC_SHOWING */
-              META_PRIORITY_RESIZE,        /* MOVE_RESIZE */
-              META_PRIORITY_BEFORE_REDRAW  /* UPDATE_ICON */
+              META_LATER_BEFORE_REDRAW, /* CALC_SHOWING */
+              META_LATER_RESIZE,        /* MOVE_RESIZE */
+              META_LATER_BEFORE_REDRAW  /* UPDATE_ICON */
             };
 
-          const GSourceFunc window_queue_idle_handler[NUMBER_OF_QUEUES] =
+          const GSourceFunc window_queue_later_handler[NUMBER_OF_QUEUES] =
             {
               idle_calc_showing,
               idle_move_resize,
@@ -1977,11 +1978,11 @@ meta_window_queue (MetaWindow *window, guint queuebits)
            * that. If not, we'll create one.
            */
 
-          if (queue_idle[queuenum] == 0)
-            queue_idle[queuenum] = g_idle_add_full
+          if (queue_later[queuenum] == 0)
+            queue_later[queuenum] = meta_later_add
               (
-                window_queue_idle_priority[queuenum],
-                window_queue_idle_handler[queuenum],
+                window_queue_later_when[queuenum],
+                window_queue_later_handler[queuenum],
                 GUINT_TO_POINTER(queuenum),
                 NULL
               );
@@ -4200,7 +4201,7 @@ idle_move_resize (gpointer data)
   copy = g_slist_copy (queue_pending[queue_index]);
   g_slist_free (queue_pending[queue_index]);
   queue_pending[queue_index] = NULL;
-  queue_idle[queue_index] = 0;
+  queue_later[queue_index] = 0;
 
   destroying_windows_disallowed += 1;
 
@@ -6262,7 +6263,7 @@ idle_update_icon (gpointer data)
   copy = g_slist_copy (queue_pending[queue_index]);
   g_slist_free (queue_pending[queue_index]);
   queue_pending[queue_index] = NULL;
-  queue_idle[queue_index] = 0;
+  queue_later[queue_index] = 0;
 
   destroying_windows_disallowed += 1;
 
diff --git a/src/include/common.h b/src/include/common.h
index b4afc83..d614542 100644
--- a/src/include/common.h
+++ b/src/include/common.h
@@ -317,8 +317,10 @@ struct _MetaButtonLayout
  * coelesce multiple things together, the appropriate place to
  * do it is usually META_PRIORITY_BEFORE_REDRAW.
  *
- * (FIXME: Use a Clutter paint() function instead, to prevent
- * starving the repaints)
+ * Note that its usually better to use meta_later_add() rather
+ * than calling g_idle_add() directly; this will make sure things
+ * get run when added from a clutter event handler without
+ * waiting for another repaint cycle.
  *
  * If something has a priority lower than the redraw priority
  * (such as a default priority idle), then it may be arbitrarily
diff --git a/src/include/util.h b/src/include/util.h
index a103104..9d0c5d1 100644
--- a/src/include/util.h
+++ b/src/include/util.h
@@ -161,6 +161,26 @@ MetaNexus *meta_nexus_new ();
  */
 extern MetaNexus *sigchld_nexus;
 
+/**
+ * MetaLaterType:
+ * @META_LATER_RESIZE: call in a resize processing phase that is done
+ *   before GTK+ repainting (including window borders) is done.
+ * @META_LATER_BEFORE_REDRAW: call before the stage is redrawn
+ * @META_LATER_IDLE: call at a very low priority (can be blocked
+ *    by running animations or redrawing applications)
+ **/
+typedef enum {
+  META_LATER_RESIZE,
+  META_LATER_BEFORE_REDRAW,
+  META_LATER_IDLE
+} MetaLaterType;
+
+guint meta_later_add    (MetaLaterType  when,
+                         GSourceFunc    func,
+                         gpointer       data,
+                         GDestroyNotify notify);
+void  meta_later_remove (guint          later_id);
+
 #endif /* META_UTIL_H */
 
 



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]