[PATCH weston] gl-renderer: fix EGL initialization steps
Manuel Bachmann
manuel.bachmann at open.eurogiciel.org
Thu Apr 2 22:03:05 PDT 2015
Agreed !
I am going send the patch again, same name ("gl-renderer: fix EGL
initialization steps") but modified in this sense and with another commit
message,
Regards,
Manuel
2015-04-02 11:51 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> On Mon, 30 Mar 2015 21:48:32 +0200
> Manuel Bachmann <manuel.bachmann at open.eurogiciel.org> wrote:
>
> > OK. Found it, thanks to your hint, Daniel.
> >
> > This version of Mesa (10.1.3) does not feature these extensions.
> >
> > They were added here :
> >
> http://cgit.freedesktop.org/mesa/mesa/commit/src/egl/main/eglglobals.c?id=468cc866b4b308cee40470f06b31002c6c56da96
> >
> > Considering that recent distributions, such as Ubuntu 14.04 LTS (Debian,
> > RHEL.. are even older) may not package a sufficiently recent version of
> > Mesa, maybe it is wise to ignore the test if "extensions" is only
> > "EGL_EXT_client_extensions" (which should not happen with recent Mesa,
> > because at least one platform should be supported there) ?
>
> Hi,
>
> I think the correct test is to check if the string contains
> "EGL_EXT_platform_base". If the client extensions string cannot be
> queried or does not contain "EGL_EXT_platform_base", then supports()
> should return 0 to indicate "maybe", which means we fall back to the old
> way.
>
>
> Thanks,
> pq
>
> > 2015-03-30 21:38 GMT+02:00 Manuel Bachmann <
> > manuel.bachmann at open.eurogiciel.org>:
> >
> > > 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)
> > >> >>
> > >> >>
>
--
Regards,
*Manuel BACHMANN Tizen Project VANNES-FR*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150403/37f58757/attachment.html>
More information about the wayland-devel
mailing list