[Cogl] [PATCH 1/3] Make it possible to call swap_buffers within a frame event callback

[Robert Bragg]

This looks good to land to me:

Reviewed-by: Robert Bragg <robert at linux.intel.com>

thanks,
- Robert

On Thu, Jan 31, 2013 at 4:53 PM, Neil Roberts <neil at linux.intel.com> wrote:
> It seems like it would be quite a reasonable design for an application
> to immediately paint the buffer and call swap_buffers within the
> handler for the sync event. This previously wouldn't work.
>
> When using the GLX winsys if swap_region is called then it immediately
> tries to set the pending notification flag. However if this is called
> from the event callback then when the callback is complete it will
> clear the flag again and the pending notification will be lost. This
> patch just makes it clear the pending flag before invoking the
> callback so that it can be safely queued again.
>
> With any winsys that doesn't directly handle the sync event
> notification it would almost work except that it was iterating the
> live list of pending events. If the callback causes another event to
> be added to this list by issuing a buffer swap then the iteration
> would never complete and cogl_poll_dispatch would never return. This
> patch just makes it steal the list before iterating so that any
> additions will be dispatched by a later call to cogl_poll_dispatch
> instead.
> ---
>  cogl/cogl-onscreen.c          | 14 +++++++++++---
>  cogl/winsys/cogl-winsys-glx.c | 23 ++++++++++++++++-------
>  2 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/cogl/cogl-onscreen.c b/cogl/cogl-onscreen.c
> index afa4f63..e79bf22 100644
> --- a/cogl/cogl-onscreen.c
> +++ b/cogl/cogl-onscreen.c
> @@ -3,7 +3,7 @@
>   *
>   * An object oriented GL/GLES Abstraction/Utility Layer
>   *
> - * Copyright (C) 2011 Intel Corporation.
> + * Copyright (C) 2011, 2013 Intel Corporation.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -418,9 +418,18 @@ void
>  _cogl_dispatch_onscreen_events (CoglContext *context)
>  {
>    CoglOnscreenEvent *event, *tmp;
> +  CoglOnscreenEventList queue;
> +
> +  /* Dispatching the event callback may cause another frame to be
> +   * drawn which in may cause another event to be queued immediately.
> +   * To make sure this loop will only dispatch one set of events we'll
> +   * steal the queue and iterate that separately */
> +  COGL_TAILQ_INIT (&queue);
> +  COGL_TAILQ_CONCAT (&queue, &context->onscreen_events_queue, list_node);
> +  COGL_TAILQ_INIT (&context->onscreen_events_queue);
>
>    COGL_TAILQ_FOREACH_SAFE (event,
> -                           &context->onscreen_events_queue,
> +                           &queue,
>                             list_node,
>                             tmp)
>      {
> @@ -432,7 +441,6 @@ _cogl_dispatch_onscreen_events (CoglContext *context)
>        cogl_object_unref (onscreen);
>        cogl_object_unref (info);
>
> -      COGL_TAILQ_REMOVE (&context->onscreen_events_queue, event, list_node);
>        g_slice_free (CoglOnscreenEvent, event);
>      }
>  }
> diff --git a/cogl/winsys/cogl-winsys-glx.c b/cogl/winsys/cogl-winsys-glx.c
> index eb13e03..ac5ea8f 100644
> --- a/cogl/winsys/cogl-winsys-glx.c
> +++ b/cogl/winsys/cogl-winsys-glx.c
> @@ -2536,21 +2536,27 @@ flush_pending_notifications_cb (void *data,
>      {
>        CoglOnscreen *onscreen = COGL_ONSCREEN (framebuffer);
>        CoglOnscreenGLX *glx_onscreen = onscreen->winsys;
> +      CoglBool pending_sync_notify = glx_onscreen->pending_sync_notify;
> +      CoglBool pending_complete_notify = glx_onscreen->pending_complete_notify;
>
> -      if (glx_onscreen->pending_sync_notify)
> +      /* If swap_region is called then notifying the sync event could
> +       * potentially immediately queue a subsequent pending notify so
> +       * we need to clear the flag before invoking the callback */
> +      glx_onscreen->pending_sync_notify = FALSE;
> +      glx_onscreen->pending_complete_notify = FALSE;
> +
> +      if (pending_sync_notify)
>          {
>            CoglFrameInfo *info = g_queue_peek_head (&onscreen->pending_frame_infos);
>
>            _cogl_onscreen_notify_frame_sync (onscreen, info);
> -          glx_onscreen->pending_sync_notify = FALSE;
>          }
>
> -      if (glx_onscreen->pending_complete_notify)
> +      if (pending_complete_notify)
>          {
>            CoglFrameInfo *info = g_queue_pop_head (&onscreen->pending_frame_infos);
>
>            _cogl_onscreen_notify_complete (onscreen, info);
> -          glx_onscreen->pending_complete_notify = FALSE;
>
>            cogl_object_unref (info);
>          }
> @@ -2579,12 +2585,15 @@ _cogl_winsys_poll_dispatch (CoglContext *context,
>        glx_display->pending_resize_notify ||
>        glx_display->pending_complete_notify)
>      {
> -      g_list_foreach (context->framebuffers,
> -                      flush_pending_notifications_cb,
> -                      NULL);
> +      /* These need to be cleared before invoking the callbacks in
> +       * case the callbacks cause them to be set again */
>        glx_display->pending_sync_notify = FALSE;
>        glx_display->pending_resize_notify = FALSE;
>        glx_display->pending_complete_notify = FALSE;
> +
> +      g_list_foreach (context->framebuffers,
> +                      flush_pending_notifications_cb,
> +                      NULL);
>      }
>  }
>
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl