Skip to content

Commit

Permalink
Raise the layout limit to 32
Browse files Browse the repository at this point in the history
  • Loading branch information
wismill committed Jan 7, 2025
1 parent 4ea9d43 commit 539d37a
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 50 deletions.
20 changes: 6 additions & 14 deletions src/keymap.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,12 @@
#include "utils.h"
#include "context.h"

/* This limit is artificially enforced, we do not depend on it any where.
* The reason it's still here is that the rules file format does not
* support multiple groups very well, and the rules shipped with
* xkeyboard-config (see rules/evdev) depend on this limit extensively.
* So just lifting this limit would cause problems for people who will use
* more than 4 layouts.
* TODO: Fix the group index syntax in the rules format, preferably in a
* backwards compatible way.
* See e.g. https://bugs.freedesktop.org/show_bug.cgi?id=14372
* Note: A limit on the number of groups we *do* depend on is imposed by
* the size of the xkb_layout_mask_t type (32). This is more than enough
* though.
*/
#define XKB_MAX_GROUPS 4
/* Note: imposed by the size of the xkb_layout_mask_t type (32).
* This is more than enough though. */
#define XKB_MAX_GROUPS 32
#define XKB_ALL_GROUPS ((1ul << XKB_MAX_GROUPS) - 1)
/* Limit imposed by X11 */
#define XKB_MAX_GROUPS_X11 4

/* Don't allow more modifiers than we can hold in xkb_mod_mask_t. */
#define XKB_MAX_MODS ((xkb_mod_index_t) (sizeof(xkb_mod_mask_t) * 8))
Expand Down
8 changes: 4 additions & 4 deletions src/state.c
Original file line number Diff line number Diff line change
Expand Up @@ -781,13 +781,13 @@ xkb_state_led_update_all(struct xkb_state *state)

if (led->which_groups != 0 && led->groups != 0) {
if (led->which_groups & XKB_STATE_LAYOUT_EFFECTIVE)
group_mask |= (1u << state->components.group);
group_mask |= (1ul << state->components.group);
if (led->which_groups & XKB_STATE_LAYOUT_DEPRESSED)
group_mask |= (1u << state->components.base_group);
group_mask |= (1ul << state->components.base_group);
if (led->which_groups & XKB_STATE_LAYOUT_LATCHED)
group_mask |= (1u << state->components.latched_group);
group_mask |= (1ul << state->components.latched_group);
if (led->which_groups & XKB_STATE_LAYOUT_LOCKED)
group_mask |= (1u << state->components.locked_group);
group_mask |= (1ul << state->components.locked_group);

if (led->groups & group_mask) {
state->components.leds |= (1u << idx);
Expand Down
2 changes: 1 addition & 1 deletion src/text.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const LookupEntry groupMaskNames[] = {
{ "Group7", 0x40 },
{ "Group8", 0x80 },
{ "none", 0x00 },
{ "all", 0xff },
{ "all", XKB_ALL_GROUPS },
{ NULL, 0 }
};

Expand Down
10 changes: 5 additions & 5 deletions src/xkbcomp/rules.c
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ matcher_mapping_set_layout_bounds(struct matcher *m)
darray_size(m->rmlvo.layouts));
m->mapping.layouts_candidates_mask =
/* All but the first layout */
((1u << m->mapping.layout_idx_max) - 1) & ~1;
(xkb_layout_mask_t)(((1ull << m->mapping.layout_idx_max) - 1) & ~1);
break;
case LAYOUT_INDEX_ANY:
m->mapping.has_layout_idx_range = true;
Expand All @@ -704,7 +704,7 @@ matcher_mapping_set_layout_bounds(struct matcher *m)
darray_size(m->rmlvo.layouts));
m->mapping.layouts_candidates_mask =
/* All layouts */
(1u << m->mapping.layout_idx_max) - 1;
(xkb_layout_mask_t)((1ull << m->mapping.layout_idx_max) - 1);
break;
case LAYOUT_INDEX_FIRST:
case XKB_LAYOUT_INVALID:
Expand All @@ -716,7 +716,7 @@ matcher_mapping_set_layout_bounds(struct matcher *m)
m->mapping.has_layout_idx_range = false;
m->mapping.layout_idx_min = idx;
m->mapping.layout_idx_max = idx + 1;
m->mapping.layouts_candidates_mask = 1u << idx;
m->mapping.layouts_candidates_mask = 1ul << idx;
}
}

Expand Down Expand Up @@ -1241,7 +1241,7 @@ matcher_rule_apply_if_matches(struct matcher *m, struct scanner *s)
idx++) \
{ \
/* Process only if index not skipped */ \
xkb_layout_mask_t mask = 1u << idx; \
xkb_layout_mask_t mask = 1ul << idx; \
if (candidate_layouts & mask) { \
to = &darray_item(m->rmlvo._component, idx); \
if (match_value_and_mark(m, value, to, match_type, \
Expand Down Expand Up @@ -1289,7 +1289,7 @@ matcher_rule_apply_if_matches(struct matcher *m, struct scanner *s)
idx < m->mapping.layout_idx_max;
idx++)
{
if (candidate_layouts & (1u << idx)) {
if (candidate_layouts & (1ul << idx)) {
for (unsigned i = 0; i < m->mapping.num_kccgst; i++) {
enum rules_kccgst kccgst = m->mapping.kccgst_at_pos[i];
struct sval value = m->rule.kccgst_value_at_pos[i];
Expand Down
2 changes: 0 additions & 2 deletions src/xkbcomp/rules.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ xkb_components_from_rules(struct xkb_context *ctx,
struct xkb_component_names *out);

/* Maximum length of a layout index string:
* [NOTE] Currently XKB_MAX_GROUPS is 4, but the following code is
* future-proof for all possible indexes.
*
* length = ceiling (bitsize(xkb_layout_index_t) * logBase 10 2)
* < ceiling (bitsize(xkb_layout_index_t) * 5 / 16)
Expand Down
35 changes: 13 additions & 22 deletions test/rules-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,8 @@ main(int argc, char *argv[])
"extra,,,extra", NULL,
"symbols_a:1+symbols_y:2+layout_c:3+layout_d(extra):4+symbols_z:3"
"+foo:1|bar:1+foo:4|bar:4", false),
/* NOTE: 5 layouts is intentional;
* will require update when raising XKB_MAX_LAYOUTS */
ENTRY("layout_a,layout_b,layout_c,layout_d,layout_e", NULL, NULL,
"symbols_a:1+symbols_y:2+layout_c:3+layout_d:4+symbols_z:3", false),
"symbols_a:1+symbols_y:2+layout_c:3+layout_d:4+layout_e:5+symbols_z:3", false),
#undef ENTRY
/* Test index ranges: too much layouts */
ENTRY2("special_indexes-limit", NULL, too_much_layouts, NULL, NULL,
Expand Down Expand Up @@ -365,16 +363,15 @@ main(int argc, char *argv[])
.rules = "all_qualifier",

.model = "my_model",
/* NOTE: 5 layouts is intentional;
* will require update when raising XKB_MAX_LAYOUTS */
.layout = "layout_a,layout_b,layout_a,layout_b,layout_c",
.variant = "",
.options = "my_option",

.keycodes = "my_keycodes", .types = "my_types",
.compat = "my_compat",
.symbols = "symbols_a:1+symbols_b:2+symbols_a:3+symbols_b:4+extra_option:1"
"+extra_option:2+extra_option:3+extra_option:4",
.symbols = "symbols_a:1+symbols_b:2+symbols_a:3+symbols_b:4+symbols_c:5"
"+extra_option:1+extra_option:2+extra_option:3+extra_option:4"
"+extra_option:5",
};
assert(test_rules(ctx, &all_qualified_alone1));

Expand All @@ -383,16 +380,14 @@ main(int argc, char *argv[])
.rules = "all_qualifier",

.model = "my_model",
/* NOTE: 5 layouts is intentional;
* will require update when raising XKB_MAX_LAYOUTS */
.layout = "layout_x,layout_a,layout_b,layout_c,layout_d",
.variant = "",
.options = "",

.keycodes = "my_keycodes", .types = "my_types",
.compat = "my_compat",
.symbols = "base:1+base:2+base:3+base:4"
"+symbols_a:2+symbols_b:3+default_symbols:4",
.symbols = "base:1+base:2+base:3+base:4+base:5"
"+symbols_a:2+symbols_b:3+default_symbols:4+default_symbols:5",
};
assert(test_rules(ctx, &all_qualified_alone2));

Expand All @@ -416,17 +411,15 @@ main(int argc, char *argv[])
.rules = "all_qualifier",

.model = "my_model",
/* NOTE: 5 layouts is intentional;
* will require update when raising XKB_MAX_LAYOUTS */
.layout = "layout_a,layout_b,layout_a,layout_b,layout_c",
.variant = "extra1,,,,",
.options = "my_option",

.keycodes = "my_keycodes", .types = "my_types",
.compat = "my_compat",
.symbols = "symbols_a:1+symbols_b:2+symbols_a:3+symbols_b:4"
"+extra_symbols:1+extra_symbols:2+extra_symbols:3+extra_symbols:4"
"+extra_option:1+extra_option:2+extra_option:3+extra_option:4",
.symbols = "symbols_a:1+symbols_b:2+symbols_a:3+symbols_b:4+symbols_c:5"
"+extra_symbols:1+extra_symbols:2+extra_symbols:3+extra_symbols:4+extra_symbols:5"
"+extra_option:1+extra_option:2+extra_option:3+extra_option:4+extra_option:5",
};
assert(test_rules(ctx, &all_qualified_with_special_indexes1));

Expand All @@ -437,20 +430,18 @@ main(int argc, char *argv[])
.rules = "all_qualifier",

.model = "my_model",
/* NOTE: 5 layouts is intentional;
* will require update when raising XKB_MAX_LAYOUTS */
.layout = "layout_a,layout_b,layout_a,layout_b,layout_c",
.variant = "extra2,,extra3,,",
.options = "my_option",

.keycodes = "my_keycodes", .types = "my_types",
.compat = "my_compat",
.symbols = "symbols_a:1+symbols_b:2+symbols_a:3+symbols_b:4"
"+extra_symbols1:1+extra_symbols2:1+extra_symbols2:2+extra_symbols2:3+extra_symbols2:4"
"+extra_symbols2:1+extra_symbols2:2+extra_symbols2:3+extra_symbols2:4"
.symbols = "symbols_a:1+symbols_b:2+symbols_a:3+symbols_b:4+symbols_c:5"
"+extra_symbols1:1+extra_symbols2:1+extra_symbols2:2+extra_symbols2:3+extra_symbols2:4+extra_symbols2:5"
"+extra_symbols2:1+extra_symbols2:2+extra_symbols2:3+extra_symbols2:4+extra_symbols2:5"
"+extra_symbols1:3"
"+extra_option:1"
"+extra_option:2+extra_option:3+extra_option:4",
"+extra_option:2+extra_option:3+extra_option:4+extra_option:5",
};
assert(test_rules(ctx, &all_qualified_with_special_indexes2));

Expand Down
23 changes: 21 additions & 2 deletions test/rulescomp.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "config.h"

#include "evdev-scancodes.h"
#include "src/keysym.h"
#include "test.h"

static int
Expand Down Expand Up @@ -140,8 +141,8 @@ main(int argc, char *argv[])
assert(test_rmlvo(ctx, "evdev", "evdev", "us", "intl", "grp:alts_toggle",
KEY_GRAVE, BOTH, XKB_KEY_dead_grave, FINISH));

/* 20 is not a legal group; make sure this is handled gracefully. */
assert(test_rmlvo(ctx, "evdev", "", "us:20", "", "",
/* 33 is not a legal group; make sure this is handled gracefully. */
assert(test_rmlvo(ctx, "evdev", "", "us:33", "", "",
KEY_A, BOTH, XKB_KEY_a, FINISH));

/* Don't choke on missing values in RMLVO. Should just skip them.
Expand Down Expand Up @@ -189,6 +190,24 @@ main(int argc, char *argv[])
assert(test_rmlvo_env(ctx, "evdev", "", "cz", "bksl", "",
KEY_A, BOTH, XKB_KEY_a, FINISH));

/* Test more than 4 groups */
#define U(cp) (XKB_KEYSYM_UNICODE_OFFSET + cp)
assert(test_rmlvo_env(ctx, "evdev-modern", "", "cz,us,ca,de,in,ru,il", ",,,,,phonetic,",
"grp:menu_toggle",
KEY_2, BOTH, XKB_KEY_ecaron, NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_Y, BOTH, XKB_KEY_y, NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_102ND, BOTH, XKB_KEY_guillemetleft, NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_Y, BOTH, XKB_KEY_z, NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_Y, BOTH, U(0x092c), NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_Y, BOTH, XKB_KEY_Cyrillic_ze, NEXT,
KEY_COMPOSE, BOTH, XKB_KEY_ISO_Next_Group, NEXT,
KEY_Y, BOTH, XKB_KEY_hebrew_tet, FINISH));

xkb_context_unref(ctx);

ctx = test_get_context(0);
Expand Down

0 comments on commit 539d37a

Please sign in to comment.