summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2013-07-01 23:54:59 +0200
committerwm4 <wm4@nowhere>2013-07-02 12:19:16 +0200
commitc4766dc3c6233d3353b79fbd226202e7b8e3fc46 (patch)
treeed8f8b06e53d5130c3a290ac0eadf873eaf0a7ec
parent2f8dcac28b64cecd537ab1fad366b2c2052c2ead (diff)
downloadmpv-c4766dc3c6233d3353b79fbd226202e7b8e3fc46.tar.bz2
mpv-c4766dc3c6233d3353b79fbd226202e7b8e3fc46.tar.xz
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".)
-rw-r--r--core/input/input.c130
-rw-r--r--core/input/joystick.c4
-rw-r--r--core/input/keycodes.h17
-rw-r--r--core/mp_fifo.c5
-rw-r--r--video/out/cocoa_common.m4
-rw-r--r--video/out/vo_caca.c2
-rw-r--r--video/out/vo_sdl.c2
-rw-r--r--video/out/w32_common.c7
-rw-r--r--video/out/wayland_common.c7
-rw-r--r--video/out/x11_common.c3
10 files changed, 94 insertions, 87 deletions
diff --git a/core/input/input.c b/core/input/input.c
index 9fdd77ddd0..d7aaf54dec 100644
--- a/core/input/input.c
+++ b/core/input/input.c
@@ -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 mp_cmd_t *interpret_key(struct input_ctx *ictx, int code)
+static int find_key_down(struct input_ctx *ictx, int code)
{
- unsigned int j;
- mp_cmd_t *ret;
+ 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)
+{
/* 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_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;
+ }
+
+ bool key_was_down = find_key_down(ictx, code) >= 0;
+
if (code & MP_KEY_STATE_DOWN) {
- 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;
- }
- 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;
+ } else if (code & MP_KEY_STATE_UP) {
+ if (key_was_down) {
+ remove_key_down(ictx, code);
+ release_down_cmd(ictx);
}
- 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--;
- 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)
diff --git a/core/input/joystick.c b/core/input/joystick.c
index 17b4279c39..e8330ffaeb 100644
--- a/core/input/joystick.c
+++ b/core/input/joystick.c
@@ -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 {
diff --git a/core/input/keycodes.h b/core/input/keycodes.h
index 80e2561514..d91465f3be 100644
--- a/core/input/keycodes.h
+++ b/core/input/keycodes.h
@@ -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 */
diff --git a/core/mp_fifo.c b/core/mp_fifo.c
index 32036749d1..1c83e99dcd 100644
--- a/core/mp_fifo.c
+++ b/core/mp_fifo.c
@@ -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) {
diff --git a/video/out/cocoa_common.m b/video/out/cocoa_common.m
index 84d8e29b86..dd166e03e1 100644
--- a/video/out/cocoa_common.m
+++ b/video/out/cocoa_common.m
@@ -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;
}
diff --git a/video/out/vo_caca.c b/video/out/vo_caca.c
index fcb0bc4322..78eaf2b395 100644
--- a/video/out/vo_caca.c
+++ b/video/out/vo_caca.c
@@ -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:
{
diff --git a/video/out/vo_sdl.c b/video/out/vo_sdl.c
index 66013e01f2..dd1e4a5053 100644
--- a/video/out/vo_sdl.c
+++ b/video/out/vo_sdl.c
@@ -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;
diff --git a/video/out/w32_common.c b/video/out/w32_common.c
index 77433db885..13ec8e7053 100644
--- a/video/out/w32_common.c
+++ b/video/out/w32_common.c
@@ -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;
}
diff --git a/video/out/wayland_common.c b/video/out/wayland_common.c
index a627100477..f3d18efc1d 100644
--- a/video/out/wayland_common.c
+++ b/video/out/wayland_common.c
@@ -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);
diff --git a/video/out/x11_common.c b/video/out/x11_common.c
index bbb6fca997..648f1d57eb 100644
--- a/video/out/x11_common.c
+++ b/video/out/x11_common.c
@@ -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);