[Mesa-dev] [RFC 1/2] glx|egl: allow to test if glthread is safe enough on X11 platform

[Matt Turner]

On Fri, Apr 28, 2017 at 2:11 PM, Gregory Hainaut
<gregory.hainaut at gmail.com> wrote:
> I extended the struct __DRIbackgroundCallableExtensionRec because
> the other function pointer is already related for glthread.
>
> DRI2/DRI3 glx code path check that display can be locked (basically
> XInitThread was called)
>
> EGL code path is more tricky as we don't want to pull X11 header. Instead
> the code will assume that it is safe if X11 isn't used or there is no display
> (i.e. 100% XCB)
>
> The new function will be used in the next commit
>
> Signed-off-by: Gregory Hainaut <gregory.hainaut at gmail.com>
> ---
>  include/GL/internal/dri_interface.h |  9 +++++++++
>  src/egl/drivers/dri2/egl_dri2.c     | 30 ++++++++++++++++++++++++++++++
>  src/glx/dri2_glx.c                  |  9 +++++++++
>  src/glx/dri3_glx.c                  |  8 ++++++++
>  4 files changed, 56 insertions(+)
>
> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h
> index 86efd1bdc9..28a52ccdb9 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -1713,13 +1713,22 @@ struct __DRIbackgroundCallableExtensionRec {
>      * non-background thread (i.e. a thread that has already been bound to a
>      * context using __DRIcoreExtensionRec::bindContext()); when this happens,
>      * the \c loaderPrivate pointer must be equal to the pointer that was
>      * passed to the driver when the currently bound context was created.
>      *
>      * This call should execute quickly enough that the driver can call it with
>      * impunity whenever a background thread starts performing drawing
>      * operations (e.g. it should just set a thread-local variable).
>      */
>     void (*setBackgroundContext)(void *loaderPrivate);
> +   /**
> +    * Indicate that it is multithread safe to use glthread. Typically
> +    * XInitThread was called in GLX setup.
> +    *
> +    * \param loaderPrivate is the value that was passed to to the driver when
> +    * the context was created.  This can be used by the loader to identify
> +    * which context any callbacks are associated with.
> +    */
> +   GLboolean (*isGlThreadSafe)(void *loaderPrivate);
>  };
>
>  #endif
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 2cab7d00c1..df2db97bcf 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -85,24 +85,54 @@
>
>  static void
>  dri_set_background_context(void *loaderPrivate)
>  {
>     _EGLContext *ctx = _eglGetCurrentContext();
>     _EGLThreadInfo *t = _eglGetCurrentThread();
>
>     _eglBindContextToThread(ctx, t);
>  }
>
> +static GLboolean
> +dri_is_glthread_safe(void *loaderPrivate)
> +{
> +#ifdef HAVE_X11_PLATFORM
> +   struct dri2_egl_surface *dri2_surf = loaderPrivate;
> +   _EGLDisplay *display =  dri2_surf->base.Resource.Display;
> +   Display *dpy;
> +
> +   // Only the libX11 isn't safe
> +   if (display->Platform != _EGL_PLATFORM_X11)
> +      return true;
> +
> +   // Will use pure XCB so no libX11 here either
> +   if (display->PlatformDisplay == NULL)
> +      return true;
> +
> +   // In an ideal world we would check the X11 lock pointer
> +   // (display->PlatformDisplay->lock_fns). Unfortunately it
> +   // requires to know the full type. And we don't want to bring X11
> +   // headers here.
> +   //
> +   // So let's assume an unsafe behavior. Modern EGL code shouldn't use
> +   // libX11 anyway.

Don't use C++ comments.