[Mesa-dev] [PATCH v3 2/2] egl: Add EGL_CHROMIUM_sync_control extension.

[Eric Anholt]

Sarah Sharp <sarah.a.sharp at linux.intel.com> writes:

> Chromium defined a new GL extension (that isn't registered with Khronos).
> We need to add an EGL extension for it, so we can migrate ChromeOS on
> Intel systems to use EGL instead of GLX.
>
> http://git.chromium.org/gitweb/?p=chromium/src/third_party/khronos.git;a=commitdiff;h=27cbfdab35c601f70aa150581ad1448d0401f447
>
> The EGL_CHROMIUM_sync_control extension is similar to the GLX extension
> OML_sync_control, but only defines one function,
> eglGetSyncValuesCHROMIUM, which is equivalent to glXGetSyncValuesOML.

> diff --git a/src/egl/drivers/dri2/egl_dri2_fallbacks.h b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
> index a5cf344..9cba001 100644
> --- a/src/egl/drivers/dri2/egl_dri2_fallbacks.h
> +++ b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
> @@ -98,3 +98,11 @@ dri2_fallback_create_wayland_buffer_from_image(_EGLDriver *drv,
>  {
>     return NULL;
>  }
> +
> +static inline EGLBoolean
> +dri2_fallback_get_sync_values(_EGLDisplay *dpy, _EGLSurface *surf,
> +                              EGLuint64KHR *ust, EGLuint64KHR *msc,
> +                              EGLuint64KHR *sbc)
> +{
> +   return EGL_FALSE;
> +}

I've stared at the code in this file, trying to figure out when it would
ever be called, and failed.  But you're following an existing pattern,
so it's fine.

> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index 219d8e6..27d0802 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -1086,6 +1086,7 @@ eglGetProcAddress(const char *procname)
>        { "eglGetPlatformDisplayEXT", (_EGLProc) eglGetPlatformDisplayEXT },
>        { "eglCreatePlatformWindowSurfaceEXT", (_EGLProc) eglCreatePlatformWindowSurfaceEXT },
>        { "eglCreatePlatformPixmapSurfaceEXT", (_EGLProc) eglCreatePlatformPixmapSurfaceEXT },
> +      { "eglGetSyncValuesCHROMIUM", (_EGLProc) eglGetSyncValuesCHROMIUM },
>        { NULL, NULL }
>     };
>     EGLint i;
> @@ -1751,3 +1752,25 @@ eglPostSubBufferNV(EGLDisplay dpy, EGLSurface surface,
>  
>     RETURN_EGL_EVAL(disp, ret);
>  }
> +
> +EGLBoolean EGLAPIENTRY
> +eglGetSyncValuesCHROMIUM(EGLDisplay display, EGLSurface surface,
> +                         EGLuint64KHR *ust, EGLuint64KHR *msc,
> +                         EGLuint64KHR *sbc)
> +{
> +   _EGLDisplay *disp = _eglLockDisplay(display);
> +   _EGLSurface *surf = _eglLookupSurface(surface, disp);
> +   _EGLDriver *drv;
> +   EGLBoolean ret;
> +
> +   _EGL_CHECK_SURFACE(disp, surf, EGL_FALSE, drv);
> +   if (!disp->Extensions.CHROMIUM_sync_control)
> +      RETURN_EGL_EVAL(disp, EGL_FALSE);
> +
> +   if (!ust || !msc || !sbc)
> +      RETURN_EGL_ERROR(disp, EGL_BAD_PARAMETER, EGL_FALSE);

This is the sort of thing that makes me uncomfortable merging extension
support without a spec.  Should we throw an error on NULL, or just not
return a value there?

It looks like GLX_OML_sync_control doesn't specify any particular
behavior for that.  Given that, throwing an error instead of crashing
seems nice.

> +   ret = drv->API.GetSyncValuesCHROMIUM(disp, surf, ust, msc, sbc);
> +
> +   RETURN_EGL_EVAL(disp, ret);
> +}

This patch is:

Reviewed-by: Eric Anholt <eric at anholt.net>

I'm a little weirded out by the custom header for what is just an EGL
extension (but unspecced) in patch 1.  I'll leave the question of
merging that up to Ian.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140507/54a1cf89/attachment.sig>