Skip to content

Commit

Permalink
state: Fix LatchGroup action with latchToLock disabled
Browse files Browse the repository at this point in the history 
A `LatchGroup` action with the `latchToLock`` option disabled can apply
its latch effect multiple times.
  • Loading branch information
@wismill
wismill committed Jan 14, 2025
1 parent d2eb860 commit 1278824
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 33 deletions.
2 changes: 2 additions & 0 deletions changes/api/577.bugfix.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed `LatchGroup` action with the `latchToLock` option disabled not applying
its latch effect multiple times.
48 changes: 23 additions & 25 deletions src/state.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,16 +240,11 @@ enum xkb_filter_result {
};

/* Modify a group component, depending on the ACTION_ABSOLUTE_SWITCH flag */
#define apply_group_delta(filter_, state_, component_) \
if (filter_->action.group.flags & ACTION_ABSOLUTE_SWITCH) \
state_->components.component_ = filter_->action.group.group; \
else \
state_->components.component_ += filter_->action.group.group

#define compute_group_delta(filter_, state_) \
(filter_->action.group.flags & ACTION_ABSOLUTE_SWITCH) \
? filter_->action.group.group - state_->components.base_group \
: filter_->action.group.group
#define apply_group_delta(filter_, state_, component_) \
if ((filter_)->action.group.flags & ACTION_ABSOLUTE_SWITCH) \
(state_)->components.component_ = (filter_)->action.group.group; \
else \
(state_)->components.component_ += (filter_)->action.group.group

static void
xkb_filter_group_set_new(struct xkb_state *state, struct xkb_filter *filter)
Expand Down Expand Up @@ -360,7 +355,9 @@ xkb_filter_group_latch_new(struct xkb_state *state, struct xkb_filter *filter)
{
const union group_latch_priv priv = {
.latch = LATCH_KEY_DOWN,
.group_delta = compute_group_delta(filter, state)
.group_delta = (filter->action.group.flags & ACTION_ABSOLUTE_SWITCH)
? filter->action.group.group - state->components.base_group
: filter->action.group.group
};
filter->priv = priv.priv;
/* Like group set */
Expand All @@ -379,8 +376,8 @@ xkb_filter_group_latch_func(struct xkb_state *state,
if (direction == XKB_KEY_DOWN && latch == LATCH_PENDING) {
/* If this is a new keypress and we're awaiting our single latched
* keypress, then either break the latch if any random key is pressed,
* or promote it to a lock or plain base set if it's the same
* group delta & flags. */
* or promote it to a lock if it's the same group delta & flags and
* latchToLock option is enabled. */
const union xkb_action *actions = NULL;
unsigned int count = xkb_key_get_actions(state, key, &actions);
for (unsigned int k = 0; k < count; k++) {
Expand All @@ -394,21 +391,20 @@ xkb_filter_group_latch_func(struct xkb_state *state,
filter->action.type = ACTION_TYPE_GROUP_LOCK;
filter->func = xkb_filter_group_lock_func;
xkb_filter_group_lock_new(state, filter);
state->components.latched_group -= priv.group_delta;
filter->key = key;
/* XXX beep beep! */
return XKB_FILTER_CONSUME;
}
else {
/* Degrade to plain set */
filter->action.type = ACTION_TYPE_GROUP_SET;
filter->func = xkb_filter_group_set_func;
xkb_filter_group_set_new(state, filter);
}
filter->key = key;
state->components.latched_group -= priv.group_delta;
/* XXX beep beep! */
return XKB_FILTER_CONSUME;
/* Do nothing if latchToLock option is not activated; if the
* latch is not broken by the following actions and the key is
* not consummed, then another latch filter will be created.
*/
continue;
}
else if (xkb_action_breaks_latch(&(actions[k]))) {
/* Breaks the latch */
state->components.latched_group = 0;
state->components.latched_group -= priv.group_delta;
filter->func = NULL;
return XKB_FILTER_CONTINUE;
}
Expand All @@ -431,7 +427,9 @@ xkb_filter_group_latch_func(struct xkb_state *state,
state->components.locked_group = 0;
filter->func = NULL;
}
else {
/* We may already have reached the latch state if pressing the
* key multiple times without latch-to-lock enabled. */
else if (latch == LATCH_KEY_DOWN) {
latch = LATCH_PENDING;
/* Switch from set to latch */
state->components.base_group -= priv.group_delta;
Expand Down
34 changes: 30 additions & 4 deletions test/common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,30 @@ test_init(void)
setbuf(stdout, NULL);
}

void
print_detailed_state(struct xkb_state *state)
{
fprintf(stderr, " Layout: base: %d, latched: %d, locked: %d, effective: %u\n",
xkb_state_serialize_layout(state, XKB_STATE_LAYOUT_DEPRESSED),
xkb_state_serialize_layout(state, XKB_STATE_LAYOUT_LATCHED),
xkb_state_serialize_layout(state, XKB_STATE_LAYOUT_LOCKED),
xkb_state_serialize_layout(state, XKB_STATE_LAYOUT_EFFECTIVE));
fprintf(stderr, " Modifiers: base: 0x%x, latched: 0x%x, locked: 0x%x, effective: 0x%x\n",
xkb_state_serialize_mods(state, XKB_STATE_MODS_DEPRESSED),
xkb_state_serialize_mods(state, XKB_STATE_MODS_LATCHED),
xkb_state_serialize_mods(state, XKB_STATE_MODS_LOCKED),
xkb_state_serialize_mods(state, XKB_STATE_MODS_EFFECTIVE));

/* There is currently no xkb_state_serialize_leds */
struct xkb_keymap *keymap = xkb_state_get_keymap(state);
xkb_led_mask_t leds = 0;
for (xkb_led_index_t led = 0; led < xkb_keymap_num_leds(keymap); led++) {
if (xkb_state_led_index_is_active(state, led) > 0)
leds |= 1u << led;
}
fprintf(stderr, " LEDs: 0x%x\n", leds);
}

/*
* Test a sequence of keysyms, resulting from a sequence of key presses,
* against the keysyms they're supposed to generate.
Expand Down Expand Up @@ -133,13 +157,13 @@ test_key_seq_va(struct xkb_keymap *keymap, va_list ap)

if (keysym == FINISH || keysym == NEXT) {
xkb_keysym_get_name(syms[i], ksbuf, sizeof(ksbuf));
fprintf(stderr, " Did not expect keysym: %s.\n", ksbuf);
fprintf(stderr, "\nERROR: Did not expect keysym: %s.\n", ksbuf);
goto fail;
}

if (keysym != syms[i]) {
xkb_keysym_get_name(keysym, ksbuf, sizeof(ksbuf));
fprintf(stderr, " Expected keysym: %s. ", ksbuf);;
fprintf(stderr, "\nERROR: Expected keysym: %s. ", ksbuf);;
xkb_keysym_get_name(syms[i], ksbuf, sizeof(ksbuf));
fprintf(stderr, " Got keysym: %s.\n", ksbuf);;
goto fail;
Expand All @@ -150,7 +174,7 @@ test_key_seq_va(struct xkb_keymap *keymap, va_list ap)
keysym = va_arg(ap, int);
if (keysym != XKB_KEY_NoSymbol) {
xkb_keysym_get_name(keysym, ksbuf, sizeof(ksbuf));
fprintf(stderr, " Expected %s, but got no keysyms.\n", ksbuf);
fprintf(stderr, "\nERROR: Expected %s, but got no keysyms.\n", ksbuf);
goto fail;
}
}
Expand All @@ -164,14 +188,16 @@ test_key_seq_va(struct xkb_keymap *keymap, va_list ap)
break;

xkb_keysym_get_name(keysym, ksbuf, sizeof(ksbuf));
fprintf(stderr, "Expected keysym: %s. Didn't get it.\n", ksbuf);
fprintf(stderr, "\nERROR: Expected keysym: %s. Didn't get it.\n", ksbuf);
goto fail;
}

xkb_state_unref(state);
return 1;

fail:
fprintf(stderr, "Current state:\n");
print_detailed_state(state);
xkb_state_unref(state);
return 0;
}
Expand Down
17 changes: 13 additions & 4 deletions test/keyseq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,14 @@ test_group_latch(struct xkb_context *ctx)
KEY_H, BOTH, XKB_KEY_h, FINISH))
test_latch_not_broken_by_modifier(keymap);

/* Mo lock */
/* No lock */
#define test_no_latch_to_lock(keymap_) \
assert(test_key_seq(keymap_, \
KEY_H, BOTH, XKB_KEY_h, NEXT, \
/* No latch-to-lock */ \
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Group_Latch, NEXT, \
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Group_Latch, NEXT, \
KEY_H, BOTH, XKB_KEY_h, NEXT, \
KEY_H, BOTH, XKB_KEY_Cyrillic_ha, NEXT, \
KEY_E, BOTH, XKB_KEY_e, NEXT, \
/* Lock the second group */ \
KEY_SCROLLLOCK, BOTH, XKB_KEY_ISO_Next_Group, NEXT, \
Expand All @@ -104,8 +104,17 @@ test_group_latch(struct xkb_context *ctx)
/* No latch-to-lock */ \
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Group_Latch, NEXT, \
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Group_Latch, NEXT, \
KEY_H, BOTH, XKB_KEY_hebrew_yod, NEXT, \
KEY_E, BOTH, XKB_KEY_hebrew_qoph, FINISH))
KEY_H, BOTH, XKB_KEY_s, NEXT, \
KEY_E, BOTH, XKB_KEY_hebrew_qoph, NEXT, \
/* Lock the third group */ \
KEY_SCROLLLOCK, BOTH, XKB_KEY_ISO_Next_Group, NEXT, \
KEY_H, BOTH, XKB_KEY_Cyrillic_ha, NEXT, \
KEY_E, BOTH, XKB_KEY_Cyrillic_ie, NEXT, \
/* No latch-to-lock */ \
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Group_Latch, NEXT, \
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Group_Latch, NEXT, \
KEY_H, BOTH, XKB_KEY_h, NEXT, \
KEY_E, BOTH, XKB_KEY_Cyrillic_ie, FINISH))
test_no_latch_to_lock(keymap);

xkb_keymap_unref(keymap);
Expand Down
3 changes: 3 additions & 0 deletions test/test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
void
test_init(void);

void
print_detailed_state(struct xkb_state *state);

/* The offset between KEY_* numbering, and keycodes in the XKB evdev
* dataset. */
#define EVDEV_OFFSET 8
Expand Down

0 comments on commit 1278824

Please sign in to comment.