[MPlayer-dev-eng] GUI and vo_x11 functions

[Alexander Strasser]

Hi,

Ingo Brückl wrote:
> Reimar Döffinger wrote on Sun, 11 Sep 2011 11:29:05 +0200:
> > On Fri, Sep 09, 2011 at 05:09:24PM +0200, Ingo Brückl wrote:
> >> Index: libvo/x11_common.c
> >> ===================================================================
> >> --- libvo/x11_common.c        (revision 34097)
> >> +++ libvo/x11_common.c        (working copy)
> >> @@ -1206,7 +1206,7 @@
> >>
> >>  void vo_x11_setlayer(Display * mDisplay, Window vo_window, int layer)
> >>  {
> >> -    if (WinID >= 0)
> >> +    if (!WinID)
> >>          return;
> >>
> >>      if (vo_fs_type & vo_wm_LAYER)
> >> @@ -1431,7 +1431,7 @@
> >>  {
> >>      vo_ontop = (!(vo_ontop));
> >>
> >> -    vo_x11_setlayer(mDisplay, vo_window, vo_ontop);
> >> +    if (WinID < 0) vo_x11_setlayer(mDisplay, vo_window, vo_ontop);
> >>  }
> 
> > I very much dislike having half the WinID check in the function and the
> > other half outside.
> 
> The inside check has a different meaning.
> 
> > I don't mind removing it completely, but IMO leaving the !WinID there is
> > confusing.
> > Even more so since it possibly should really check the vo_window parameter
> > instead of WinID.
> 
> Actually, it is checked whether vo_window is the root window (although
> indirectly, because vo_window is the root window if WinID is 0). The same
> check is done in vo_x11_decoration().

  I believe Ingo is right on this one. But I also do find the code
misleading and confusing.

> Alexander Strasser wrote on Sat, 10 Sep 2011 15:33:52 +0200:
> 
> > In general I am a bit concerned about the WinID handling inside the
> > x11_common and the x11 based vo drivers.
> 
> Me too.

  As I see it, we are trying to tackle to many issues at once. It should
be possible to break this down into smaller self contained changes.

  This is my first attempt to do it:

  1. Generalize some functions a bit, so they could be used from the GUI
  2. Don't automatically handle cursor auto hide feature when an external
     window ID is specified).
  3. Clean up/improve the code handling around the WinID variable.

  Point 3 isn't yet worked out. Point 1 and 2 should be possible right
now with Ingo's patches iff Reimar changed his mind about the confusing
WinID usage.

  The other alternative I currently see is: reorder priorities and do
point 3 before 1 and 2. After point 3 is done, the objections should
no longer hold but the patches would need to get updated to the future
version of x11_common code.

[...]

  Alexander