[MPlayer-dev-eng] [PATCH] double click to switch to full screen in GUI

[Reimar Döffinger]

Hello,
On Thu, Sep 14, 2006 at 11:11:07PM +0200, laurent wozniak wrote:
>   gtkInit();
> +// --- initialize some input constants 
> + g_object_get ( G_OBJECT ( gtk_settings_get_default() ), 
> +    "gtk-double-click-time", &double_click_time, NULL );
> + mp_input_set_double_click_time(double_click_time);

I don't care much, but it should be tested if this works with GTK1

> @@ -305,6 +307,7 @@
>  
>  static mp_cmd_bind_t def_cmd_binds[] = {
>  
> +  { {  MOUSE_BTN0_DBL, 0 }, "vo_fullscreen" },

I'd prefer it not to be bound by default.

> +// Mouse button double click time
> +// Default value may be overridden during the gui initialization 
> +static unsigned int double_click_time = 300;
> +

probably should be a command line option.

> @@ -877,9 +884,14 @@
>    
>  
>  static char*
> -mp_input_find_bind_for_key(mp_cmd_bind_t* binds, int n,int* keys) {
> +mp_input_find_bind_for_key(int n,int* keys) {
> +  mp_cmd_bind_t* binds;
>    int j;
>  
> +  binds = cmd_binds;
> +  if (binds == NULL)
> +    return NULL;
> +  
>    for(j = 0; binds[j].cmd != NULL; j++) {
>      if(n > 0) {
>        int found = 1,s;
> @@ -903,24 +915,27 @@
>  
>  static mp_cmd_t*
>  mp_input_get_cmd_from_keys(int n,int* keys, int paused) {
> -  char* cmd = NULL;
> +  char* cmd;
>    mp_cmd_t* ret;
>  
> -  if(cmd_binds)
> -    cmd = mp_input_find_bind_for_key(cmd_binds,n,keys);
> -  if(cmd == NULL)
> -    cmd = mp_input_find_bind_for_key(def_cmd_binds,n,keys);
> +  for(;;) {
> +    cmd = mp_input_find_bind_for_key(n,keys);
> +    // found a command
> +    if(cmd != NULL) {
> +      if (strcmp(cmd, MP_CMD_TXT_NO_COMMAND) == 0)
> +        return NULL;
> +      break;
> +    }
>  
> -  if(cmd == NULL) {
> +    // ignore unbound key
>      mp_msg(MSGT_INPUT,MSGL_WARN,MSGTR_NoBindFound,mp_input_get_key_name(keys[0]));
> -    if(n > 1) {
> -      int s;
> -      for(s=1; s < n; s++)
> -	mp_msg(MSGT_INPUT,MSGL_WARN,"-%s",mp_input_get_key_name(keys[s]));
> -    }
>      mp_msg(MSGT_INPUT,MSGL_WARN,"                         \n");
> -    return NULL;
> +    keys++;
> +    n--;
> +    if(n<=0)
> +      return NULL;


Why does a no_command key bind have to behave completely than other key
binds?
I would simply add the

> +      if (strcmp(cmd, MP_CMD_TXT_NO_COMMAND) == 0)
> +        return NULL;

And leave everything else as is.

> +  // When binding a command to a mouse button double click,
> +  // if the corresponding single click is not bound,
> +  // bind it to the command which does nothing:
> +  // Avoid logging command not found on the first click of the double click.
> +  for(j = 0 ; (key = keys[j]) != 0 ; j++) {
> +    if (MOUSE_BTN0_DBL <= key && key <= MOUSE_BTN_LAST_DBL) {
> +      key = MOUSE_BTN0 + key - MOUSE_BTN0_DBL;
> +      other_cmd = mp_input_find_bind_for_key(1, &key);
> +      if (other_cmd == NULL) {
> +        int other_cmd_keys[2];      	
> +        other_cmd_keys[0] = key;
> +        other_cmd_keys[1] = 0;      	
> +        mp_input_bind_keys(other_cmd_keys, MP_CMD_TXT_NO_COMMAND);
> +      }
> +    }
> +  }
> +

I find the (IMHO doubtful since possibly confusing) convenience of not having
to add a no_command keybind not worth even that few lines of code.

> +// This put key layer filter events
> +// It can translate the event into another one 

Should be doxygen-compatible comments.
Also I think that translating is not such a great idea, won't it break
thinks for people who have bound a mouse button and click real fast?
They would only get half the events compared to before.
IMHO instead the double click event should be generated in addition.

> +void mplayer_put_key(int code) {
> +  unsigned int now;
> +  int btn;
> +  int ignore;
> +  
> +  ignore = code & MP_VETOED_KEY;
> +  code &= ~MP_VETOED_KEY;
> +
> +  // Check mouse button press event
> +  if ((MOUSE_BTN0 | MP_KEY_DOWN) <= code && code <= (MOUSE_BTN_LAST | MP_KEY_DOWN)) {  
> +    btn = code - (MOUSE_BTN0 | MP_KEY_DOWN);    

trailing whitespace...

> +    // Translate mouse wheel: Interpret press event as mouse button release event
> +    if (btn == 3 || btn == 4) {
> +      code = MOUSE_BTN0 + btn;
> +    }

I don't really like all these mouse wheel changes, first because the do
not seem absolutely necessary for now (just BTN0-BTN2 double clicks is probably
enough for now), and some mice do have more than one mouse wheel (though
the old code didn't behave entirely right in that case either).

> Index: libvo/vo_sdl.c
> ===================================================================
> --- libvo/vo_sdl.c	(revision 19814)
> +++ libvo/vo_sdl.c	(working copy)
> @@ -1189,10 +1189,7 @@
>  			case SDL_MOUSEBUTTONDOWN:
>  				if(vo_nomouse_input)
>  				    break;
> -				if(event.button.button == 4 || event.button.button == 5)
> -					mplayer_put_key(MOUSE_BASE+event.button.button-1);
> -				else
> -					mplayer_put_key((MOUSE_BASE+event.button.button-1) | MP_KEY_DOWN);
> +				mplayer_put_key((MOUSE_BTN0+event.button.button-1) | MP_KEY_DOWN);
>  				break;			    
>  		
>  			case SDL_MOUSEBUTTONUP:
> @@ -1198,7 +1195,7 @@
>  			case SDL_MOUSEBUTTONUP:
>  				if(vo_nomouse_input)
>  				    break;
> -				mplayer_put_key(MOUSE_BASE+event.button.button-1);
> +				mplayer_put_key(MOUSE_BTN0+event.button.button-1);
>  				break;
>  	
>  			/* graphics mode selection shortcuts */

Why this MOUSE_BASE -> MOUSE_BTN0 change?

Greetings,
Reimar Döffinger