[MPlayer-dev-eng] GUI and vo_x11 functions

[Ingo Brückl]

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().

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.

After reconsidering the patches, I've come to the conclusion that we don't
need the various WinID < 0 conditions - in fact shouldn't have them.
vo_x11_ontop() and vo_x11_border() are only called via control/VOCTRL which
should be allowed. Patching vo_dxr2 would retain its current behavior, but
I'd say it currently has the same ontop bug as the GUI (ontop doesn't work)
and not patching vo_dxr2 would enable ontop now.

So the only remaining patch is:

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)

and - maybe - the vo_x11_sizehint() patch to add parameters (but I'll check
whether I can do this with existing GUI code first.)

Other WinID conditions could be prevented by the simple vo_hidecursor() and
vo_showcursor() patch (see the "GUI cursor control" topic).

Ingo