[vlc-devel] [vlc-commits] macosx: re-implement 'video-deco' support (close #6426)

[David Fuhrmann]

Hi Felix,

Nice feature. I wrote some remarks below.

Am 17.05.2012 um 16:01 schrieb Felix Paul Kühne:

> vlc | branch: master | Felix Paul Kühne <fkuehne at videolan.org> | Thu May 17 16:01:19 2012 +0200| [f01f7eca64bd6f007afeaaeed623990e76a43de8] | committer: Felix Paul Kühne
> 
> macosx: re-implement 'video-deco' support (close #6426)
> 
> disabling this option will get you a separate video output window without any visible playback controls or window border. resizing is supported through hotkeys and Lion's native mechanism if available. Control the playback through menu and main window controller. Note that 'embedded' is obviously ignored in this mode.
> 
>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=f01f7eca64bd6f007afeaaeed623990e76a43de8
> ---
> 
> modules/gui/macosx/MainWindow.h |    2 +
> modules/gui/macosx/MainWindow.m |  121 ++++++++++++++++++++++++++++-----------
> 2 files changed, 88 insertions(+), 35 deletions(-)


>      BOOL b_splitviewShouldBeHidden = NO;
> +
>      /* setup the styled interface */
> +    b_video_deco = config_GetInt( VLCIntf, "video-deco" );
> +    if (b_video_deco)
> +        b_nonembedded = YES;

Why do you set b_nonembeddded to yes here? This does not really make sense for me, b_nonembedded should be already set to the right value in setupVideoView.

> -    self = [super initWithContentRect:contentRect styleMask:styleMask
> +    b_video_deco = config_GetInt( VLCIntf, "video-deco" );
> +    if (b_video_deco)
> +        self = [super initWithContentRect:contentRect styleMask:styleMask
>                                backing:backingType defer:flag];
> +    else
> +        self = [super initWithContentRect:contentRect styleMask:styleMask
> +                                  backing:backingType defer:flag];

There is no functional change here. And "b_video_deco = ..." should be moved to the beginning of the method (its already used above).

Also, I noticed that you still see the fullscreen controller in fullscreen mode. Should that be also disabled in this mode?

Furthermore disabling video-deco and enabling lion fullscreen mode is not a good idea currently. Nonembedded window in conjunction with lion fullscreen is already blocked for the same reason (lion fullscreen is prioritized over nonembedded window here), therefore I would suggest to do the same for !(video-deco).

Best regards,
David