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

[Pekka Paalanen]

On Fri, 21 Aug 2015 22:32:40 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> On Fri, Aug 21, 2015 at 04:05:34PM +0300, Pekka Paalanen wrote:
> > 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(+)

> > > +{
> > > +	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.
> 
> Right. The purpose was to not knowingly let the subsurface demo dead
> lock. Now it will warn it might dead lock, and then dead lock. The point
> was to make it clear that one cannot use an EGL surface as a subsurface
> reliably if eglSwapInterval(0) is not supported. But since it's a demo
> client, I guess it doesn't really matter if we dead lock, at least when
> we warn about it first.

Yeah. We'd need even more code in toytoolkit to ensure that the app
actually did set swap interval to zero, or set it in the toolkit. I
think it would be going too far in pretending that toytoolkit is
actually a robust toolkit.


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/20150824/6761b6c4/attachment.sig>