[vlc-devel] [vlc-commits] qt: remove dubious and non functional call

[Romain Vimont]

On Mon, Jun 11, 2018 at 11:56:11PM +0200, erwan.tulou at gmail.com wrote:
> 
> 
> Le 11/06/2018 à 23:51, erwan.tulou at gmail.com a écrit :
> > Hello,
> > 
> > Le 11/06/2018 à 23:31, Jean-Baptiste Kempf a écrit :
> > > This commits is weird.
> > Without the patch, vlc on Wayland with Gnome Mutter freezes completely.
> > 
> > The original two lines seem to ensure that the stack is resized with the
> > size of the video widget, but the video widget is never inserted into
> > the Qmap.
> > So these two lines are just a call to resizeStack( -1, -1 ). At best, a
> > no op, at worst a error protocol for Wayland.

But when width != -1 && height != -1, it resizes VLC to the previous
component size.

For instance, the playlist has a size, you play a video, the window is
autoresized to fit the video, and when you stop, it is resized back to
the playlist size.

With your patch, it does not autoresize to the playlist size anymore.

Instead, I suggest to avoid the call only if width == -1 || height == -1
(or always call it and return early in resizeStack() in that case).

Regards

> > The thing is that autoresize seems to be implemented elsewhere, and
> > those two lines were just no op.
> > 
> > regards
> > > On Mon, 11 Jun 2018, at 23:12, Erwan Tulou wrote:
> > > > vlc | branch: master | Erwan Tulou <erwan10 at videolan.org> | Fri May 25
> > > > 12:37:21 2018 +0200| [5ea9b98e29b518dadfc10e710eaa18f2c42468e9] |
> > > > committer: Erwan Tulou
> > > > 
> > > > qt: remove dubious and non functional call
> > > > 
> > > > As no videoWidget is ever inserted into the stackWidgetsSizes QMap, a
> > > > default QSize is provided with width and height = -1. These
> > > > uninitialized
> > > > values are then propagated and end up as a no op at some point.
> > > > 
> > > > Yet, on Wayland, this -1 default values can end up being transmitted to
> > > > the Wayland server, that treats them as invalid values (protocol
> > > > error).
> > > > At least the Gnome server(mutter) does.
> > > > 
> > > > This fix simply removes the call.
> > > > 
> > > > > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=5ea9b98e29b518dadfc10e710eaa18f2c42468e9
> > > > > 
> > > > ---
> > > > 
> > > >   modules/gui/qt/main_interface.cpp | 2 --
> > > >   1 file changed, 2 deletions(-)
> > > > 
> > > > diff --git a/modules/gui/qt/main_interface.cpp b/modules/gui/qt/
> > > > main_interface.cpp
> > > > index 4b07c33bf9..d14ae32b74 100644
> > > > --- a/modules/gui/qt/main_interface.cpp
> > > > +++ b/modules/gui/qt/main_interface.cpp
> > > > @@ -678,8 +678,6 @@ inline void MainInterface::showTab( QWidget
> > > > *widget,
> > > > bool video_closing )
> > > >       }
> > > >         stackCentralW->setCurrentWidget( widget );
> > > > -    if( b_autoresize )
> > > > -        resizeStack( stackWidgetsSizes[widget].width(),
> > > > stackWidgetsSizes[widget].height() );
> > > >     #ifdef DEBUG_INTF
> > > >       msg_Dbg( p_intf, "Stack state changed to %s, index %i",
> > > > 
> > > > _______________________________________________
> > > > vlc-commits mailing list
> > > > vlc-commits at videolan.org
> > > > https://mailman.videolan.org/listinfo/vlc-commits
> > > 
> > 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel