[Cogl] Residual diff between frame-synchronization branches, annotated

Owen Taylor otaylor at redhat.com
Thu Jan 24 22:01:54 PST 2013


 diff --git a/cogl/cogl-frame-info-private.h b/cogl/cogl-frame-info-private.h
 index e9bf1f4..79b2a41 100644
 --- a/cogl/cogl-frame-info-private.h
 +++ b/cogl/cogl-frame-info-private.h
 @@ -26,7 +26,6 @@
  
  #include "cogl-frame-info.h"
  #include "cogl-object-private.h"
 -#include "cogl-output.h"
  
  struct _CoglFrameInfo
  {

This was an extra introduced include, cogl-frame-info.h already includes cogl-output.h

----------------------------------------------------------------------
diff --git a/cogl/cogl-frame-info.h b/cogl/cogl-frame-info.h
index 2cb4e00..1f5de5f 100644
--- a/cogl/cogl-frame-info.h
+++ b/cogl/cogl-frame-info.h
@@ -92,7 +92,7 @@ int64_t cogl_frame_info_get_frame_counter (CoglFrameInfo *info);
 int64_t cogl_frame_info_get_presentation_time (CoglFrameInfo *info);
 
 /**
- * cogl_frame_info_get_refresh_interval:
+ * cogl_frame_info_get_refresh_rate:
  * @info: a #CoglFrameInfo object
  *
  * Gets the refresh rate in Hertz for the output that the frame was on

Your changes forgot to rename this doc comment

----------------------------------------------------------------------
 diff --git a/cogl/cogl-glx-display-private.h b/cogl/cogl-glx-display-private.h
 index fb7f395..69b1570 100644
 --- a/cogl/cogl-glx-display-private.h
 +++ b/cogl/cogl-glx-display-private.h
 @@ -50,9 +50,9 @@ typedef struct _CoglGLXDisplay
    GLXContext glx_context;
    GLXWindow dummy_glxwin;
    Window dummy_xwin;
 -  CoglBool pending_swap_notify;
 +  CoglBool pending_sync_notify;
 +  CoglBool pending_complete_notify;
    CoglBool pending_resize_notify;
 -  CoglBool pending_frame_info_notify;
  } CoglGLXDisplay;
 
 #endif /* __COGL_DISPLAY_GLX_PRIVATE_H */

You stopped using pending_frame_info_notify, but didn't remove it. We're going to have
to separate out the pending_sync and pending_complete when we hook up to
_NET_WM_FRAME_TIMINGS, so it makes more sense to keep them separate.

----------------------------------------------------------------------
 diff --git a/cogl/cogl-onscreen.c b/cogl/cogl-onscreen.c
 index c3a95e1..64f8a40 100644
 --- a/cogl/cogl-onscreen.c
 +++ b/cogl/cogl-onscreen.c
 @@ -343,18 +343,16 @@ shim_swap_buffers_callback (CoglOnscreen *onscreen,
  {
    SwapBufferCallbackState *state = user_data;
  
 -  /* XXX: To maintain the original semantics of the swap buffer
 -   * callbacks we issue swap buffer callbacks based on
 -   * _FRAME_EVENT_COMPLETE events because these currently effectively
 -   * correspond to the SwapComplete events from GLX.
 +  /* XXX: Note that technically it is a change in semantics for this
 +   * interface to forward _SYNC events here and also makes the api
 +   * name somewhat missleading.
     *
 -   * Note: We anticipate making Cogl issue _SYNC events immediatly on
 -   * calling cogl_onscreen_swap_buffers() when there are no pending
 -   * SwapComplete events and if we do that we would confuse Clutter if
 -   * we were to instead dispatch swap buffer callbacks based on the
 -   * _SYNC events.
 +   * In practice though this interface is currently used by
 +   * applications for throttling, not because they are strictly
 +   * interested in knowing when a frame has been presented and so
 +   * forwarding _SYNC events should serve them better.
     */
 -  if (event == COGL_FRAME_EVENT_COMPLETE)
 +  if (event == COGL_FRAME_EVENT_SYNC)
      state->callback (COGL_FRAMEBUFFER (onscreen), state->user_data);
 }

Change you paste-binned in IRC that I merged in.

----------------------------------------------------------------------
 diff --git a/cogl/winsys/cogl-winsys-egl-kms.c b/cogl/winsys/cogl-winsys-egl-kms.c
 index 1c185c6..1867e56 100644
 --- a/cogl/winsys/cogl-winsys-egl-kms.c
 +++ b/cogl/winsys/cogl-winsys-egl-kms.c
 @@ -756,8 +756,6 @@ _cogl_winsys_egl_context_init (CoglContext *context,
                    COGL_FEATURE_ID_FRAME_SYNC, TRUE);
    COGL_FLAGS_SET (context->features,
                    COGL_FEATURE_ID_SWAP_BUFFERS_EVENT, TRUE);
 -  COGL_FLAGS_SET (context->features,
 -                  COGL_FEATURE_ID_PRESENTATION_TIME, TRUE);
    COGL_FLAGS_SET (context->winsys_features,
                    COGL_WINSYS_FEATURE_SWAP_BUFFERS_EVENT,
                    TRUE);

the EGL backend shouldn't set the PRESENTATION_TIME feature since it doesn't provide it.

----------------------------------------------------------------------
 diff --git a/cogl/winsys/cogl-winsys-glx.c b/cogl/winsys/cogl-winsys-glx.c
 index 9e096fb..a0b72dc 100644
 --- a/cogl/winsys/cogl-winsys-glx.c
 +++ b/cogl/winsys/cogl-winsys-glx.c
 @@ -84,9 +84,9 @@ typedef struct _CoglOnscreenGLX
    CoglOnscreenXlib _parent;
    GLXDrawable glxwin;
    uint32_t last_swap_vsync_counter;
 -  CoglBool pending_swap_notify;
 +  CoglBool pending_sync_notify;
 +  CoglBool pending_complete_notify;
    CoglBool pending_resize_notify;
 -  CoglBool pending_frame_info_notify;
  } CoglOnscreenGLX;

  typedef struct _CoglTexturePixmapGLX
 
More from keeping two separate pending flags.

---------------------------------------------------------------------- 
 @@ -251,13 +251,20 @@ ust_to_nanoseconds (CoglRenderer *renderer,
       case COGL_GLX_UST_IS_GETTIMEOFDAY:
         {
          struct timeval tv;
 +        struct timespec ts;
 +        int64_t current_system_time;
 +        int64_t current_monotonic_time;
 
 -        gettimeofday (&tv, NULL);
 -        return tv.tv_sec * G_GINT64_CONSTANT (1000000000) +
 -          tv.tv_usec * G_GINT64_CONSTANT (1000);
 +        gettimeofday(&tv, NULL);
 +        clock_gettime (CLOCK_MONOTONIC, &ts);
 +        current_system_time = (tv.tv_sec * G_GINT64_CONSTANT (1000000)) + tv.tv_usec;
 +        current_monotonic_time = 
 +          ts.tv_sec * G_GINT64_CONSTANT (1000000000) + ts.tv_nsec;
 +
 +        return current_monotonic_time + 1000 * (ust - current_system_time);
        }

This is the sine qua non for me, I *really* *really* want to be able to correlate
timestamps. I have no interest in timestamps with an unknown offset.

----------------------------------------------------------------------
      case COGL_GLX_UST_IS_MONOTONIC_TIME:
 -      return ust;
 +      return 1000 * ust;
      case COGL_GLX_UST_IS_OTHER:
        /* In this case the scale of UST is undefined so we can't easily
         * scale to nanoseconds.

I double checked and the new DRM UST's as exposed via Xorg are definitely in usec - if
you want nanosecond units you need to multiple. (Your code for detecting
COGL_GLX_UST_IS_MONOTONIC still looks for usec UST)

----------------------------------------------------------------------
 @@ -276,6 +283,17 @@ ust_to_nanoseconds (CoglRenderer *renderer,
  }
 
  static void
 +set_complete_pending (CoglOnscreen *onscreen)
 +{
 +  CoglOnscreenGLX *glx_onscreen = onscreen->winsys;
 +  CoglContext *context = COGL_FRAMEBUFFER (onscreen)->context;
 +  CoglGLXDisplay *glx_display = context->display->winsys;
 +
 +  glx_display->pending_complete_notify = TRUE;
 +  glx_onscreen->pending_complete_notify = TRUE;
 +}
 +
 +static void
 notify_swap_buffers (CoglContext *context, GLXBufferSwapComplete *swap_event)

 @@ -290,17 +308,20 @@ 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_swap_notify = TRUE;
 -  glx_onscreen->pending_swap_notify = TRUE;
 +  glx_display->pending_sync_notify = TRUE;
 +  glx_onscreen->pending_sync_notify = TRUE;
  
    if (swap_event->ust != 0)
      {
        CoglFrameInfo *info = g_queue_peek_tail (&onscreen->pending_frame_infos);
 +
        info->presentation_time =
          ust_to_nanoseconds (context->display->renderer,
                             glx_onscreen->glxwin,
                             swap_event->ust);
     }
 +
 +  set_complete_pending (onscreen);
  }
 
  static void

More from keeping two separate pending flags

----------------------------------------------------------------------
 @@ -1447,14 +1469,18 @@ _cogl_winsys_get_vsync_counter (CoglContext *ctx)
  }
 
  static void
 -set_refresh_rate_from_output (CoglOnscreen *onscreen,
 -                              CoglOutput *output)
 +set_frame_info_output (CoglOnscreen *onscreen,
 +                       CoglOutput *output)
  {
 -  float refresh_rate = cogl_output_get_refresh_rate (output);
 -  if (refresh_rate != 0.0)
 +  CoglFrameInfo *info = g_queue_peek_tail (&onscreen->pending_frame_infos);
 +
 +  info->output = output;
 +
 +  if (output)
      {
 -      CoglFrameInfo *info = g_queue_peek_tail (&onscreen->pending_frame_infos);
 -      info->refresh_rate = refresh_rate;
 +      float refresh_rate = cogl_output_get_refresh_rate (output);
 +      if (refresh_rate != 0.0)
 +        info->refresh_rate = refresh_rate;
      }
  }
 
 @@ -1644,23 +1670,24 @@ _cogl_winsys_onscreen_swap_region (CoglOnscreen *onscreen,
  
    if (!xlib_onscreen->is_foreign_xwin)
      {
 -      CoglFrameInfo *info = g_queue_peek_tail (&onscreen->pending_frame_infos);
 +      CoglOutput *output;
  
        x_min = CLAMP (x_min, 0, framebuffer_width);
        x_max = CLAMP (x_max, 0, framebuffer_width);
        y_min = CLAMP (y_min, 0, framebuffer_width);
        y_max = CLAMP (y_max, 0, framebuffer_height);
  
 -      info->output =
 +      output =
          _cogl_xlib_renderer_output_for_rectangle (context->display->renderer,
                                                    xlib_onscreen->x + x_min,
                                                    xlib_onscreen->y + y_min,
                                                    x_max - x_min,
                                                    y_max - y_min);
  
 -      if (info->output)
 -        set_refresh_rate_from_output (onscreen, info->output);
 +      set_frame_info_output (onscreen, output);
      }
 +
 +  set_complete_pending (onscreen);
  }
 
  static void
 @@ -1741,11 +1768,11 @@ _cogl_winsys_onscreen_swap_buffers (CoglOnscreen *onscreen)
      glx_onscreen->last_swap_vsync_counter =
        _cogl_winsys_get_vsync_counter (context);
  
 -  if (xlib_onscreen->output)
 -    {
 -      CoglFrameInfo *info = g_queue_peek_tail (&onscreen->pending_frame_infos);
 -      info->output = xlib_onscreen->output;
 -    }
 +  set_frame_info_output (onscreen, xlib_onscreen->output);
 +
 +  if (!(glx_renderer->glXSwapInterval &&
 +        _cogl_winsys_has_feature (COGL_WINSYS_FEATURE_VBLANK_WAIT)))
 +    set_complete_pending (onscreen);
  }
 
  static uint32_t

Code refactoring, fixes a bug where the refresh rate wasn't set set when using
swap_buffers() rather than swap_region()

----------------------------------------------------------------------
 @@ -2408,9 +2435,9 @@ _cogl_winsys_poll_get_info (CoglContext *context,
  
    /* If we've already got a pending swap notify then we'll dispatch
       immediately */
 -  if (glx_display->pending_swap_notify ||
 +  if (glx_display->pending_sync_notify ||
        glx_display->pending_resize_notify ||
 -      glx_display->pending_frame_info_notify)
 +      glx_display->pending_complete_notify)
     *timeout = 0;
  }
 
 @@ -2425,13 +2452,20 @@ flush_pending_notifications_cb (void *data,
        CoglOnscreen *onscreen = COGL_ONSCREEN (framebuffer);
        CoglOnscreenGLX *glx_onscreen = onscreen->winsys;
  
 -      if (glx_onscreen->pending_swap_notify)
 +      if (glx_onscreen->pending_sync_notify)
          {
 -          CoglFrameInfo *info = g_queue_pop_tail (&onscreen->pending_frame_infos);
 +          CoglFrameInfo *info = g_queue_peek_tail (&onscreen->pending_frame_infos);
 
            _cogl_onscreen_notify_frame_sync (onscreen, info);
 +          glx_onscreen->pending_sync_notify = FALSE;
 +        }
 +
 +      if (glx_onscreen->pending_complete_notify)
 +        {
 +          CoglFrameInfo *info = g_queue_pop_tail (&onscreen->pending_frame_infos);
 +
            _cogl_onscreen_notify_complete (onscreen, info);
 -          glx_onscreen->pending_swap_notify = FALSE;
 +          glx_onscreen->pending_complete_notify = FALSE;
  
            cogl_object_unref (info);
          }
 @@ -2456,16 +2490,16 @@ _cogl_winsys_poll_dispatch (CoglContext *context,
                                       poll_fds,
                                       n_poll_fds);
  
 -  if (glx_display->pending_swap_notify ||
 +  if (glx_display->pending_sync_notify ||
        glx_display->pending_resize_notify ||
 -      glx_display->pending_frame_info_notify)
 +      glx_display->pending_complete_notify)
      {
        g_list_foreach (context->framebuffers,
                        flush_pending_notifications_cb,
                        NULL);
 -      glx_display->pending_swap_notify = FALSE;
 +      glx_display->pending_sync_notify = FALSE;
        glx_display->pending_resize_notify = FALSE;
 -      glx_display->pending_frame_info_notify = FALSE;
 +      glx_display->pending_complete_notify = FALSE;
      }
  }
 
More from keeping two separate pending flags



More information about the Cogl mailing list