weston-simple-egl fullscreen broken?

[Pekka Paalanen]

On Thu, 9 Mar 2017 12:53:35 +0000
Vincent ABRIOU <vincent.abriou at st.com> wrote:

> Hi Pekka,
> 
> On 03/09/2017 11:32 AM, Pekka Paalanen wrote:
> > On Wed, 8 Mar 2017 17:16:31 +0000
> > Vincent ABRIOU <vincent.abriou at st.com> wrote:
> >  
> >> Hi,
> >>
> >> I have investigate deeper the issue and it comes from the Mali400
> >> wayland library (at least in the r6p1-01rel0 version).
> >>
> >> Actually, the Mali400 wayland library is creating very early the output
> >> surface taking into account the 250x250 default size because fullscreen
> >> information is not yet available at this moment.
> >>
> >> Code from simple-egl.c:
> >>
> >> static void
> >> create_surface(struct window *window)
> >> {
> >> 	struct display *display = window->display;
> >> 	EGLBoolean ret;
> >>
> >> 	window->surface =
> >> 		wl_compositor_create_surface(display->compositor);
> >>
> >> 	window->native =
> >> 		wl_egl_window_create(window->surface,
> >> 				     window->geometry.width,
> >> 				     window->geometry.height);
> >> 	window->egl_surface =
> >> 		weston_platform_create_egl_surface(display->egl.dpy,
> >> 						   display->egl.conf,
> >> 						   window->native,
> >> 						   NULL);
> >> 	/* Mali400 has already created its first 250x250 surface */
> >>
> >> 	...
> >> 	...
> >>
> >> 	/* Full screen information comes too late */
> >> 	if (window->fullscreen)
> >> 		zxdg_toplevel_v6_set_fullscreen(window->xdg_toplevel,
> >> 		NULL);
> >> }
> >>
> >>
> >> The mali400 library lock occurs while the eglSwapBuffer when the 250x250
> >> surface need to be committed whereas a fullscreen surface is expected.
> >>
> >> I fix this issue in the Mali400 wayland library by avoiding to commit
> >> the surface if the size does not match the expected output surface size.  
> >
> > Hi,
> >
> > you do know that sometimes not committing from eglSwapBuffers() is
> > going to break applications, right?
> >
> > The commit is used for a lot more than just pushing a buffer.
> >
> > Applications can be expecting other state to be latched in by calling
> > eglSwapBuffers() since it must never return without having sent the
> > wl_surface.commit, and wait for events triggered by that commit before
> > continuing.  
> 
> Ok I see. So my patch is unsafe.
> 
> >
> > In particular, applications that throttle to wl_surface.frame callbacks
> > would freeze forever if any single commit was skipped.
> >  
> >> Once the 250x250 surface is skipped during eglSwapBuffer it is then
> >> destroyed and created again with the right fullscreen parameters.  
> >  
> >>>>>> On 6 February 2017 at 16:56, Fabien DESSENNE <fabien.dessenne at st.com>  
> >>>> wrote:  
> >>>>>>> I remember I used to get « weston-simple-egl -f » working fine.
> >>>>>>> But it does not work anymore : nothing is displayed. From the logs
> >>>>>>> I can see (among others) zxdg_toplevel_v6.configure and
> >>>>>>> wl_surface.commit Testing with another client works fine:
> >>>>>>> weston-terminal -f -> OK There may be something wrong with my
> >>>>>>> environment since I am testing with the Daniel’s atomic version
> >>>>>>> (forked from weston-1.12.0+), but I guess this is broken also with the official  
> >>>> version.  
> >
> > I think there are two different things here.
> >
> > First, I believe that simple-egl is maybe a little off when starting
> > with -f. It could negotiate the fullscreen size before creating the
> > wl_egl_window. It actually does already use window->wait_for_configure
> > AFAICT, it would just need to postpone creating the wl_egl_window later
> > (e.g. on-demand just before calling redraw()), which means it needs to
> > postpone also GL init and the other dependent things.
> >
> > The code guarantees, that there are no draw calls before the call to
> > wl_egl_window_resize() setting the size for the first frame. This works
> > fine for EGL implementations that get the buffer on the first draw call
> > or EGL_EXT_buffer_age query, but unfortunately we don't probably have a
> > spec to point to and say the EGL implementation got it wrong.
> >
> > Second, using a wrong sized buffer for the first commit should cause a
> > glitch at most, and start running just fine fullscreen after the first
> > correctly sized buffer has been committed. But it sounds like it just
> > fails to show at all, ever?
> >  
> 
> I was expecting the first frame is wrong and then others will be good. 
> But as you mention, the Mali400 library is stuck and any of the frames 
> are shown. The second time the weston-simple-egl redraw function is 
> called it is stuck in glClear.

I wonder if that's actually Weston's problem/feature of not mapping a
fullscreen window with a "wrong" size... Quentin?

Vincent, would you be able to check what the Mali library is waiting
for? Is it a wl_surface.frame callback or something else?

In any case, I think we are assuming in lots of places that EGL
allocates the buffer on the first draw call since the size might change
until then. After all, it is quite likely that something triggers a
resize after you have shown a frame, i.e. after eglSwapBuffers. One
should not need to draw and commit one more buffer in the old size to
get a new size. You can't assume all GL apps are games that just push
frames unthrottled as fast as possible, some might actually need user
input before updating again since there is nothing to change on screen
otherwise.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170309/182b3a6a/attachment.sig>