Conversation
📝 WalkthroughWalkthroughThe pull request transitions input forwarding from XTest-based injection to a unified wlroots compositor architecture. It replaces XWaylandServer with CompositorServer, removes X11/XTest library dependencies, introduces thread-safe event marshaling via SPSCQueue and eventfd between main and compositor threads, and updates the public API to expose separate X11 and Wayland display accessors. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Main as Main Thread
participant Queue as SPSCQueue
participant EventFD as eventfd
participant Comp as Compositor Thread
participant Seat as wlr_seat
participant WaylandApp as Wayland App
participant XWaylandApp as X11 App (via wlr_xwm)
User->>Main: forward_key(scancode)
Main->>Main: sdl_to_linux_keycode()
Main->>Queue: enqueue InputEvent{type:key, code, pressed}
Main->>EventFD: write (signal)
EventFD-->>Comp: wakeup
Comp->>Queue: dequeue InputEvent
alt Keyboard Enter (on first event)
Comp->>Seat: wlr_seat_keyboard_enter()
end
Comp->>Seat: wlr_seat_keyboard_notify_key()
alt Wayland Surface Active
Seat->>WaylandApp: wl_keyboard::key event
else XWayland Surface Active
Seat->>Seat: wlr_xwm translate to X11 KeyPress
Seat->>XWaylandApp: X11 KeyPress (via wlr_xwm)
end
Note over Comp,Seat: Pointer events follow similar path<br/>(inject_pointer_motion, _button, _axis)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Add CompositorServer (headless wlroots) for Wayland/XWayland hosting - Forward SDL keyboard + mouse events via wlr_seat_*_notify_* injection - Remove legacy XWaylandServer implementation - Vendor xdg-shell protocol header for compositor/client plumbing - Split input tests into dedicated X11 + Wayland binaries and update CMake - Update input forwarding docs and OpenSpec change proposal
PR Compliance Guide 🔍(Compliance updated until commit 47116b4)Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label Previous compliance checksCompliance check up to commit e37b8f0
|
|||||||||||||||||||||||||||||||||||||||||||||||
PR Code Suggestions ✨Latest suggestions up to 47116b4
Previous suggestions✅ Suggestions up to commit e37b8f0
|
|||||||||||||||||||||||||||||||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @openspec/changes/add-wayland-input-forwarding/proposal.md:
- Around line 7-17: Mark the two breaking changes in the "What Changes" list by
prefixing them with **BREAKING**: update the bullet "Replace XTest injection
with unified `wlr_seat_*` APIs for both X11 and Wayland apps" to begin with
"**BREAKING**" and similarly prefix "Remove X11/XTest dependencies from input
forwarding" with "**BREAKING**"; ensure the first note briefly mentions the API
impact (removes `display_number()` and adds `x11_display()`/`wayland_display()`)
and the second note mentions the build impact (removes libX11/libXtst link
targets) so readers immediately see the scope of the breaking change.
In @src/input/compositor_server.cpp:
- Around line 53-54: The error message returned from
make_error<int>(ErrorCode::input_init_failed, ...) incorrectly refers to X11
"DISPLAY" numbers; update the string to reference Wayland socket names instead
(e.g., "No available Wayland socket names (wayland-1..9 all bound)" or similar),
keeping the same ErrorCode and make_error<int> usage so the return statement and
semantics remain unchanged.
🧹 Nitpick comments (11)
openspec/changes/add-wayland-input-forwarding/design.md (2)
40-57: Consider adding language identifiers to diagram code blocks.The static analysis tool flags these blocks for missing language specifiers. For ASCII diagrams, you can use
textorplaintextto satisfy the linter while accurately describing the content.🔎 Proposed fix
-``` +```text SDL Event (main thread) |
184-232: Class definition in design doc is incomplete vs. implementation.The
CompositorServerclass definition shown here is missing several members that exist in the actual implementation (compositor_server.cpp), including:
m_focused_xsurface(wlr_xwayland_surface*)m_outputandm_output_layoutm_xkb_contextm_event_sourcem_last_pointer_x/yThis is acceptable for a design doc (showing essential API), but consider adding a note that this is a simplified view.
src/input/compositor_server.cpp (2)
315-327: Consider logging failed eventfd writes.When
write(m_event_fd.get(), &val, sizeof(val))fails (returns != 8), the event is successfully queued but the compositor thread may not wake immediately. While not a data loss issue (next write will wake it), this could cause input latency. Consider adding a debug log for monitoring.
609-613: Dead code: XWayland surfaces are never in m_surfaces.Per the design, XWayland surfaces are intentionally NOT added to
m_surfaces(see Line 573-575). Thisstd::findanderasewill never find anything. Consider removing this dead code or adding a comment explaining it's defensive.🔎 Proposed fix
void CompositorServer::handle_xwayland_surface_destroy() { // Remove all XWayland-related listeners wl_list_remove(&m_listeners->xwayland_surface_destroy.link); wl_list_init(&m_listeners->xwayland_surface_destroy.link); wl_list_remove(&m_listeners->xwayland_surface_associate.link); wl_list_init(&m_listeners->xwayland_surface_associate.link); wl_list_remove(&m_listeners->xwayland_surface_map.link); wl_list_init(&m_listeners->xwayland_surface_map.link); // Clear focus if this was an XWayland surface if (m_focused_xsurface) { - auto it = std::find(m_surfaces.begin(), m_surfaces.end(), m_focused_surface); - if (it != m_surfaces.end()) { - m_surfaces.erase(it); - } - m_focused_xsurface = nullptr; m_focused_surface = nullptr; wlr_seat_keyboard_clear_focus(m_seat); wlr_seat_pointer_clear_focus(m_seat); } m_listeners->pending_xsurface = nullptr; }docs/input_forwarding.md (1)
113-144: Add language specifier to thread model diagram.The static analysis tool flags this code block for missing language specifier. Use
textfor ASCII diagrams.🔎 Proposed fix
-``` +```text Main Thread Compositor Threadtests/input/goggles_input_test_wayland.cpp (1)
8-89: Consider extracting shared RAII wrappers to reduce duplication.
VulkanInstanceandSDLContextare identical betweengoggles_input_test_wayland.cppandgoggles_input_test_x11.cpp(per the relevant code snippets). Consider extracting these to a shared header liketests/input/test_helpers.hppto eliminate duplication.This is a nice-to-have for maintainability but not blocking.
src/input/input_forwarder.cpp (3)
12-119: Keycode mapping is incomplete.The mapping covers basic alphanumeric keys and modifiers but is missing commonly used keys such as:
- Function keys (F1-F12)
- Punctuation (
,,.,/,;,',[,],\,-,=)- Navigation keys (Delete, Insert, Home, End, Page Up/Down)
- Caps Lock, Num Lock, Scroll Lock
This may cause user frustration when these keys are silently dropped (returning 0). Consider expanding the mapping or adding a warning log for unmapped scancodes to aid debugging.
184-191: TODO: Coordinate mapping may cause incorrect cursor positioning.The TODO comment indicates coordinate mapping for different window sizes is not implemented. If the goggles window size differs from the target surface size, the cursor will be positioned incorrectly. Consider tracking this as a follow-up task.
Would you like me to open an issue to track implementing coordinate mapping for window size differences?
193-210: Magic number 15.0 for scroll delta should be documented or named.The scroll multiplier
15.0lacks explanation. This value likely represents pixels-per-scroll-unit but the rationale should be documented. Consider extracting to a named constant:🔎 Suggested improvement
+namespace { +// Scroll delta multiplier: converts discrete scroll units to axis delta. +// Wayland axis events use pixel-like values; 15 pixels per discrete step +// approximates a typical scroll experience. +constexpr double kScrollDeltaMultiplier = 15.0; +} // namespace auto InputForwarder::forward_mouse_wheel(const SDL_MouseWheelEvent& event) -> Result<void> { if (event.y != 0) { // SDL: positive = up, Wayland: positive = down; negate to match - double value = static_cast<double>(-event.y) * 15.0; + double value = static_cast<double>(-event.y) * kScrollDeltaMultiplier; if (!m_impl->server.inject_pointer_axis(value, false)) { GOGGLES_LOG_DEBUG("Input queue full, dropped axis event"); } } if (event.x != 0) { - double value = static_cast<double>(event.x) * 15.0; + double value = static_cast<double>(event.x) * kScrollDeltaMultiplier; if (!m_impl->server.inject_pointer_axis(value, true)) { GOGGLES_LOG_DEBUG("Input queue full, dropped axis event"); } } return {}; }src/input/compositor_server.hpp (1)
55-60: Display accessors return invalid values beforestart()is called.
m_display_numberis initialized to-1, so callingx11_display()orwayland_display()beforestart()returns invalid display strings like":-1"or"wayland--1". Consider:
- Documenting this as a precondition, or
- Returning
std::optional<std::string>to signal unavailability, or- Adding an assertion/check
🔎 Option: Return empty string if not started
[[nodiscard]] auto x11_display() const -> std::string { + if (m_display_number < 0) return {}; return ":" + std::to_string(m_display_number); } [[nodiscard]] auto wayland_display() const -> std::string { + if (m_display_number < 0) return {}; return "wayland-" + std::to_string(m_display_number); }src/input/xdg-shell-protocol.h (1)
1-6: Vendored generated protocol header - consider documenting regeneration process.This is an auto-generated file from
wayland-scanner 1.24.0. Consider:
- Adding a comment at the top noting this is vendored and should not be manually edited
- Documenting how to regenerate (e.g., from which
xdg-shell.xmlversion and with what command)- Alternatively, generating this at build time via CMake's
wayland_scannerintegrationThis helps maintainers update the protocol when needed and prevents accidental manual edits.
🔎 Suggested header comment
/* Generated by wayland-scanner 1.24.0 */ /* * DO NOT EDIT - This file is generated from xdg-shell.xml * * To regenerate: * wayland-scanner server-header \ * /usr/share/wayland-protocols/stable/xdg-shell/xdg-shell.xml \ * src/input/xdg-shell-protocol.h */
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
docs/input_forwarding.mdopenspec/changes/add-wayland-input-forwarding/design.mdopenspec/changes/add-wayland-input-forwarding/proposal.mdopenspec/changes/add-wayland-input-forwarding/specs/input-forwarding/spec.mdopenspec/changes/add-wayland-input-forwarding/tasks.mdsrc/app/main.cppsrc/input/CMakeLists.txtsrc/input/compositor_server.cppsrc/input/compositor_server.hppsrc/input/input_forwarder.cppsrc/input/input_forwarder.hppsrc/input/xdg-shell-protocol.hsrc/input/xwayland_server.cppsrc/input/xwayland_server.hpptests/CMakeLists.txttests/input/goggles_input_test_wayland.cpptests/input/goggles_input_test_x11.cpp
💤 Files with no reviewable changes (2)
- src/input/xwayland_server.hpp
- src/input/xwayland_server.cpp
🧰 Additional context used
📓 Path-based instructions (3)
openspec/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/**/*.md: Use#### Scenario:format (4 hashtags) for scenario headers in requirements, not bullets or bold text, with at least one scenario per requirement
Use format- **WHEN** [condition] - **THEN** [expected result]for scenario steps in requirements
Files:
openspec/changes/add-wayland-input-forwarding/tasks.mdopenspec/changes/add-wayland-input-forwarding/proposal.mdopenspec/changes/add-wayland-input-forwarding/design.mdopenspec/changes/add-wayland-input-forwarding/specs/input-forwarding/spec.md
openspec/changes/**/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
List spec deltas in
proposal.mdunder 'What Changes' section, marking breaking changes with BREAKING
Files:
openspec/changes/add-wayland-input-forwarding/proposal.md
openspec/changes/**/specs/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/changes/**/specs/**/*.md: Write spec deltas using## ADDED|MODIFIED|REMOVED|RENAMED Requirementsformat with at least one#### Scenario:per requirement in spec files
When modifying existing requirements in a MODIFIED delta, paste the full requirement block including the header and all scenarios, as the archiver will replace the entire requirement
Use ADDED for new capabilities that can stand alone; use MODIFIED for changes to existing requirement behavior, scope, or acceptance criteria; use RENAMED for name-only changes
Files:
openspec/changes/add-wayland-input-forwarding/specs/input-forwarding/spec.md
🧠 Learnings (9)
📚 Learning: 2025-12-29T15:44:30.520Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: openspec/project.md:0-0
Timestamp: 2025-12-29T15:44:30.520Z
Learning: Applies to openspec/tests/**/*.{cpp,hpp} : Focus unit tests on non-GPU logic: utility functions, configuration parsing, and pipeline graph logic
Applied to files:
tests/CMakeLists.txttests/input/goggles_input_test_x11.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/tests/**/*.cpp : Only non-GPU logic is tested initially: in scope are utility functions, configuration parsing, error handling, pipeline graph logic. Out of scope: Vulkan initialization, GPU resource management, rendering, capture layer.
Applied to files:
tests/CMakeLists.txttests/input/goggles_input_test_x11.cpptests/input/goggles_input_test_wayland.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/src/capture/vk_layer/**/*.{cpp,hpp} : Vulkan layer code in `src/capture/vk_layer/` has strict logging constraints: use only `error` and `critical` levels, no file I/O or blocking operations in hot paths, prefix all logs with `[goggles_vklayer]`, never log in `vkQueuePresentKHR` hot path.
Applied to files:
tests/CMakeLists.txttests/input/goggles_input_test_x11.cpptests/input/goggles_input_test_wayland.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/**/*.{cpp,hpp} : All `vulkan-hpp` calls returning `vk::Result` must be explicitly checked. Prohibited: masking with `static_cast<void>()`. Use `VK_TRY` macro for early return with `vk::Result`, `GOGGLES_TRY` for Result<T> propagation (expression-style), `GOGGLES_MUST` for internal invariants (aborts on failure).
Applied to files:
tests/input/goggles_input_test_x11.cpptests/input/goggles_input_test_wayland.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/src/**/*.{cpp,hpp} : For Vulkan resources, follow RAII guidelines. Use `vk::Unique*` only for appropriate resource types (per D.2 table). Call `device.waitIdle()` or wait on appropriate fences before destroying resources with GPU-async lifetime. Store creation info with resources for debugging. Never leak Vulkan objects. Declare members in reverse destruction order (device before instance).
Applied to files:
tests/input/goggles_input_test_x11.cpptests/input/goggles_input_test_wayland.cpp
📚 Learning: 2025-12-29T15:44:30.520Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: openspec/project.md:0-0
Timestamp: 2025-12-29T15:44:30.520Z
Learning: Applies to openspec/src/render/**/*.{cpp,hpp} : Use `vk::Unique*` for long-lived Vulkan resources; use plain handles for per-frame/GPU-async resources
Applied to files:
tests/input/goggles_input_test_x11.cpptests/input/goggles_input_test_wayland.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/src/**/*.{cpp,hpp} : Due to async GPU execution, RAII handles (`vk::Unique*`) cannot automatically handle synchronization. Use selectively: `vk::Unique*` for Instance, Device, Surface (long-lived singletons), Swapchain, Pipelines, Layouts (created once). Use plain handles for Command buffers, per-frame sync primitives (Fence, Semaphore), Imported external images.
Applied to files:
tests/input/goggles_input_test_x11.cpptests/input/goggles_input_test_wayland.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/**/*.{cpp,hpp} : All resources must be managed via RAII. Wrap Vulkan objects in RAII types (custom or from libraries like `vk-bootstrap`, `vulkan-hpp`). Use RAII wrappers for file handles, sockets, etc. (`std::fstream`, `UniqueFd`, custom types). No manual cleanup in destructors unless encapsulated in RAII.
Applied to files:
tests/input/goggles_input_test_wayland.cpp
📚 Learning: 2025-12-29T15:44:30.520Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: openspec/project.md:0-0
Timestamp: 2025-12-29T15:44:30.520Z
Learning: Applies to openspec/src/capture/vk_layer/**/*.{cpp,hpp} : Do not perform blocking operations or file I/O in capture layer performance-critical paths (e.g., vkQueuePresentKHR)
Applied to files:
tests/input/goggles_input_test_wayland.cpp
🧬 Code graph analysis (6)
src/input/input_forwarder.hpp (1)
src/input/compositor_server.hpp (2)
nodiscard(55-57)nodiscard(58-60)
tests/input/goggles_input_test_x11.cpp (1)
tests/input/goggles_input_test_wayland.cpp (6)
init_vulkan(93-113)init_vulkan(93-93)init_sdl(115-146)init_sdl(115-115)run_input_loop(182-210)run_input_loop(182-183)
src/input/compositor_server.cpp (3)
src/util/error.hpp (1)
make_error(50-52)src/input/compositor_server.hpp (15)
CompositorServer(45-45)CompositorServer(46-46)CompositorServer(48-48)CompositorServer(50-50)toplevel(99-99)xsurface(104-104)xsurface(105-105)xsurface(110-110)linux_keycode(62-62)sx(63-63)button(64-64)value(65-65)surface(103-103)surface(107-107)surface(109-109)src/input/input_forwarder.hpp (4)
event(19-19)event(20-20)event(21-21)event(22-22)
src/input/input_forwarder.cpp (2)
src/input/compositor_server.hpp (3)
linux_keycode(62-62)button(64-64)value(65-65)src/input/input_forwarder.hpp (4)
event(19-19)event(20-20)event(21-21)event(22-22)
src/input/compositor_server.hpp (1)
src/input/compositor_server.cpp (2)
CompositorServer(73-81)CompositorServer(83-85)
tests/input/goggles_input_test_wayland.cpp (1)
tests/input/goggles_input_test_x11.cpp (20)
VulkanInstance(11-11)VulkanInstance(13-17)VulkanInstance(19-19)VulkanInstance(22-24)other(26-35)other(26-26)other(67-88)other(67-67)SDLContext(43-43)SDLContext(45-55)SDLContext(57-57)SDLContext(60-65)print_event_info(146-178)print_event_info(146-146)init_vulkan(93-113)init_vulkan(93-93)init_sdl(115-144)init_sdl(115-115)run_input_loop(180-208)run_input_loop(180-181)
🪛 markdownlint-cli2 (0.18.1)
openspec/changes/add-wayland-input-forwarding/design.md
40-40: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
63-63: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
103-103: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
139-139: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
297-297: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/input_forwarding.md
113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Static Analysis (clang-tidy)
- GitHub Check: Build and Test (test preset)
🔇 Additional comments (25)
src/input/CMakeLists.txt (1)
3-18: LGTM! Clean migration from XTest to wlroots.The CMake configuration correctly reflects the architectural changes:
compositor_server.cppreplacesxwayland_server.cpp, X11/XTest dependencies are removed, and wlroots/wayland-server dependencies remain in place for the new seat-based input model.src/app/main.cpp (1)
244-245: LGTM! Improved logging for dual-display awareness.The updated log message correctly uses the new
x11_display()andwayland_display()accessors, providing clear visibility into both the XWayland and native Wayland display identifiers exposed by the unified CompositorServer.src/input/input_forwarder.hpp (1)
23-24: LGTM! API properly exposes dual-display identifiers.The new
x11_display()andwayland_display()methods correctly expose the distinct display identifiers for X11 (via XWayland) and native Wayland clients, replacing the previous singledisplay_number()accessor. This aligns with the unified CompositorServer architecture.tests/input/goggles_input_test_x11.cpp (4)
98-98: LGTM! Consistent branding for X11 test variant.The test binary is correctly branded with "(X11)" suffixes in application name, window title, and log prefixes (
goggles_input_test_x11), clearly distinguishing it from the Wayland variant.Also applies to: 108-108, 121-121, 127-127, 129-129, 135-135
139-139: Nice visual differentiation with background color.The reddish background color (40,0,0,255) provides a clear visual distinction from the Wayland variant's greenish background, making it easy to identify which test is running at a glance.
183-185: LGTM! Clear test context in header.The updated header accurately describes the test's purpose: verifying input forwarding via XWayland on the X11 backend, consistent with the PR's unified seat-based architecture.
220-223: Good diagnostic practice logging both display variables.Logging both
DISPLAYandWAYLAND_DISPLAYprovides useful context for debugging the compositor's display configuration, even though this is the X11 test variant. This mirrors the approach in the Wayland variant and aids in troubleshooting dual-display scenarios.openspec/changes/add-wayland-input-forwarding/specs/input-forwarding/spec.md (1)
1-157: Spec follows required format and covers all key requirements.The specification correctly uses
#### Scenario:headers and the- **WHEN** ... - **THEN** ...format as per coding guidelines. Each requirement has at least one scenario with clear acceptance criteria.The documentation of XWayland surface handling (Lines 34, 53-58) correctly explains the design decision to avoid destroy listeners for XWayland surfaces, which aligns with the implementation in
compositor_server.cpp.tests/CMakeLists.txt (1)
92-122: Test targets are well-structured and consistent.Both
goggles_input_test_x11andgoggles_input_test_waylandshare identical linking and definitions, which is appropriate for their parallel purpose. Sanitizers are correctly enabled for both.Note: These graphical test targets are not registered with CTest via
add_test(), which appears intentional for manual testing as documented indocs/input_forwarding.md.openspec/changes/add-wayland-input-forwarding/tasks.md (1)
1-97: Comprehensive task tracking with clear rationale.The task list provides excellent traceability for the implementation work. The note on Line 25 explaining why XWayland destroy listeners are not used is particularly valuable for future maintainers.
src/input/compositor_server.cpp (6)
59-81: Listener initialization pattern is correct.The
Listenersstruct properly aggregates allwl_listenerinstances, and the constructor correctly initializes all listener links withwl_list_init()before any signal registration. This prevents undefined behavior from uninitialized list pointers.
163-166: Manual memory management for wlr_keyboard is acceptable here.The
new wlr_keyboard{}is paired withdelete m_keyboardinstop()(Line 268). This is appropriate because wlroots requires the keyboard to outlive the seat, and the struct must be allocated on the heap for wlroots signal handling.
87-89: RAII cleanup guard pattern is well-implemented.The
cleanup_on_errorlambda withstd::unique_ptrguard ensuresstop()is called if any initialization step fails, preventing resource leaks. Theguard.release()at Line 234 correctly disables cleanup on success.
238-313: Destruction order and thread synchronization are correct.The
stop()method properly:
- Terminates the event loop before joining the thread (Line 243-248)
- Removes the event source before closing the eventfd (Line 251-254)
- Destroys resources in reverse initialization order
- Correctly leaves
m_xdg_shellandm_compositoras nullptr without explicit destroy (owned bywl_display)
382-393: XWayland re-activation on every key event is intentional.The pattern of calling
wlr_xwayland_surface_activate()andwlr_seat_keyboard_notify_enter()before each key event ensures wlr_xwm maintains correct X11 focus state. While seemingly redundant, this is documented in the design as necessary for reliable XWayland input delivery.
637-658: Focus transition safely handles stale XWayland pointers.The implementation correctly clears
m_focused_xsurface(Line 644) before making any wlroots API calls that might try to send leave events to the old surface. This prevents crashes when switching from XWayland to native Wayland, as documented in the design.docs/input_forwarding.md (1)
1-238: Documentation is comprehensive and well-organized.The updated documentation clearly explains:
- Unified input path via wlr_seat APIs
- Thread model and cross-thread marshaling
- Focus transition behavior and limitations
- Testing procedures for both X11 and Wayland backends
- Removed dependencies (libX11, libXtst)
The diagrams effectively illustrate the architecture and data flow.
tests/input/goggles_input_test_wayland.cpp (2)
93-146: Initialization functions are well-structured.The
init_vulkan()andinit_sdl()functions properly handle errors with clear messages and use RAII for resource management. The Wayland-specificsetenv("SDL_VIDEODRIVER", "wayland", 1)correctly forces SDL to use the Wayland backend.
212-231: Main function provides useful diagnostic output.Logging both
DISPLAYandWAYLAND_DISPLAYenvironment variables (Lines 221-225) helps verify the test is connected to the correct compositor, matching the documentation indocs/input_forwarding.md.src/input/input_forwarder.cpp (3)
121-136: LGTM!The mouse button mapping correctly translates SDL buttons to Linux input codes, including the extra buttons (X1/X2) mapped to BTN_SIDE/BTN_EXTRA.
160-170: LGTM!The function correctly handles unmapped scancodes by returning early and logs dropped events without failing. This is appropriate for input forwarding where occasional event drops are acceptable.
212-218: LGTM!Clean delegation to the underlying compositor server for display accessors.
src/input/compositor_server.hpp (3)
1-28: LGTM!Forward declarations avoid pulling in heavy Wayland/wlroots headers into translation units that include this header. The NOLINT comments appropriately suppress naming convention warnings for external C library types.
32-41: LGTM!The
InputEventstruct is appropriately designed for cross-thread event marshaling via the SPSC queue. The flat layout with type-discriminated field usage is efficient and simple.
67-111: LGTM!The private implementation is well-structured:
- Raw pointers are appropriate for wlroots-managed objects
- PIMPL pattern for
Listenerskeeps wl_listener details out of the headerstd::jthreadensures automatic thread joining on destruction- SPSC queue with capacity 64 provides adequate buffering for input events
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @src/input/compositor_server.cpp:
- Around line 641-652: The method handle_xwayland_surface_map and its associated
listener state listeners.xwayland_surface_map are dead code because no
wl_signal_add registers that listener (only xwayland_surface_associate is
added); remove the unused method handle_xwayland_surface_map, the
listeners.xwayland_surface_map member and any related cleanup/initialization
(the wl_list_remove/wl_list_init calls and any other references such as the
cleanup sites around where pending_xsurface is handled) and ensure only the
active listener xwayland_surface_associate and its handlers remain to avoid
no-op cleanup logic.
♻️ Duplicate comments (1)
src/input/compositor_server.cpp (1)
56-57: Error message refers to wrong display type.This was flagged in a previous review. The error message mentions "DISPLAY numbers" but this function binds Wayland sockets (
wayland-N), not X11 displays.
🧹 Nitpick comments (4)
src/input/compositor_server.hpp (1)
12-19: Consider documenting which fields are valid for each event type.The
InputEventstruct has fields that are only meaningful for specific event types (e.g.,x/yfor pointer_motion,value/horizontalfor pointer_axis). While using a flat struct is fine for SPSC queue efficiency, a brief comment documenting the field-to-type mapping would improve maintainability.🔎 Suggested documentation
+/// Event data for input injection. Fields used depend on `type`: +/// - key: code (linux keycode), pressed +/// - pointer_motion: x, y +/// - pointer_button: code (button code), pressed +/// - pointer_axis: value, horizontal struct InputEvent { InputEventType type; - uint32_t code; - bool pressed; - double x, y; - double value; - bool horizontal; + uint32_t code; ///< Linux keycode (key) or button code (pointer_button) + bool pressed; ///< Press state (key, pointer_button) + double x, y; ///< Surface coordinates (pointer_motion) + double value; ///< Scroll amount (pointer_axis) + bool horizontal; ///< Axis orientation (pointer_axis) };src/input/compositor_server.cpp (3)
295-295: Thread captures reference to local - verify lifetime.The lambda captures
&implwhich is a reference to the local variableimpl(which is itself a reference to*m_impl). This is safe because:
implreferencesm_impl's pointeem_imploutlives the thread (joined instop())stop()is called in destructorHowever, capturing by value (
m_impl.get()stored in a local pointer) would be more explicit about the intent.🔎 Alternative: capture pointer explicitly
- impl.compositor_thread = std::jthread([&impl] { wl_display_run(impl.display); }); + auto* display = impl.display; + impl.compositor_thread = std::jthread([display] { wl_display_run(display); });
444-454: Re-entering keyboard focus on every key event may have performance implications.The code re-activates the XWayland surface and re-notifies keyboard enter before every key event. While comments indicate this is required for XWayland, this generates additional Wayland protocol traffic. Consider if this could be optimized by tracking whether re-entry is actually needed (e.g., after focus loss).
That said, if this pattern was arrived at through debugging XWayland quirks, the current approach is pragmatic. A comment explaining why this is needed would help future maintainers.
663-668: Dead code: XWayland surfaces are never added to thesurfacesvector.Lines 628-630 explicitly state that XWayland surfaces are NOT added to the
surfacesvector. Therefore, thestd::findanderasehere will never find anything. This code path appears to be dead code.🔎 Suggested removal
// Clear focus if this was an XWayland surface if (focused_xsurface) { - auto it = std::find(surfaces.begin(), surfaces.end(), focused_surface); - if (it != surfaces.end()) { - surfaces.erase(it); - } - focused_xsurface = nullptr; focused_surface = nullptr; wlr_seat_keyboard_clear_focus(seat); wlr_seat_pointer_clear_focus(seat); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/input/compositor_server.cppsrc/input/compositor_server.hpp
🧰 Additional context used
🧬 Code graph analysis (1)
src/input/compositor_server.hpp (1)
src/input/compositor_server.cpp (2)
CompositorServer(137-137)CompositorServer(139-141)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Static Analysis (clang-tidy)
- GitHub Check: Build and Test (test preset)
🔇 Additional comments (7)
src/input/compositor_server.hpp (1)
21-44: LGTM!The class design follows best practices: PImpl for ABI stability and hiding wlroots details, deleted copy/move semantics to prevent accidental duplication of compositor resources, and
[[nodiscard]]on methods where ignoring the return value would be a bug.src/input/compositor_server.cpp (6)
42-46: LGTM!Correct use of
CLOCK_MONOTONICfor obtaining timestamps, which is the appropriate clock for event timing as it's not affected by system time changes.
72-135: LGTM!The
Implstruct is well-organized with proper initialization of Wayland listener links in the constructor. The use ofstd::jthreadensures proper thread joining, and theUniqueKeyboardRAII wrapper correctly handles keyboard cleanup.
301-376: LGTM!The shutdown sequence correctly reverses the initialization order with appropriate comments explaining the ordering constraints. Key aspects done right:
- Thread joined before destroying objects it accesses
- Event source removed before eventfd closed
- XWayland destroyed before compositor/seat
- Resources nullified after destruction
384-389: Consider edge case where queue push succeeds but eventfd write fails.If
try_pushsucceeds butwritefails (e.g., interrupted by signal), the event is queued but the event loop won't be woken up until the next successful write. This is likely benign in practice since the event will be processed on the next notification, but the return value may mislead callers into thinking the event wasn't queued.You could restructure to only return false when the push fails, or document the semantics more clearly. For robustness, consider retrying the write on
EINTR:🔎 Optional: retry on EINTR
if (m_impl->event_queue.try_push(event)) { uint64_t val = 1; - auto ret = write(m_impl->event_fd.get(), &val, sizeof(val)); - return ret == sizeof(val); + ssize_t ret; + do { + ret = write(m_impl->event_fd.get(), &val, sizeof(val)); + } while (ret < 0 && errno == EINTR); + return ret == sizeof(val); }
508-524: LGTM!The multi-stage XDG toplevel handling (commit → map → focus) correctly follows the xdg-shell protocol lifecycle. The
offsetofpattern is the idiomatic wlroots approach for accessing the containing struct from a listener.
678-729: LGTM!The focus management correctly handles transitions between native Wayland and XWayland surfaces:
- Clears stale pointers before wlroots calls that might access them
- Cleans up dangling listeners from the other surface type
- Activates XWayland surfaces before setting keyboard/pointer focus
The defensive approach of clearing listeners when switching focus types prevents use-after-free issues.
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @src/input/compositor_server.cpp:
- Around line 56-57: The error text returned by
make_error<int>(ErrorCode::input_init_failed, ...) in compositor_server.cpp
incorrectly references "DISPLAY numbers" even though the code binds Wayland
sockets; update the message to mention Wayland socket names (e.g., "No available
wayland-N sockets (1-9 all bound)" or similar) so it accurately reflects the
resource being bound while keeping the same ErrorCode and return expression.
- Around line 638-658: Remove dead XWayland destroy handling: delete the
Impl::handle_xwayland_surface_destroy() method implementation and the unused
wl_listener member listeners.xwayland_surface_destroy, plus its wl_list_init
call and any forward declaration handle_xwayland_surface_destroy(); since the
listener is never registered and XWayland surfaces aren't added to surfaces,
remove related cleanup/initialization for listeners.xwayland_surface_destroy and
any references to it to avoid unused/dead code.
- Around line 442-447: The code currently re-activates XWayland surfaces and
calls wlr_seat_keyboard_notify_enter()/wlr_seat_pointer_notify_enter() on every
input event (see focused_xsurface, wlr_xwayland_surface_activate,
wlr_seat_set_keyboard, wlr_seat_keyboard_notify_enter,
wlr_seat_pointer_notify_enter), which is unnecessary; change the logic to only
perform XWayland activation and wlr_seat_*_notify_enter() when the focus
actually changes or when a device is added/reconfigured—implement a focus
comparison (track last_entered_surface/last_entered_pointer_target) and call
those functions only when focused_surface/focused_pointer_target differs, and if
you must retain re-entry as a workaround, add a clear comment referencing the
specific XWayland bug and why wlroots’ normal semantics don’t apply.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/input/compositor_server.cpp
🧰 Additional context used
🧬 Code graph analysis (1)
src/input/compositor_server.cpp (5)
src/util/error.hpp (1)
make_error(50-52)src/input/compositor_server.hpp (8)
CompositorServer(23-23)CompositorServer(24-24)CompositorServer(26-26)CompositorServer(28-28)linux_keycode(36-36)sx(37-37)button(38-38)value(39-39)src/input/input_forwarder.cpp (4)
x11_display(212-214)x11_display(212-212)wayland_display(216-218)wayland_display(216-216)src/util/config.hpp (1)
to_string(19-19)src/input/input_forwarder.hpp (4)
event(19-19)event(20-20)event(21-21)event(22-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (test preset)
- GitHub Check: Static Analysis (clang-tidy)
🔇 Additional comments (4)
src/input/compositor_server.cpp (4)
660-679: Well-structured focus management with defensive cleanup.The
focus_surfacemethod demonstrates good defensive programming:
- Clears stale XWayland pointers before any wlroots calls that might access them (lines 665-668), preventing potential use-after-free bugs
- Proactively cleans up pending XWayland listeners (lines 670-673) to avoid dangling references
This pattern prevents crashes when switching from XWayland to native Wayland clients, as noted in the comment on line 666.
681-709: Good defensive cleanup pattern for XWayland focus.The
focus_xwayland_surfacemethod follows a safe pattern:
- Clears seat focus first (lines 686-688) to prevent wlroots from sending leave events to stale surfaces
- Cleans up all pending XDG listeners (lines 690-697) before establishing new focus
- Properly activates the XWayland surface (line 703) as required for wlr_xwm to send focus events
This symmetric approach to focus management (mirroring
focus_surface) ensures clean state transitions.
193-193: xdg-shell protocol version 3 is compatible with wlroots 0.18. wlroots 0.18 supports xdg_wm_base versions up to v6, so version 3 is well-supported. No action needed.
183-183: Verify that compositor protocol version 6 is compatible with wlroots 0.18.The project requires wlroots 0.18.3 (from pixi.lock), and the compositor is created with protocol version 6. While the XDG shell uses version 3 (suggesting intentional version selection), the choice of version 6 for the compositor lacks documentation or justification. Confirm this version aligns with wlroots 0.18's supported compositor protocol versions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/input/compositor_server.cpp (1)
115-122: Inconsistent listener initialization pattern—not a bug but worth noting.Some listeners are pre-initialized in the constructor (lines 117-121:
xdg_surface_commit,xdg_surface_map,xdg_surface_destroy,xwayland_surface_associate,xwayland_surface_destroy), while others are initialized on-demand before first use (line 200:new_xdg_toplevel, line 266:new_xwayland_surface).This is intentional: the pre-initialized listeners are registered/unregistered multiple times (once per surface), while the on-demand ones are registered once at startup. The pattern is correct but could be clarified with a comment.
Optional: Add clarifying comment
In the constructor or near the listener declarations, add:
// Listeners registered/unregistered per-surface are pre-initialized // to avoid repeated wl_list_init calls. One-time listeners (new_xdg_toplevel, // new_xwayland_surface) are initialized on-demand before registration.Also applies to: 200-200, 266-266
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
openspec/changes/add-wayland-input-forwarding/proposal.mdsrc/input/compositor_server.cpp
🧰 Additional context used
📓 Path-based instructions (2)
openspec/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/**/*.md: Use#### Scenario:format (4 hashtags) for scenario headers in requirements, not bullets or bold text, with at least one scenario per requirement
Use format- **WHEN** [condition] - **THEN** [expected result]for scenario steps in requirements
Files:
openspec/changes/add-wayland-input-forwarding/proposal.md
openspec/changes/**/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
List spec deltas in
proposal.mdunder 'What Changes' section, marking breaking changes with BREAKING
Files:
openspec/changes/add-wayland-input-forwarding/proposal.md
🧠 Learnings (1)
📚 Learning: 2025-12-19T08:54:18.412Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-12-19T08:54:18.412Z
Learning: Applies to openspec/changes/**/proposal.md : List spec deltas in `proposal.md` under 'What Changes' section, marking breaking changes with **BREAKING**
Applied to files:
openspec/changes/add-wayland-input-forwarding/proposal.md
🧬 Code graph analysis (1)
src/input/compositor_server.cpp (4)
src/util/error.hpp (1)
make_error(50-52)src/input/input_forwarder.cpp (4)
x11_display(212-214)x11_display(212-212)wayland_display(216-218)wayland_display(216-216)src/util/config.hpp (1)
to_string(19-19)src/input/input_forwarder.hpp (4)
event(19-19)event(20-20)event(21-21)event(22-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (test preset)
- GitHub Check: Static Analysis (clang-tidy)
🔇 Additional comments (4)
src/input/compositor_server.cpp (2)
649-660: Acknowledge the documented XWayland surface lifecycle limitation.The comments correctly document that XWayland surfaces cannot safely listen to the destroy event (lines 653-654: "fires unexpectedly during normal operation, breaking X11 input entirely"). The consequence is that
focused_xsurfaceandfocused_surfacecan become dangling pointers when X11 clients disconnect.The mitigation strategy (lines 590-591) steals focus to new Wayland surfaces, clearing stale XWayland focus. This works for typical multi-app workflows but leaves a window where dangling pointers exist if:
- An X11 app disconnects
- No new Wayland surface is created
- Input events arrive before focus is updated
Given that the destroy listener was tested and found to break input (per line 654), this trade-off appears necessary. The risk is acceptable since crashing on every input event would be worse than occasionally using stale focus.
1-743: Overall implementation is well-structured with proper resource management.The code demonstrates:
- ✅ Proper RAII patterns (UniqueKeyboard, UniqueFd, cleanup guard in start())
- ✅ Correct thread synchronization using SPSCQueue (single-producer-single-consumer) with eventfd for wake-up
- ✅ Appropriate cleanup order in stop() (terminate display → join thread → destroy resources)
- ✅ Defensive listener lifecycle management with cleanup in multiple code paths (lines 697-699, 721-727)
- ✅ Documented architectural trade-offs for XWayland surface management
The two issues flagged in separate comments (dead code removal and focus re-entry pattern) are carry-overs from previous reviews.
openspec/changes/add-wayland-input-forwarding/proposal.md (2)
7-17: Breaking changes are now properly marked.The past review comment flagging missing BREAKING markers has been addressed. Both breaking changes in the 'What Changes' section are now correctly prefixed:
- Line 9: API breaking change (removes
display_number(), addsx11_display()/wayland_display())- Line 16: Build breaking change (removes libX11/libXtst link targets)
The document follows the coding guidelines for
openspec/changes/**/proposal.md.
1-60: Well-structured proposal with comprehensive impact analysis.The proposal clearly documents:
- ✅ Motivation (Why): Current XTest limitation for Wayland-native apps
- ✅ Technical changes (What Changes): Unified wlr_seat API approach with proper breaking change markers
- ✅ Migration path (Old Code Cleanup): Complete removal of XTest code with file renames
- ✅ Testing strategy: Separate test binaries for X11 and Wayland paths
- ✅ Impact assessment: Affected files, removed dependencies, and new capabilities
The decision to completely remove XTest rather than maintain dual paths is appropriate for a pre-release project.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/input/compositor_server.cpp (1)
442-518: Consider extracting XWayland re-activation pattern to reduce duplication.The XWayland surface re-activation logic (checking
focused_xsurface, callingwlr_xwayland_surface_activate()andwlr_seat_*_notify_enter()) is repeated identically for all four input event types. While the pattern is clear and correct, extracting to a helper method likeensure_xwayland_focus_active()would reduce the ~50 lines of duplication and make the quirk easier to maintain.Example refactor
void Impl::ensure_keyboard_focus() { if (focused_xsurface) { wlr_xwayland_surface_activate(focused_xsurface, true); wlr_seat_set_keyboard(seat, keyboard.get()); wlr_seat_keyboard_notify_enter(seat, focused_surface, keyboard->keycodes, keyboard->num_keycodes, &keyboard->modifiers); } else if (keyboard_entered_surface != focused_surface) { wlr_seat_set_keyboard(seat, keyboard.get()); wlr_seat_keyboard_notify_enter(seat, focused_surface, keyboard->keycodes, keyboard->num_keycodes, &keyboard->modifiers); keyboard_entered_surface = focused_surface; } } void Impl::ensure_pointer_focus(double x, double y) { if (focused_xsurface) { wlr_xwayland_surface_activate(focused_xsurface, true); wlr_seat_pointer_notify_enter(seat, focused_surface, x, y); } else if (pointer_entered_surface != focused_surface) { wlr_seat_pointer_notify_enter(seat, focused_surface, x, y); pointer_entered_surface = focused_surface; } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/input/compositor_server.cpp
🧰 Additional context used
🧬 Code graph analysis (1)
src/input/compositor_server.cpp (5)
src/util/error.hpp (1)
make_error(50-52)src/input/compositor_server.hpp (5)
CompositorServer(23-23)CompositorServer(24-24)CompositorServer(26-26)CompositorServer(28-28)button(38-38)src/input/input_forwarder.cpp (2)
x11_display(212-214)x11_display(212-212)src/util/config.hpp (1)
to_string(19-19)src/input/input_forwarder.hpp (4)
event(19-19)event(20-20)event(21-21)event(22-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Static Analysis (clang-tidy)
- GitHub Check: Build and Test (test preset)
🔇 Additional comments (6)
src/input/compositor_server.cpp (6)
147-295: Excellent initialization with comprehensive error handling.The
start()method demonstrates best practices:
- RAII guard for automatic cleanup on any initialization failure
- Detailed error messages for each step
- Correct resource creation order (dependencies created first)
- Compositor thread launched only after all resources are ready
297-374: Exemplary resource cleanup with correct destruction order.The
stop()method properly handles:
- Thread synchronization (join before destroying shared objects)
- Dependency-ordered cleanup (XWayland before compositor, seat before display)
- Event source removal before closing eventfd
- Clearing all surface pointers to prevent dangling references
376-430: Clean input injection pattern with proper thread synchronization.All injection methods consistently:
- Use lock-free queue for thread-safe event passing
- Signal compositor thread via eventfd only after successful queue push
- Return boolean status for caller feedback
524-617: Well-structured XDG surface lifecycle management.The three-stage handling (new_toplevel → commit → map) correctly follows the xdg_shell protocol. Listener management is defensive—always removing and re-initializing before registration to ensure clean state. The focus policy (prefer Wayland surfaces over potentially stale XWayland) is a pragmatic workaround for the inability to detect X11 disconnects.
619-657: XWayland handling correctly accounts for protocol limitations.The asymmetric treatment of XWayland surfaces (not added to
surfacesvector, no destroy listener) is well-documented and addresses real limitations:
- The destroy event fires unexpectedly during normal operation
- X11 client disconnects cannot be reliably detected
- The conservative focus policy (only steal when no focus exists) prevents disrupting Wayland clients
These are pragmatic workarounds given wlroots/XWayland constraints.
659-714: Robust focus switching with careful state management.Both focus methods demonstrate defensive programming:
- Guard against redundant focus changes
- Clear stale pointers before wlroots calls to prevent crashes
- Clean up pending listeners from the other surface type to avoid dangling references
- For XWayland: clear seat focus first to prevent leave events to stale surfaces
- For XWayland: activate X11 window to ensure wlr_xwm sends focus events correctly
The careful ordering and cleanup prevent crashes when switching between native Wayland and XWayland clients.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @src/input/compositor_server.cpp:
- Around line 598-621: The destroy handler handle_xdg_surface_destroy
incorrectly removes focused_surface instead of the actual destroyed surface and
reuses a single destroy listener across surfaces; change the callback to use the
data parameter (cast to wlr_surface* or the appropriate surface wrapper) to find
and erase that specific surface from the surfaces vector, and only clear
focus/entered pointers if the destroyed surface equals focused_surface (preserve
focused_xsurface logic); mirror the approach used in
new_xdg_toplevel/new_xwayland_surface where the destroy callback operates on the
passed-in surface instance and avoid relying on a shared
listeners.xdg_surface_destroy for multiple surfaces.
🧹 Nitpick comments (1)
src/input/compositor_server.hpp (1)
10-19: Consider moving InputEvent to internal implementation.
InputEventTypeandInputEventare exposed in the public namespace but not used by any public methods (they're only used internally for cross-thread queue communication in the .cpp file). Exposing internal types unnecessarily enlarges the API surface and may cause users to depend on implementation details.Consider moving these to an internal namespace or directly into the anonymous namespace in the .cpp file.
🔎 Suggested approach
In the .cpp file, move the types into the anonymous namespace or an internal detail namespace:
namespace goggles::input { namespace { // or: namespace detail { enum class InputEventType : std::uint8_t { key, pointer_motion, pointer_button, pointer_axis }; struct InputEvent { InputEventType type; uint32_t code; bool pressed; double x, y; double value; bool horizontal; }; } // namespace // ... rest of implementation }Remove these types from the public header entirely.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/input/compositor_server.cppsrc/input/compositor_server.hpp
🧰 Additional context used
🧬 Code graph analysis (1)
src/input/compositor_server.hpp (1)
src/input/compositor_server.cpp (2)
CompositorServer(133-133)CompositorServer(135-137)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (test preset)
- GitHub Check: Static Analysis (clang-tidy)
🔇 Additional comments (5)
src/input/compositor_server.cpp (4)
42-70: LGTM: Helper functions are well-designed.The time function, socket binding, and keyboard deleter all follow correct patterns. The socket binding range (0-9) is reasonable for the use case, and the error message correctly references "goggles sockets" (addressed from past review).
150-299: LGTM: Lifecycle management follows proper patterns.The
start()method uses a cleanup guard to ensure proper teardown on error, and thestop()method carefully orders destruction (thread join before object destruction, xwayland before compositor, seat before display, event_source before eventfd close). This prevents use-after-free and ensures safe shutdown.Also applies to: 301-378
436-526: LGTM: Input event processing correctly handles XWayland quirks.The comments at lines 446-448 and 469 clearly explain why XWayland surfaces require re-activation and re-entry before each event (wlr_xwm requirement), while native Wayland clients only receive enter events when focus actually changes. This addresses the concern from the previous review about the re-entry pattern. The SPSCQueue + eventfd cross-thread signaling is also correctly implemented.
623-718: LGTM: XWayland surface handling correctly addresses lifecycle asymmetry.The comments at lines 650-655 clearly explain why XWayland surfaces are not added to the
surfacesvector and why destroy listeners are not registered (they would break X11 input). The focus management functions (focus_surfaceandfocus_xwayland_surface) properly clear stale pointers and clean up pending listeners when switching between Wayland and XWayland clients.src/input/compositor_server.hpp (1)
21-44: LGTM: Clean public API design with proper RAII and encapsulation.The
CompositorServerclass follows best practices:
- PImpl pattern hides wlroots dependencies from public headers
[[nodiscard]]on all methods that return status (ensures callers check for errors)- Deleted copy/move operations enforce single ownership
- Clear lifecycle with
start()/stop()methods- Clean separation between X11 and Wayland display accessors
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @docs/input_forwarding.md:
- Around line 113-144: The fenced code block containing the thread model ASCII
diagram (the triple-backtick block that starts before "Main Thread
Compositor Thread") needs a language specifier to satisfy Markdown lint rule
MD040; update that block to start with ```text (or ```plaintext) instead of just
``` so the ASCII diagram is treated as plain text.
🧹 Nitpick comments (8)
tests/input/auto_input_forwarding_x11.cpp (2)
18-34: Consider extracting shared test utilities.
make_xdg_runtime_dir()is duplicated betweenauto_input_forwarding_x11.cppandauto_input_forwarding_wayland.cpp. Consider extracting to a shared test utility header to reduce maintenance burden.Also, the created temporary directory is never cleaned up. For test hygiene, consider adding cleanup on exit (or document that cleanup is intentional to allow post-mortem inspection).
105-122: Partially initialized SDL_KeyboardEvent may contain garbage fields.The
SDL_KeyboardEventis value-initialized but onlyscancodeanddownare set. Other fields liketimestamp,windowID,which,key, andmodremain uninitialized. While the forwarder only usesscancodeanddown, this could cause issues if the API evolves.🔎 Suggested improvement
- SDL_KeyboardEvent key{}; - key.scancode = SDL_SCANCODE_A; - key.down = true; + SDL_KeyboardEvent key{}; + key.type = SDL_EVENT_KEY_DOWN; + key.scancode = SDL_SCANCODE_A; + key.down = true; + key.key = SDLK_A;tests/input/manual_input_x11.cpp (1)
8-89: Significant code duplication withmanual_input_wayland.cpp.The
VulkanInstanceandSDLContextRAII structs are identical between this file andmanual_input_wayland.cpp. Similarly,print_event_info()andrun_input_loop()are duplicated with only minor string differences.Consider extracting shared components to a common header (e.g.,
tests/input/test_common.hpp) to reduce maintenance overhead.tests/input/auto_input_forwarding_wayland.cpp (1)
31-47: SDLGuard inconsistency with X11 variant.This
SDLGuardincludes arenderermember (line 33), but the X11 variant (auto_input_forwarding_x11.cpp) does not. While not a bug (X11 test doesn't create a renderer), maintaining consistent struct definitions across tests would improve maintainability.This reinforces the recommendation to extract shared test utilities.
docs/input_forwarding.md (1)
63-68: Clarify socket naming.Line 65 states the headless backend "Creates Wayland sockets (wayland-N, DISPLAY :N)" but DISPLAY :N is provided by XWayland, not the headless backend directly. Consider clarifying:
-- Creates Wayland sockets (wayland-N, DISPLAY :N) without requiring GPU/display +- Creates Wayland socket (wayland-N) without requiring GPU/display +- XWayland provides DISPLAY :N for X11 clientssrc/input/input_forwarder.cpp (2)
197-214: Document or extract the scroll multiplier constant.The value
15.0on lines 200 and 207 is a magic number for converting SDL wheel units to Wayland axis values. Consider extracting to a named constant with documentation explaining the rationale.🔎 Suggested improvement
+namespace { +// Wayland axis events use "logical scroll units" (typically 15 per notch) +// See: https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_pointer +constexpr double kScrollMultiplier = 15.0; +} // namespace + auto InputForwarder::forward_mouse_wheel(const SDL_MouseWheelEvent& event) -> Result<void> { if (event.y != 0) { // SDL: positive = up, Wayland: positive = down; negate to match - double value = static_cast<double>(-event.y) * 15.0; + double value = static_cast<double>(-event.y) * kScrollMultiplier;
160-174: Consider propagating queue-full condition in return value.When
inject_key()returns false (queue full), the method logs a debug message but still returns success. For callers that need to know if input was actually delivered, consider returning an error or at least a distinct success-with-warning state.Current behavior is acceptable for MVP (best-effort delivery), but worth noting for future reliability improvements.
tests/CMakeLists.txt (1)
142-146: Consider using LSAN suppressions file for more targeted leak filtering.Disabling all leak detection with
ASAN_OPTIONS=detect_leaks=0is a valid workaround for external library leaks, but it also hides any real leaks in the test code itself. For more precise control, consider:
- Create a
lsan.suppsuppressions file listing Mesa/EGL leak patterns- Use
ASAN_OPTIONS=suppressions=lsan.suppinsteadThis preserves leak detection for project code while suppressing known external library leaks.
📋 Example suppressions file approach
Create
tests/lsan.supp:# Mesa/EGL driver leaks during SDL initialization leak:libEGL leak:libGL leak:libmesaThen update the test property:
set_property(TEST goggles_auto_input_forwarding_x11 PROPERTY ENVIRONMENT "ASAN_OPTIONS=suppressions=${CMAKE_CURRENT_SOURCE_DIR}/lsan.supp")This approach is optional—the current blanket suppression is acceptable for integration tests, but suppressions provide better leak coverage for test logic.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/input_forwarding.mdopenspec/changes/add-wayland-input-forwarding/proposal.mdopenspec/changes/add-wayland-input-forwarding/tasks.mdsrc/input/compositor_server.cppsrc/input/input_forwarder.cpptests/CMakeLists.txttests/input/auto_input_forwarding_wayland.cpptests/input/auto_input_forwarding_x11.cpptests/input/manual_input_wayland.cpptests/input/manual_input_x11.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- openspec/changes/add-wayland-input-forwarding/tasks.md
🧰 Additional context used
📓 Path-based instructions (2)
openspec/**/*.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
openspec/**/*.md: Use#### Scenario:format (4 hashtags) for scenario headers in requirements, not bullets or bold text, with at least one scenario per requirement
Use format- **WHEN** [condition] - **THEN** [expected result]for scenario steps in requirements
Files:
openspec/changes/add-wayland-input-forwarding/proposal.md
openspec/changes/**/proposal.md
📄 CodeRabbit inference engine (openspec/AGENTS.md)
List spec deltas in
proposal.mdunder 'What Changes' section, marking breaking changes with BREAKING
Files:
openspec/changes/add-wayland-input-forwarding/proposal.md
🧠 Learnings (11)
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/config/goggles.toml : Development/testing configuration: `config/goggles.toml` in repository root. Structure should include sections for [capture], [pipeline], [render], and [logging].
Applied to files:
docs/input_forwarding.md
📚 Learning: 2025-12-19T08:54:18.412Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-12-19T08:54:18.412Z
Learning: Applies to openspec/changes/**/proposal.md : List spec deltas in `proposal.md` under 'What Changes' section, marking breaking changes with **BREAKING**
Applied to files:
openspec/changes/add-wayland-input-forwarding/proposal.md
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/tests/**/*.cpp : Only non-GPU logic is tested initially: in scope are utility functions, configuration parsing, error handling, pipeline graph logic. Out of scope: Vulkan initialization, GPU resource management, rendering, capture layer.
Applied to files:
tests/input/manual_input_x11.cpptests/input/manual_input_wayland.cpptests/CMakeLists.txt
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/src/capture/vk_layer/**/*.{cpp,hpp} : Vulkan layer code in `src/capture/vk_layer/` has strict logging constraints: use only `error` and `critical` levels, no file I/O or blocking operations in hot paths, prefix all logs with `[goggles_vklayer]`, never log in `vkQueuePresentKHR` hot path.
Applied to files:
tests/input/manual_input_x11.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/**/*.{cpp,hpp} : All `vulkan-hpp` calls returning `vk::Result` must be explicitly checked. Prohibited: masking with `static_cast<void>()`. Use `VK_TRY` macro for early return with `vk::Result`, `GOGGLES_TRY` for Result<T> propagation (expression-style), `GOGGLES_MUST` for internal invariants (aborts on failure).
Applied to files:
tests/input/manual_input_x11.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/src/**/*.{cpp,hpp} : Due to async GPU execution, RAII handles (`vk::Unique*`) cannot automatically handle synchronization. Use selectively: `vk::Unique*` for Instance, Device, Surface (long-lived singletons), Swapchain, Pipelines, Layouts (created once). Use plain handles for Command buffers, per-frame sync primitives (Fence, Semaphore), Imported external images.
Applied to files:
tests/input/manual_input_x11.cpptests/input/manual_input_wayland.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/src/**/*.{cpp,hpp} : For Vulkan resources, follow RAII guidelines. Use `vk::Unique*` only for appropriate resource types (per D.2 table). Call `device.waitIdle()` or wait on appropriate fences before destroying resources with GPU-async lifetime. Store creation info with resources for debugging. Never leak Vulkan objects. Declare members in reverse destruction order (device before instance).
Applied to files:
tests/input/manual_input_x11.cpptests/input/manual_input_wayland.cpp
📚 Learning: 2025-12-29T15:44:30.520Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: openspec/project.md:0-0
Timestamp: 2025-12-29T15:44:30.520Z
Learning: Applies to openspec/src/render/**/*.{cpp,hpp} : Use `vk::Unique*` for long-lived Vulkan resources; use plain handles for per-frame/GPU-async resources
Applied to files:
tests/input/manual_input_x11.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/src/**/*.{cpp,hpp} : Application code MUST use `vulkan-hpp` (C++ bindings), NOT the raw Vulkan C API. Use `vk::` types (e.g., `vk::Instance`, `vk::Device`, `vk::Image`). Never use raw C handles (`VkInstance`, `VkDevice`, etc.) in application code. Required configuration: `#define VULKAN_HPP_NO_EXCEPTIONS` and `#define VULKAN_HPP_DISPATCH_LOADER_DYNAMIC 1`.
Applied to files:
tests/input/manual_input_x11.cpp
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/**/*.{cpp,hpp} : All resources must be managed via RAII. Wrap Vulkan objects in RAII types (custom or from libraries like `vk-bootstrap`, `vulkan-hpp`). Use RAII wrappers for file handles, sockets, etc. (`std::fstream`, `UniqueFd`, custom types). No manual cleanup in destructors unless encapsulated in RAII.
Applied to files:
tests/input/manual_input_wayland.cpp
📚 Learning: 2025-12-29T15:44:30.520Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: openspec/project.md:0-0
Timestamp: 2025-12-29T15:44:30.520Z
Learning: Applies to openspec/tests/**/*.{cpp,hpp} : Focus unit tests on non-GPU logic: utility functions, configuration parsing, and pipeline graph logic
Applied to files:
tests/CMakeLists.txt
🧬 Code graph analysis (4)
tests/input/auto_input_forwarding_x11.cpp (2)
tests/input/auto_input_forwarding_wayland.cpp (1)
SDLGuard(36-46)src/input/input_forwarder.cpp (2)
create(148-158)create(148-148)
tests/input/manual_input_wayland.cpp (1)
tests/input/manual_input_x11.cpp (20)
VulkanInstance(11-11)VulkanInstance(13-17)VulkanInstance(19-19)VulkanInstance(22-24)other(26-35)other(26-26)other(67-88)other(67-67)SDLContext(43-43)SDLContext(45-55)SDLContext(57-57)SDLContext(60-65)print_event_info(147-179)print_event_info(147-147)init_vulkan(93-113)init_vulkan(93-93)init_sdl(115-145)init_sdl(115-115)run_input_loop(181-209)run_input_loop(181-182)
src/input/input_forwarder.cpp (3)
src/input/input_forwarder.hpp (4)
event(19-19)event(20-20)event(21-21)event(22-22)src/input/compositor_server.hpp (3)
linux_keycode(36-36)button(38-38)value(39-39)src/input/compositor_server.cpp (4)
x11_display(146-151)x11_display(146-146)wayland_display(153-155)wayland_display(153-153)
tests/input/auto_input_forwarding_wayland.cpp (2)
src/input/input_forwarder.cpp (4)
create(148-158)create(148-148)wayland_display(220-222)wayland_display(220-220)src/input/compositor_server.cpp (2)
wayland_display(153-155)wayland_display(153-153)
🪛 markdownlint-cli2 (0.18.1)
docs/input_forwarding.md
113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Static Analysis (clang-tidy)
- GitHub Check: Build and Test (test preset)
🔇 Additional comments (12)
tests/input/auto_input_forwarding_x11.cpp (1)
52-131: LGTM! Test structure is sound.The test flow is well-structured: skip if Xwayland unavailable, create runtime dir, initialize forwarder, configure environment, create window, forward events, and verify receipt. The 5-second timeout with 25ms polling interval is reasonable for CI environments.
tests/input/manual_input_x11.cpp (1)
98-140: LGTM! String updates correctly reflect X11 context.The log messages, window titles, and application names have been appropriately updated to indicate X11/XWayland backend. The reddish background color (40, 0, 0) provides visual distinction from the Wayland variant.
tests/input/auto_input_forwarding_wayland.cpp (2)
77-98: Appropriate use of skip semantics for optional Wayland support.Returning 0 (skip) for SDL initialization failures is correct - Wayland backend may not be available in all CI environments. The renderer creation and initial present to force a buffer commit is the correct approach for Wayland surface mapping.
109-126: LGTM! Test loop logic is correct.The synthetic key injection and event polling loop is well-structured with appropriate timeout handling.
tests/input/manual_input_wayland.cpp (1)
1-232: LGTM! Well-structured Wayland manual input test.The test is correctly configured for Wayland with appropriate environment setup, RAII resource management, and event handling. The green-tinted background (0, 40, 0) provides clear visual distinction from the X11 variant.
As noted in the X11 review, the significant code duplication with
manual_input_x11.cppcould be reduced by extracting common utilities.docs/input_forwarding.md (1)
1-238: Comprehensive documentation update.The documentation thoroughly covers the new unified seat-based architecture, thread model, data flow, and testing procedures. The diagrams are clear and the limitations section is appropriately honest about current MVP scope.
openspec/changes/add-wayland-input-forwarding/proposal.md (2)
7-17: LGTM! Breaking changes are now properly marked.The BREAKING prefixes on lines 9 and 16 correctly identify API-breaking changes (accessor rename) and build-breaking changes (removed dependencies) per the coding guidelines.
1-60: Comprehensive proposal document.The proposal clearly documents the rationale, breaking changes, cleanup steps, and impact. The archive path for old XTest design (
openspec/changes/archive/2026-01-04-add-input-forwarding-x11/) provides good historical reference.src/input/input_forwarder.cpp (4)
12-119: LGTM! Clean keycode mapping implementation.The SDL scancode to Linux keycode mapping covers the documented subset (A-Z, 0-9, arrows, modifiers, common keys). The switch statement is straightforward and the default case returning 0 for unmapped keys is properly handled by the caller.
121-136: LGTM! Button mapping is correct.The SDL to Linux button mapping correctly maps:
- Left/Middle/Right to BTN_LEFT/MIDDLE/RIGHT
- X1/X2 (side buttons) to BTN_SIDE/BTN_EXTRA
This follows the standard evdev button code conventions.
140-158: LGTM! Clean initialization flow.The
create()factory correctly initializes the forwarder and propagates server start failures. The Impl struct cleanly encapsulates the CompositorServer dependency.
216-222: LGTM! Display accessors correctly delegate to server.The new
x11_display()andwayland_display()accessors properly delegate to the CompositorServer, maintaining the separation of concerns.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @docs/input_forwarding.md:
- Around line 19-49: The fenced ASCII diagram currently starts with a bare
triple-backtick; update that fence to include a language specifier (e.g., change
``` to ```text or ```plaintext) so the ASCII art is treated as plain text and
satisfies MD040; locate the triple-backtick fence that wraps the diagram (the
block beginning with ``` and ending with ```) and replace the opening fence
accordingly.
- Around line 113-144: The fenced ASCII diagram starting with "Main Thread
Compositor Thread" is missing a language specifier; update that code fence to
include a language token such as "text" or "plaintext" (i.e., change the leading
``` to ```text) so the block passes markdown-lint rule MD040 while leaving the
ASCII art and surrounding content intact.
In @src/input/compositor_server.cpp:
- Around line 654-657: Add a clarifying comment above the if-statement that
checks focused_surface and focused_xsurface to document that native Wayland
surfaces are allowed to steal focus from XWayland surfaces; specifically,
annotate the block around the condition using the same rationale as the existing
handle_xwayland_surface_associate comment and include the suggested text: "Focus
Wayland surface if: - No current focus, OR - Current focus is XWayland
(prioritize native Wayland over XWayland)". Place this comment immediately
before the if (!focused_surface || focused_xsurface) { ... } that calls
wlr_xdg_toplevel_set_activated(hooks->toplevel, true) and
focus_surface(hooks->surface).
🧹 Nitpick comments (4)
docs/input_forwarding.md (1)
63-68: Clarify socket/display naming distinction.Line 65 conflates Wayland socket naming (
wayland-N) with X11 display notation (DISPLAY :N). These serve different purposes and should be distinguished. Consider rephrasing to clarify that the backend creates both:
- Wayland socket (
wayland-N) for native Wayland clients- X11 display socket (
:N) for XWayland/X11 appssrc/input/compositor_server.cpp (3)
393-447: Consider logging injection failures for diagnostics.When injection fails (either queue full or eventfd write error), the methods return false without indicating the cause. Adding debug-level logging would help diagnose issues during development and testing.
Example enhancement for inject_key
auto CompositorServer::inject_key(uint32_t linux_keycode, bool pressed) -> bool { InputEvent event{}; event.type = InputEventType::key; event.code = linux_keycode; event.pressed = pressed; if (m_impl->event_queue.try_push(event)) { uint64_t val = 1; auto ret = write(m_impl->event_fd.get(), &val, sizeof(val)); + if (ret != sizeof(val)) { + GOGGLES_LOG_DEBUG("Failed to signal eventfd for key injection: code={}", linux_keycode); + } return ret == sizeof(val); + } else { + GOGGLES_LOG_DEBUG("Event queue full, dropping key injection: code={}", linux_keycode); } return false; }Apply similar changes to the other three injection methods.
449-587: Significant code duplication in event processing—extract common patterns.The four input event handlers (key, pointer_motion, pointer_button, pointer_axis) duplicate two distinct patterns:
Target surface resolution (lines 456-470, 492-507, 523-537, 553-567): Each case repeats the logic to resolve
target_surfacefromfocused_surface/focused_xsurface, check for dissociated XWayland surfaces, and log warnings.XWayland re-activation (lines 472-479, 508-514, 539-544, 569-574): Each case repeats the XWayland quirk workaround (activate + re-enter) vs. native Wayland enter-once logic.
Extracting these into helper methods would:
- Reduce ~60 lines to ~20 lines with two helpers
- Ensure consistency if the logic needs adjustment
- Make the XWayland quirk workaround centralized
Proposed helper methods
Add to
Impl:// Returns target surface for input, handling XWayland dissociation. // Returns nullptr if no valid target exists. auto resolve_target_surface() -> wlr_surface* { wlr_surface* target = focused_surface; if (focused_xsurface) { target = focused_xsurface->surface; if (!target) { if (!xwayland_surface_detached) { GOGGLES_LOG_DEBUG("Dropping input: focused XWayland surface is dissociated (surface=null)"); xwayland_surface_detached = true; } return nullptr; } xwayland_surface_detached = false; } return target; } // Ensures keyboard focus is correctly set for the target surface. // Handles XWayland quirk requiring re-activation on every event. void ensure_keyboard_focus(wlr_surface* target) { if (focused_xsurface) { wlr_xwayland_surface_activate(focused_xsurface, true); wlr_seat_set_keyboard(seat, keyboard.get()); wlr_seat_keyboard_notify_enter(seat, target, keyboard->keycodes, keyboard->num_keycodes, &keyboard->modifiers); } else if (keyboard_entered_surface != target) { wlr_seat_set_keyboard(seat, keyboard.get()); wlr_seat_keyboard_notify_enter(seat, target, keyboard->keycodes, keyboard->num_keycodes, &keyboard->modifiers); keyboard_entered_surface = target; } } // Ensures pointer focus is correctly set for the target surface. // Handles XWayland quirk requiring re-activation on every event. void ensure_pointer_focus(wlr_surface* target, double x, double y) { if (focused_xsurface) { wlr_xwayland_surface_activate(focused_xsurface, true); wlr_seat_pointer_notify_enter(seat, target, x, y); } else if (pointer_entered_surface != target) { wlr_seat_pointer_notify_enter(seat, target, x, y); pointer_entered_surface = target; } }Then simplify each case, e.g., for
key:case InputEventType::key: { wlr_surface* target_surface = resolve_target_surface(); if (!target_surface) { break; } ensure_keyboard_focus(target_surface); auto state = event.pressed ? WL_KEYBOARD_KEY_STATE_PRESSED : WL_KEYBOARD_KEY_STATE_RELEASED; wlr_seat_keyboard_notify_key(seat, time, event.code, state); break; }Apply similar simplifications to the other three cases.
741-746: Clarify "breaks X11 input" comment with specific symptoms.The comment states that registering a destroy listener on
xsurface->surface->events.destroy"breaks X11 input entirely," but doesn't explain the failure mode. Adding details about what "breaks" means (e.g., "causes events to drop," "fires unexpectedly during normal operation," "prevents focus updates") would help future maintainers understand why this pattern is necessary.If you have specifics about the failure (from testing or wlroots documentation), consider expanding the comment:
- // NOTE: Do NOT register destroy listener on xsurface->surface->events.destroy - // It fires unexpectedly during normal operation, breaking X11 input entirely. + // NOTE: Do NOT register destroy listener on xsurface->surface->events.destroy + // wlroots fires this signal during X11 client reconnection/reparenting events, + // not just on true destruction. Listening causes premature focus clearing and + // subsequent input events to be dropped by wlr_xwm.(Adjust the explanation based on your actual observations.)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/input_forwarding.mdsrc/input/compositor_server.cpp
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/config/goggles.toml : Development/testing configuration: `config/goggles.toml` in repository root. Structure should include sections for [capture], [pipeline], [render], and [logging].
Applied to files:
docs/input_forwarding.md
🧬 Code graph analysis (1)
src/input/compositor_server.cpp (1)
src/util/error.hpp (1)
make_error(50-52)
🪛 markdownlint-cli2 (0.18.1)
docs/input_forwarding.md
113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (test preset)
- GitHub Check: Static Analysis (clang-tidy)
🔇 Additional comments (7)
docs/input_forwarding.md (1)
1-244: LGTM on technical content and structure.The documentation comprehensively explains the new wlroots-based input forwarding architecture, thread model, and cross-thread marshaling strategy. Limitations are transparent, dependencies are clearly listed, and testing guidance covers both X11 and Wayland backends. The architecture aligns well with the PR objectives (CompositorServer, wlr_seat APIs, XWayland integration, SPSCQueue+eventfd marshaling).
src/input/compositor_server.cpp (6)
43-59: LGTM on helper functions.The time retrieval and socket binding logic are correctly implemented. The error message accurately describes the resource being bound (goggles sockets), and the socket naming follows the expected pattern.
163-312: LGTM on initialization sequence.The
start()method correctly initializes all wlroots components in the proper dependency order, uses RAII guard for cleanup on error, and successfully establishes both Wayland and XWayland servers with appropriate input capabilities.
314-391: LGTM on cleanup sequence.The
stop()method follows the correct destruction order (terminate, join thread, remove event source, clear pointers, destroy XWayland before compositor, seat before display), preventing use-after-free and ensuring all resources are properly released.
589-697: LGTM on XDG surface lifecycle management.The per-surface hooks pattern correctly tracks each toplevel's lifecycle with dedicated listeners. Memory management, listener registration/removal, focus clearing, and vector cleanup are all properly implemented. The destroy handler correctly removes the surface from the vector regardless of focus state.
699-769: LGTM on XWayland surface lifecycle management.The per-surface hooks pattern, listener registration, override_redirect filtering, and focused surface cleanup are correctly implemented. The intentional decision not to register a destroy listener on the wlr_surface is properly documented, preventing input breakage.
771-822: LGTM on focus management with careful state transitions.Both
focus_surface()andfocus_xwayland_surface()correctly manage the transition between focus states. The ordering of operations (clear stale pointers before wlroots calls, clear seat focus before setting new focus) prevents use-after-free and invalid leave events. The comments explaining these orderings are valuable for maintainability.
| if (!focused_surface || focused_xsurface) { | ||
| wlr_xdg_toplevel_set_activated(hooks->toplevel, true); | ||
| focus_surface(hooks->surface); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/input && wc -l compositor_server.cppRepository: K1ngst0m/Goggles
Length of output: 85
🏁 Script executed:
cd src/input && sed -n '650,660p' compositor_server.cppRepository: K1ngst0m/Goggles
Length of output: 354
🏁 Script executed:
cd src/input && sed -n '745,755p' compositor_server.cppRepository: K1ngst0m/Goggles
Length of output: 466
🏁 Script executed:
cd src/input && rg -i "focus.*steal|steal.*focus|xwayland.*priority|wayland.*priority|focus.*policy" -C 2Repository: K1ngst0m/Goggles
Length of output: 42
🏁 Script executed:
cd src/input && sed -n '640,680p' compositor_server.cppRepository: K1ngst0m/Goggles
Length of output: 1307
🏁 Script executed:
cd src/input && sed -n '700,760p' compositor_server.cppRepository: K1ngst0m/Goggles
Length of output: 2570
🏁 Script executed:
rg "focused_xsurface|focused_surface" src/input/compositor_server.cpp -B 2 -A 2Repository: K1ngst0m/Goggles
Length of output: 4994
Add comment explaining native Wayland priority over XWayland.
The condition !focused_surface || focused_xsurface at lines 654-657 allows native Wayland surfaces to steal focus from XWayland surfaces. This asymmetric policy is intentional: XWayland surfaces cannot steal focus from Wayland surfaces (as documented in the handle_xwayland_surface_associate comment at lines 745-748), but Wayland surfaces can steal from XWayland.
Add a comment similar to the one at lines 745-748 to document this prioritization:
// Focus Wayland surface if:
// - No current focus, OR
// - Current focus is XWayland (prioritize native Wayland over XWayland)
This makes the focus policy explicit and symmetric with the XWayland handler documentation.
🤖 Prompt for AI Agents
In @src/input/compositor_server.cpp around lines 654 - 657, Add a clarifying
comment above the if-statement that checks focused_surface and focused_xsurface
to document that native Wayland surfaces are allowed to steal focus from
XWayland surfaces; specifically, annotate the block around the condition using
the same rationale as the existing handle_xwayland_surface_associate comment and
include the suggested text: "Focus Wayland surface if: - No current focus, OR -
Current focus is XWayland (prioritize native Wayland over XWayland)". Place this
comment immediately before the if (!focused_surface || focused_xsurface) { ... }
that calls wlr_xdg_toplevel_set_activated(hooks->toplevel, true) and
focus_surface(hooks->surface).
K1ngst0m
force-pushed
the
dev/wayland-input-forward
branch
from
8beeac3 to
47116b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tests/CMakeLists.txt:
- Around line 166-167: Add the same explanatory comment used for the X11 test to
document why leak detection is disabled for the Wayland test: locate the
set_property(TEST goggles_auto_input_forwarding_wayland PROPERTY ENVIRONMENT
"ASAN_OPTIONS=detect_leaks=0") line and prepend a brief comment (or
cross-reference) explaining the rationale for using ASAN_OPTIONS=detect_leaks=0
(e.g., expected intentional leaks from the test harness or third-party libs) so
maintainers understand why leak suppression is applied.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-29T15:44:30.520Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: openspec/project.md:0-0
Timestamp: 2025-12-29T15:44:30.520Z
Learning: Applies to openspec/tests/**/*.{cpp,hpp} : Focus unit tests on non-GPU logic: utility functions, configuration parsing, and pipeline graph logic
Applied to files:
tests/CMakeLists.txt
📚 Learning: 2026-01-02T07:02:32.720Z
Learnt from: CR
Repo: K1ngst0m/Goggles PR: 0
File: docs/project_policies.md:0-0
Timestamp: 2026-01-02T07:02:32.720Z
Learning: Applies to docs/tests/**/*.cpp : Only non-GPU logic is tested initially: in scope are utility functions, configuration parsing, error handling, pipeline graph logic. Out of scope: Vulkan initialization, GPU resource management, rendering, capture layer.
Applied to files:
tests/CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (test preset)
- GitHub Check: Static Analysis (clang-tidy)
🔇 Additional comments (2)
tests/CMakeLists.txt (2)
92-122: LGTM: Manual test executables well-structured.The manual test executables for X11 and Wayland variants are properly configured with consistent dependencies (SDL3, Vulkan, goggles_util), sanitizers enabled, and appropriately not registered with CTest since they're interactive tools.
124-146: Integration test dependencies are correct; setup is solid.The X11 integration test properly links only SDL3 and goggles_input, which is exactly what the test needs. The source uses InputForwarder for key event forwarding and SDL for window/event handling—no Vulkan or utility functions. The CTest registration, working directory, and sanitizer configuration are all appropriate.
The ASAN leak suppression with documented reasoning (Mesa/EGL leaks during SDL startup) is a reasonable pragmatic workaround for an external-library issue.
| set_property(TEST goggles_auto_input_forwarding_wayland | ||
| PROPERTY ENVIRONMENT "ASAN_OPTIONS=detect_leaks=0") |
There was a problem hiding this comment.
Document the leak suppression rationale.
The Wayland integration test applies the same ASAN_OPTIONS=detect_leaks=0 setting as the X11 test, but lacks the explanatory comment present at lines 142-144. For maintainability, either duplicate the comment or add a cross-reference explaining why leak detection is disabled.
🔎 Suggested fix: Add explanatory comment
+# LeakSanitizer currently reports leaks inside Mesa/EGL during SDL startup in this
+# headless Wayland integration test. Disable leak detection for this test so it
+# can run under the ASAN preset without failing on external-library leaks.
set_property(TEST goggles_auto_input_forwarding_wayland
PROPERTY ENVIRONMENT "ASAN_OPTIONS=detect_leaks=0")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set_property(TEST goggles_auto_input_forwarding_wayland | |
| PROPERTY ENVIRONMENT "ASAN_OPTIONS=detect_leaks=0") | |
| # LeakSanitizer currently reports leaks inside Mesa/EGL during SDL startup in this | |
| # headless Wayland integration test. Disable leak detection for this test so it | |
| # can run under the ASAN preset without failing on external-library leaks. | |
| set_property(TEST goggles_auto_input_forwarding_wayland | |
| PROPERTY ENVIRONMENT "ASAN_OPTIONS=detect_leaks=0") |
🤖 Prompt for AI Agents
In @tests/CMakeLists.txt around lines 166 - 167, Add the same explanatory
comment used for the X11 test to document why leak detection is disabled for the
Wayland test: locate the set_property(TEST goggles_auto_input_forwarding_wayland
PROPERTY ENVIRONMENT "ASAN_OPTIONS=detect_leaks=0") line and prepend a brief
comment (or cross-reference) explaining the rationale for using
ASAN_OPTIONS=detect_leaks=0 (e.g., expected intentional leaks from the test
harness or third-party libs) so maintainers understand why leak suppression is
applied.
| if(DEFINED ENV{CI} OR DEFINED ENV{GITHUB_ACTIONS}) | ||
| set_property(TEST goggles_auto_input_forwarding_x11 PROPERTY DISABLED TRUE) | ||
| set_property(TEST goggles_auto_input_forwarding_wayland PROPERTY DISABLED TRUE) | ||
| endif() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking GitHub Actions workflows for display/compositor support ==="
find .github/workflows -type f \( -name "*.yml" -o -name "*.yaml" \) 2>/dev/null | head -20
echo -e "\n=== Looking for CI configuration content ==="
if [ -d .github/workflows ]; then
for file in .github/workflows/*.{yml,yaml}; do
if [ -f "$file" ]; then
echo "File: $file"
rg -n "xvfb|Xvfb|DISPLAY|wayland|weston|headless|integration.*test|Test goggles" "$file" || echo " (no matches)"
fi
done
fi
echo -e "\n=== Checking for project documentation ==="
fd -t f -e md . . -name README -o -name TESTING -o -name "*.md" 2>/dev/null | head -20
echo -e "\n=== Searching documentation for CI testing strategy ==="
if [ -f README.md ]; then
echo "=== README.md CI mentions ==="
rg -n -i "ci.*test|headless|integration" README.md || echo " (no matches)"
fi
echo -e "\n=== Looking at tests/CMakeLists.txt context ==="
sed -n '150,190p' tests/CMakeLists.txtRepository: K1ngst0m/Goggles
Length of output: 1417
🏁 Script executed:
#!/bin/bash
echo "=== Checking the actual integration test implementations ==="
fd -t f -name "*input_forwarding*" tests/
echo -e "\n=== Examining the test code for environmental dependencies ==="
fd -t f -name "*.cpp" -o -name "*.h" tests/ -exec rg -l "input_forwarding" {} \;
echo -e "\n=== Looking for any TODO/FIXME comments around CI disabling ==="
rg -B5 -A5 "DISABLED TRUE" tests/CMakeLists.txtRepository: K1ngst0m/Goggles
Length of output: 1065
🏁 Script executed:
#!/bin/bash
echo "=== Checking if this is a known issue in tracking ==="
fd -t f -e md . . -name "*.md" -exec rg -l "input.*forward|x11.*wayland|test.*disable" {} \;
echo -e "\n=== Checking git history for context ==="
git log --oneline -n 30 -- tests/CMakeLists.txt 2>/dev/null || echo "Git history not available"
echo -e "\n=== Checking for any issue references in comments ==="
rg -B10 -A5 "auto_input_forwarding" tests/CMakeLists.txtRepository: K1ngst0m/Goggles
Length of output: 2512
Integration tests disabled in CI need documentation and a remediation plan.
These are legitimate integration tests validating the CompositorServer → X11/Wayland → SDL input forwarding paths. Disabling them in CI removes automated regression coverage for this critical functionality.
The comment in CMakeLists.txt reveals the actual issue: "LeakSanitizer currently reports leaks inside Mesa/EGL during SDL startup in this headless XWayland integration test." This indicates the problem is not simply a missing display server, but EGL/Mesa behavior in headless environments.
Required before merge:
- Document the constraint: Add a comment explaining why these tests are disabled in CI and what environmental issues prevent them from running
- Create a tracking issue: File a GitHub issue to track re-enabling these tests (e.g., "Re-enable X11/Wayland integration tests in CI")
- Link the issue: Reference it in a TODO comment for future developers
The suggested alternatives (xvfb-run, mocking, nightly builds) may require deeper investigation of the EGL/Mesa leak issue to be effective, but without tracking this, it risks being forgotten indefinitely.
1 participant
User description
Why
The current input forwarding implementation only supports X11 applications via XTest injection to XWayland. Wayland-native applications connecting to the nested compositor cannot receive input events. Additionally, the XTest approach bypasses wlroots' seat/focus management system and requires maintaining a separate X11 connection.
What Changes
wlr_seat_*APIs for both X11 and Wayland appswlr_xwayland_set_seat()so wlr_xwm handles X11 translationXWaylandServertoCompositorServerto reflect its broader roleOld Code Cleanup
Since the project is pre-release, we will completely remove the XTest-based implementation rather than maintaining dual paths:
Files to rename:
src/input/xwayland_server.hpp->compositor_server.hppsrc/input/xwayland_server.cpp->compositor_server.cppCode to remove from
input_forwarder.cpp:#include <X11/Xlib.h>and#include <X11/extensions/XTest.h>Display* x11_displaymember andXOpenDisplay()/XCloseDisplay()callslinux_to_x11_keycode()function (wlr_xwm handles X11 keycode translation)XTestFakeKeyEvent(),XTestFakeButtonEvent(),XTestFakeMotionEvent()callsXFlush()callsDependencies to remove from
src/input/CMakeLists.txt:X11::X11link targetX11::Xtstlink targetfind_package(X11)if local to this moduleArchive for reference:
openspec/changes/archive/2026-01-04-add-input-forwarding-x11/Test app updates (
tests/input/goggles_input_test.cpp):setenv("SDL_VIDEODRIVER", "x11", 1)goggles_input_test_x11- tests X11/XWayland input via wlr_seat -> wlr_xwm pathgoggles_input_test_wayland- tests native Wayland input via wlr_seat pathSDL_VIDEODRIVER(x11orwayland)WAYLAND_DISPLAYin addition toDISPLAYImpact
input-forwardingcapabilitysrc/input/xwayland_server.*->compositor_server.*(rename + refactor)src/input/input_forwarder.hpp(minimal changes)src/input/input_forwarder.cpp(major refactor - remove X11, add wlr_seat)src/input/CMakeLists.txt(remove X11/XTest deps)tests/input/goggles_input_test.cpp-> split into_x11and_waylandvariantstests/input/CMakeLists.txt(build both test binaries)docs/input_forwarding.md(update architecture)libX11,libXtst(from input forwarding module)PR Type
Enhancement, Bug fix
Description
Replace XTest injection with unified
wlr_seat_*APIs for both X11 and Wayland applications, eliminating X11/XTest dependencies from input forwardingRename
XWaylandServertoCompositorServerto reflect its broader role managing both native Wayland and XWayland clientsImplement thread-safe input event marshaling using
SPSCQueueand eventfd for cross-thread communication between main thread and compositor threadAdd virtual keyboard and pointer devices to wlroots seat with proper xkb keymap support for unified input delivery
Track surfaces from both xdg_shell and XWayland clients with proper focus management and lifecycle handling
Connect XWayland to seat via
wlr_xwayland_set_seat()so wlr_xwm automatically handles X11 protocol translationRemove X11 and XTest library dependencies from input forwarding module
Split input test applications into separate X11 and Wayland variants for better test isolation and clarity
Comprehensive documentation including architecture diagrams, design rationale, specifications, and implementation tasks
Diagram Walkthrough
File Walkthrough
4 files
compositor_server.hpp
New unified compositor server with wlr_seat inputsrc/input/compositor_server.hpp
xwayland_server.hppwith unified inputarchitecture
CompositorServerclass managing headless wlroots compositorwith seat, keyboard, and XWayland
inject_key,inject_pointer_*) usingSPSCQueueand eventfdx11_display()andwayland_display()methods for environmentvariable setup
compositor_server.cpp
Compositor server implementation with unified wlr_seat inputsrc/input/compositor_server.cpp
and native Wayland support
and XWayland clients with proper focus handling
SPSCQueueand eventfdto compositor thread
wlr_seat_keyboard_notify_key(),wlr_seat_pointer_notify_*()APIs for both X11 and Wayland appsinput_forwarder.hpp
Update display accessor methods for dual display supportsrc/input/input_forwarder.hpp
display_number()method tox11_display()andwayland_display()returning strings
identifiers
main.cpp
Update logging for dual display informationsrc/app/main.cpp
input forwarder
display_number()call to dualx11_display()andwayland_display()calls1 files
input_forwarder.cpp
Remove X11/XTest, use unified wlr_seat APIssrc/input/input_forwarder.cpp
#include,XOpenDisplay,XTestFakeKeyEvent, etc.)CompositorServerinjection methodsinstead of XTest
linux_to_x11_keycode()with direct Linux keycode constantsfrom
sdl_to_linux_button()helper for pointer button mapping2 files
goggles_input_test_x11.cpp
X11 backend test app with improved loggingtests/input/goggles_input_test_x11.cpp
goggles_input_test.cppwith X11-specific brandinggoggles_input_test_x11forclarity
from Wayland variant
DISPLAYandWAYLAND_DISPLAYenvironmentvariables
goggles_input_test_wayland.cpp
New Wayland backend test app for native inputtests/input/goggles_input_test_wayland.cpp
test structure)
SDL_VIDEODRIVER=waylandto test native Wayland backendDISPLAYandWAYLAND_DISPLAYenvironment variables1 files
CMakeLists.txt
Split input test into X11 and Wayland variantstests/CMakeLists.txt
goggles_input_testtarget into two separate binariesgoggles_input_test_x11for X11/XWayland input testinggoggles_input_test_waylandfor native Wayland input testing2 files
CMakeLists.txt
Remove X11/XTest library dependenciessrc/input/CMakeLists.txt
X11_INCLUDE_DIRfrom target include directories${X11_LIBRARIES}and${X11_XTest_LIB}from target linklibraries
forwarding)
xdg-shell-protocol.h
Add Wayland xdg_shell protocol header for native surface supportsrc/input/xdg-shell-protocol.h
lines)
xdg_surface, xdg_toplevel, and xdg_popup
surface management
specification
5 files
input_forwarding.md
Update documentation for unified wlr_seat architecturedocs/input_forwarding.md
wlr_seat_*API instead of XTest injection
both Wayland and XWayland surfaces
input marshaling
to X11 protocol
instructions
design.md
Design document for unified input forwarding architectureopenspec/changes/add-wayland-input-forwarding/design.md
wlr_seat_*APIswith focus mutual exclusion
main-to-compositor thread marshaling
and native Wayland surfaces
spec.md
Specification for input forwarding capabilityopenspec/changes/add-wayland-input-forwarding/specs/input-forwarding/spec.md
forwarding system
keyboard/pointer input, thread-safe marshaling
management, and disconnection
wlr_seat_*APIstasks.md
Implementation task checklist for input forwardingopenspec/changes/add-wayland-input-forwarding/tasks.md
marshaling, and cleanup
destroy listener handling
proposal.md
Change proposal for Wayland native input forwardingopenspec/changes/add-wayland-input-forwarding/proposal.md
unified input forwarding
2 files
Summary by CodeRabbit
Release Notes
New Features
Refactor
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.