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

[erwan.tulou at gmail.com]

Hello,

Le 12/06/2018 à 08:57, Romain Vimont a écrit :
> On Tue, Jun 12, 2018 at 08:49:54AM +0200, Romain Vimont wrote:
>> 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).
> Or better than checking width and height, check QSize::isValid()
> instead.

Yes, this call could be valid. Testing isValid() is the clean way to go. 
I just applied a correction to the patch. Thanks


>> 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
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel