[clutter/wip/master-next: 2/31] Deprecate clutter_threads_enter()/leave()

From: Emmanuele Bassi <ebassi src gnome org>
To: commits-list gnome org
Cc:
Subject: [clutter/wip/master-next: 2/31] Deprecate clutter_threads_enter()/leave()
Date: Wed, 11 Jul 2012 14:39:40 +0000 (UTC)
commit 0e4c6d0a87c827ff69853ec24d99ef9b838a6f47
Author: Emmanuele Bassi <ebassi gnome org>
Date:   Thu Jul 5 14:30:26 2012 +0100

    Deprecate clutter_threads_enter()/leave()
    
    Acquiring the Clutter lock to mark critical sections is not portable,
    and not recommended to implement threaded applications with Clutter.
    
    The recommended pattern is to use worker threads, and schedule UI
    updates inside idle or timeout handlers within the main loop. We should
    enforce this pattern by deprecating the threads_enter()/leave()
    functions. For compatibility concerns, we need internal API to acquire
    the main lock during frame processing dispatch.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=679450

 clutter/clutter-main.c                       |   40 ++++++++++++++++++++-----
 clutter/clutter-main.h                       |    2 -
 clutter/clutter-master-clock.c               |   13 ++++----
 clutter/clutter-private.h                    |    3 ++
 clutter/deprecated/clutter-main.h            |    6 ++++
 clutter/deprecated/clutter-timeout-pool.c    |    1 +
 clutter/evdev/clutter-device-manager-evdev.c |   12 ++++----
 clutter/gdk/clutter-event-gdk.c              |    4 +-
 clutter/osx/clutter-event-loop-osx.c         |   33 ++++++++++++---------
 clutter/tslib/clutter-event-tslib.c          |   13 ++++----
 clutter/wayland/clutter-event-wayland.c      |   18 ++++++-----
 clutter/win32/clutter-event-win32.c          |   12 ++++----
 clutter/x11/clutter-event-x11.c              |   17 ++++++-----
 13 files changed, 107 insertions(+), 67 deletions(-)
---
diff --git a/clutter/clutter-main.c b/clutter/clutter-main.c
index 1e9e8a0..4504499 100644
--- a/clutter/clutter-main.c
+++ b/clutter/clutter-main.c
@@ -940,9 +940,9 @@ clutter_main (void)
 
   if (g_main_loop_is_running (main_loops->data))
     {
-      clutter_threads_leave ();
+      _clutter_threads_release_lock ();
       g_main_loop_run (loop);
-      clutter_threads_enter ();
+      _clutter_threads_acquire_lock ();
     }
 
   main_loops = g_slist_remove (main_loops, loop);
@@ -1024,12 +1024,12 @@ _clutter_threads_dispatch (gpointer data)
   ClutterThreadsDispatch *dispatch = data;
   gboolean ret = FALSE;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   if (!g_source_is_destroyed (g_main_current_source ()))
     ret = dispatch->func (dispatch->data);
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return ret;
 }
@@ -1265,18 +1265,37 @@ clutter_threads_add_timeout (guint       interval,
                                            NULL);
 }
 
+void
+_clutter_threads_acquire_lock (void)
+{
+  if (clutter_threads_lock != NULL)
+    (* clutter_threads_lock) ();
+}
+
+void
+_clutter_threads_release_lock (void)
+{
+  if (clutter_threads_unlock != NULL)
+    (* clutter_threads_unlock) ();
+}
+
 /**
  * clutter_threads_enter:
  *
  * Locks the Clutter thread lock.
  *
  * Since: 0.4
+ *
+ * Deprecated: 1.12: This function should not be used by application
+ *   code; marking critical sections is not portable on various
+ *   platforms. Instead of acquiring the Clutter lock, schedule UI
+ *   updates from the main loop using clutter_threads_add_idle() or
+ *   clutter_threads_add_timeout().
  */
 void
 clutter_threads_enter (void)
 {
-  if (clutter_threads_lock != NULL)
-    (* clutter_threads_lock) ();
+  _clutter_threads_acquire_lock ();
 }
 
 /**
@@ -1285,12 +1304,17 @@ clutter_threads_enter (void)
  * Unlocks the Clutter thread lock.
  *
  * Since: 0.4
+ *
+ * Deprecated: 1.12: This function should not be used by application
+ *   code; marking critical sections is not portable on various
+ *   platforms. Instead of acquiring the Clutter lock, schedule UI
+ *   updates from the main loop using clutter_threads_add_idle() or
+ *   clutter_threads_add_timeout().
  */
 void
 clutter_threads_leave (void)
 {
-  if (clutter_threads_unlock != NULL)
-    (* clutter_threads_unlock) ();
+  _clutter_threads_release_lock ();
 }
 
 
diff --git a/clutter/clutter-main.h b/clutter/clutter-main.h
index 51ce9e1..134ac3c 100644
--- a/clutter/clutter-main.h
+++ b/clutter/clutter-main.h
@@ -101,8 +101,6 @@ void                    clutter_do_event                        (ClutterEvent *e
 gboolean                clutter_get_accessibility_enabled       (void);
 
 /* Threading functions */
-void                    clutter_threads_enter                   (void);
-void                    clutter_threads_leave                   (void);
 void                    clutter_threads_set_lock_functions      (GCallback enter_fn,
                                                                  GCallback leave_fn);
 guint                   clutter_threads_add_idle                (GSourceFunc    func,
diff --git a/clutter/clutter-master-clock.c b/clutter/clutter-master-clock.c
index 9496548..32655ee 100644
--- a/clutter/clutter-master-clock.c
+++ b/clutter/clutter-master-clock.c
@@ -429,7 +429,7 @@ clutter_clock_prepare (GSource *source,
   ClutterMasterClock *master_clock = clock_source->master_clock;
   int delay;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   if (G_UNLIKELY (clutter_paint_debug_flags &
                   CLUTTER_DEBUG_CONTINUOUS_REDRAW))
@@ -445,7 +445,8 @@ clutter_clock_prepare (GSource *source,
     }
 
   delay = master_clock_next_frame_delay (master_clock);
-  clutter_threads_leave ();
+
+  _clutter_threads_release_lock ();
 
   *timeout = delay;
 
@@ -459,9 +460,9 @@ clutter_clock_check (GSource *source)
   ClutterMasterClock *master_clock = clock_source->master_clock;
   int delay;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
   delay = master_clock_next_frame_delay (master_clock);
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return delay == 0;
 }
@@ -487,7 +488,7 @@ clutter_clock_dispatch (GSource     *source,
 
   CLUTTER_NOTE (SCHEDULER, "Master clock [tick]");
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   /* Get the time to use for this frame */
   master_clock->cur_tick = g_source_get_time (source);
@@ -528,7 +529,7 @@ clutter_clock_dispatch (GSource     *source,
 
   master_clock->prev_tick = master_clock->cur_tick;
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   CLUTTER_TIMER_STOP (_clutter_uprof_context, master_dispatch_timer);
 
diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h
index 25090c6..8fd30b0 100644
--- a/clutter/clutter-private.h
+++ b/clutter/clutter-private.h
@@ -193,6 +193,9 @@ typedef struct
 gboolean _clutter_threads_dispatch      (gpointer data);
 void     _clutter_threads_dispatch_free (gpointer data);
 
+void                    _clutter_threads_acquire_lock                   (void);
+void                    _clutter_threads_release_lock                   (void);
+
 ClutterMainContext *    _clutter_context_get_default                    (void);
 void                    _clutter_context_lock                           (void);
 void                    _clutter_context_unlock                         (void);
diff --git a/clutter/deprecated/clutter-main.h b/clutter/deprecated/clutter-main.h
index d05d94b..257ac5a 100644
--- a/clutter/deprecated/clutter-main.h
+++ b/clutter/deprecated/clutter-main.h
@@ -34,6 +34,12 @@ G_BEGIN_DECLS
 CLUTTER_DEPRECATED_IN_1_10
 void                    clutter_threads_init                    (void);
 
+CLUTTER_DEPRECATED_IN_1_12
+void                    clutter_threads_enter                   (void);
+
+CLUTTER_DEPRECATED_IN_1_12
+void                    clutter_threads_leave                   (void);
+
 CLUTTER_DEPRECATED_IN_1_6
 guint                   clutter_threads_add_frame_source        (guint             fps,
                                                                  GSourceFunc       func,
diff --git a/clutter/deprecated/clutter-timeout-pool.c b/clutter/deprecated/clutter-timeout-pool.c
index 9071697..ad2b313 100644
--- a/clutter/deprecated/clutter-timeout-pool.c
+++ b/clutter/deprecated/clutter-timeout-pool.c
@@ -35,6 +35,7 @@
 #endif
 
 #define CLUTTER_DISABLE_DEPRECATION_WARNINGS
+#include "deprecated/clutter-main.h"
 
 #include "clutter-timeout-pool.h"
 
diff --git a/clutter/evdev/clutter-device-manager-evdev.c b/clutter/evdev/clutter-device-manager-evdev.c
index d3eb8e2..7b090ac 100644
--- a/clutter/evdev/clutter-device-manager-evdev.c
+++ b/clutter/evdev/clutter-device-manager-evdev.c
@@ -120,12 +120,12 @@ clutter_event_prepare (GSource *source,
 {
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   *timeout = -1;
   retval = clutter_events_pending ();
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -136,12 +136,12 @@ clutter_event_check (GSource *source)
   ClutterEventSource *event_source = (ClutterEventSource *) source;
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   retval = ((event_source->event_poll_fd.revents & G_IO_IN) ||
             clutter_events_pending ());
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -297,7 +297,7 @@ clutter_event_dispatch (GSource     *g_source,
   uint32_t _time;
   ClutterStage *stage;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   stage = _clutter_input_device_get_stage (input_device);
 
@@ -428,7 +428,7 @@ clutter_event_dispatch (GSource     *g_source,
     }
 
 out:
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return TRUE;
 }
diff --git a/clutter/gdk/clutter-event-gdk.c b/clutter/gdk/clutter-event-gdk.c
index c01461b..1663138 100644
--- a/clutter/gdk/clutter-event-gdk.c
+++ b/clutter/gdk/clutter-event-gdk.c
@@ -102,7 +102,7 @@ clutter_gdk_handle_event (GdkEvent *gdk_event)
   if (stage == NULL)
     return GDK_FILTER_CONTINUE;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   switch (gdk_event->type)
     {
@@ -315,7 +315,7 @@ clutter_gdk_handle_event (GdkEvent *gdk_event)
       result = GDK_FILTER_REMOVE;
     }
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return result;
 }
diff --git a/clutter/osx/clutter-event-loop-osx.c b/clutter/osx/clutter-event-loop-osx.c
index d8e6dd6..6b3a561 100644
--- a/clutter/osx/clutter-event-loop-osx.c
+++ b/clutter/osx/clutter-event-loop-osx.c
@@ -32,7 +32,9 @@
 #include <sys/types.h>
 #include <sys/uio.h>
 #include <unistd.h>
-#include <clutter/clutter-debug.h>
+
+#include "clutter-debug.h"
+#include "clutter-private.h"
 
 /* 
  * This file implementations integration between the GLib main loop and
@@ -643,13 +645,13 @@ clutter_event_prepare (GSource *source,
 {
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
   
   *timeout = -1;
 
   retval = (clutter_events_pending () || _clutter_osx_event_loop_check_pending ());
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -659,7 +661,7 @@ clutter_event_check (GSource *source)
 {
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   /* XXX: This check isn't right it won't handle a recursive GLib main
    * loop run within an outer CFRunLoop run. Such loops will pile up
@@ -678,7 +680,7 @@ clutter_event_check (GSource *source)
   
   retval = (clutter_events_pending () || _clutter_osx_event_loop_check_pending ());
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -691,16 +693,19 @@ clutter_event_dispatch (GSource     *source,
   NSEvent *nsevent;
   ClutterEvent *event;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   nsevent = _clutter_osx_event_loop_get_pending ();
-  if (nsevent) {
-    clutter_threads_leave ();
-    [NSApp sendEvent:nsevent];
-    clutter_threads_enter ();
-    
-    _clutter_osx_event_loop_release_event (nsevent);
-  }
+  if (nsevent)
+    {
+      _clutter_threads_release_lock ();
+
+      [NSApp sendEvent:nsevent];
+
+      _clutter_threads_acquire_lock ();
+
+      _clutter_osx_event_loop_release_event (nsevent);
+    }
   
   event = clutter_event_get ();
 
@@ -711,7 +716,7 @@ clutter_event_dispatch (GSource     *source,
       clutter_event_free (event);
     }
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return TRUE;
 }
diff --git a/clutter/tslib/clutter-event-tslib.c b/clutter/tslib/clutter-event-tslib.c
index 045412f..2605625 100644
--- a/clutter/tslib/clutter-event-tslib.c
+++ b/clutter/tslib/clutter-event-tslib.c
@@ -173,12 +173,12 @@ clutter_event_prepare (GSource *source,
 {
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   *timeout = -1;
   retval = clutter_events_pending ();
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -189,12 +189,12 @@ clutter_event_check (GSource *source)
   ClutterEventSource *event_source = (ClutterEventSource *) source;
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   retval = ((event_source->event_poll_fd.revents & G_IO_IN) ||
             clutter_events_pending ());
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -208,7 +208,7 @@ clutter_event_dispatch (GSource     *source,
   struct ts_sample tsevent;
   ClutterEvent *event;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   /* FIXME while would be better here but need to deal with lockups */
   if ((!clutter_events_pending()) &&
@@ -272,8 +272,7 @@ clutter_event_dispatch (GSource     *source,
     }
 
 out:
-
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return TRUE;
 }
diff --git a/clutter/wayland/clutter-event-wayland.c b/clutter/wayland/clutter-event-wayland.c
index 7e5e8f9..08adcca 100644
--- a/clutter/wayland/clutter-event-wayland.c
+++ b/clutter/wayland/clutter-event-wayland.c
@@ -32,8 +32,10 @@
 #include <stdlib.h>
 #include <wayland-client.h>
 
-#include "../clutter-event.h"
-#include "../clutter-main.h"
+#include "clutter-event.h"
+#include "clutter-main.h"
+#include "clutter-private.h"
+
 #include "clutter-event-wayland.h"
 
 typedef struct _ClutterEventSourceWayland
@@ -50,7 +52,7 @@ clutter_event_source_wayland_prepare (GSource *base, gint *timeout)
   ClutterEventSourceWayland *source = (ClutterEventSourceWayland *) base;
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   *timeout = -1;
 
@@ -61,7 +63,7 @@ clutter_event_source_wayland_prepare (GSource *base, gint *timeout)
 
   retval = clutter_events_pending ();
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -72,11 +74,11 @@ clutter_event_source_wayland_check (GSource *base)
   ClutterEventSourceWayland *source = (ClutterEventSourceWayland *) base;
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   retval = clutter_events_pending () || source->pfd.revents;
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -89,7 +91,7 @@ clutter_event_source_wayland_dispatch (GSource *base,
   ClutterEventSourceWayland *source = (ClutterEventSourceWayland *) base;
   ClutterEvent *event;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   if (source->pfd.revents)
     {
@@ -106,7 +108,7 @@ clutter_event_source_wayland_dispatch (GSource *base,
       clutter_event_free (event);
     }
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return TRUE;
 }
diff --git a/clutter/win32/clutter-event-win32.c b/clutter/win32/clutter-event-win32.c
index eab1991..c31697b 100644
--- a/clutter/win32/clutter-event-win32.c
+++ b/clutter/win32/clutter-event-win32.c
@@ -246,12 +246,12 @@ clutter_event_prepare (GSource *source,
 {
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   *timeout = -1;
   retval = (clutter_events_pending () || check_msg_pending ());
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -262,14 +262,14 @@ clutter_event_check (GSource *source)
   ClutterEventSource *event_source = (ClutterEventSource *) source;
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   if ((event_source->event_poll_fd.revents & G_IO_IN))
     retval = (clutter_events_pending () || check_msg_pending ());
   else
     retval = FALSE;
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -282,7 +282,7 @@ clutter_event_dispatch (GSource     *source,
   ClutterEvent *event;
   MSG msg;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   /* Process Windows messages until we've got one that translates into
      the clutter event queue */
@@ -298,7 +298,7 @@ clutter_event_dispatch (GSource     *source,
       clutter_event_free (event);
     }
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return TRUE;
 }
diff --git a/clutter/x11/clutter-event-x11.c b/clutter/x11/clutter-event-x11.c
index 3af161c..16a34bc 100644
--- a/clutter/x11/clutter-event-x11.c
+++ b/clutter/x11/clutter-event-x11.c
@@ -33,6 +33,7 @@
 #include "clutter-debug.h"
 #include "clutter-event-private.h"
 #include "clutter-main.h"
+#include "clutter-private.h"
 
 #include <string.h>
 
@@ -182,7 +183,7 @@ clutter_x11_handle_event (XEvent *xevent)
 
   result = CLUTTER_X11_FILTER_CONTINUE;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   backend = clutter_get_default_backend ();
 
@@ -230,7 +231,7 @@ out:
     XFreeEventData (xdisplay, &xevent->xcookie);
 #endif
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return result;
 }
@@ -242,12 +243,12 @@ clutter_event_prepare (GSource *source,
   ClutterBackendX11 *backend = ((ClutterEventSource *) source)->backend;
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   *timeout = -1;
   retval = (clutter_events_pending () || XPending (backend->xdpy));
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -259,14 +260,14 @@ clutter_event_check (GSource *source)
   ClutterBackendX11 *backend = event_source->backend;
   gboolean retval;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   if (event_source->event_poll_fd.revents & G_IO_IN)
     retval = (clutter_events_pending () || XPending (backend->xdpy));
   else
     retval = FALSE;
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return retval;
 }
@@ -308,7 +309,7 @@ clutter_event_dispatch (GSource     *source,
   ClutterBackendX11 *backend = ((ClutterEventSource *) source)->backend;
   ClutterEvent *event;
 
-  clutter_threads_enter ();
+  _clutter_threads_acquire_lock ();
 
   /*  Grab the event(s), translate and figure out double click.
    *  The push onto queue (stack) if valid.
@@ -324,7 +325,7 @@ clutter_event_dispatch (GSource     *source,
       clutter_event_free (event);
     }
 
-  clutter_threads_leave ();
+  _clutter_threads_release_lock ();
 
   return TRUE;
 }
[Date Prev][Date Next] [Thread Prev][Thread Next] [Thread Index] [Date Index] [Author Index]