summaryrefslogtreecommitdiffstats
path: root/core
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 /core
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".)
Diffstat (limited to 'core')
-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
4 files changed, 80 insertions, 76 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) {