[Cogl] [PATCH] onscreen: pointer not id for frame_callback closures

Robert Bragg robert at sixbynine.org
Tue Jan 15 08:54:15 PST 2013


From: Robert Bragg <robert at linux.intel.com>

To be consistent with a new fencing api being discussed on bugzilla[1]
a.t.m this changes the cogl_onscreen_add_frame_callback() api to return
a CoglFrameClosure instead of an unsigned int. In addition to being
consistent this lets us directly unlink the closure without walking the
list of closures.

kind regards,
- Robert

[1] https://bugzilla.gnome.org/show_bug.cgi?id=691752

-- >8 --
This changes the cogl_onscreen_add_frame_callback() api to return a
CoglFrameClosure pointer instead of an unsigned int. A CoglFrameClosure
pointer can be passed to cogl_onscreen_remove_frame_callback(). One
advantage compared to using an integer is that the closure can be
unlinked from the internal list without scanning the list. This change
is also intended to be consistent with a new fences api that will
need to track callbacks for notifying when a fence has completed.
---
 cogl/cogl-context-private.h  |    1 +
 cogl/cogl-context.c          |    6 +++
 cogl/cogl-onscreen-private.h |   11 ++----
 cogl/cogl-onscreen.c         |   70 ++++++++++++++++++++++++-----------------
 cogl/cogl-onscreen.h         |   32 +++++++++++++++----
 5 files changed, 77 insertions(+), 43 deletions(-)

diff --git a/cogl/cogl-context-private.h b/cogl/cogl-context-private.h
index 4070932..fb8a1d4 100644
--- a/cogl/cogl-context-private.h
+++ b/cogl/cogl-context-private.h
@@ -184,6 +184,7 @@ struct _CoglContext
   gboolean have_last_offscreen_allocate_flags;
   CoglOffscreenAllocateFlags last_offscreen_allocate_flags;
 
+  GHashTable *swap_callback_closures;
   int next_frame_callback_id;
 
   CoglGLES2Context *current_gles2_context;
diff --git a/cogl/cogl-context.c b/cogl/cogl-context.c
index f148464..7e6a32b 100644
--- a/cogl/cogl-context.c
+++ b/cogl/cogl-context.c
@@ -310,6 +310,9 @@ cogl_context_new (CoglDisplay *display,
   context->current_draw_buffer_state_flushed = 0;
   context->current_draw_buffer_changes = COGL_FRAMEBUFFER_STATE_ALL;
 
+  context->swap_callback_closures =
+    g_hash_table_new (g_direct_hash, g_direct_equal);
+
   g_queue_init (&context->gles2_context_stack);
 
   context->journal_flush_attributes_array =
@@ -481,6 +484,9 @@ _cogl_context_free (CoglContext *context)
   if (context->blit_texture_pipeline)
     cogl_object_unref (context->blit_texture_pipeline);
 
+  if (context->swap_callback_closures)
+    g_hash_table_destroy (context->swap_callback_closures);
+
   g_warn_if_fail (context->gles2_context_stack.length == 0);
 
   if (context->journal_flush_attributes_array)
diff --git a/cogl/cogl-onscreen-private.h b/cogl/cogl-onscreen-private.h
index d9c9945..08cb535 100644
--- a/cogl/cogl-onscreen-private.h
+++ b/cogl/cogl-onscreen-private.h
@@ -24,6 +24,7 @@
 #ifndef __COGL_ONSCREEN_PRIVATE_H
 #define __COGL_ONSCREEN_PRIVATE_H
 
+#include "cogl-onscreen.h"
 #include "cogl-framebuffer-private.h"
 #include "cogl-queue.h"
 
@@ -33,20 +34,16 @@
 #include <windows.h>
 #endif
 
-typedef struct _CoglFrameCallbackEntry CoglFrameCallbackEntry;
+COGL_TAILQ_HEAD (CoglFrameCallbackList, CoglFrameClosure);
 
-COGL_TAILQ_HEAD (CoglFrameCallbackList, CoglFrameCallbackEntry);
-
-struct _CoglFrameCallbackEntry
+struct _CoglFrameClosure
 {
-  COGL_TAILQ_ENTRY (CoglFrameCallbackEntry) list_node;
+  COGL_TAILQ_ENTRY (CoglFrameClosure) list_node;
 
   CoglFrameCallback callback;
 
   void *user_data;
   CoglUserDataDestroyCallback destroy;
-
-  unsigned int id;
 };
 
 typedef struct _CoglResizeNotifyEntry CoglResizeNotifyEntry;
diff --git a/cogl/cogl-onscreen.c b/cogl/cogl-onscreen.c
index 6efcdb9..b7d443d 100644
--- a/cogl/cogl-onscreen.c
+++ b/cogl/cogl-onscreen.c
@@ -116,7 +116,7 @@ _cogl_onscreen_free (CoglOnscreen *onscreen)
   CoglFramebuffer *framebuffer = COGL_FRAMEBUFFER (onscreen);
   const CoglWinsysVtable *winsys = _cogl_framebuffer_get_winsys (framebuffer);
   CoglResizeNotifyEntry *resize_entry;
-  CoglFrameCallbackEntry *frame_entry;
+  CoglFrameClosure *frame_closure;
   CoglFrameInfo *frame_info;
 
   while ((resize_entry = COGL_TAILQ_FIRST (&onscreen->resize_callbacks)))
@@ -125,14 +125,14 @@ _cogl_onscreen_free (CoglOnscreen *onscreen)
       g_slice_free (CoglResizeNotifyEntry, resize_entry);
     }
 
-  while ((frame_entry = COGL_TAILQ_FIRST (&onscreen->frame_callbacks)))
+  while ((frame_closure = COGL_TAILQ_FIRST (&onscreen->frame_callbacks)))
     {
       COGL_TAILQ_REMOVE (&onscreen->frame_callbacks, frame_entry, list_node);
 
       if (frame_entry->destroy)
         frame_entry->destroy (frame_entry->user_data);
 
-      g_slice_free (CoglFrameCallbackEntry, frame_entry);
+      g_slice_free (CoglFrameClosure, frame_closure);
     }
 
   while ((frame_info = g_queue_pop_tail (&onscreen->pending_frame_infos)))
@@ -298,36 +298,32 @@ cogl_onscreen_add_frame_callback (CoglOnscreen *onscreen,
                                   void *user_data,
                                   CoglUserDataDestroyCallback destroy)
 {
-  CoglFrameCallbackEntry *entry = g_slice_new0 (CoglFrameCallbackEntry);
+  CoglFrameClosure *closure = g_slice_new0 (CoglFrameClosure);
   CoglContext *ctx = COGL_FRAMEBUFFER (onscreen)->context;
 
-  entry->callback = callback;
-  entry->user_data = user_data;
-  entry->destroy = destroy;
-  entry->id = ctx->next_frame_callback_id++;
+  closure->callback = callback;
+  closure->user_data = user_data;
+  closure->destroy = destroy;
 
-  COGL_TAILQ_INSERT_TAIL (&onscreen->frame_callbacks, entry, list_node);
+  COGL_TAILQ_INSERT_TAIL (&onscreen->frame_callbacks, closure, list_node);
 
-  return entry->id;
+  return entry;
 }
 
 void
 cogl_onscreen_remove_frame_callback (CoglOnscreen *onscreen,
-                                     unsigned int id)
+                                     CoglFrameClosure *closure)
 {
-  CoglFrameCallbackEntry *entry;
+  CoglFrameClosure *closure;
 
-  COGL_TAILQ_FOREACH (entry, &onscreen->frame_callbacks, list_node)
-    {
-      if (entry->id == id)
-        {
-          COGL_TAILQ_REMOVE (&onscreen->frame_callbacks, entry, list_node);
-          if (entry->destroy)
-            entry->destroy (entry->user_data);
-          g_slice_free (CoglFrameCallbackEntry, entry);
-          return;
-        }
-    }
+  _COGL_RETURN_IF_FAIL (closure);
+
+  if (closure->destroy)
+    closure->destroy (closure->user_data);
+
+  COGL_TAILQ_REMOVE (&onscreen->frame_callbacks, closure, list_node);
+
+  g_slice_free (CoglFrameClosure, closure);
 }
 
 typedef struct _SwapBufferCallbackState
@@ -359,22 +355,38 @@ cogl_onscreen_add_swap_buffers_callback (CoglOnscreen *onscreen,
                                          CoglSwapBuffersNotify callback,
                                          void *user_data)
 {
+  CoglContext *ctx = COGL_FRAMEBUFFER (onscreen)->context;
   SwapBufferCallbackState *state = g_slice_new (SwapBufferCallbackState);
+  CoglFrameClosure *closure;
+  unsigned int id = ctx->next_frame_callback_id++;
 
   state->callback = callback;
   state->user_data = user_data;
 
-  return cogl_onscreen_add_frame_callback (onscreen,
-                                           shim_swap_buffers_callback,
-                                           state,
-                                           destroy_swap_buffers_callback_state);
+  closure =
+    cogl_onscreen_add_frame_callback (onscreen,
+                                      shim_swap_buffers_callback,
+                                      state,
+                                      destroy_swap_buffers_callback_state);
+
+  g_hash_table_insert (ctx->swap_callback_closures,
+                       GINT_TO_POINTER (id),
+                       closure);
+
+  return id;
 }
 
 void
 cogl_onscreen_remove_swap_buffers_callback (CoglOnscreen *onscreen,
                                             unsigned int id)
 {
-  cogl_onscreen_remove_frame_callback (onscreen, id);
+  CoglContext *ctx = COGL_FRAMEBUFFER (onscreen)->context;
+  CoglFrameClosure *closure = g_hash_table_lookup (ctx->swap_callback_closures,
+                                                   GINT_TO_POINTER (id));
+
+  _COGL_RETURN_IF_FAIL (closure);
+
+  cogl_onscreen_remove_frame_callback (onscreen, closure);
 }
 
 void
@@ -425,7 +437,7 @@ cogl_onscreen_hide (CoglOnscreen *onscreen)
 void
 _cogl_onscreen_notify_swap_buffers (CoglOnscreen *onscreen)
 {
-  CoglFrameCallbackEntry *entry, *tmp;
+  CoglFrameClosure *entry, *tmp;
   CoglFrameInfo *info = g_queue_pop_tail (&onscreen->pending_frame_infos);
 
   COGL_TAILQ_FOREACH_SAFE (entry,
diff --git a/cogl/cogl-onscreen.h b/cogl/cogl-onscreen.h
index 501819d..39a3ee0 100644
--- a/cogl/cogl-onscreen.h
+++ b/cogl/cogl-onscreen.h
@@ -394,6 +394,19 @@ typedef void (*CoglFrameCallback) (CoglOnscreen *onscreen,
                                    void *user_data);
 
 /**
+ * CoglFrameClosure:
+ *
+ * An opaque type that tracks a #CoglFrameCallback and associated user
+ * data. A #CoglFrameClosure pointer will be returned from
+ * cogl_onscreen_add_frame_callback() and it allows you to remove a
+ * callback later using cogl_onscreen_remove_frame_callback().
+ *
+ * Since: 1.14
+ * Stability: unstable
+ */
+typedef struct _CoglFrameClosure CoglFrameClosure;
+
+/**
  * cogl_onscreen_add_frame_callback:
  * @onscreen: A #CoglOnscreen framebuffer
  * @callback: A callback function to call for frame events
@@ -429,12 +442,12 @@ typedef void (*CoglFrameCallback) (CoglOnscreen *onscreen,
  * %COGL_FRAME_EVENT_SYNC if the compositor is agressively throttling
  * your application.</note>
  *
- * Return value: a unique identifier that can be used to remove to
- *               remove the callback later.
+ * Return value: a #CoglFrameClosure pointer that can be used to
+ *               remove the callback and associated @user_data later.
  * Since: 1.14
  * Stability: unstable
  */
-unsigned int
+CoglFrameClosure *
 cogl_onscreen_add_frame_callback (CoglOnscreen *onscreen,
                                   CoglFrameCallback callback,
                                   void *user_data,
@@ -443,17 +456,22 @@ cogl_onscreen_add_frame_callback (CoglOnscreen *onscreen,
 /**
  * cogl_onscreen_remove_frame_callback:
  * @onscreen: A #CoglOnscreen
- * @id: An identifier returned from cogl_onscreen_add_frame_callback()
+ * @closure: A #CoglFrameClosure returned from
+ *           cogl_onscreen_add_frame_callback()
  *
- * Removes a callback that was previously registered
- * using cogl_onscreen_add_frame_callback().
+ * Removes a callback and associated user data that were previously
+ * registered using cogl_onscreen_add_frame_callback().
+ *
+ * If a destroy callback was passed to
+ * cogl_onscreen_add_frame_callback() to destroy the user data then
+ * this will get called.
  *
  * Since: 1.14
  * Stability: unstable
  */
 void
 cogl_onscreen_remove_frame_callback (CoglOnscreen *onscreen,
-                                     unsigned int id);
+                                     CoglFrameClosure *closure);
 
 typedef void (*CoglSwapBuffersNotify) (CoglFramebuffer *framebuffer,
                                        void *user_data);
-- 
1.7.7.6



More information about the Cogl mailing list