[PATCH weston v2] clients: Require EGL_MIN_SWAP_INTERVAL to be 0 for subsurfaces

[Pekka Paalanen]

On Tue, 27 Jan 2015 10:47:29 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> Warn and fail when assert to create sub surfaces when swap interval 0 is
> not supported by the EGL platform.
> 
> Programs wanting to add subsurfaces need to check whether it is
> subsurfaces are supported by the display with the new
> display_is_subsurfaces_supported() function.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> 
> Changed since v1:
> 
> Coding style fix.
> 
> Require clients to check whether subsurfaces is supported or not prior to
> adding them.
> 
> 
>  clients/nested.c      |  6 ++++++
>  clients/subsurfaces.c |  6 ++++++
>  clients/window.c      | 30 ++++++++++++++++++++++++++++++
>  clients/window.h      |  4 ++++
>  4 files changed, 46 insertions(+)
> 
> diff --git a/clients/nested.c b/clients/nested.c
> index f094237..f52173e 100644
> --- a/clients/nested.c
> +++ b/clients/nested.c
> @@ -1127,6 +1127,12 @@ main(int argc, char *argv[])
>  		return -1;
>  	}
>  
> +	if (!display_is_subsurfaces_supported(display)) {
> +		fprintf(stderr, "subsurface support required\n");
> +		display_destroy(display);
> +		return -1;
> +	}
> +
>  	nested = nested_create(display);
>  
>  	launch_client(nested, "weston-nested-client");
> diff --git a/clients/subsurfaces.c b/clients/subsurfaces.c
> index fcbe496..8376399 100644
> --- a/clients/subsurfaces.c
> +++ b/clients/subsurfaces.c
> @@ -792,6 +792,12 @@ main(int argc, char *argv[])
>  		return -1;
>  	}
>  
> +	if (!display_is_subsurfaces_supported(display)) {
> +		fprintf(stderr, "subsurface support required\n");
> +		display_destroy(display);
> +		return -1;
> +	}
> +
>  	app = demoapp_create(display);
>  
>  	display_run(display);
> diff --git a/clients/window.c b/clients/window.c
> index b45b499..3b95896 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -2023,6 +2023,9 @@ window_create_tooltip(struct tooltip *tooltip)
>  
>  	tooltip->widget = window_add_subsurface(parent->window, tooltip, SUBSURFACE_DESYNCHRONIZED);
>  
> +	if (!tooltip->widget)
> +		return -1;
> +
>  	extents = get_text_extents(display, tooltip);
>  	widget_set_redraw_handler(tooltip->widget, tooltip_redraw_handler);
>  	widget_set_allocation(tooltip->widget,
> @@ -4845,6 +4848,8 @@ window_add_subsurface(struct window *window, void *data,
>  	struct wl_surface *parent;
>  	struct wl_subcompositor *subcompo = window->display->subcompositor;
>  
> +	assert(display_is_subsurfaces_supported(window->display));
> +
>  	surface = surface_create(window);
>  	surface->buffer_type = window_get_buffer_type(window);
>  	widget = widget_create(window, surface, data);
> @@ -5620,6 +5625,31 @@ display_has_subcompositor(struct display *display)
>  	return display->subcompositor != NULL;
>  }
>  
> +bool
> +display_is_subsurfaces_supported(struct display *display)

Hi,

this name is quite misleading. It's more like "is EGL supported for
sub-surfaces?"

> +{
> +	GLint min_swap_interval;
> +
> +	if (!display_has_subcompositor(display)) {
> +		return false;
> +	}
> +
> +	if (!eglGetConfigAttrib(display->dpy,
> +				display->argb_config,
> +				EGL_MIN_SWAP_INTERVAL,
> +				&min_swap_interval)) {
> +		fprintf(stderr, "failed to retrieve EGL_MIN_SWAP_INTERVAL\n");
> +		return false;
> +	}
> +
> +	if (min_swap_interval > 0) {
> +		fprintf(stderr, "warning: EGLSwapInterval(0) not supported\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  cairo_device_t *
>  display_get_cairo_device(struct display *display)
>  {
> diff --git a/clients/window.h b/clients/window.h
> index 5247f19..b47e0b2 100644
> --- a/clients/window.h
> +++ b/clients/window.h
> @@ -28,6 +28,7 @@
>  #include <xkbcommon/xkbcommon.h>
>  #include <wayland-client.h>
>  #include <cairo.h>
> +#include <stdbool.h>
>  #include "../shared/config-parser.h"
>  #include "../shared/zalloc.h"
>  
> @@ -84,6 +85,9 @@ display_get_display(struct display *display);
>  int
>  display_has_subcompositor(struct display *display);
>  
> +bool
> +display_is_subsurfaces_supported(struct display *display);
> +
>  cairo_device_t *
>  display_get_cairo_device(struct display *display);
>  

clients/subsurfaces.c already has egl_make_swapbuffers_nonblock(), and
I'm not sure nested.c even needs this. Nested.c does not attempt to set
zero swap interval either.

Therefore I don't think this patch really adds anything worthwhile.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150821/5a178dce/attachment-0001.sig>