weston-simple-egl fullscreen broken?

[Vincent ABRIOU]

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.

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

One point is that starting weston-simple-egl in normal mode (250x250) 
and then switching to fullscreen by pressing F11 key is working well 
with Mali library.

Vincent

>
> Thanks,
> pq
>