[vlc-devel] [RFC PATCH] qt: Use XCB events to notify of a video widget resize

[Hugo Beauzée-Luyssen]

Hi,

On Tue, Apr 25, 2017, at 06:36 PM, Rémi Denis-Courmont wrote:
> Le tiistaina 25. huhtikuuta 2017, 11.12.10 EEST Hugo Beauzée-Luyssen a
> écrit :
> > This obviously lacks support for wayland,
> 
> At least Fedora has switched, so Wayland is no longer merely optional,
> AFAICT.

Sure, but this was just a RFC to check if that would be an appropriate
way to fix #18211

> 
> > and seems to be unrequired on windows.
> 
> That is rather suspicious/dubious. Maybe the Windows video outputs
> duplicate 
> size events internally, which would be another bug.
> 

It doesn't seem to do so. It might very well be that windows doesn't
wait for the change to have been processed to report the new size, but
frankly, I have no idea.
In any case, I'll try to replicate the same behavior for windows, so
that all the resize handling gets done in the same place.

> > Ref #18211
> > ---
> >  configure.ac                                    |  1 +
> >  modules/gui/qt/components/interface_widgets.cpp | 23
> > +++++++++++++++++++++++ modules/gui/qt/components/interface_widgets.hpp | 
> > 8 ++++++++
> >  3 files changed, 32 insertions(+)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index cc54fad48c..696ef77749 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -3010,6 +3010,7 @@ AS_IF([test "${enable_xcb}" != "no"], [
> >    PKG_CHECK_MODULES(XCB_KEYSYMS, [xcb-keysyms >= 0.3.4],
> > [have_xcb_keysyms="yes"], [ AC_MSG_WARN([${XCB_KEYSYMS_PKG_ERRORS}. Hotkeys
> > will not work.]) ])
> > +  AC_DEFINE([HAVE_XCB], [1], [Define to 1 if XCB is available.])
> >  ])
> >  AM_CONDITIONAL([HAVE_XCB], [test "${have_xcb}" = "yes"])
> >  AM_CONDITIONAL([HAVE_XCB_KEYSYMS], [test "${have_xcb_keysyms}" = "yes"])
> 
> Why clutter configure.ac and config.h when you already have a
> conditional?

I tend to favor adding configuration defines in config.h rather than in
the Makefile.am, although I'll grant that it's more localized and
doesn't require a full rebuild when it's specified there. I'll make the
change.

> 
> > diff --git a/modules/gui/qt/components/interface_widgets.cpp
> > b/modules/gui/qt/components/interface_widgets.cpp index
> > 081985dc2c..56869ff87d 100644
> > --- a/modules/gui/qt/components/interface_widgets.cpp
> > +++ b/modules/gui/qt/components/interface_widgets.cpp
> > @@ -53,6 +53,9 @@
> >  #if defined (QT5_HAS_X11)
> >  # include <X11/Xlib.h>
> >  # include <QX11Info>
> > +# if HAVE_XCB
> > +# include <xcb/xproto.h>
> > +#endif
> >  #endif
> >  #ifdef QT5_HAS_WAYLAND
> >  # include QPNI_HEADER
> > @@ -257,12 +260,32 @@ void VideoWidget::setSize( unsigned int w, unsigned
> > int h ) sync();
> >  }
> > 
> > +#if RESIZE_FROM_NATIVE_EVENTS
> > +bool VideoWidget::nativeEvent( const QByteArray& eventType, void* message,
> > long* ) +{
> > +#if HAVE_XCB
> > +    if ( eventType == "xcb_generic_event_t" )
> > +    {
> > +        const xcb_generic_event_t* xev = reinterpret_cast<const
> > xcb_generic_event_t*>( message ); +
> > +        if ( xev->response_type == XCB_CONFIGURE_NOTIFY )
> > +        {
> > +            msg_Err( p_intf, "COUCOU");
> > +            reportSize();
> > +        }
> > +    }
> > +#endif
> > +    // Let Qt handle that event in any case
> > +    return false;
> > +}
> > +#else
> >  void VideoWidget::resizeEvent( QResizeEvent *event )
> >  {
> >      QWidget::resizeEvent( event );
> > 
> >      reportSize();
> >  }
> > +#endif
> > 
> >  int VideoWidget::qtMouseButton2VLC( Qt::MouseButton qtButton )
> >  {
> > diff --git a/modules/gui/qt/components/interface_widgets.hpp
> > b/modules/gui/qt/components/interface_widgets.hpp index
> > ca2926c49c..1a1dbdfe5c 100644
> > --- a/modules/gui/qt/components/interface_widgets.hpp
> > +++ b/modules/gui/qt/components/interface_widgets.hpp
> > @@ -45,6 +45,10 @@
> >  #include <QPropertyAnimation>
> >  #include <QLinkedList>
> > 
> > +#if HAVE_XCB
> > +# define RESIZE_FROM_NATIVE_EVENTS 1
> > +#endif
> > +
> >  class QMenu;
> >  class QSlider;
> >  class QWidgetAction;
> > @@ -69,7 +73,11 @@ protected:
> >          return NULL;
> >      }
> > 
> > +#if RESIZE_FROM_NATIVE_EVENTS
> > +    bool nativeEvent(const QByteArray &eventType, void *message, long
> > *result) Q_DECL_OVERRIDE; +#else
> >      virtual void resizeEvent(QResizeEvent *) Q_DECL_OVERRIDE;
> > +#endif
> >      void mousePressEvent(QMouseEvent *) Q_DECL_OVERRIDE;
> >      void mouseMoveEvent(QMouseEvent *) Q_DECL_OVERRIDE;
> >      void mouseReleaseEvent(QMouseEvent *) Q_DECL_OVERRIDE;
> 
> 
> -- 
> 雷米‧德尼-库尔蒙
> https://www.remlab.net/
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

Regards,

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr