[PATCH weston] gl-renderer: fix EGL initialization steps

[Pekka Paalanen]

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