[MPlayer-dev-eng] GUI and vo_x11_create_vo_window()

[Ingo Brückl]

Reimar Döffinger wrote on Fri, 19 Aug 2011 08:04:09 +0200:

> On 19 Aug 2011, at 00:08, Ingo Brückl <ib at wupperonline.de> wrote:
>> Reimar Döffinger wrote on Thu, 18 Aug 2011 23:39:21 +0200:
>>
>>> I still do not see the difference in behaviour between the global
>>> fullscreen variable and messing with the flags.
>>
>>> Not really, it goes through the fullscreen global variable first.
>>> And I don't see what you think the advantage of that flag manipulation
>>> is.
>>
>> This is the flow:
>>
>> config_video_out(...flags...)   <-flags already according to global fullscreen
>>    -> GUI_SETUP_VIDEO_WINDOW   <-currently no flags
>>    -> vo->config(...flags...)
>>        -> vo_x11_create_vo_window(...flags...)
>>

> I just don't see why you can't set it before or after, both of which
> should work.
> GUI_SETUP_VIDEO_WINDOW doesn't really seem quite the right place, there
> has to be additional code anyway e.g. when switching from/to fullscreen.

Maybe we are talking at cross purposes. It is not about switching from/to
fullscreen (this is GUI internal stuff and has already happened, because the
window is under GUI's control), it is about telling the vo driver what state
the window is in it is going to handle. And I think GUI_SETUP_VIDEO_WINDOW is
exactly the right place to do it, because that is why GUI_SETUP_VIDEO_WINDOW
exists at all.

To clear things up, here is the patch I have in mind:

  Index: libvo/video_out.c
  ===================================================================
  --- libvo/video_out.c	(revision 34002)
  +++ libvo/video_out.c	(working copy)
  @@ -372,7 +372,7 @@
   #ifdef CONFIG_GUI
       if (use_gui) {
         // GUI creates and manages window for us
  -      gui(GUI_SETUP_VIDEO_WINDOW, 0);
  +      gui(GUI_SETUP_VIDEO_WINDOW, &flags);
       }
   #endif
     }
  Index: gui/interface.c
  ===================================================================
  --- gui/interface.c	(revision 34002)
  +++ gui/interface.c	(working copy)
  @@ -758,6 +758,11 @@

       case GUI_SETUP_VIDEO_WINDOW:

  +        if (guiApp.subWindow.isFullScreen)
  +            *(uint32_t *)data |= VOFLAG_FULLSCREEN;
  +        else
  +            *(uint32_t *)data &= ~VOFLAG_FULLSCREEN;
  +

As you can see, it is just about telling vo_x11_create_vo_window() the right
thing. Because vo_x11_create_vo_window() sets vo_fs according to the flags it
gets, this information has to be correct:

  void vo_x11_create_vo_window(XVisualInfo *vis, int x, int y,
                               unsigned int width, unsigned int height, int flags,
                               Colormap col_map,
                               const char *classname, const char *title)
  {
    if (WinID >= 0) {
      vo_fs = flags & VOFLAG_FULLSCREEN;
      ...

Concerning setting/unsetting fullscreen there isn't any further hack nor any
other interference with vo code. This is just it. The window is prepared, has
a state and the vo driver prior to vo->config() just gets the correct flags
information now.

IMHO this is rather a bug fix than a hack.

Ingo