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

[Derek Foreman]

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.  :)

> 
> 
> 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);
>>>
>>