weston-simple-egl fullscreen broken?

[Pekka Paalanen]

On Thu, 9 Mar 2017 15:01:03 +0000
Vincent ABRIOU <vincent.abriou at st.com> wrote:

> On 03/09/2017 02:48 PM, Pekka Paalanen wrote:
> > 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?  
> 
> After a check, Mali library is waiting for a wl_surface_frame callback.
> This callback is used to warn mali that the swap has been completed.

Oh, just occurred to me - if you still have your patch to not issue
wl_surface.commit from eglSwapBuffers, then the compositor will never
ack the Mali library's wl_surface.frame either. :-)

Btw. it does not mean "the swap has been completed", or, well, not the
swap you just posted. The previous swap has been completed. It just
means the compositor has taken the frame into compositing and it would
be an appropriate time to start drawing again to *some other* buffer.

Buffer availability is signalled by wl_buffer.release instead. Frame
callbacks provide throttling, release events tell you which buffer is
free for re-use.


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/ae1c5ca8/attachment.sig>