input: require VOs to send key up events, redo input key lookup

Making key up events implicit was sort-of a nice idea, but it's too
tricky and unreliable and makes the key lookup code (interpret_keys())
hard to reason about. See e.g. previous commit for subtle bugs and
issues this caused.

Make key-up events explicit instead. Add key up events to all VOs.
Any time MP_KEY_STATE_DOWN is used, the matching key up event must
use MP_KEY_STATE_UP.

Rewrite the key lookup code. It should be simpler and more robust now.
(Even though the LOC increases, because the new code is less "compact".)
This commit is contained in:
wm4 2013-07-01 23:54:59 +02:00
parent 2f8dcac28b
commit c4766dc3c6
10 changed files with 93 additions and 86 deletions

View File

@ -624,7 +624,7 @@ static char *get_key_name(int key, char *ret)
static char *get_key_combo_name(int *keys, int max)
{
char *ret = talloc_strdup(NULL, "");
while (1) {
while (max > 0) {
ret = get_key_name(*keys, ret);
if (--max && *++keys)
ret = talloc_asprintf_append_buffer(ret, "-");
@ -1280,36 +1280,52 @@ static void release_down_cmd(struct input_ctx *ictx)
ictx->ar_state = -1;
}
static int find_key_down(struct input_ctx *ictx, int code)
{
code &= ~(MP_KEY_STATE_UP | MP_KEY_STATE_DOWN);
for (int j = 0; j < ictx->num_key_down; j++) {
if (ictx->key_down[j] == code)
return j;
}
return -1;
}
static void remove_key_down(struct input_ctx *ictx, int code)
{
int index = find_key_down(ictx, code);
if (index >= 0) {
memmove(&ictx->key_down[index], &ictx->key_down[index + 1],
(ictx->num_key_down - (index + 1)) * sizeof(int));
ictx->num_key_down -= 1;
}
}
static mp_cmd_t *interpret_key(struct input_ctx *ictx, int code)
{
unsigned int j;
mp_cmd_t *ret;
/* On normal keyboards shift changes the character code of non-special
* keys, so don't count the modifier separately for those. In other words
* we want to have "a" and "A" instead of "a" and "Shift+A"; but a separate
* shift modifier is still kept for special keys like arrow keys.
*/
int unmod = code & ~(MP_KEY_MODIFIER_MASK | MP_KEY_STATE_DOWN);
int unmod = code & ~MP_KEY_MODIFIER_MASK;
if (unmod >= 32 && unmod < MP_KEY_BASE)
code &= ~MP_KEY_MODIFIER_SHIFT;
if (code & MP_KEY_STATE_DOWN) {
if (ictx->num_key_down >= MP_MAX_KEY_DOWN) {
if (!(code & MP_KEY_STATE_UP) && ictx->num_key_down >= MP_MAX_KEY_DOWN) {
mp_tmsg(MSGT_INPUT, MSGL_ERR, "Too many key down events "
"at the same time\n");
return NULL;
}
code &= ~MP_KEY_STATE_DOWN;
bool key_was_down = find_key_down(ictx, code) >= 0;
if (code & MP_KEY_STATE_DOWN) {
// Check if we don't already have this key as pushed
for (j = 0; j < ictx->num_key_down; j++) {
if (ictx->key_down[j] == code)
break;
}
if (j != ictx->num_key_down)
if (key_was_down)
return NULL;
// Cancel current down-event (there can be only one)
release_down_cmd(ictx);
ictx->key_down[ictx->num_key_down] = code;
ictx->key_down[ictx->num_key_down] = code & ~MP_KEY_STATE_DOWN;
ictx->num_key_down++;
ictx->last_key_down = mp_time_us();
ictx->ar_state = 0;
@ -1318,52 +1334,35 @@ static mp_cmd_t *interpret_key(struct input_ctx *ictx, int code)
if (ictx->current_down_cmd && (code & MP_KEY_EMIT_ON_UP))
ictx->current_down_cmd->key_up_follows = true;
return mp_cmd_clone(ictx->current_down_cmd);
}
// button released or press of key with no separate down/up events
for (j = 0; j < ictx->num_key_down; j++) {
if (ictx->key_down[j] == code)
break;
}
bool emit_key = false;
bool doubleclick = MP_KEY_IS_MOUSE_BTN_DBL(code);
if (doubleclick) {
int btn = code - MP_MOUSE_BTN0_DBL + MP_MOUSE_BTN0;
if (!ictx->num_key_down
|| ictx->key_down[ictx->num_key_down - 1] != btn)
return NULL;
j = ictx->num_key_down - 1;
ictx->key_down[j] = code;
emit_key = true;
}
if (j == ictx->num_key_down) { // was not already down; add temporarily
if (ictx->num_key_down > MP_MAX_KEY_DOWN) {
mp_tmsg(MSGT_INPUT, MSGL_ERR, "Too many key down events "
"at the same time\n");
return NULL;
}
ictx->key_down[ictx->num_key_down] = code;
ictx->num_key_down++;
emit_key = true;
}
// This is a key up event, but the key up command is added by
// release_down_cmd(), not by this code.
if ((code & MP_KEY_EMIT_ON_UP) && ictx->current_down_cmd)
emit_key = false;
// Interpret only maximal point of multibutton event
ret = NULL;
if (emit_key)
ret = get_cmd_from_keys(ictx, NULL, ictx->num_key_down, ictx->key_down);
if (doubleclick) {
ictx->key_down[j] = code - MP_MOUSE_BTN0_DBL + MP_MOUSE_BTN0;
return ret;
}
// Remove the key
if (j + 1 < ictx->num_key_down)
memmove(&ictx->key_down[j], &ictx->key_down[j + 1],
(ictx->num_key_down - (j + 1)) * sizeof(int));
ictx->num_key_down--;
} else if (code & MP_KEY_STATE_UP) {
if (key_was_down) {
remove_key_down(ictx, code);
release_down_cmd(ictx);
return ret;
}
return NULL;
} else {
// Press of key with no separate down/up events
if (key_was_down) {
// Mixing press events and up/down with the same key is not allowed
mp_tmsg(MSGT_INPUT, MSGL_WARN, "Mixing key presses and up/down.\n");
}
// Add temporarily (include ongoing down/up events)
int num_key_down = ictx->num_key_down;
int key_down[MP_MAX_KEY_DOWN];
memcpy(key_down, ictx->key_down, num_key_down * sizeof(int));
// Assume doubleclick events never use down/up, while button events do
if (MP_KEY_IS_MOUSE_BTN_DBL(code)) {
// Don't emit "MOUSE_BTN0+MOUSE_BTN0_DBL", just "MOUSE_BTN0_DBL"
int btn = code - MP_MOUSE_BTN0_DBL + MP_MOUSE_BTN0;
if (!num_key_down || key_down[num_key_down - 1] != btn)
return NULL;
key_down[num_key_down - 1] = code;
} else {
key_down[num_key_down] = code;
num_key_down++;
}
return get_cmd_from_keys(ictx, NULL, num_key_down, key_down);
}
}
static mp_cmd_t *check_autorepeat(struct input_ctx *ictx)
@ -1419,16 +1418,15 @@ static bool key_updown_ok(enum mp_command_type cmd)
void mp_input_feed_key(struct input_ctx *ictx, int code)
{
ictx->got_new_events = true;
int unmod = code & ~(MP_KEY_MODIFIER_MASK | MP_KEY_STATE_DOWN);
if (MP_KEY_DEPENDS_ON_MOUSE_POS(unmod))
ictx->mouse_event_counter++;
if (code == MP_INPUT_RELEASE_ALL) {
mp_msg(MSGT_INPUT, MSGL_V, "input: release all\n");
memset(ictx->key_down, 0, sizeof(ictx->key_down));
ictx->num_key_down = 0;
release_down_cmd(ictx);
return;
}
int unmod = code & ~MP_KEY_MODIFIER_MASK;
if (MP_KEY_DEPENDS_ON_MOUSE_POS(unmod))
ictx->mouse_event_counter++;
mp_msg(MSGT_INPUT, MSGL_V, "input: key code=%#x\n", code);
struct mp_cmd *cmd = interpret_key(ictx, code);
if (!cmd)

View File

@ -139,7 +139,7 @@ int mp_input_joystick_read(void *ctx, int fd) {
if(ev.value == 1)
return (MP_JOY_BTN0 + ev.number) | MP_KEY_STATE_DOWN;
else
return MP_JOY_BTN0 + ev.number;
return (MP_JOY_BTN0 + ev.number) | MP_KEY_STATE_UP;
} else if(ev.type & JS_EVENT_AXIS) {
if(ev.value < -JOY_AXIS_DELTA && axis[ev.number] != -1) {
axis[ev.number] = -1;
@ -150,7 +150,7 @@ int mp_input_joystick_read(void *ctx, int fd) {
} else if(ev.value <= JOY_AXIS_DELTA && ev.value >= -JOY_AXIS_DELTA && axis[ev.number] != 0) {
int r = axis[ev.number] == 1 ? MP_JOY_AXIS0_PLUS+(2*ev.number) : MP_JOY_AXIS0_MINUS+(2*ev.number);
axis[ev.number] = 0;
return r;
return r | MP_KEY_STATE_UP;
} else
return MP_INPUT_NOTHING;
} else {

View File

@ -220,21 +220,28 @@
#define MP_KEY_MODIFIER_META (1<<25)
#define MP_KEY_MODIFIER_MASK (MP_KEY_MODIFIER_SHIFT | MP_KEY_MODIFIER_CTRL | \
MP_KEY_MODIFIER_ALT | MP_KEY_MODIFIER_META)
MP_KEY_MODIFIER_ALT | MP_KEY_MODIFIER_META | \
MP_KEY_STATE_DOWN | MP_KEY_STATE_UP)
// Flag for key events. Multiple down events are idempotent. Release keys by
// sending the key code without this flag, or by sending MP_INPUT_RELEASE_ALL
// as key code.
// sending the key code with KEY_STATE_UP set, or by sending
// MP_INPUT_RELEASE_ALL as key code.
#define MP_KEY_STATE_DOWN (1<<26)
// Flag for key events. Releases a key previously held down with
// MP_KEY_STATE_DOWN. Do not sending redundant UP events and do not forget to
// release keys at all with UP. If input is unreliable, use MP_INPUT_RELEASE_ALL
// or don't use MP_KEY_STATE_DOWN in the first place.
#define MP_KEY_STATE_UP (1<<27)
// The following flags are not modifiers, but are part of the keycode itself.
// Emit a command even on key-up (normally key-up is ignored). The command
// handling code has to ignore unwanted commands specifically.
#define MP_KEY_EMIT_ON_UP (1<<27)
#define MP_KEY_EMIT_ON_UP (1<<28)
// Use this when the key shouldn't be auto-repeated (like mouse buttons)
// Also means both key-down key-up events produce emit bound commands.
#define MP_NO_REPEAT_KEY (1<<28)
#define MP_NO_REPEAT_KEY (1<<29)
#endif /* MPLAYER_KEYCODES_H */

View File

@ -53,9 +53,8 @@ void mplayer_put_key(struct mp_fifo *fifo, int code)
double now = mp_time_sec();
int doubleclick_time = fifo->opts->doubleclick_time;
// ignore system-doubleclick if we generate these events ourselves
if (doubleclick_time
&& (code & ~MP_KEY_STATE_DOWN) >= MP_MOUSE_BTN0_DBL
&& (code & ~MP_KEY_STATE_DOWN) < MP_MOUSE_BTN_DBL_END)
int unmod = code & ~MP_KEY_MODIFIER_MASK;
if (doubleclick_time && MP_KEY_IS_MOUSE_BTN_DBL(unmod))
return;
mp_input_feed_key(fifo->input, code);
if (code & MP_KEY_STATE_DOWN) {

View File

@ -893,14 +893,14 @@ int vo_cocoa_cgl_color_size(struct vo *vo)
// doing the second click in a double click. Put in the key_fifo
// the key that would be put from the MouseUp handling code.
if([theEvent clickCount] == 2) {
cocoa_put_key(MP_MOUSE_BTN0 + buttonNumber);
cocoa_put_key((MP_MOUSE_BTN0 + buttonNumber) | MP_KEY_STATE_UP);
self.mouseDown = NO;
}
break;
case NSLeftMouseUp:
case NSRightMouseUp:
case NSOtherMouseUp:
cocoa_put_key(MP_MOUSE_BTN0 + buttonNumber);
cocoa_put_key((MP_MOUSE_BTN0 + buttonNumber) | MP_KEY_STATE_UP);
self.mouseDown = NO;
break;
}

View File

@ -182,7 +182,7 @@ static void check_events(struct vo *vo)
case CACA_EVENT_MOUSE_RELEASE:
if (!vo->opts->nomouse_input)
mplayer_put_key(vo->key_fifo,
MP_MOUSE_BTN0 + cev.data.mouse.button - 1);
(MP_MOUSE_BTN0 + cev.data.mouse.button - 1) | MP_KEY_STATE_UP);
break;
case CACA_EVENT_KEY_PRESS:
{

View File

@ -541,7 +541,7 @@ static void check_events(struct vo *vo)
break;
case SDL_MOUSEBUTTONUP:
mplayer_put_key(vo->key_fifo,
(MP_MOUSE_BTN0 + ev.button.button - 1));
(MP_MOUSE_BTN0 + ev.button.button - 1) | MP_KEY_STATE_UP);
break;
case SDL_MOUSEWHEEL:
break;

View File

@ -225,19 +225,19 @@ static LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam,
mouse_button = MP_MOUSE_BTN0 | MP_KEY_STATE_DOWN;
break;
case WM_LBUTTONUP:
mouse_button = MP_MOUSE_BTN0;
mouse_button = MP_MOUSE_BTN0 | MP_KEY_STATE_UP;
break;
case WM_MBUTTONDOWN:
mouse_button = MP_MOUSE_BTN1 | MP_KEY_STATE_DOWN;
break;
case WM_MBUTTONUP:
mouse_button = MP_MOUSE_BTN1;
mouse_button = MP_MOUSE_BTN1 | MP_KEY_STATE_UP;
break;
case WM_RBUTTONDOWN:
mouse_button = MP_MOUSE_BTN2 | MP_KEY_STATE_DOWN;
break;
case WM_RBUTTONUP:
mouse_button = MP_MOUSE_BTN2;
mouse_button = MP_MOUSE_BTN2 | MP_KEY_STATE_UP;
break;
case WM_MOUSEWHEEL: {
int x = GET_WHEEL_DELTA_WPARAM(wParam);
@ -250,6 +250,7 @@ static LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam,
break;
case WM_XBUTTONUP:
mouse_button = HIWORD(wParam) == 1 ? MP_MOUSE_BTN5 : MP_MOUSE_BTN6;
mouse_button |= MP_KEY_STATE_UP;
break;
}

View File

@ -278,7 +278,7 @@ static void keyboard_handle_key(void *data,
if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
mplayer_put_key(wl->vo->key_fifo, mpkey | MP_KEY_STATE_DOWN);
else
mplayer_put_key(wl->vo->key_fifo, mpkey);
mplayer_put_key(wl->vo->key_fifo, mpkey | MP_KEY_STATE_UP);
}
}
@ -320,7 +320,7 @@ static void pointer_handle_enter(void *data,
/* Release the left button on pointer enter again
* because after moving the shell surface no release event is sent */
mplayer_put_key(wl->vo->key_fifo, MP_MOUSE_BTN0);
mplayer_put_key(wl->vo->key_fifo, MP_MOUSE_BTN0 | MP_KEY_STATE_UP);
show_cursor(wl);
}
@ -355,7 +355,8 @@ static void pointer_handle_button(void *data,
struct vo_wayland_state *wl = data;
mplayer_put_key(wl->vo->key_fifo, MP_MOUSE_BTN0 + (button - BTN_LEFT) |
((state == WL_POINTER_BUTTON_STATE_PRESSED) ? MP_KEY_STATE_DOWN : 0));
((state == WL_POINTER_BUTTON_STATE_PRESSED)
? MP_KEY_STATE_DOWN : MP_KEY_STATE_UP));
if ((button == BTN_LEFT) && (state == WL_POINTER_BUTTON_STATE_PRESSED))
wl_shell_surface_move(wl->window->shell_surface, wl->input->seat, serial);

View File

@ -766,7 +766,8 @@ int vo_x11_check_events(struct vo *vo)
break;
case ButtonRelease:
mplayer_put_key(vo->key_fifo,
MP_MOUSE_BTN0 + Event.xbutton.button - 1);
(MP_MOUSE_BTN0 + Event.xbutton.button - 1)
| MP_KEY_STATE_UP);
break;
case PropertyNotify: {
char *name = XGetAtomName(display, Event.xproperty.atom);