[vlc-devel] [PATCH] xdg_shell needs to be fully configured

[erwan.tulou at gmail.com]

Hello,

On 18/06/2018 20:22, Rémi Denis-Courmont wrote:
> Le tiistaina 12. kesäkuuta 2018, 17.14.43 EEST erwan.tulou at gmail.com a écrit :
>> Hello,
>>
>>      One more (important) patch to improve xdg_shell.
>>
>>      the Wayland documentation makes it explicit that no buffer must be
>> attached or committed before an xdg surface is fully configured.
>> (ack_configure sent).
>>      Therefore, the xdg_shell vout window must ensure that the
>> ack_configure() has been sent before handing over the surface handler to
>> be used.
>>
>>      On Weston, the bug is straightforward with the following message :
>> error: xdg_surface at 9: error 3: xdg_surface has never been configured
> It is true that VLC violates the letter of the protocol specification, but I
> cannot reproduce the problem with Weston 4.0.0

There was also a problem with Gnome Mutter (tested on Debian Buster). 
Though no error messages were issued, if not fully configured, the xdg 
toplevel  couldn't remember its previous geometry after returning from 
fullscreen.

The test case was the following:  launch a video, go fullscreen, come 
back normal :

With the patch :
The xdg toplevel remembers its geometry prior to fullscreen. The video 
comes back to the right size without any need to reconfigure it.

Without the patch:
The video doesn't come back. Actually, the geometry is reset to width = 
1, height = 1. It seems that since the xdg toplevel did not reach full 
configuration, a default w=1, h=1 is applied instead of the previous size.
>
> In any case, it looks to me that the patch will lock-up the wl_shell plugin.
>
The patch will have indeed the vout thread wait in the Open() function 
of the vout window plugin till the xdg surface configure callback is 
received and executed. As long as the wl_display_dispatch() is done 
before vlc_clone, no race is to be afraid of.  This is actually a 
pattern similar to the qt or skins2 vout window that block the vout 
thread till a fully configured window handler is ready.

I think the issue must be addressed. The way to address it is not 
debatable. The patch I sent is one viable solution. Another one could be 
based on the usual vlc cond_wait/lock mechanisms.
Also the patch I sent should definitely check wl_display_dispatch() for 
possible errors to avoid an infinite loop, which I forgot.

Rgds

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus