[PATCH weston] gl-renderer: fix EGL initialization steps
Manuel Bachmann
manuel.bachmann at open.eurogiciel.org
Mon Mar 30 12:38:20 PDT 2015
Hi Daniel, and thanks,
"What is the complete client extension string on that platform?"
It is simply "EGL_EXT_client_extensions".
Here are the details : kernel 3.13.0 - Mesa 10.1.3 (built with
"with-egl-platforms=wayland,x11,drm") - kernel module is "i915" - EGL is :
1.4 (DRI2) - GL vendor is : Intel Open Source Technology Center - GL
renderer is : Mesa DRI Intel(R) Ivybridge Model.
GPU seems to be : Intel HP Graphics 4000 (core i7 CPU)
2015-03-30 20:52 GMT+02:00 Daniel Stone <daniel at fooishbar.org>:
> Hi,
>
> On 30 March 2015 at 17:38, Manuel Bachmann
> <manuel.bachmann at open.eurogiciel.org> wrote:
> > "You removed the early return here...
> > ...which means this check may be accessing NULL extensions, no?"
> >
> > You are right ! Sorry for that, did not even look at the #ifdef, will
> rework
> > this part.
>
> Apart from the configless_context damage, the rest (don't make lack of
> client extensions query fatal) is very much correct. I think that got
> lost in review. :\
>
> > "You do want to keep the -1 return as fatal here. gl_renderer_supports()
> > should already return 0 meaning "maybe" if client extensions cannot be
> > queried. The supports function is very nicely documented."
> >
> > Well, I have here a pratical case of a Ubuntu 14.04 laptop (Intel GPU,
> i915
> > driver, will let you know the hardware, Mesa and libs versions soon when
> I
> > get my hands on it), where the supports() function is able to find an
> > "*extensions" strings, but then fails to locate these precise extensions,
> > and thus returns -1. It prevents the X11 and DRM compositors to run, but
> it
> > works nicely when removing this check. Does it make sense to you ?
> > (regarding the purpose of the function, is that when building EGL with
> X11
> > support, it will always report X11 as a supported platform ? Maybe there
> are
> > versions of Mesa which do not honor this. Will check.)
>
> Weird - the only way I can see that happening is if Mesa advertises
> support for platform_base, but not for platform_{x11,gbm}, which seems
> very odd indeed. Ever since the addition of platform_base support[0],
> Mesa has supported EXT variants of x11 and wayland, and a MESA variant
> of gbm. So given the (fairly exhaustive) checks in
> gl_renderer_supports, I can't see how we'd end up in this situation.
> What is the complete client extension string on that platform?
>
> Cheers,
> Daniel
>
> [0]:
> http://cgit.freedesktop.org/mesa/mesa/commit/src/egl/main/eglglobals.c?id=468cc866b4b308cee40470f06b31002c6c56da96
>
> > 2015-03-30 14:40 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> >>
> >> On Sun, 29 Mar 2015 05:03:09 +0200
> >> Manuel Bachmann <manuel.bachmann at open.eurogiciel.org> wrote:
> >>
> >> > From: Manuel Bachmann <manuel.bachmann at open.eurogiciel.org>
> >> >
> >> > We should not prevent gl-renderer to initalize if client
> >> > extensions were not found. Practically, this prevented
> >> > Weston from running with GL on i915 DRI platforms.
> >> >
> >> > Some DRI drivers, including VMware vmwgfx, do not support
> >> > calling eglQueryString() with a EGL_NO_DISPLAY parameter.
> >> > Just as we do in gl_renderer_supports(), which returns 0
> >> > but does not fail in this case, do not fail in
> >> > gl_renderer_setup_egl_extensions().
> >> > ---
> >> > src/gl-renderer.c | 18 +++++++-----------
> >> > 1 file changed, 7 insertions(+), 11 deletions(-)
> >> >
> >> > diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> >> > index b3b2364..28ecc63 100644
> >> > --- a/src/gl-renderer.c
> >> > +++ b/src/gl-renderer.c
> >> > @@ -2136,15 +2136,14 @@ gl_renderer_setup_egl_extensions(struct
> >> > weston_compositor *ec)
> >> > (const char *) eglQueryString(EGL_NO_DISPLAY,
> >> > EGL_EXTENSIONS);
> >> > if (!extensions) {
> >> > weston_log("Retrieving EGL client extension string
> >> > failed.\n");
> >> > - return -1;
> >> > + } else {
> >> > + if (strstr(extensions, "EGL_EXT_platform_base"))
> >> > + gr->create_platform_window =
> >> > + (void *)
> >> > eglGetProcAddress("eglCreatePlatformWindowSurfaceEXT");
> >> > + else
> >> > + weston_log("warning: EGL_EXT_platform_base not
> >> > supported.\n");
> >> > }
> >>
> >> You removed the early return here...
> >>
> >> >
> >> > - if (strstr(extensions, "EGL_EXT_platform_base"))
> >> > - gr->create_platform_window =
> >> > - (void *)
> >> > eglGetProcAddress("eglCreatePlatformWindowSurfaceEXT");
> >> > - else
> >> > - weston_log("warning: EGL_EXT_platform_base not
> >> > supported.\n");
> >> > -
> >> > #ifdef EGL_MESA_configless_context
> >> > if (strstr(extensions, "EGL_MESA_configless_context"))
> >> > gr->has_configless_context = 1;
> >>
> >> ...which means this check may be accessing NULL extensions, no?
> >>
> >> Hmm, I wonder if the configless context check is looking at the right
> >> extensions string here.
> >>
> >> > @@ -2256,12 +2255,9 @@ gl_renderer_create(struct weston_compositor
> *ec,
> >> > EGLenum platform,
> >> > EGLint major, minor;
> >> > int supports = 0;
> >> >
> >> > - if (platform) {
> >> > + if (platform)
> >> > supports = gl_renderer_supports(
> >> > ec, platform_to_extension(platform));
> >> > - if (supports < 0)
> >> > - return -1;
> >> > - }
> >>
> >> Just make sure that if EXT_platform_base exists, but the specific
> >> platform we are looking for does not, we fail this function.
> >>
> >> You do want to keep the -1 return as fatal here. gl_renderer_supports()
> >> should already return 0 meaning "maybe" if client extensions cannot be
> >> queried. The supports function is very nicely documented.
> >>
> >> >
> >> > gr = zalloc(sizeof *gr);
> >> > if (gr == NULL)
> >>
> >>
> >> Thanks,
> >> pq
> >
> >
> >
> >
> > --
> > Regards,
> >
> > Manuel BACHMANN
> > Tizen Project
> > VANNES-FR
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
>
--
Regards,
*Manuel BACHMANN Tizen Project VANNES-FR*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150330/d9d7a279/attachment.html>
More information about the wayland-devel
mailing list