[PATCH weston] Remove cairo-gles2 support from toytoolkit

Wed Mar 16 10:28:40 UTC 2016

Hi Bryce

On Tue, 15 Mar 2016 19:04:41 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> This is a first step towards stopping use of cairo-egl in demo apps.
> See:  https://bugs.freedesktop.org/show_bug.cgi?id=83878

Also note that is a GSoC project idea:
https://phabricator.freedesktop.org/w/wayland/gsoc_2016/ideas/

I'd like to see the GSoC students and their projects announced before
other people tackle these topics, since they are not urgent.

> The nested client had required cairo-gles2 but looks like it ought to
> work with plain cairo-gl (at least, it compiles fine).

A compile test is not fine. The failure mode of badly mixing GL and
GLES2 in the same process (it may be enough to simply load both
libGL.so and libGLESv2.so to cause problems) is a runtime crash or
misbehaviour. Yes, it's the worst kind.

nested.c is clearly written to call GLES2 functions. Mesa might expose
similar named functions in libGL, but it's not supposed to run and work.
nested.c relies on toytoolkit to pull in libGLESv2.so and not libGL.so.
It certainly does not help that both GL and GLES2 have some same named
functions, too, which may then explode when called under a wrong API
context.

This GL vs. GLES2 issue is probably also why Cairo itself has to choose
at build time which one it supports, since the infrastructure required
to choose one at runtime with a single binary is too much (or use
Waffle).

Therefore NAK for this patch. You are essentially trying convert
nested.c from GLESv2 to GL, but many embedded platforms expose GLESv2
and not GL, so conversion in this direction makes less sense than in
the other direction. Though we probably don't want to convert our GL
demos to GLESv2 either, since it's good to have demos showing that also
desktop-GL works with Wayland.

Thanks,
pq

> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> ---
>  Makefile.am      |  2 --
>  clients/window.c | 19 +------------------
>  configure.ac     | 29 ++++-------------------------
>  3 files changed, 5 insertions(+), 45 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index fe08d94..3ac4da2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -626,7 +626,6 @@ weston_scaler_SOURCES = clients/scaler.c
>  weston_scaler_LDADD = libtoytoolkit.la
>  weston_scaler_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
>  
> -if HAVE_CAIRO_GLESV2
>  demo_clients += weston-nested weston-nested-client
>  
>  weston_nested_SOURCES = 				\
> @@ -638,7 +637,6 @@ weston_nested_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
>  weston_nested_client_SOURCES = clients/nested-client.c
>  weston_nested_client_LDADD = $(SIMPLE_EGL_CLIENT_LIBS) -lm
>  weston_nested_client_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
> -endif
>  
>  weston_eventdemo_SOURCES = 				\
>  	clients/eventdemo.c				\
> diff --git a/clients/window.c b/clients/window.c
> index ced867e..b7d831e 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -44,12 +44,7 @@
>  #ifdef HAVE_CAIRO_EGL
>  #include <wayland-egl.h>
>  
> -#ifdef USE_CAIRO_GLESV2
> -#include <GLES2/gl2.h>
> -#include <GLES2/gl2ext.h>
> -#else
>  #include <GL/gl.h>
> -#endif
>  #include <EGL/egl.h>
>  #include <EGL/eglext.h>
>  
> @@ -5537,11 +5532,7 @@ init_egl(struct display *d)
>  	EGLint major, minor;
>  	EGLint n;
>  
> -#ifdef USE_CAIRO_GLESV2
> -#  define GL_BIT EGL_OPENGL_ES2_BIT
> -#else
> -#  define GL_BIT EGL_OPENGL_BIT
> -#endif
> +#define GL_BIT EGL_OPENGL_ES2_BIT
>  
>  	static const EGLint argb_cfg_attribs[] = {
>  		EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
> @@ -5554,16 +5545,8 @@ init_egl(struct display *d)
>  		EGL_NONE
>  	};
>  
> -#ifdef USE_CAIRO_GLESV2
> -	static const EGLint context_attribs[] = {
> -		EGL_CONTEXT_CLIENT_VERSION, 2,
> -		EGL_NONE
> -	};
> -	EGLint api = EGL_OPENGL_ES_API;
> -#else
>  	EGLint *context_attribs = NULL;
>  	EGLint api = EGL_OPENGL_API;
> -#endif
>  
>  	d->dpy =
>  		weston_platform_get_egl_display(EGL_PLATFORM_WAYLAND_KHR,
> diff --git a/configure.ac b/configure.ac
> index 9e8115a..87b9856 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -258,7 +258,7 @@ if test x$enable_screen_sharing = xyes; then
>  fi
>  
>  AC_ARG_WITH(cairo,
> -	    AS_HELP_STRING([--with-cairo=@<:@image|gl|glesv2@:>@]
> +	    AS_HELP_STRING([--with-cairo=@<:@image|gl@:>@]
>  			   [Which Cairo renderer to use for the clients]),
>  			   [],[with_cairo="image"])
>  
> @@ -269,27 +269,9 @@ if test "x$with_cairo" = "xgl"; then
>  	cairo_modules="cairo-gl"
>  	AC_MSG_WARN([The --with-cairo=gl option can cause increased resource usage and potential instability, and thus is not recommended.  It is needed only for a few special demo programs.])
>  else
> -if test "x$with_cairo" = "xglesv2"; then
> -	cairo_modules="cairo-glesv2"
> -	AC_MSG_WARN([The --with-cairo=gles2 option can cause increased resource usage and potential instability, and thus is not recommended.  It is needed only for a few special demo programs.])
> -else
>  	AC_ERROR([Unknown cairo renderer requested])
>  fi
>  fi
> -fi
> -
> -# Included for legacy compat
> -AC_ARG_WITH(cairo-glesv2,
> -            AS_HELP_STRING([--with-cairo-glesv2],
> -                           [Use GLESv2 cairo]))
> -if test "x$with_cairo_glesv2" = "xyes"; then
> -  cairo_modules="cairo-glesv2"
> -  with_cairo="glesv2"
> -fi
> -
> -if test "x$cairo_modules" = "xcairo-glesv2"; then
> -AC_DEFINE([USE_CAIRO_GLESV2], [1], [Use the GLESv2 GL cairo backend])
> -fi
>  
>  PKG_CHECK_MODULES(PIXMAN, [pixman-1])
>  PKG_CHECK_MODULES(PNG, [libpng])
> @@ -391,8 +373,8 @@ if test x$enable_clients = xyes; then
>    PKG_CHECK_MODULES(SERVER, [wayland-server])
>    PKG_CHECK_MODULES(WESTON_INFO, [wayland-client >= 1.5.91])
>  
> -  # Only check for cairo-egl if a GL or GLES renderer requested
> -  AS_IF([test "x$cairo_modules" = "xcairo-gl" -o "x$cairo_modules" = "xcairo-glesv2"], [
> +  # Only check for cairo-egl if a GL renderer requested
> +  AS_IF([test "x$cairo_modules" = "xcairo-gl"], [
>      PKG_CHECK_MODULES(CAIRO_EGL, [wayland-egl egl cairo-egl >= 1.11.3 $cairo_modules],
>                        [have_cairo_egl=yes], [have_cairo_egl=no])
>      AS_IF([test "x$have_cairo_egl" = "xyes"],
> @@ -423,14 +405,11 @@ fi
>  
>  AM_CONDITIONAL(HAVE_PANGO, test "x$have_pango" = "xyes")
>  
> -AM_CONDITIONAL(HAVE_CAIRO_GLESV2,
> -	       [test "x$have_cairo_egl" = "xyes" -a "x$cairo_modules" = "xcairo-glesv2" -a "x$enable_egl" = "xyes"])
> -
>  AM_CONDITIONAL(BUILD_FULL_GL_CLIENTS,
>  	       test x$cairo_modules = "xcairo-gl" -a "x$have_cairo_egl" = "xyes" -a "x$enable_egl" = "xyes")
>  
>  AM_CONDITIONAL(BUILD_SUBSURFACES_CLIENT,
> -	       [test '(' "x$have_cairo_egl" != "xyes" -o "x$cairo_modules" = "xcairo-glesv2" ')' -a "x$enable_simple_egl_clients" = "xyes"])
> +	       [test "x$have_cairo_egl" != "xyes" -a "x$enable_simple_egl_clients" = "xyes"])
>  
>  AM_CONDITIONAL(ENABLE_DESKTOP_SHELL, true)
>  

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160316/bd7eca27/attachment-0001.sig>