[cogl/wip/rib/frame-synchronization: 2/2] onscreen: pointer not id for frame_callback closures

[Robert Bragg <rbragg src gnome org>]

commit cc5cd11cbab0e9d2f063d21c498bba5a4819ad8f
Author: Robert Bragg <robert linux intel com>
Date:   Tue Jan 15 16:32:54 2013 +0000

    onscreen: pointer not id for frame_callback closures
    
    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 |   13 +++----
 cogl/cogl-onscreen.c         |   81 +++++++++++++++++++++++------------------
 cogl/cogl-onscreen.h         |   32 +++++++++++++----
 5 files changed, 82 insertions(+), 51 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..fd516c1 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;
@@ -78,7 +75,7 @@ struct _CoglOnscreen
 
   CoglBool swap_throttled;
 
-  CoglFrameCallbackList frame_callbacks;
+  CoglFrameCallbackList frame_closures;
 
   CoglBool resizable;
   CoglResizeNotifyList resize_callbacks;
diff --git a/cogl/cogl-onscreen.c b/cogl/cogl-onscreen.c
index 6efcdb9..f45d26a 100644
--- a/cogl/cogl-onscreen.c
+++ b/cogl/cogl-onscreen.c
@@ -46,7 +46,7 @@ _cogl_onscreen_init_from_template (CoglOnscreen *onscreen,
 {
   CoglFramebuffer *framebuffer = COGL_FRAMEBUFFER (onscreen);
 
-  COGL_TAILQ_INIT (&onscreen->frame_callbacks);
+  COGL_TAILQ_INIT (&onscreen->frame_closures);
   COGL_TAILQ_INIT (&onscreen->resize_callbacks);
 
   framebuffer->config = onscreen_template->config;
@@ -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_closures)))
     {
-      COGL_TAILQ_REMOVE (&onscreen->frame_callbacks, frame_entry, list_node);
+      COGL_TAILQ_REMOVE (&onscreen->frame_closures, frame_closure, list_node);
 
-      if (frame_entry->destroy)
-        frame_entry->destroy (frame_entry->user_data);
+      if (frame_closure->destroy)
+        frame_closure->destroy (frame_closure->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)))
@@ -292,42 +292,35 @@ cogl_win32_onscreen_get_window (CoglOnscreen *onscreen)
 
 #endif /* COGL_HAS_WIN32_SUPPORT */
 
-unsigned int
+CoglFrameClosure *
 cogl_onscreen_add_frame_callback (CoglOnscreen *onscreen,
                                   CoglFrameCallback callback,
                                   void *user_data,
                                   CoglUserDataDestroyCallback destroy)
 {
-  CoglFrameCallbackEntry *entry = g_slice_new0 (CoglFrameCallbackEntry);
-  CoglContext *ctx = COGL_FRAMEBUFFER (onscreen)->context;
+  CoglFrameClosure *closure = g_slice_new0 (CoglFrameClosure);
 
-  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_closures, closure, list_node);
 
-  return entry->id;
+  return closure;
 }
 
 void
 cogl_onscreen_remove_frame_callback (CoglOnscreen *onscreen,
-                                     unsigned int id)
+                                     CoglFrameClosure *closure)
 {
-  CoglFrameCallbackEntry *entry;
+  _COGL_RETURN_IF_FAIL (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;
-        }
-    }
+  if (closure->destroy)
+    closure->destroy (closure->user_data);
+
+  COGL_TAILQ_REMOVE (&onscreen->frame_closures, closure, list_node);
+
+  g_slice_free (CoglFrameClosure, closure);
 }
 
 typedef struct _SwapBufferCallbackState
@@ -359,22 +352,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,11 +434,11 @@ 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,
-                           &onscreen->frame_callbacks,
+                           &onscreen->frame_closures,
                            list_node,
                            tmp)
     {
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);