Skip to content

Input triggers#4410

Merged
Saviq merged 6 commits intomainfrom
input-triggers-spike
Mar 17, 2026
Merged

Input triggers#4410
Saviq merged 6 commits intomainfrom
input-triggers-spike

Conversation

@tarek-y-ismail
Copy link
Contributor

@tarek-y-ismail tarek-y-ismail commented Nov 13, 2025

What's new?

Initial implementation of the input triggers protocols. Allows privileged clients to reserve a key combination (modifiers + symkey/keycode) and be notified whenever that combination is pressed/released, regardless of which client has focus.

How to test

Direct communication with privileged client:

  • Run mir with --add-wayland-extensions=ext_input_trigger_action_manager_v1:ext_input_trigger_registration_manager_v1
  • Inside the running compositor, launch a terminal
  • Run ./build/bin/trigger_client
  • At the moment, it's hardcoded to request Ctrl + Shift + C and ALT + x for key syms, and Alt + z for scan codes. If you press any of them. "Hey from " will be printed
  • Letting go will result in "Bye from " being printed.

To test Alt + z, run mir with --keymap fr. This should use the azerty layout and make the z key print w, and the q key print z. Even in the azerty layout, Alt + z with the azerty z will not work, but the same combination with the qwerty z will. Note: Right alt is used for special letters, so use left alt :)


Gatekeeper:

  • Run mir with --add-wayland-extensions=ext_input_trigger_action_manager_v1:ext_input_trigger_registration_manager_v1
  • Inside the running compositor, launch a terminal
  • Run python3 examples/gatekeeper/gatekeeper.py --show-ui
  • Run python3 examples/gatekeeper/test_client.py
  • The gatekeeper should request to register two shortcuts. You're free to reject, modify them, or accept them as is.
  • Pressing the shortcuts with the test client not focused will print different messages when begin and end are received.

Checklist

  • Tests added and pass
    • WLCS tests require the addition of keyboard access. Probably for another PR.
  • Final rebase: once the PR is approved

@tarek-y-ismail tarek-y-ismail self-assigned this Nov 13, 2025
@tarek-y-ismail tarek-y-ismail force-pushed the input-triggers-spike branch 2 times, most recently from 76a9c52 to 29f6da5 Compare November 14, 2025 12:32
@tarek-y-ismail
Copy link
Contributor Author

tarek-y-ismail commented Nov 14, 2025

  • The alphabetic key specified in the client request has to be the last key to be pressed. Pressing C + Shift + Ctrl or other combinations where C isn't the last character will not work.

We could solve this in two ways: Explicitly require users to press the alpha key at the end, or manually keep track of the pressed keysyms, consuming only the event where all the keys the user specified are down, which seems to be what i3 does on my machine. For example:

  1. ctrl is pressed, goes to the focused client
  2. c is pressed, goes to the focused client. We record that c is down
  3. shift is pressed. We check if all modifiers and alpha keys are down and if so, we consume this event and call begin.

Edit: I'm opting to implement the second solution

@tarek-y-ismail tarek-y-ismail force-pushed the MIRENG-1493-input-triggers branch from 41c94a9 to 3599e06 Compare November 18, 2025 08:01
@tarek-y-ismail tarek-y-ismail force-pushed the input-triggers-spike branch 2 times, most recently from f488d7b to 5e01560 Compare December 4, 2025 17:17
@tarek-y-ismail
Copy link
Contributor Author

Big TODO: The token authority should be modified to create a new token type (issue_trigger_token?) to allow multiple triggers to be registered.

Right now, tokens are used to store the relationship between a trigger, the input filter, and the corresponding action (that's what I remember, the architecture is a bit hazy in my head right now). So having tokens that are invalidated after a short time would require replacing this with something else.

The first idea that pops up into my head is using the token to just establish the relationship between the three objects, but not holding onto it as I do right now.

@AlanGriffiths AlanGriffiths force-pushed the MIRENG-1493-input-triggers branch from 7c58613 to 90f9c87 Compare January 16, 2026 11:09
Base automatically changed from MIRENG-1493-input-triggers to main January 20, 2026 11:07
@tarek-y-ismail tarek-y-ismail force-pushed the input-triggers-spike branch 4 times, most recently from 1cf3f74 to f8c1c47 Compare January 20, 2026 14:41
Comment on lines +70 to +71
input_trigger_registration_v1.cpp input_trigger_registration_v1.h
input_trigger_action_v1.cpp input_trigger_action_v1.h
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugly indentation. But probably something for another PR

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a spike/prototype for an input trigger protocol that allows Wayland clients to register global keyboard shortcuts. The protocol consists of two parts: trigger registration (defining shortcuts) and action management (handling shortcut events).

Changes:

  • Implements Wayland protocol extensions for input trigger registration and action management
  • Adds support infrastructure including token-based action tracking and event filtering
  • Includes example client and Python-based gatekeeper for testing the protocol

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/server/frontend_wayland/wayland_default_configuration.cpp Registers the two new input trigger protocol extensions in the Wayland extension system
src/server/frontend_wayland/wayland_connector.h Adds InputTriggerData and Clock dependencies to WaylandExtensions context
src/server/frontend_wayland/wayland_connector.cpp Instantiates InputTriggerData and passes it to extension context
src/server/frontend_wayland/input_trigger_registration_v1.h Defines protocol interfaces for trigger registration and KeyboardSymTrigger class
src/server/frontend_wayland/input_trigger_registration_v1.cpp Implements registration manager, action controls, and modifier mapping logic
src/server/frontend_wayland/input_trigger_data.h Defines shared data structure for tracking pending trigger actions
src/server/frontend_wayland/input_trigger_action_v1.h Defines protocol interfaces for action management
src/server/frontend_wayland/input_trigger_action_v1.cpp Implements action manager, keyboard event filtering, and modifier matching
src/server/frontend_wayland/CMakeLists.txt Adds new source files to build system
src/common/input/xkb_mapper.cpp Moves expand_modifiers function to namespace scope for external use
include/common/mir/input/xkb_mapper.h Exports expand_modifiers function for use in trigger protocol
examples/trigger_client/main.cpp Implements standalone C++ Wayland client to test trigger registration
examples/trigger_client/CMakeLists.txt Build configuration for trigger_client example
examples/gatekeeper/test_client.py Python test client using D-Bus portal interface
examples/gatekeeper/generate_protocols.py Script to generate Python bindings for Wayland protocols
examples/gatekeeper/gatekeeper.py Python-based UI for managing global shortcuts via D-Bus portal
examples/CMakeLists.txt Includes trigger_client subdirectory in build
debian/mir-demos.install Adds trigger_client to package install list
rpm/mir.spec Adds trigger_client to RPM package
CMakeLists.txt Adds coverage build configuration and fixes platform path in LGPL test

Copy link
Contributor Author

@tarek-y-ismail tarek-y-ismail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick review for the gatekeeper and test client

Gatekeeper is a bit spaghettified, test client is alright.

Comment on lines +13 to +14
# Add current directory to path to find generated protocols
sys.path.append(os.path.dirname(os.path.abspath(__file__)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary?

Comment on lines +62 to +69
# Minimal Keysym definitions (XKB)
KEY_A = 0x0061
# ... add more as needed

MOD_SHIFT = 0x08
MOD_CTRL = 0x100
MOD_ALT = 0x01
MOD_META = 0x800
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, maybe there's a library to handle this?

Comment on lines +100 to +101
lbl = Gtk.Label(label="An application wants to register the following shortcuts:")
box.append(lbl)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary variable.

Suggested change
lbl = Gtk.Label(label="An application wants to register the following shortcuts:")
box.append(lbl)
box.append(Gtk.Label(label="An application wants to register the following shortcuts:"))

class GlobalShortcutsPortal:
def __init__(self, app):
self.app = app
self.sessions = {} # session_handle -> { 'app_id': str, 'shortcuts': dict, 'sender': str }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we need a comment with the structure, might as well use type hints.

Comment on lines +206 to +212
# [CHANGE] Added sender argument
def CreateSession(self, request_handle, session_handle, app_id, options, sender):
logger.info(f"CreateSession: {session_handle} for {app_id} (Sender: {sender})")
self.sessions[session_handle] = {
'app_id': app_id,
'shortcuts': {},
'sender': sender # [CHANGE] Store the sender
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftovers from when I was vibing this

Suggested change
# [CHANGE] Added sender argument
def CreateSession(self, request_handle, session_handle, app_id, options, sender):
logger.info(f"CreateSession: {session_handle} for {app_id} (Sender: {sender})")
self.sessions[session_handle] = {
'app_id': app_id,
'shortcuts': {},
'sender': sender # [CHANGE] Store the sender
def CreateSession(self, request_handle, session_handle, app_id, options, sender):
logger.info(f"CreateSession: {session_handle} for {app_id} (Sender: {sender})")
self.sessions[session_handle] = {
'app_id': app_id,
'shortcuts': {},
'sender': sender

from gi.repository import Gio, GLib

# XML for the interface we (the client) will expose to receive events
# [FIXED] Removed session_handle argument to match Gatekeeper's call signature (sua{sv})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# [FIXED] Removed session_handle argument to match Gatekeeper's call signature (sua{sv})

def on_client_method_call(self, connection, sender, object_path, interface_name, method_name, parameters, invocation):
# Handle the callbacks from Gatekeeper
if method_name == "Activated":
# [FIXED] Unpack 3 arguments: shortcut_id, timestamp, options
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# [FIXED] Unpack 3 arguments: shortcut_id, timestamp, options

invocation.return_value(None)

elif method_name == "Deactivated":
# [FIXED] Unpack 3 arguments
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# [FIXED] Unpack 3 arguments

def setup_client_listener(self):
# We simply register the object. Gatekeeper knows our bus unique name (e.g. :1.99)
# because it captured it when we called CreateSession.
session_path = "/org/freedesktop/portal/desktop/session/test_client/1"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably means we can't have multiple test clients?

@tarek-y-ismail
Copy link
Contributor Author

Ok, I think this is ready to review now. One thing that could be improved is that the data inside InputTriggerData could be encapsulated better instead of just being public, but that's a minor thing in my opinion.

@tarek-y-ismail tarek-y-ismail marked this pull request as ready for review January 27, 2026 16:25
@tarek-y-ismail tarek-y-ismail requested a review from a team as a code owner January 27, 2026 16:25
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK as a throwaway spike: it works without too much pain.

But for maintainable code that we want to land:

  1. There are apparently spurious changes to xkb_mapper;
  2. poor encapsulation of logic; and,
  3. a failure to leverage the threading mode.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

@tarek-y-ismail tarek-y-ismail force-pushed the input-triggers-spike branch 2 times, most recently from 69fa308 to 3b07faa Compare March 12, 2026 15:57
@tarek-y-ismail tarek-y-ismail requested a review from Copilot March 12, 2026 16:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

validity and availability

Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have a few comments, but looks sensible!

@tarek-y-ismail
Copy link
Contributor Author

tarek-y-ismail commented Mar 13, 2026

Bug I just thought. Throwing it here before I forget.

If key release order is not the same as key press order, clients will see inconsistent key states. For example with Ctrl + Shift + C. The first to will go to the client, "C" will be consumed as it completes the combo. If the combo isn't broken with "C", then the client will see a key up event without a corresponding key down event. And for the key breaking the combo, it should see a key up event.

Will probably have to synthesize events in this case.

Edit: After some contemplation, I think the following is in order:

  1. A new class will be created to keep track of which events were consumed and which were passed to clients. This should allow us to look them up on key up events and also consume or pass them
  2. Related to this, we also need to handle repeat events. If a user presses "C" then Shift then Ctrl. I believe that "c" should repeat, then "C", then as the event is activated, repeat events should be consumed. If Ctrl is let go of, then "C" repeat events should continue to the clients, and "c" events should do the same if Shift is let go. To facilitate this, I will probably need to inform this new class of EventOutcomes so they know when triggers are active or not.

The high level overview is that any_trigger_handled would return the event outcome instead of a boolean. This new class would use that, the state it's maintaining, and the given event to determine what to do with it (points 1 and 2 above)

Edit: handled in 70355cf

@tarek-y-ismail
Copy link
Contributor Author

Got some commits stuck. Can't push right now. Will see to it later.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.

void InputTriggerActionControlV1::cancel()
{
action_group->cancel();
delete this;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't originally sure if this was the right call or not. The thunk code does look like it calls wl_resource_destroy which I will assume handles freeing everything. Will leave it unresolved if anyone wants to chime in.

Comment on lines +460 to +465
delete this;
}

void InputTriggerActionControlV1::destroy()
{
delete this;
@tarek-y-ismail tarek-y-ismail requested a review from Copilot March 13, 2026 22:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.

Comment on lines +73 to +85
bool mf::InputTriggerRegistry::register_trigger(Trigger* trigger)
{
// Housekeeping
std::erase_if(triggers, [](auto const& weak_trigger) { return !weak_trigger; });

auto const already_registered = sr::any_of(
triggers, [trigger](auto const& other_trigger) { return trigger->is_same_trigger(&other_trigger.value()); });

if (already_registered)
return false;

triggers.push_back(wayland::make_weak(trigger));
return true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends on whether we should handle generic + sided modifiers being specified, which is currently being discussed in #4410 (comment)

Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider my feedback, but I will not block :)

@github-actions
Copy link

TICS Quality Gate

✔️ Passed

mir

Coding Standards: ✔️ Passed

✔️ Condition “No new Coding Standard Violations for level 1, 2, 3 with respect to Previous analysis” passed.

See the results in the TICS Viewer

The following files have been checked for this project
  • src/server/frontend_wayland/input_trigger_action_v1.cpp
  • src/server/frontend_wayland/input_trigger_action_v1.h
  • src/server/frontend_wayland/input_trigger_registration_v1.cpp
  • src/server/frontend_wayland/input_trigger_registration_v1.h
  • src/server/frontend_wayland/input_trigger_registry.cpp
  • src/server/frontend_wayland/input_trigger_registry.h
  • src/server/frontend_wayland/keyboard_state_tracker.cpp
  • src/server/frontend_wayland/keyboard_state_tracker.h
  • src/server/frontend_wayland/wayland_connector.cpp
  • src/server/frontend_wayland/wayland_connector.h
  • src/server/frontend_wayland/wayland_default_configuration.cpp
  • src/server/frontend_wayland/wl_seat.cpp
  • src/server/frontend_wayland/wl_seat.h

TICS / TICS / Run TICS analysis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants