[2/2] egl: Express eglGetDisplay in terms of _eglGetPlatformDisplayCommon

Submitted by Adam Jackson on Nov. 16, 2017, 6:27 p.m.

Details

Message ID 20171116182728.10591-2-ajax@redhat.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Adam Jackson Nov. 16, 2017, 6:27 p.m.
The latter is now the one place where we initialize an _EGLDisplay.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 src/egl/main/eglapi.c | 67 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 22 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 2a6513a95c..b8e91005bd 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -352,28 +352,6 @@  _eglConvertAttribsToInt(const EGLAttrib *attr_list)
    return int_attribs;
 }
 
-
-/**
- * This is typically the first EGL function that an application calls.
- * It associates a private _EGLDisplay object to the native display.
- */
-EGLDisplay EGLAPIENTRY
-eglGetDisplay(EGLNativeDisplayType nativeDisplay)
-{
-   _EGLPlatformType plat;
-   _EGLDisplay *dpy;
-   void *native_display_ptr;
-
-   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
-
-   STATIC_ASSERT(sizeof(void*) == sizeof(nativeDisplay));
-   native_display_ptr = (void*) nativeDisplay;
-
-   plat = _eglGetNativePlatform(native_display_ptr);
-   dpy = _eglFindDisplay(plat, native_display_ptr);
-   return _eglGetDisplayHandle(dpy);
-}
-
 static EGLDisplay
 _eglGetPlatformDisplayCommon(EGLenum platform, void *native_display,
                              const EGLAttrib *attrib_list)
@@ -402,6 +380,11 @@  _eglGetPlatformDisplayCommon(EGLenum platform, void *native_display,
    case EGL_PLATFORM_SURFACELESS_MESA:
       dpy = _eglGetSurfacelessDisplay(native_display, attrib_list);
       break;
+#endif
+#if defined(HAVE_HAIKU_PLATFORM) || defined(HAVE_ANDROID_PLATFORM)
+   case _EGL_NATIVE_PLATFORM:
+      dpy = _eglFindDisplay(_EGL_NATIVE_PLATFORM, native_display);
+      break;
 #endif
    default:
       RETURN_EGL_ERROR(NULL, EGL_BAD_PARAMETER, NULL);
@@ -435,6 +418,46 @@  eglGetPlatformDisplay(EGLenum platform, void *native_display,
    return _eglGetPlatformDisplayCommon(platform, native_display, attrib_list);
 }
 
+static EGLenum
+_eglTranslatePlatform(_EGLPlatformType plat)
+{
+   /* mirror of _EGLPlatformType, kinda */
+   EGLenum platform[] = {
+      EGL_PLATFORM_X11_KHR,
+      EGL_PLATFORM_WAYLAND_KHR,
+      EGL_PLATFORM_GBM_KHR,
+      _EGL_NATIVE_PLATFORM,
+      _EGL_NATIVE_PLATFORM,
+      EGL_PLATFORM_SURFACELESS_MESA,
+   };
+
+   if (plat < 0 || plat >= _EGL_NUM_PLATFORMS)
+      return _EGL_INVALID_PLATFORM;
+
+   return platform[plat];
+}
+
+/**
+ * This is typically the first EGL function that an application calls.
+ * It associates a private _EGLDisplay object to the native display.
+ */
+EGLDisplay EGLAPIENTRY
+eglGetDisplay(EGLNativeDisplayType nativeDisplay)
+{
+   _EGLPlatformType plat;
+   EGLenum platform;
+   void *native_display_ptr;
+
+   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
+
+   STATIC_ASSERT(sizeof(void*) == sizeof(nativeDisplay));
+   native_display_ptr = (void*) nativeDisplay;
+
+   plat = _eglGetNativePlatform(native_display_ptr);
+   platform = _eglTranslatePlatform(plat);
+   return _eglGetPlatformDisplayCommon(platform, native_display_ptr, NULL);
+}
+
 /**
  * Copy the extension into the string and update the string pointer.
  */

Comments

On Thursday, 2017-11-16 13:27:28 -0500, Adam Jackson wrote:
> The latter is now the one place where we initialize an _EGLDisplay.
> 
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> ---
>  src/egl/main/eglapi.c | 67 ++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index 2a6513a95c..b8e91005bd 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -352,28 +352,6 @@ _eglConvertAttribsToInt(const EGLAttrib *attr_list)
>     return int_attribs;
>  }
>  
> -
> -/**
> - * This is typically the first EGL function that an application calls.
> - * It associates a private _EGLDisplay object to the native display.
> - */
> -EGLDisplay EGLAPIENTRY
> -eglGetDisplay(EGLNativeDisplayType nativeDisplay)
> -{
> -   _EGLPlatformType plat;
> -   _EGLDisplay *dpy;
> -   void *native_display_ptr;
> -
> -   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
> -
> -   STATIC_ASSERT(sizeof(void*) == sizeof(nativeDisplay));
> -   native_display_ptr = (void*) nativeDisplay;
> -
> -   plat = _eglGetNativePlatform(native_display_ptr);
> -   dpy = _eglFindDisplay(plat, native_display_ptr);
> -   return _eglGetDisplayHandle(dpy);
> -}
> -
>  static EGLDisplay
>  _eglGetPlatformDisplayCommon(EGLenum platform, void *native_display,
>                               const EGLAttrib *attrib_list)
> @@ -402,6 +380,11 @@ _eglGetPlatformDisplayCommon(EGLenum platform, void *native_display,
>     case EGL_PLATFORM_SURFACELESS_MESA:
>        dpy = _eglGetSurfacelessDisplay(native_display, attrib_list);
>        break;
> +#endif
> +#if defined(HAVE_HAIKU_PLATFORM) || defined(HAVE_ANDROID_PLATFORM)
> +   case _EGL_NATIVE_PLATFORM:
> +      dpy = _eglFindDisplay(_EGL_NATIVE_PLATFORM, native_display);
> +      break;

Should this be in a second commit?

Also, I would add a separate EGL_PLATFORM_ANDROID_KHR case; android can
be used without being the default platform (I'm thinking surfaceless
cros with arc++, or android-x86 living side by side with eg. wayland)

>  #endif
>     default:
>        RETURN_EGL_ERROR(NULL, EGL_BAD_PARAMETER, NULL);
> @@ -435,6 +418,46 @@ eglGetPlatformDisplay(EGLenum platform, void *native_display,
>     return _eglGetPlatformDisplayCommon(platform, native_display, attrib_list);
>  }
>  
> +static EGLenum
> +_eglTranslatePlatform(_EGLPlatformType plat)
> +{
> +   /* mirror of _EGLPlatformType, kinda */
> +   EGLenum platform[] = {
> +      EGL_PLATFORM_X11_KHR,
> +      EGL_PLATFORM_WAYLAND_KHR,
> +      EGL_PLATFORM_GBM_KHR,
> +      _EGL_NATIVE_PLATFORM,
> +      _EGL_NATIVE_PLATFORM,
> +      EGL_PLATFORM_SURFACELESS_MESA,
> +   };

Since you're using those as indexes, you should probably also define
them using those same indexes:

   EGLenum platform[] = {
      [_EGL_PLATFORM_ANDROID]     = EGL_PLATFORM_ANDROID_KHR,
      [_EGL_PLATFORM_DRM]         = EGL_PLATFORM_GBM_KHR,
      [_EGL_PLATFORM_HAIKU]       = _EGL_NATIVE_PLATFORM,
      [_EGL_PLATFORM_SURFACELESS] = EGL_PLATFORM_SURFACELESS_MESA,
      [_EGL_PLATFORM_WAYLAND]     = EGL_PLATFORM_WAYLAND_KHR,
      [_EGL_PLATFORM_X11]         = EGL_PLATFORM_X11_KHR,
   };

With that:
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>

> +
> +   if (plat < 0 || plat >= _EGL_NUM_PLATFORMS)
> +      return _EGL_INVALID_PLATFORM;
> +
> +   return platform[plat];
> +}
> +
> +/**
> + * This is typically the first EGL function that an application calls.
> + * It associates a private _EGLDisplay object to the native display.
> + */
> +EGLDisplay EGLAPIENTRY
> +eglGetDisplay(EGLNativeDisplayType nativeDisplay)
> +{
> +   _EGLPlatformType plat;
> +   EGLenum platform;
> +   void *native_display_ptr;
> +
> +   _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL, EGL_NO_DISPLAY);
> +
> +   STATIC_ASSERT(sizeof(void*) == sizeof(nativeDisplay));
> +   native_display_ptr = (void*) nativeDisplay;
> +
> +   plat = _eglGetNativePlatform(native_display_ptr);
> +   platform = _eglTranslatePlatform(plat);
> +   return _eglGetPlatformDisplayCommon(platform, native_display_ptr, NULL);
> +}
> +
>  /**
>   * Copy the extension into the string and update the string pointer.
>   */
> -- 
> 2.14.3
>
On Fri, 2017-11-17 at 16:26 +0000, Eric Engestrom wrote:
> On Thursday, 2017-11-16 13:27:28 -0500, Adam Jackson wrote:
> 
> > @@ -402,6 +380,11 @@ _eglGetPlatformDisplayCommon(EGLenum platform, void *native_display,
> >     case EGL_PLATFORM_SURFACELESS_MESA:
> >        dpy = _eglGetSurfacelessDisplay(native_display, attrib_list);
> >        break;
> > +#endif
> > +#if defined(HAVE_HAIKU_PLATFORM) || defined(HAVE_ANDROID_PLATFORM)
> > +   case _EGL_NATIVE_PLATFORM:
> > +      dpy = _eglFindDisplay(_EGL_NATIVE_PLATFORM, native_display);
> > +      break;
> 
> Should this be in a second commit?

Without this change, eglGetDisplay would be entirely broken on Haiku
and Android. It maybe belongs in a prior commit?

> Also, I would add a separate EGL_PLATFORM_ANDROID_KHR case; android can
> be used without being the default platform (I'm thinking surfaceless
> cros with arc++, or android-x86 living side by side with eg. wayland)

Yeah, fair point, the Android path should use the extension written for
that purpose. I'll fix that up and resubmit.
 
> > +static EGLenum
> > +_eglTranslatePlatform(_EGLPlatformType plat)
> > +{
> > +   /* mirror of _EGLPlatformType, kinda */
> > +   EGLenum platform[] = {
> > +      EGL_PLATFORM_X11_KHR,
> > +      EGL_PLATFORM_WAYLAND_KHR,
> > +      EGL_PLATFORM_GBM_KHR,
> > +      _EGL_NATIVE_PLATFORM,
> > +      _EGL_NATIVE_PLATFORM,
> > +      EGL_PLATFORM_SURFACELESS_MESA,
> > +   };
> 
> Since you're using those as indexes, you should probably also define
> them using those same indexes:

Will do.

- ajax