[PATCH weston 2/3] clients: Require EGL_MIN_SWAP_INTERVAL to be 0 for subsurfaces

[Jonas Ådahl]

On Tue, Jan 27, 2015 at 09:29:52AM -0600, Derek Foreman wrote:
> On 26/01/15 08:24 PM, Jonas Ådahl wrote:
> > On Mon, Jan 26, 2015 at 02:27:51PM -0600, Derek Foreman wrote:
> >> On 26/01/15 04:19 AM, Jonas Ådahl wrote:
> >>> Warn and fail when trying to create sub surfaces when swap interval 0 is
> >>> not supported by the EGL platform.
> >>>
> >>> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> >>> ---
> >>>  clients/nested.c      |  1 +
> >>>  clients/subsurfaces.c |  2 ++
> >>>  clients/window.c      | 19 +++++++++++++++++++
> >>>  3 files changed, 22 insertions(+)
> >>>
> >>> diff --git a/clients/nested.c b/clients/nested.c
> >>> index f094237..696b727 100644
> >>> --- a/clients/nested.c
> >>> +++ b/clients/nested.c
> >>> @@ -970,6 +970,7 @@ ss_surface_init(struct nested_surface *surface)
> >>>  		window_add_subsurface(nested->window,
> >>>  				      nested,
> >>>  				      SUBSURFACE_SYNCHRONIZED);
> >>> +	assert(ss_surface->widget);
> >>
> >> Should we really be using assert() for these?
> > 
> > I'll change the API to make users need to query if subsurfaces are
> > supported or not, making the asserts actual programming errors. Do you
> > think that is enough? Otherwise we need to handle this error, meaning
> > adding an error path that needs to kill the nested client, and we don't
> > have "internal error" error code to kill client connections with.
> 
> Honestly, I just figured something like:
> if (!ss_surface->widget) {
> 	fprintf(stderr, "Eternal sadness :(\n");
> 	exit(EXIT_FAILURE);
> }
> 
> (And maybe a warn_unused_result attribute if we want to be really
> paranoid...)
> 
> I'm happy with exit() on failure, but if we use asserts and compile with
> NDEBUG then it just turns into a segfault on failure.
> 
> What you suggested sounds nice, but I think you know better than I do if
> it's overkill or not.  :)

I sent an updated version, but did the mistake of making it a reply to
patch 1/3. It has slightly better error handling.


Jonas

> 
> > 
> > 
> > Jonas
> > 
> >>
> >>>  
> >>>  	widget_set_use_cairo(ss_surface->widget, 0);
> >>>  
> >>> diff --git a/clients/subsurfaces.c b/clients/subsurfaces.c
> >>> index 833030f..e0a5010 100644
> >>> --- a/clients/subsurfaces.c
> >>> +++ b/clients/subsurfaces.c
> >>> @@ -500,6 +500,7 @@ triangle_create(struct window *window, struct egl_state *egl)
> >>>  	tri->egl = egl;
> >>>  	tri->widget = window_add_subsurface(window, tri,
> >>>  		int_to_mode(option_triangle_mode));
> >>> +	assert(tri->widget);
> >>>  	widget_set_use_cairo(tri->widget, 0);
> >>>  	widget_set_resize_handler(tri->widget, triangle_resize_handler);
> >>>  	widget_set_redraw_handler(tri->widget, triangle_redraw_handler);
> >>> @@ -737,6 +738,7 @@ demoapp_create(struct display *display)
> >>>  
> >>>  	app->subsurface = window_add_subsurface(app->window, app,
> >>>  		int_to_mode(option_red_mode));
> >>> +	assert(app->subsurface);
> >>>  	widget_set_redraw_handler(app->subsurface, sub_redraw_handler);
> >>>  	widget_set_resize_handler(app->subsurface, sub_resize_handler);
> >>>  
> >>> diff --git a/clients/window.c b/clients/window.c
> >>> index b45b499..beb28ca 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,
> >>> @@ -4844,6 +4847,22 @@ window_add_subsurface(struct window *window, void *data,
> >>>  	struct surface *surface;
> >>>  	struct wl_surface *parent;
> >>>  	struct wl_subcompositor *subcompo = window->display->subcompositor;
> >>> +	GLint min_swap_interval;
> >>> +
> >>> +	if (EGL_FALSE == eglGetConfigAttrib(window->display->dpy,
> >>> +					    window->display->argb_config,
> >>> +					    EGL_MIN_SWAP_INTERVAL,
> >>> +					    &min_swap_interval)) {
> >>> +		fprintf(stderr, "failed to add subsurface, couldn't retrieve "
> >>> +			"EGL_MIN_SWAP_INTERVAL\n");
> >>> +		return NULL;
> >>> +	}
> >>> +
> >>> +	if (min_swap_interval > 0) {
> >>> +		fprintf(stderr, "failed to add subsurface, no "
> >>> +			"EGLSwapInterval(0) support\n");
> >>> +		return NULL;
> >>> +	}
> >>>  
> >>>  	surface = surface_create(window);
> >>>  	surface->buffer_type = window_get_buffer_type(window);
> >>>
> >>
>