From da3ee823a65cec1db04e356385a72ffc3868bf56 Mon Sep 17 00:00:00 2001 From: Pierre Le Marre Date: Thu, 19 Dec 2024 18:21:01 +0100 Subject: [PATCH] symbols: Fix key symbols/actions merge --- src/keymap-priv.c | 24 ++++++++ src/keymap.h | 6 ++ src/xkbcomp/symbols.c | 124 +++++++++++++++++++++++++++++------------- 3 files changed, 115 insertions(+), 39 deletions(-) diff --git a/src/keymap-priv.c b/src/keymap-priv.c index 41f4c754..c615134c 100644 --- a/src/keymap-priv.c +++ b/src/keymap-priv.c @@ -152,6 +152,30 @@ XkbLevelsSameSyms(const struct xkb_level *a, const struct xkb_level *b) return memcmp(a->s.syms, b->s.syms, sizeof(*a->s.syms) * a->num_syms) == 0; } +bool +XkbLevelHasNoKeysym(const struct xkb_level *level) +{ + if (level->num_syms == 0) + return true; + if (level->num_syms == 1) + return level->s.sym == XKB_KEY_NoSymbol; + for (unsigned int k = 0; k < level->num_syms; k++) { + if (level->s.syms[k] != XKB_KEY_NoSymbol) + return false; + } + return true; +} + +bool +XkbLevelsSameActions(const struct xkb_level *a, const struct xkb_level *b) +{ + if (a->num_syms != b->num_syms) + return false; + if (a->num_syms <= 1) + return memcmp(&a->a.action, &b->a.action, sizeof(a->a.action)) == 0; + return memcmp(a->a.actions, b->a.actions, sizeof(*a->a.actions) * a->num_syms) == 0; +} + bool XkbLevelHasNoAction(const struct xkb_level *level) { diff --git a/src/keymap.h b/src/keymap.h index 218a389e..1ddf5448 100644 --- a/src/keymap.h +++ b/src/keymap.h @@ -498,6 +498,12 @@ XkbModNameToIndex(const struct xkb_mod_set *mods, xkb_atom_t name, bool XkbLevelsSameSyms(const struct xkb_level *a, const struct xkb_level *b); +bool +XkbLevelHasNoKeysym(const struct xkb_level *level); + +bool +XkbLevelsSameActions(const struct xkb_level *a, const struct xkb_level *b); + bool XkbLevelHasNoAction(const struct xkb_level *level); diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c index b9517286..2149490a 100644 --- a/src/xkbcomp/symbols.c +++ b/src/xkbcomp/symbols.c @@ -302,17 +302,52 @@ MergeGroups(SymbolsInfo *info, GroupInfo *into, GroupInfo *from, bool clobber, struct xkb_level *intoLevel = &darray_item(into->levels, i); struct xkb_level *fromLevel = &darray_item(from->levels, i); - if (fromLevel->num_syms == 0) { - /* it's empty for consistency with other comparisons */ + const bool fromHasNoKeysym = XkbLevelHasNoKeysym(fromLevel); + const bool fromHasNoAction = XkbLevelHasNoAction(fromLevel); + if (fromHasNoKeysym && fromHasNoAction) { + /* Empty `from`: do nothing */ + continue; } - else if (intoLevel->num_syms == 0) { + + const bool intoHasNoKeysym = XkbLevelHasNoKeysym(intoLevel); + const bool intoHasNoAction = XkbLevelHasNoAction(intoLevel); + if (intoHasNoKeysym && intoHasNoAction) { + /* Empty `into`: use `from` keysyms and actions */ StealLevelInfo(intoLevel, fromLevel); + continue; + } + + if (intoLevel->num_syms != fromLevel->num_syms) { + /* Handle different keysyms/actions count */ + assert(intoLevel->num_syms > 0); + assert(fromLevel->num_syms > 0); + if (report) { + log_warn(info->ctx, XKB_WARNING_CONFLICTING_KEY_SYMBOL, + "Multiple definitions for level %d/group %u on key %s " + "with incompatible keysyms/actions count; " + "Using %s (count: %u), ignoring %s (count: %u)\n", + i + 1, group + 1, KeyNameText(info->ctx, key_name), + (clobber ? "from" : "to"), + (clobber ? fromLevel->num_syms : intoLevel->num_syms), + (clobber ? "to" : "from"), + (clobber ? intoLevel->num_syms : fromLevel->num_syms)); + } + if (clobber) { + /* There is no obvious way to deal with this case other than + * just cloning `from` */ + StealLevelInfo(intoLevel, fromLevel); + } + continue; } else { - bool actions_replaced = false; + /* Possible level conflict */ + assert(intoLevel->num_syms > 0); + assert(fromLevel->num_syms == intoLevel->num_syms); + /* Handle keysyms */ if (!XkbLevelsSameSyms(fromLevel, intoLevel)) { - if (report) { + /* Incompatible keysyms */ + if (report && !(intoHasNoKeysym || fromHasNoKeysym)) { log_warn(info->ctx, XKB_WARNING_CONFLICTING_KEY_SYMBOL, "Multiple symbols for level %d/group %u on key %s; " "Using %s, ignoring %s\n", @@ -322,45 +357,34 @@ MergeGroups(SymbolsInfo *info, GroupInfo *into, GroupInfo *from, bool clobber, } if (clobber) { - /* Use `from` keysyms and actions */ - if (report && !XkbLevelHasNoAction(intoLevel)) { - if (fromLevel->num_syms > 1) { - log_warn( - info->ctx, XKB_WARNING_CONFLICTING_KEY_ACTION, - "Multiple actions for level %d/group %u " - "on key %s; Using from, ignoring to\n", - i + 1, group + 1, - KeyNameText(info->ctx, key_name)); - } else { - log_warn( - info->ctx, XKB_WARNING_CONFLICTING_KEY_ACTION, - "Multiple actions for level %d/group %u " - "on key %s; Using %s, ignoring %s\n", - i + 1, group + 1, - KeyNameText(info->ctx, key_name), - ActionTypeText(fromLevel->a.action.type), - ActionTypeText(intoLevel->a.action.type)); + /* Override: copy any defined keysym from `from` */ + if (unlikely(intoLevel->num_syms > 1)) { + for (unsigned int k = 0; k < fromLevel->num_syms; k++) { + if (fromLevel->s.syms[k] != XKB_KEY_NoSymbol) + intoLevel->s.syms[k] = fromLevel->s.syms[k]; + } + } else if (fromLevel->s.sym != XKB_KEY_NoSymbol) { + intoLevel->s.sym = fromLevel->s.sym; + } + } else { + /* Augment: copy only the keysyms from `from` that are + * undefined in `into` */ + if (unlikely(intoLevel->num_syms > 1)) { + for (unsigned int k = 0; k < intoLevel->num_syms; k++) { + if (intoLevel->s.syms[k] == XKB_KEY_NoSymbol) + intoLevel->s.syms[k] = fromLevel->s.syms[k]; } + } else if (intoLevel->s.sym == XKB_KEY_NoSymbol) { + intoLevel->s.sym = fromLevel->s.sym; } - StealLevelInfo(intoLevel, fromLevel); - actions_replaced = true; } } /* Handle actions */ - if (actions_replaced) { - /* Already handled, included incompatible keysyms/actions count */ - } else if (XkbLevelHasNoAction(fromLevel)) { - /* It's empty for consistency with other comparisons */ - } else if (XkbLevelHasNoAction(intoLevel)) { - /* Take actions from `from` */ - assert(intoLevel->num_syms == fromLevel->num_syms); - StealLevelInfo(intoLevel, fromLevel); - } else { + if (!XkbLevelsSameActions(intoLevel, fromLevel)) { /* Incompatible actions */ - assert(intoLevel->num_syms == fromLevel->num_syms); - if (report) { - if (intoLevel->num_syms > 1) { + if (report && !(intoHasNoAction || fromHasNoAction)) { + if (unlikely(intoLevel->num_syms > 1)) { log_warn( info->ctx, XKB_WARNING_CONFLICTING_KEY_ACTION, "Multiple actions for level %d/group %u on key %s; " @@ -386,14 +410,36 @@ MergeGroups(SymbolsInfo *info, GroupInfo *into, GroupInfo *from, bool clobber, ActionTypeText(ignore->type)); } } - if (clobber) - StealLevelInfo(intoLevel, fromLevel); + if (clobber) { + /* Override: copy any defined action from `from` */ + if (unlikely(fromLevel->num_syms > 1)) { + for (unsigned int k = 0; k < fromLevel->num_syms; k++) { + if (fromLevel->a.actions[k].type != ACTION_TYPE_NONE) + intoLevel->a.actions[k] = fromLevel->a.actions[k]; + } + } else if (fromLevel->a.action.type != ACTION_TYPE_NONE) { + intoLevel->a.action = fromLevel->a.action; + } + } else { + /* Augment: copy only the actions from `from` that are + * undefined in `into` */ + if (unlikely(intoLevel->num_syms > 1)) { + for (unsigned int k = 0; k < intoLevel->num_syms; k++) { + if (intoLevel->a.actions[k].type == ACTION_TYPE_NONE) + intoLevel->a.actions[k] = fromLevel->a.actions[k]; + } + } else if (intoLevel->a.action.type == ACTION_TYPE_NONE) { + intoLevel->a.action = fromLevel->a.action; + } + } } } } /* If @from has extra levels, get them as well. */ darray_foreach_from(level, from->levels, levels_in_both) { darray_append(into->levels, *level); + /* We may have stolen keysyms or actions arrays: + * do not free them when clearing `from` */ level->num_syms = 0; } into->defined |= (from->defined & GROUP_FIELD_ACTS);