[PATCH weston] gl-renderer: fix EGL initialization steps
Manuel Bachmann
manuel.bachmann at open.eurogiciel.org
Mon Mar 30 12:48:32 PDT 2015
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) ?
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)
>> >>
>> >>
>> >> 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*
>
--
Regards,
*Manuel BACHMANN Tizen Project VANNES-FR*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150330/f973a486/attachment-0001.html>
More information about the wayland-devel
mailing list