[MPlayer-dev-eng] [PATCH] GUI cursor control

[Alexander Strasser]

Hi Ingo!

Ingo Brückl wrote:
> Alexander Strasser wrote on Sun, 4 Sep 2011 17:02:25 +0200:
> 
> > I agree that this is a good idea and that hidecursor should be disabled
> > too.
> 
> vo_hidecursor() never gets called in case of WinID >= 0. The "goto final"
> prevents that. This leads to an annoying "cursor visible bug" in the GUI.
> (While making the cursor invisible is no problem, the GUI currently has no
> control of it remaining invisible.)

  I looked at the code again, and now I think I know what you are after.

> As there is no hidecursor for WinIDs, we all seem to agree that there
> shouldn't be a showcursor either.

  Yes, we all agree in that point. But after re-reading your explanation
and the code I think you might be trying to fix the symptom but not the
problem. Wouldn't it be better to have the X11 VOs not interfere with
cursor management if an external WinID is used? In that case you would
would be in control of it remaining invisible and responsible for making
it visible.

> 
> > Maybe it is a more viable approach to disable them from inside the
> > vo_showcursor/vo_hidecursor routines themselves and document them while at
> > it.
> 
> I don't want to do that because this would prevent the useful autohide
> feature.

  I guess this is the reason you don't want to get into full control of
the cursor visibility.

  In case you want to keep this functionality, which depends on internal
VO state, then I would expect it to be better to make it just work out
without your interference. Maybe by doing the opposite change and letting
the VO code make it invisible.

> > Also I could not find any of these called outside the x11_common object
> > module, so it may be a good idea to remove the prototypes and make them
> > private to x11_common.c by marking them static.
> 
> Good point, so I revised the patch.

  Thanks for implementing it quickly, but stuffing this into a separate
commit would be much preferable. It could also get committed before your
show/hide cursor patch. But choose the order as you see fit in your work 
flow.

> Index: libvo/x11_common.c
> ===================================================================
> --- libvo/x11_common.c	(revision 34053)
> +++ libvo/x11_common.c	(working copy)
> @@ -178,7 +178,7 @@
>      }
>  }
>  
> -void vo_hidecursor(Display * disp, Window win)
> +static void vo_hidecursor(Display * disp, Window win)
>  {
>      Cursor no_ptr;
>      Pixmap bm_no;
> @@ -203,7 +203,7 @@
>      XFreeColors(disp,colormap,&black.pixel,1,0);
>  }
>  
> -void vo_showcursor(Display * disp, Window win)
> +static void vo_showcursor(Display * disp, Window win)
>  {
>      if (WinID == 0)
>          return;
> @@ -752,7 +752,7 @@
>  void vo_x11_uninit(void)
>  {
>      saver_on(mDisplay);
> -    if (vo_window != None)
> +    if (vo_window != None && WinID < 0)
>          vo_showcursor(mDisplay, vo_window);
>  
>      if (f_gc != None)
> Index: libvo/x11_common.h
> ===================================================================
> --- libvo/x11_common.h	(revision 34053)
> +++ libvo/x11_common.h	(working copy)
> @@ -63,8 +63,6 @@
>  
>  int vo_init( void );
>  void vo_uninit( void );
> -void vo_hidecursor ( Display* , Window );
> -void vo_showcursor( Display *disp, Window win );
>  void vo_x11_decoration( Display * vo_Display,Window w,int d );
>  void vo_x11_classhint( Display * display,Window window,const char *name );
>  void vo_x11_nofs_sizepos(int x, int y, int width, int height);

  Alexander