[vlc-devel] [3.0 PATCH 1/2] qt: create another indirection X11 window

[Alexandre Janniaux]

Hi,

No issue on XWayland with Sway. About #4606, did you have
time to check whether it was really still the case with
Qt5 or is it hypothetically?

As a workaround, LGTM, but I haven't checked with X11 while
trying to reproduce the bug yet.

Regards,
--
Alexandre Janniaux
Videolabs

On Fri, Feb 05, 2021 at 07:58:14PM +0200, remi at remlab.net wrote:
> From: Rémi Denis-Courmont <remi at remlab.net>
>
> The main window may be destroyed before the video window. This notably
> occurs if the user requests to close the main UI via window decorations.
> While Qt allows those requests to be rejected, doing so would
> reintroduce obnoxious bug #4606.
>
> The Qt-X11 display connection will be closed as well as it belongs to
> the QApplication instance.
>
> This creates a separate window belonging to a separate display
> connection, and which is not tied to the QApplication and QMainWindow
> instances. Unfortunately, this adds yet another connection to the X11
> display server in the VLC process in addition to QApplication's and the
> video display's. And that connection won't process events.
>
> Refs #21875.
> ---
>  .../gui/qt/components/interface_widgets.cpp   |  4 +-
>  modules/gui/qt/qt.cpp                         | 64 ++++++++++++++++++-
>  2 files changed, 64 insertions(+), 4 deletions(-)
>
> diff --git a/modules/gui/qt/components/interface_widgets.cpp b/modules/gui/qt/components/interface_widgets.cpp
> index ced7d9f8c7..74e82eab58 100644
> --- a/modules/gui/qt/components/interface_widgets.cpp
> +++ b/modules/gui/qt/components/interface_widgets.cpp
> @@ -228,13 +228,15 @@ QSize VideoWidget::physicalSize() const
>      return current_size;
>  }
>
> +void WindowResized(vout_window_t *, const QSize&);
> +
>  void VideoWidget::reportSize()
>  {
>      if( !p_window )
>          return;
>
>      QSize size = physicalSize();
> -    vout_window_ReportSize( p_window, size.width(), size.height() );
> +    WindowResized(p_window, size);
>  }
>
>  /* Set the Widget to the correct Size */
> diff --git a/modules/gui/qt/qt.cpp b/modules/gui/qt/qt.cpp
> index cbd197750a..6e7056b10b 100644
> --- a/modules/gui/qt/qt.cpp
> +++ b/modules/gui/qt/qt.cpp
> @@ -361,6 +361,7 @@ static void Abort( void *obj )
>
>  #if defined (QT5_HAS_X11)
>  # include <vlc_xlib.h>
> +# include <QX11Info>
>
>  static void *ThreadXCB( void *data )
>  {
> @@ -708,6 +709,10 @@ static void ShowDialog( intf_thread_t *p_intf, int i_dialog_event, int i_arg,
>   */
>  static int WindowControl( vout_window_t *, int i_query, va_list );
>
> +#ifdef QT5_HAS_X11
> +static Display *dpy;
> +#endif
> +
>  static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
>  {
>      if( cfg->is_standalone )
> @@ -740,15 +745,61 @@ static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
>      MainInterface *p_mi = p_intf->p_sys->p_mi;
>      msg_Dbg( p_wnd, "requesting video window..." );
>
> +#ifdef QT5_HAS_X11
> +    Window xid;
> +
> +    if (QX11Info::isPlatformX11())
> +    {
> +        dpy = XOpenDisplay(NULL);
> +        if (unlikely(dpy == NULL))
> +            return VLC_EGENERIC;
> +
> +        int snum = DefaultScreen(dpy);
> +        unsigned long black = BlackPixel(dpy, snum);
> +
> +        xid = XCreateSimpleWindow(dpy, RootWindow(dpy, snum),
> +                                  0, 0, cfg->width, cfg->height,
> +                                  0, black, black);
> +    }
> +#endif
> +
>      if( !p_mi->getVideo( p_wnd, cfg->width, cfg->height, cfg->is_fullscreen ) )
> +    {
> +#ifdef QT5_HAS_X11
> +        if (QX11Info::isPlatformX11())
> +            XCloseDisplay(dpy);
> +#endif
>          return VLC_EGENERIC;
> +    }
>
> +#ifdef QT5_HAS_X11
> +    if (QX11Info::isPlatformX11())
> +    {
> +        XReparentWindow(dpy, xid, p_wnd->handle.xid, 0, 0);
> +        XMapWindow(dpy, xid);
> +        XSync(dpy, True);
> +        p_wnd->handle.xid = xid;
> +    }
> +#endif
>      p_wnd->info.has_double_click = true;
>      p_wnd->control = WindowControl;
>      p_wnd->sys = (vout_window_sys_t*)p_mi;
>      return VLC_SUCCESS;
>  }
>
> +void WindowResized(vout_window_t *wnd, const QSize& size)
> +{
> +#ifdef QT5_HAS_X11
> +    if (QX11Info::isPlatformX11())
> +    {
> +        XResizeWindow(dpy, wnd->handle.xid, size.width(), size.height());
> +        XClearWindow(dpy, wnd->handle.xid);
> +        XSync(dpy, True);
> +    }
> +#endif
> +    vout_window_ReportSize(wnd, size.width(), size.height());
> +}
> +
>  static int WindowControl( vout_window_t *p_wnd, int i_query, va_list args )
>  {
>      MainInterface *p_mi = (MainInterface *)p_wnd->sys;
> @@ -779,8 +830,15 @@ static void WindowClose( vout_window_t *p_wnd )
>      if (unlikely(!active))
>      {
>          msg_Warn (p_wnd, "video already released");
> -        return;
>      }
> -    msg_Dbg (p_wnd, "releasing video...");
> -    p_mi->releaseVideo();
> +    else
> +    {
> +        msg_Dbg(p_wnd, "releasing video...");
> +        p_mi->releaseVideo();
> +    }
> +
> +#if defined (QT5_HAS_X11)
> +    if (QX11Info::isPlatformX11())
> +        XCloseDisplay(dpy);
> +#endif
>  }
> --
> 2.30.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel