[Cogl] [PATCH] ensure _SYNC and _COMPLETE events are always dispatched
Owen Taylor
otaylor at redhat.com
Tue Jan 29 10:04:29 PST 2013
In the commit message you say:
> [...] Previously applications would check
> for the _SWAP_BUFFERS_EVENT feature to see if they could use
> cogl_onscreen_add_swap_buffers_callback() as a means to throttle their
> paint cycle but they would also have to have a fallback that simply used
> and idle callback for painting. This should simplify the boilerplate
> required for using Cogl.
But an app shouldn't just draw as fast as possible as soon as they get
the sync message - Clutter looks at COGL_WINSYS_FEATURE_SWAP_THROTTLE
and if it's absent will always wait until 1/60th of a second after the
start of the last frame before starting a new frame. Does the
"boilerplate" need to include logic like this? should Cogl throttle if
COGL_WINSYS_FEATURE_SWAP_THROTTLE is missing? is it pathological if
COGL_WINSYS_FEATURE_SWAP_THROTTLE is missing?
I was initially worried that sending the SYNC message always would be
problematical since an app would care whether SYNC messages are
throttled or not, but it seems that at least for Clutter it doesn't
matter - the behavior it wants is that if the swap is throttled, it will
start drawing immediately unless it is waiting for a SYNC message, so
an immediate SYNC message will have the same effect.
- Owen
> This patch removes COGL_FEATURE_ID_FRAME_SYNC feature since we now
> guarantee that this event is always dispatched.
>
> TODO: squash this back into _add_frame_callback patch
> ---
> cogl/cogl-context-private.h | 3 +
> cogl/cogl-context.h | 3 -
> cogl/cogl-onscreen-private.h | 16 ++++++
> cogl/cogl-onscreen.c | 76 +++++++++++++++++++++++++
> cogl/cogl-onscreen.h | 38 ++++++-------
> cogl/cogl-poll.c | 10 ++++
> cogl/cogl-sdl.c | 2 +-
> cogl/cogl-types.h | 3 +
> cogl/winsys/cogl-winsys-egl-kms.c | 6 +-
> cogl/winsys/cogl-winsys-glx-feature-functions.h | 3 +-
> cogl/winsys/cogl-winsys-glx.c | 35 ++++++++----
> 11 files changed, 156 insertions(+), 39 deletions(-)
>
> diff --git a/cogl/cogl-context-private.h b/cogl/cogl-context-private.h
> index ae8e3dc..fc528e7 100644
> --- a/cogl/cogl-context-private.h
> +++ b/cogl/cogl-context-private.h
> @@ -49,6 +49,7 @@
> #include "cogl-gpu-info-private.h"
> #include "cogl-gl-header.h"
> #include "cogl-framebuffer-private.h"
> +#include "cogl-onscreen-private.h"
>
> typedef struct
> {
> @@ -175,6 +176,8 @@ struct _CoglContext
> GHashTable *swap_callback_closures;
> int next_swap_callback_id;
>
> + CoglOnscreenEventList onscreen_events_queue;
> +
> CoglGLES2Context *current_gles2_context;
> GQueue gles2_context_stack;
>
> diff --git a/cogl/cogl-context.h b/cogl/cogl-context.h
> index 2b4eaf2..1b6b878 100644
> --- a/cogl/cogl-context.h
> +++ b/cogl/cogl-context.h
> @@ -204,8 +204,6 @@ cogl_is_context (void *object);
> * suported.
> * @COGL_FEATURE_ID_DEPTH_TEXTURE: Whether #CoglFramebuffer support rendering
> * the depth buffer to a texture.
> - * @COGL_FEATURE_ID_FRAME_SYNC: Whether %COGL_FRAME_EVENT_SYNC events
> - * will be sent to registered #CoglFrameCallback functions.
> * @COGL_FEATURE_ID_PRESENTATION_TIME: Whether frame presentation
> * time stamps will be recorded in #CoglFrameInfo objects.
> *
> @@ -237,7 +235,6 @@ typedef enum _CoglFeatureID
> COGL_FEATURE_ID_SWAP_BUFFERS_EVENT,
> COGL_FEATURE_ID_GLES2_CONTEXT,
> COGL_FEATURE_ID_DEPTH_TEXTURE,
> - COGL_FEATURE_ID_FRAME_SYNC,
> COGL_FEATURE_ID_PRESENTATION_TIME,
>
> /*< private >*/
> diff --git a/cogl/cogl-onscreen-private.h b/cogl/cogl-onscreen-private.h
> index 7751d80..89efb0e 100644
> --- a/cogl/cogl-onscreen-private.h
> +++ b/cogl/cogl-onscreen-private.h
> @@ -59,6 +59,19 @@ struct _CoglResizeNotifyEntry
> unsigned int id;
> };
>
> +typedef struct _CoglOnscreenEvent CoglOnscreenEvent;
> +
> +COGL_TAILQ_HEAD (CoglOnscreenEventList, CoglOnscreenEvent);
> +
> +struct _CoglOnscreenEvent
> +{
> + COGL_TAILQ_ENTRY (CoglOnscreenEvent) list_node;
> +
> + CoglOnscreen *onscreen;
> + CoglFrameInfo *info;
> + CoglFrameEvent type;
> +};
> +
> struct _CoglOnscreen
> {
> CoglFramebuffer _parent;
> @@ -102,4 +115,7 @@ _cogl_onscreen_notify_complete (CoglOnscreen *onscreen, CoglFrameInfo *info);
> void
> _cogl_onscreen_notify_resize (CoglOnscreen *onscreen);
>
> +void
> +_cogl_dispatch_onscreen_events (CoglContext *context);
> +
> #endif /* __COGL_ONSCREEN_PRIVATE_H */
> diff --git a/cogl/cogl-onscreen.c b/cogl/cogl-onscreen.c
> index ba5efbb..3887a1a 100644
> --- a/cogl/cogl-onscreen.c
> +++ b/cogl/cogl-onscreen.c
> @@ -119,6 +119,22 @@ _cogl_onscreen_free (CoglOnscreen *onscreen)
> g_free (onscreen);
> }
>
> +static void
> +_cogl_onscreen_queue_event (CoglOnscreen *onscreen,
> + CoglFrameEvent type,
> + CoglFrameInfo *info)
> +{
> + CoglContext *ctx = COGL_FRAMEBUFFER (onscreen)->context;
> +
> + CoglOnscreenEvent *event = g_slice_new (CoglOnscreenEvent);
> +
> + event->onscreen = cogl_object_ref (onscreen);
> + event->info = cogl_object_ref (info);
> + event->type = type;
> +
> + COGL_TAILQ_INSERT_TAIL (&ctx->onscreen_events_queue, event, list_node);
> +}
> +
> void
> cogl_onscreen_swap_buffers (CoglOnscreen *onscreen)
> {
> @@ -140,6 +156,21 @@ cogl_onscreen_swap_buffers (CoglOnscreen *onscreen)
> COGL_BUFFER_BIT_COLOR |
> COGL_BUFFER_BIT_DEPTH |
> COGL_BUFFER_BIT_STENCIL);
> +
> + if (!_cogl_winsys_has_feature (COGL_WINSYS_FEATURE_SYNC_AND_COMPLETE_EVENT))
> + {
> + CoglFrameInfo *info;
> +
> + g_warn_if_fail (onscreen->pending_frame_infos.length == 1);
> +
> + info = g_queue_pop_tail (&onscreen->pending_frame_infos);
> +
> + _cogl_onscreen_queue_event (onscreen, COGL_FRAME_EVENT_SYNC, info);
> + _cogl_onscreen_queue_event (onscreen, COGL_FRAME_EVENT_COMPLETE, info);
> +
> + cogl_object_unref (info);
> + }
> +
> onscreen->frame_counter++;
> }
>
> @@ -174,6 +205,21 @@ cogl_onscreen_swap_region (CoglOnscreen *onscreen,
> COGL_BUFFER_BIT_COLOR |
> COGL_BUFFER_BIT_DEPTH |
> COGL_BUFFER_BIT_STENCIL);
> +
> + if (!_cogl_winsys_has_feature (COGL_WINSYS_FEATURE_SYNC_AND_COMPLETE_EVENT))
> + {
> + CoglFrameInfo *info;
> +
> + g_warn_if_fail (onscreen->pending_frame_infos.length == 1);
> +
> + info = g_queue_pop_tail (&onscreen->pending_frame_infos);
> +
> + _cogl_onscreen_queue_event (onscreen, COGL_FRAME_EVENT_SYNC, info);
> + _cogl_onscreen_queue_event (onscreen, COGL_FRAME_EVENT_COMPLETE, info);
> +
> + cogl_object_unref (info);
> + }
> +
> onscreen->frame_counter++;
> }
>
> @@ -422,6 +468,36 @@ cogl_onscreen_hide (CoglOnscreen *onscreen)
> winsys->onscreen_set_visibility (onscreen, FALSE);
> }
> }
> +void
> +_cogl_dispatch_onscreen_events (CoglContext *context)
> +{
> + CoglOnscreenEvent *event, *tmp;
> +
> + COGL_TAILQ_FOREACH_SAFE (event,
> + &context->onscreen_events_queue,
> + list_node,
> + tmp)
> + {
> + CoglOnscreen *onscreen = event->onscreen;
> + CoglFrameInfo *info = event->info;
> +
> + switch (event->type)
> + {
> + case COGL_FRAME_EVENT_SYNC:
> + _cogl_onscreen_notify_frame_sync (onscreen, info);
> + break;
> + case COGL_FRAME_EVENT_COMPLETE:
> + _cogl_onscreen_notify_complete (onscreen, info);
> + break;
> + }
> +
> + cogl_object_unref (onscreen);
> + cogl_object_unref (info);
> +
> + COGL_TAILQ_REMOVE (&context->onscreen_events_queue, event, list_node);
> + g_slice_free (CoglOnscreenEvent, event);
> + }
> +}
>
> void
> _cogl_onscreen_notify_frame_sync (CoglOnscreen *onscreen, CoglFrameInfo *info)
> diff --git a/cogl/cogl-onscreen.h b/cogl/cogl-onscreen.h
> index d079cd7..0a59eb8 100644
> --- a/cogl/cogl-onscreen.h
> +++ b/cogl/cogl-onscreen.h
> @@ -409,9 +409,7 @@ cogl_onscreen_swap_region (CoglOnscreen *onscreen,
> * CoglFrameEvent:
> * @COGL_FRAME_EVENT_SYNC: Notifies that the system compositor has
> * acknowledged a frame and is ready for a
> - * new frame to be created. Only delivered
> - * if the %COGL_FEATURE_ID_FRAME_SYNC
> - * feature is supported.
> + * new frame to be created.
> * @COGL_FRAME_EVENT_COMPLETE: Notifies that a frame has ended. This
> * is a good time for applications to
> * collect statistics about the frame
> @@ -493,30 +491,26 @@ typedef struct _CoglFrameClosure CoglFrameClosure;
> * Installs a @callback function that will be called for significant
> * events relating to the given @onscreen framebuffer.
> *
> - * If the %COGL_FEATURE_ID_FRAME_SYNC feature is supported
> - * then @callback will be used to notify when the system compositor is
> + * The @callback will be used to notify when the system compositor is
> * ready for this application to render a new frame. In this case
> * %COGL_FRAME_EVENT_SYNC will be passed as the event argument to the
> * given @callback in addition to the #CoglFrameInfo corresponding to
> * the frame beeing acknowledged by the compositor.
> *
> - * If the %COGL_FEATURE_ID_PRESENTATION_EVENTS is available then
> - * @callback will be called to notify when the frame has become
> - * visible to the user. In this case %COGL_FRAME_EVENT_PRESENTED will
> - * be passed as the event argument to the given @callback in addition
> - * to the #CoglFrameInfo corresponding to the newly presented frame.
> - *
> - * We recommend throttling your application according to
> - * %COGL_FRAME_EVENT_SYNC events whenever the
> - * %COGL_FEATURE_ID_FRAME_SYNC feature is available. This allows your
> - * application to avoid being blocked by the driver which will block
> - * your applications mainloop.
> - *
> - * <note>Applications should not make assumptions about the relative
> - * ordering of different types of frame events. A
> - * %COGL_FRAME_EVENT_PRESENTED event can be received before a
> - * %COGL_FRAME_EVENT_SYNC if the compositor is agressively throttling
> - * your application.</note>
> + * The @callback will also be called to notify when the frame has
> + * ended. In this case %COGL_FRAME_EVENT_COMPLETE will be passed as
> + * the event argument to the given @callback in addition to the
> + * #CoglFrameInfo corresponding to the newly presented frame. The
> + * meaning of "ended" here simply means that no more timing
> + * information will be collected within the corresponding
> + * #CoglFrameInfo and so this is a good opportunity to analyse the
> + * given info. It does not necessarily mean that the GPU has finished
> + * rendering the corresponding frame.
> + *
> + * We highly recommend throttling your application according to
> + * %COGL_FRAME_EVENT_SYNC events so that your application can avoid
> + * wasting resources, drawing more frames than your system compositor
> + * can display.
> *
> * Return value: a #CoglFrameClosure pointer that can be used to
> * remove the callback and associated @user_data later.
> diff --git a/cogl/cogl-poll.c b/cogl/cogl-poll.c
> index 86be2cf..c6d19fb 100644
> --- a/cogl/cogl-poll.c
> +++ b/cogl/cogl-poll.c
> @@ -44,6 +44,13 @@ cogl_poll_get_info (CoglContext *context,
> _COGL_RETURN_IF_FAIL (n_poll_fds != NULL);
> _COGL_RETURN_IF_FAIL (timeout != NULL);
>
> + if (!COGL_TAILQ_EMPTY (&context->onscreen_events_queue))
> + {
> + *n_poll_fds = 0;
> + *timeout = 0;
> + return;
> + }
> +
> winsys = _cogl_context_get_winsys (context);
>
> if (winsys->poll_get_info)
> @@ -70,6 +77,9 @@ cogl_poll_dispatch (CoglContext *context,
>
> _COGL_RETURN_IF_FAIL (cogl_is_context (context));
>
> + if (!COGL_TAILQ_EMPTY (&context->onscreen_events_queue))
> + _cogl_dispatch_onscreen_events (context);
> +
> winsys = _cogl_context_get_winsys (context);
>
> if (winsys->poll_dispatch)
> diff --git a/cogl/cogl-sdl.c b/cogl/cogl-sdl.c
> index 03afeb7..b49890f 100644
> --- a/cogl/cogl-sdl.c
> +++ b/cogl/cogl-sdl.c
> @@ -83,5 +83,5 @@ cogl_sdl_handle_event (CoglContext *context, SDL_Event *event)
> void
> cogl_sdl_idle (CoglContext *context)
> {
> - /* NOP since Cogl doesn't currently need to do anything when idle */
> + _cogl_dispatch_onscreen_events (context);
> }
> diff --git a/cogl/cogl-types.h b/cogl/cogl-types.h
> index 01ad4c9..e4d8aae 100644
> --- a/cogl/cogl-types.h
> +++ b/cogl/cogl-types.h
> @@ -658,6 +658,9 @@ typedef enum _CoglWinsysFeature
> /* Avaiable if the age of the back buffer can be queried */
> COGL_WINSYS_FEATURE_BUFFER_AGE,
>
> + /* Avaiable if the winsys directly handles _SYNC and _COMPLETE events */
> + COGL_WINSYS_FEATURE_SYNC_AND_COMPLETE_EVENT,
> +
> COGL_WINSYS_FEATURE_N_FEATURES
> } CoglWinsysFeature;
>
> diff --git a/cogl/winsys/cogl-winsys-egl-kms.c b/cogl/winsys/cogl-winsys-egl-kms.c
> index a984afb..db2b496 100644
> --- a/cogl/winsys/cogl-winsys-egl-kms.c
> +++ b/cogl/winsys/cogl-winsys-egl-kms.c
> @@ -753,12 +753,14 @@ _cogl_winsys_egl_context_init (CoglContext *context,
> CoglError **error)
> {
> COGL_FLAGS_SET (context->features,
> - COGL_FEATURE_ID_FRAME_SYNC, TRUE);
> - COGL_FLAGS_SET (context->features,
> COGL_FEATURE_ID_SWAP_BUFFERS_EVENT, TRUE);
> + /* TODO: remove this deprecated feature */
> COGL_FLAGS_SET (context->winsys_features,
> COGL_WINSYS_FEATURE_SWAP_BUFFERS_EVENT,
> TRUE);
> + COGL_FLAGS_SET (context->winsys_features,
> + COGL_WINSYS_FEATURE_SYNC_AND_COMPLETE_EVENT,
> + TRUE);
>
> return TRUE;
> }
> diff --git a/cogl/winsys/cogl-winsys-glx-feature-functions.h b/cogl/winsys/cogl-winsys-glx-feature-functions.h
> index d0d2de3..156b58f 100644
> --- a/cogl/winsys/cogl-winsys-glx-feature-functions.h
> +++ b/cogl/winsys/cogl-winsys-glx-feature-functions.h
> @@ -169,7 +169,8 @@ COGL_WINSYS_FEATURE_BEGIN (255, 255,
> swap_event,
> "INTEL\0",
> "swap_event\0",
> - COGL_WINSYS_FEATURE_SWAP_BUFFERS_EVENT)
> + COGL_WINSYS_FEATURE_SWAP_BUFFERS_EVENT |
> + COGL_WINSYS_FEATURE_SYNC_AND_COMPLETE_EVENT)
> COGL_WINSYS_FEATURE_END ()
>
> COGL_WINSYS_FEATURE_BEGIN (255, 255,
> diff --git a/cogl/winsys/cogl-winsys-glx.c b/cogl/winsys/cogl-winsys-glx.c
> index 2dbe0d7..0893af7 100644
> --- a/cogl/winsys/cogl-winsys-glx.c
> +++ b/cogl/winsys/cogl-winsys-glx.c
> @@ -301,6 +301,17 @@ _cogl_winsys_get_clock_time (CoglContext *context)
> }
>
> static void
> +set_sync_pending (CoglOnscreen *onscreen)
> +{
> + CoglOnscreenGLX *glx_onscreen = onscreen->winsys;
> + CoglContext *context = COGL_FRAMEBUFFER (onscreen)->context;
> + CoglGLXDisplay *glx_display = context->display->winsys;
> +
> + glx_display->pending_sync_notify = TRUE;
> + glx_onscreen->pending_sync_notify = TRUE;
> +}
> +
> +static void
> set_complete_pending (CoglOnscreen *onscreen)
> {
> CoglOnscreenGLX *glx_onscreen = onscreen->winsys;
> @@ -315,8 +326,6 @@ static void
> notify_swap_buffers (CoglContext *context, GLXBufferSwapComplete *swap_event)
> {
> CoglOnscreen *onscreen = find_onscreen_for_xid (context, (uint32_t)swap_event->drawable);
> - CoglDisplay *display = context->display;
> - CoglGLXDisplay *glx_display = display->winsys;
> CoglOnscreenGLX *glx_onscreen;
>
> if (!onscreen)
> @@ -326,8 +335,7 @@ notify_swap_buffers (CoglContext *context, GLXBufferSwapComplete *swap_event)
> /* We only want to notify that the swap is complete when the
> application calls cogl_context_dispatch so instead of immediately
> notifying we'll set a flag to remember to notify later */
> - glx_display->pending_sync_notify = TRUE;
> - glx_onscreen->pending_sync_notify = TRUE;
> + set_sync_pending (onscreen);
>
> if (swap_event->ust != 0)
> {
> @@ -716,11 +724,9 @@ update_winsys_features (CoglContext *context, CoglError **error)
> COGL_FLAGS_SET (context->winsys_features,
> COGL_WINSYS_FEATURE_SWAP_REGION_THROTTLE, TRUE);
>
> - if (_cogl_winsys_has_feature (COGL_WINSYS_FEATURE_SWAP_BUFFERS_EVENT))
> + if (_cogl_winsys_has_feature (COGL_WINSYS_FEATURE_SYNC_AND_COMPLETE_EVENT))
> {
> - COGL_FLAGS_SET (context->features,
> - COGL_FEATURE_ID_FRAME_SYNC,
> - TRUE);
> + /* TODO: remove this deprecated feature */
> COGL_FLAGS_SET (context->features,
> COGL_FEATURE_ID_SWAP_BUFFERS_EVENT,
> TRUE);
> @@ -1266,7 +1272,7 @@ _cogl_winsys_onscreen_init (CoglOnscreen *onscreen,
> }
>
> #ifdef GLX_INTEL_swap_event
> - if (_cogl_winsys_has_feature (COGL_WINSYS_FEATURE_SWAP_BUFFERS_EVENT))
> + if (_cogl_winsys_has_feature (COGL_WINSYS_FEATURE_SYNC_AND_COMPLETE_EVENT))
> {
> GLXDrawable drawable =
> glx_onscreen->glxwin ? glx_onscreen->glxwin : xlib_onscreen->xwin;
> @@ -1749,7 +1755,16 @@ _cogl_winsys_onscreen_swap_region (CoglOnscreen *onscreen,
> set_frame_info_output (onscreen, output);
> }
>
> - set_complete_pending (onscreen);
> + /* XXX: we don't get SwapComplete events based on how we implement
> + * the _swap_region() API but if cogl-onscreen.c knows we are
> + * handling _SYNC and _COMPLETE events in the winsys then we need to
> + * send fake events in this case.
> + */
> + if (_cogl_winsys_has_feature (COGL_WINSYS_FEATURE_SYNC_AND_COMPLETE_EVENT))
> + {
> + set_sync_pending (onscreen);
> + set_complete_pending (onscreen);
> + }
> }
>
> static void
More information about the Cogl
mailing list