Preserve ordering when placing synthetic events in the queue

When we are in the the event translation function sometimes we need to
synthesise events: the double and triple click events are synthetic events
placed on the queue after a sequence of events has been received, for
instance.

Until now, the events were placed on the queue after the translation from
the native events was successful. This led to a loss of ordering because
we put the synthesised event on the queue before the last event that
triggered it.

This patch puts the events on the queue before translating them, with a
"pending" flag set; if the translation sequence is completed then the flag
is removed - otherwise the event is removed from the queue altogether. The
queue manipulation functions have been modified to ignore the "pending"
flag when looking for events.

This patch also adds a private structure overlayed on the ClutterEvent
struct so that we can extend the events with private data without exposing
it in the public API.

diff --git a/clutter/clutter-event.c b/clutter/clutter-event.c
index 709710e..c532351 100644 (file)
--- a/clutter/clutter-event.c
+++ b/clutter/clutter-event.c
@@ -350,8 +350,6 @@ clutter_event_get_type (void)
   return our_type;
 }
 
-static GHashTable *event_hash = NULL;
-
 /**
  * clutter_event_new:
  * @type: The type of event.
@@ -363,17 +361,15 @@ static GHashTable *event_hash = NULL;
 ClutterEvent *
 clutter_event_new (ClutterEventType type)
 {
+  ClutterEventPrivate *real_event;
   ClutterEvent *new_event;
 
-  if (!event_hash)
-    event_hash = g_hash_table_new (g_direct_hash, NULL);
+  real_event = g_slice_new0 (ClutterEventPrivate);
+  real_event->flags = 0;
 
-  new_event = g_slice_new0 (ClutterEvent);
+  new_event = (ClutterEvent *) real_event;
   new_event->type = new_event->any.type = type;
 
-  /* FIXME: why do we put in a hash ? */
-  g_hash_table_insert (event_hash, new_event, GUINT_TO_POINTER (1));
-
   return new_event;
 }
 
@@ -409,8 +405,7 @@ clutter_event_free (ClutterEvent *event)
 {
   if (G_LIKELY (event))
     {
-      g_hash_table_remove (event_hash, event);
-      g_slice_free (ClutterEvent, event);
+      g_slice_free (ClutterEventPrivate, (ClutterEventPrivate *) event);
     }
 }
 
@@ -424,12 +419,34 @@ clutter_event_free (ClutterEvent *event)
  *
  * Since: 0.4
  */
-ClutterEvent*
+ClutterEvent *
 clutter_event_get (void)
 {
   ClutterMainContext *context = clutter_context_get_default ();
+  GList *item;
+
+  if (!context->events_queue)
+    return NULL;
+
+  if (g_queue_is_empty (context->events_queue))
+    return NULL;
+
+  /* find the first non pending item */
+  item = context->events_queue->tail;
+  while (item)
+    {
+      ClutterEventPrivate *event = item->data;
+
+      if (!(event->flags & CLUTTER_EVENT_PENDING))
+        {
+          g_queue_remove (context->events_queue, event);
+          return (ClutterEvent *) event;
+        }
+
+      item = item->prev;
+    }
 
-  return g_queue_pop_tail (context->events_queue);
+  return NULL;
 }
 
 /**
@@ -446,13 +463,26 @@ ClutterEvent *
 clutter_event_peek (void)
 {
   ClutterMainContext *context = clutter_context_get_default ();
+  GList *item;
 
-  g_return_val_if_fail (context != NULL, NULL);
-  
-  if (context->events_queue == NULL)
+  if (!context->events_queue)
+    return NULL;
+
+  if (g_queue_is_empty (context->events_queue))
     return NULL;
 
-  return g_queue_peek_tail (context->events_queue);
+  /* find the first non pending item */
+  item = context->events_queue->tail;
+  while (item)
+    {
+      ClutterEventPrivate *event = item->data;
+      if (!(event->flags & CLUTTER_EVENT_PENDING))
+        return (ClutterEvent *) event;
+
+      item = item->prev;
+    }
+
+  return NULL;
 }
 
 /**
@@ -487,13 +517,28 @@ gboolean
 clutter_events_pending (void)
 {
   ClutterMainContext *context = clutter_context_get_default ();
+  GList *item;
 
   g_return_val_if_fail (context != NULL, FALSE);
 
   if (!context->events_queue)
     return FALSE;
 
-  return g_queue_is_empty (context->events_queue) == FALSE;
+  if (g_queue_is_empty (context->events_queue))
+    return FALSE;
+
+  /* find the first non pending item */
+  item = context->events_queue->head;
+  while (item)
+    {
+      ClutterEventPrivate *event = item->data;
+      if (!(event->flags & CLUTTER_EVENT_PENDING))
+        return TRUE;
+
+      item = item->next;
+    }
+
+  return FALSE;
 }
 
 /* Backend helpers (private) */
@@ -536,10 +581,10 @@ _clutter_event_button_generate (ClutterBackend *backend,
       button_x[0] = button_x[1] = 0;
       button_y[0] = button_y[1] = 0;
     }
-  else if ((event->button.time < (button_click_time[0] + double_click_time)) &&
-      (event->button.button == button_number[0]) &&
-      (ABS (event->button.x - button_x[0]) <= double_click_distance) &&
-      (ABS (event->button.y - button_y[0]) <= double_click_distance))
+  else if ((event->button.time < (button_click_time[0] + double_click_time))
+           && (event->button.button == button_number[0])
+           && (ABS (event->button.x - button_x[0]) <= double_click_distance)
+           && (ABS (event->button.y - button_y[0]) <= double_click_distance))
     {
       synthesize_click (backend, event, 2);
       

diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h
index ac42da4..b97023b 100644 (file)
--- a/clutter/clutter-private.h
+++ b/clutter/clutter-private.h
@@ -61,7 +61,25 @@ typedef enum {
   CLUTTER_PICK_ALL
 } ClutterPickMode;
 
-typedef struct _ClutterMainContext ClutterMainContext;
+typedef enum {
+  /* this flag is set when an event has been put on the queue but still
+   * needs processing; the event queue must ignore every event with this
+   * flag set
+   */
+  CLUTTER_EVENT_PENDING = 1 << 0
+} ClutterEventFlags;
+
+typedef struct _ClutterEventPrivate     ClutterEventPrivate;
+typedef struct _ClutterMainContext      ClutterMainContext;
+
+/* Private structure, to be used for extending ClutterEvent without
+ * exposing new members and breaking compatibility.
+ */
+struct _ClutterEventPrivate
+{
+  ClutterEvent event;
+  guint flags;
+};
 
 struct _ClutterMainContext
 {

diff --git a/clutter/eglx/clutter-event-egl.c b/clutter/eglx/clutter-event-egl.c
index 59843cf..afc97dd 100644 (file)
--- a/clutter/eglx/clutter-event-egl.c
+++ b/clutter/eglx/clutter-event-egl.c
@@ -295,25 +295,30 @@ static void
 events_queue (ClutterBackend *backend)
 {
   ClutterBackendEGL   *backend_egl = CLUTTER_BACKEND_EGL (backend);
-  ClutterEvent        *event;
+  Display             *xdisplay = backend_egl->xdpy;
   XEvent               xevent;
   ClutterMainContext  *clutter_context;
 
   clutter_context = clutter_context_get_default ();
 
-  Display *xdisplay = backend_egl->xdpy;
-
   while (!clutter_events_pending () && XPending (xdisplay))
     {
+      ClutterEvent *event;
+
       XNextEvent (xdisplay, &xevent);
 
       event = clutter_event_new (CLUTTER_NOTHING);
+      ((ClutterEventPrivate *) event)->flags |= CLUTTER_EVENT_PENDING;
+
+      g_queue_push_head (clutter_context->events_queue, event);
+
       if (clutter_event_translate (backend, event, &xevent))
         {
-         g_queue_push_head (clutter_context->events_queue, event);
+          ((ClutterEventPrivate *) event)->flags &= ~CLUTTER_EVENT_PENDING;
         }
       else
         {
+          g_queue_remove (clutter_context->events_queue, event);
           clutter_event_free (event);
         }
     }

diff --git a/clutter/glx/clutter-event-glx.c b/clutter/glx/clutter-event-glx.c
index 8636601..dc09342 100644 (file)
--- a/clutter/glx/clutter-event-glx.c
+++ b/clutter/glx/clutter-event-glx.c
@@ -517,7 +517,6 @@ static void
 events_queue (ClutterBackend *backend)
 {
   ClutterBackendGLX *backend_glx = CLUTTER_BACKEND_GLX (backend);
-  ClutterEvent      *event;
   Display           *xdisplay = backend_glx->xdpy;
   XEvent             xevent;
   ClutterMainContext  *clutter_context;
@@ -526,17 +525,27 @@ events_queue (ClutterBackend *backend)
 
   while (!clutter_events_pending () && XPending (xdisplay))
     {
+      ClutterEvent *event;
+
       XNextEvent (xdisplay, &xevent);
 
       event = clutter_event_new (CLUTTER_NOTHING);
 
+      /* mark the event as pending and push it so that event_translate()
+       * can put events on the queue without tampering with the ordering
+       */
+      ((ClutterEventPrivate *) event)->flags |= CLUTTER_EVENT_PENDING;
+      g_queue_push_head (clutter_context->events_queue, event);
+      
       if (event_translate (backend, event, &xevent))
         {
-         /* push directly here to avoid copy of queue_put */
-         g_queue_push_head (clutter_context->events_queue, event);
+          /* translation successful, the event is done */
+         ((ClutterEventPrivate *) event)->flags &= ~CLUTTER_EVENT_PENDING;
         }
       else
         {
+          /* remove the event from the queue */
+          g_queue_remove (clutter_context->events_queue, event);
           clutter_event_free (event);
         }
     }

diff --git a/clutter/sdl/clutter-event-sdl.c b/clutter/sdl/clutter-event-sdl.c
index f9c05e6..388fa38 100644 (file)
--- a/clutter/sdl/clutter-event-sdl.c
+++ b/clutter/sdl/clutter-event-sdl.c
@@ -326,13 +326,17 @@ clutter_event_dispatch (GSource     *source,
        {
          event = clutter_event_new (CLUTTER_NOTHING);
 
+          ((ClutterEventPrivate *) event)->flags |= CLUTTER_EVENT_PENDING;
+
+         g_queue_push_head (clutter_context->events_queue, event);
+
          if (event_translate (backend, event, &sdl_event))
            {
-             /* push directly here to avoid copy of queue_put */
-             g_queue_push_head (clutter_context->events_queue, event);
-           }
+             ((ClutterEventPrivate *) event)->flags &= ~CLUTTER_EVENT_PENDING;
+            }
          else
            {
+              g_queue_remove (clutter_context->events_queue, event);
              clutter_event_free (event);
            }
        }
@@ -342,7 +346,7 @@ clutter_event_dispatch (GSource     *source,
 
   if (event)
     {
-      clutter_do_event(event);
+      clutter_do_event (event);
       clutter_event_free (event);
     }