Re: [waffle] [PATCH 7/7] egl: Use eglGetPlatformDisplay when possible

[Emil Velikov]

On 19 October 2016 at 22:52, Chad Versace  wrote:
> On Wed 19 Oct 2016, Emil Velikov wrote:
>> On 18 October 2016 at 17:58, Chad Versace  wrote:
>> > Tested against Mesa master@8c78fdb with `ninja check-func` on Linux.
>> > ---
>> >  src/waffle/egl/wegl_display.c  | 22 ++
>> >  src/waffle/egl/wegl_platform.c | 35 +--
>> >  src/waffle/egl/wegl_platform.h |  8 
>> >  3 files changed, 59 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/src/waffle/egl/wegl_display.c b/src/waffle/egl/wegl_display.c
>> > index 7a7986c..c924f2a 100644
>> > --- a/src/waffle/egl/wegl_display.c
>> > +++ b/src/waffle/egl/wegl_display.c
>> > @@ -104,10 +104,24 @@ wegl_display_init(struct wegl_display *dpy,
>> >  if (!ok)
>> >  goto fail;
>> >
>> > -dpy->egl = plat->eglGetDisplay((EGLNativeDisplayType) native_display);
>> > -if (!dpy->egl) {
>> > -wegl_emit_error(plat, "eglGetDisplay");
>> > -goto fail;
>> > +if (wegl_platform_can_use_eglGetPlatformDisplay(plat)) {
>> > +void *fixed_native_dpy = native_display;
>> > +if (plat->egl_platform == EGL_PLATFORM_X11_KHR)
>> > +fixed_native_dpy = _display;
>> > +
>
>> Silly question: wasn't the fixup applicable only for the window/pixmap 
>> surface ?
>
> Thanks! You're right.
>
>> > +dpy->egl = plat->eglGetPlatformDisplay(plat->egl_platform,
>> > +   fixed_native_dpy,
>> > +   NULL);
>> > +if (!dpy->egl) {
>> > +wegl_emit_error(plat, "eglGetPlatformDisplay");
>> > +goto fail;
>> > +}
>
>> Wondering if falling back to eglGetDisplay() is a smart idea in this case?
>> How about EGL_EXT_platform_base/eglGetPlatformDisplayEXT (admittedly
>> there's no support for it atm, but it's trivial to do so) ?
>
> I want to keep the old eglGetDisplay behavior. Otherwise, upgrading
> Waffle would regress functionality on systems with old Mesa (thinking
> about Ubunut LTS). And it's a no-no for upgrades to cause a gross
> regression like that.
>
> I agree that the fallback path should include eglGetPlatformDisplayEXT.
> But the lack of eglGetPlatformDisplayEXT doesn't make the patch
> incorrect. That can be done as a simple follow-up patch.
>
Agreed.

It just hit me that current mesa does not advertise the KHR (EGL 1.5)
extensions so with this series one is still using the old
eglGetDisplay. Having a closer look at Mesa - advertising them won't
be able in the short term, if at all. That's due to the fact that we
depend on quering the dri module(s) in order to advertise some of the
EGL 1.5 requirements/extensions.

Regardless, this is not something we should worry about in waffle.
Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 7/7] egl: Use eglGetPlatformDisplay when possible

[Chad Versace]

On Wed 19 Oct 2016, Emil Velikov wrote:
> On 18 October 2016 at 17:58, Chad Versace  wrote:
> > Tested against Mesa master@8c78fdb with `ninja check-func` on Linux.
> > ---
> >  src/waffle/egl/wegl_display.c  | 22 ++
> >  src/waffle/egl/wegl_platform.c | 35 +--
> >  src/waffle/egl/wegl_platform.h |  8 
> >  3 files changed, 59 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/waffle/egl/wegl_display.c b/src/waffle/egl/wegl_display.c
> > index 7a7986c..c924f2a 100644
> > --- a/src/waffle/egl/wegl_display.c
> > +++ b/src/waffle/egl/wegl_display.c
> > @@ -104,10 +104,24 @@ wegl_display_init(struct wegl_display *dpy,
> >  if (!ok)
> >  goto fail;
> >
> > -dpy->egl = plat->eglGetDisplay((EGLNativeDisplayType) native_display);
> > -if (!dpy->egl) {
> > -wegl_emit_error(plat, "eglGetDisplay");
> > -goto fail;
> > +if (wegl_platform_can_use_eglGetPlatformDisplay(plat)) {
> > +void *fixed_native_dpy = native_display;
> > +if (plat->egl_platform == EGL_PLATFORM_X11_KHR)
> > +fixed_native_dpy = _display;
> > +

> Silly question: wasn't the fixup applicable only for the window/pixmap 
> surface ?

Thanks! You're right.

> > +dpy->egl = plat->eglGetPlatformDisplay(plat->egl_platform,
> > +   fixed_native_dpy,
> > +   NULL);
> > +if (!dpy->egl) {
> > +wegl_emit_error(plat, "eglGetPlatformDisplay");
> > +goto fail;
> > +}

> Wondering if falling back to eglGetDisplay() is a smart idea in this case?
> How about EGL_EXT_platform_base/eglGetPlatformDisplayEXT (admittedly
> there's no support for it atm, but it's trivial to do so) ?

I want to keep the old eglGetDisplay behavior. Otherwise, upgrading
Waffle would regress functionality on systems with old Mesa (thinking
about Ubunut LTS). And it's a no-no for upgrades to cause a gross
regression like that.

I agree that the fallback path should include eglGetPlatformDisplayEXT.
But the lack of eglGetPlatformDisplayEXT doesn't make the patch
incorrect. That can be done as a simple follow-up patch.

I'll submit a v2 in-reply-to. (But not now. I'll be off of work until
Monday).
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 7/7] egl: Use eglGetPlatformDisplay when possible

[Emil Velikov]

On 18 October 2016 at 17:58, Chad Versace  wrote:
> Tested against Mesa master@8c78fdb with `ninja check-func` on Linux.
> ---
>  src/waffle/egl/wegl_display.c  | 22 ++
>  src/waffle/egl/wegl_platform.c | 35 +--
>  src/waffle/egl/wegl_platform.h |  8 
>  3 files changed, 59 insertions(+), 6 deletions(-)
>
> diff --git a/src/waffle/egl/wegl_display.c b/src/waffle/egl/wegl_display.c
> index 7a7986c..c924f2a 100644
> --- a/src/waffle/egl/wegl_display.c
> +++ b/src/waffle/egl/wegl_display.c
> @@ -104,10 +104,24 @@ wegl_display_init(struct wegl_display *dpy,
>  if (!ok)
>  goto fail;
>
> -dpy->egl = plat->eglGetDisplay((EGLNativeDisplayType) native_display);
> -if (!dpy->egl) {
> -wegl_emit_error(plat, "eglGetDisplay");
> -goto fail;
> +if (wegl_platform_can_use_eglGetPlatformDisplay(plat)) {
> +void *fixed_native_dpy = native_display;
> +if (plat->egl_platform == EGL_PLATFORM_X11_KHR)
> +fixed_native_dpy = _display;
> +
Silly question: wasn't the fixup applicable only for the window/pixmap surface ?


> +dpy->egl = plat->eglGetPlatformDisplay(plat->egl_platform,
> +   fixed_native_dpy,
> +   NULL);
> +if (!dpy->egl) {
> +wegl_emit_error(plat, "eglGetPlatformDisplay");
> +goto fail;
> +}
Wondering if falling back to eglGetDisplay() is a smart idea in this case?
How about EGL_EXT_platform_base/eglGetPlatformDisplayEXT (admittedly
there's no support for it atm, but it's trivial to do so) ?

Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

[waffle] [PATCH 7/7] egl: Use eglGetPlatformDisplay when possible

[Chad Versace]

Tested against Mesa master@8c78fdb with `ninja check-func` on Linux.
---
 src/waffle/egl/wegl_display.c  | 22 ++
 src/waffle/egl/wegl_platform.c | 35 +--
 src/waffle/egl/wegl_platform.h |  8 
 3 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/src/waffle/egl/wegl_display.c b/src/waffle/egl/wegl_display.c
index 7a7986c..c924f2a 100644
--- a/src/waffle/egl/wegl_display.c
+++ b/src/waffle/egl/wegl_display.c
@@ -104,10 +104,24 @@ wegl_display_init(struct wegl_display *dpy,
 if (!ok)
 goto fail;
 
-dpy->egl = plat->eglGetDisplay((EGLNativeDisplayType) native_display);
-if (!dpy->egl) {
-wegl_emit_error(plat, "eglGetDisplay");
-goto fail;
+if (wegl_platform_can_use_eglGetPlatformDisplay(plat)) {
+void *fixed_native_dpy = native_display;
+if (plat->egl_platform == EGL_PLATFORM_X11_KHR)
+fixed_native_dpy = _display;
+
+dpy->egl = plat->eglGetPlatformDisplay(plat->egl_platform,
+   fixed_native_dpy,
+   NULL);
+if (!dpy->egl) {
+wegl_emit_error(plat, "eglGetPlatformDisplay");
+goto fail;
+}
+} else {
+dpy->egl = plat->eglGetDisplay((EGLNativeDisplayType) native_display);
+if (!dpy->egl) {
+wegl_emit_error(plat, "eglGetDisplay");
+goto fail;
+}
 }
 
 ok = plat->eglInitialize(dpy->egl, >major_version, 
>minor_version);
diff --git a/src/waffle/egl/wegl_platform.c b/src/waffle/egl/wegl_platform.c
index 669fe38..71eb29e 100644
--- a/src/waffle/egl/wegl_platform.c
+++ b/src/waffle/egl/wegl_platform.c
@@ -65,7 +65,8 @@ wegl_platform_teardown(struct wegl_platform *self)
 bool ok = true;
 int error = 0;
 
-unsetenv("EGL_PLATFORM");
+if (!wegl_platform_can_use_eglGetPlatformDisplay(self))
+unsetenv("EGL_PLATFORM");
 
 if (self->eglHandle) {
 error = dlclose(self->eglHandle);
@@ -149,10 +150,40 @@ wegl_platform_init(struct wegl_platform *self, EGLenum 
egl_platform)
 self->client_extensions =
 self->eglQueryString(EGL_NO_DISPLAY, EGL_EXTENSIONS);
 
-setup_env(self);
+if (!wegl_platform_can_use_eglGetPlatformDisplay(self))
+setup_env(self);
 
 error:
 // On failure the caller of wegl_platform_init will trigger it's own
 // destruction which will execute wegl_platform_teardown.
 return ok;
 }
+
+bool
+wegl_platform_can_use_eglGetPlatformDisplay(const struct wegl_platform *plat)
+{
+const char *ext;
+
+if (!plat->eglGetPlatformDisplay)
+return false;
+
+switch (plat->egl_platform) {
+case EGL_PLATFORM_ANDROID_KHR:
+ext = "EGL_KHR_platform_android";
+break;
+case EGL_PLATFORM_GBM_KHR:
+ext = "EGL_KHR_platform_gbm";
+break;
+case EGL_PLATFORM_WAYLAND_KHR:
+ext = "EGL_KHR_platform_wayland";
+break;
+case EGL_PLATFORM_X11_KHR:
+ext = "EGL_KHR_platform_x11";
+break;
+default:
+assert(!"bad egl_platform enum");
+return false;
+}
+
+return waffle_is_extension_in_string(plat->client_extensions, ext);
+}
diff --git a/src/waffle/egl/wegl_platform.h b/src/waffle/egl/wegl_platform.h
index 4573ec2..d6788eb 100644
--- a/src/waffle/egl/wegl_platform.h
+++ b/src/waffle/egl/wegl_platform.h
@@ -103,3 +103,11 @@ wegl_platform_teardown(struct wegl_platform *self);
 
 bool
 wegl_platform_init(struct wegl_platform *self, EGLenum egl_platform);
+
+
+// Can eglGetPlatformDisplay can be used for this platform?
+//
+// True if libEGL exposes the eglGetPlatformDisplay function; and if EGL
+// supports the needed platform extension.
+bool
+wegl_platform_can_use_eglGetPlatformDisplay(const struct wegl_platform *plat);
-- 
2.10.0

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle