feat: add treeland-input-manager-unstable-v1 protocol - #57
Conversation
|
Skipping CI for Draft Pull Request. |
|
@copilot review下这个pr, |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Wayland protocol definition (treeland_input_manager_unstable_v1) for privileged clients to discover per-seat input capabilities and apply batched (atomic) settings for pointer, mouse, touchpad, and keyboard devices in the treeland compositor environment.
Changes:
- Added a new unstable protocol XML defining
treeland_input_manager_v1plus settings interfaces for pointer/mouse/touchpad/keyboard. - Wired the new XML into the install list via
CMakeLists.txt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
xml/treeland-input-manager-unstable-v1.xml |
New protocol specification for compositor-managed input device configuration (capabilities + per-device settings objects + apply model). |
CMakeLists.txt |
Installs the new protocol XML with the rest of the protocol set. |
Comments suppressed due to low confidence (2)
xml/treeland-input-manager-unstable-v1.xml:139
wl_keyboardis not a global object (it is created fromwl_seat.get_keyboard), so “wl_keyboard global is removed” is misleading/incorrect. Please reword this example to match Wayland object lifetimes (e.g., seat removed, client destroyswl_keyboard, or the server destroys the object).
<arg name="keyboard" type="object" interface="wl_keyboard"
summary="keyboard object identifying the target keyboard"/>
</request>
xml/treeland-input-manager-unstable-v1.xml:171
- This sentence is unclear/grammatically incorrect: “treeland_pointer_settings_v1 creating just one is enough.” Please rephrase so it clearly states what the client should do (e.g., that one pointer settings object per seat is sufficient).
</description>
<arg name="type" type="uint" enum="treeland_input_manager_v1.device_type"
summary="type of the device capability"/>
<arg name="seat" type="object" interface="wl_seat"
summary="seat associated with the capability"/>
</event>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8241da1 to
0a8adc7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (3)
xml/treeland-input-manager-unstable-v1.xml:743
- Grammar issue in the interface description: "settings for all keyboard associated with the target seat" should be plural (e.g., "all keyboards" / "all keyboard devices").
<description summary="keyboard settings object">
The treeland_keyboard_settings_v1 interface represents pending
settings for all keyboard associated with the target seat.
xml/treeland-input-manager-unstable-v1.xml:271
- set_speed doesn’t define the valid range/interpretation of the fixed-point speed value (e.g., libinput typically uses a bounded range) or what happens for out-of-range inputs. Please document the range/behavior so clients can safely set speed without relying on compositor-specific behavior.
<request name="set_speed">
<description summary="set the pointer speed">
Set the pointer motion speed. The change takes effect on the
next apply request.
This request updates only the pending state tracked by this
object. Sending this request, or the following apply request,
does not by itself cause a speed event to be emitted.
</description>
<arg name="speed" type="fixed" summary="desired pointer motion speed"/>
</request>
xml/treeland-input-manager-unstable-v1.xml:782
- set_repeat uses signed ints for rate/delay but the protocol doesn’t specify whether negative values are allowed, and what the compositor does for invalid inputs (protocol error vs clamp). Please document the accepted ranges and behavior to avoid clients guessing.
<request name="set_repeat">
<description summary="set keyboard repeat rate and delay">
Set the keyboard repeat rate and delay to apply on the next
apply request.
This request updates only the pending state tracked by this
object. Sending this request, or the following apply request,
does not by itself cause a repeat event to be emitted.
</description>
<arg name="rate" type="int"
summary="repeat rate in characters per second"/>
<arg name="delay" type="int"
summary="delay in milliseconds before repeating starts"/>
</request>
fd2953d to
f903195
Compare
ca7e713 to
3be5963
Compare
|
TAG Bot New tag: 0.5.7 |
94f7b50 to
cc9940e
Compare
6943cb1 to
8b1832c
Compare
Introduce a new Wayland protocol, treeland_input_manager_v1, to allow privileged clients to configure compositor-managed input devices. Key design features: - Capability-based: Compositor advertises mouse/touchpad/keyboard availability via capability events. - Atomic configuration: Uses a "request-apply" model where settings are batched and applied via an explicit apply request. - Multi-seat support: Settings objects are bound to specific wl_seat. This protocol enables fine-grained control over input peripheral behavior within the treeland compositor environment. Log: Added treeland-input-manager-unstable-v1.xml. Tasks: https://pms.uniontech.com/task-view-389477.html Influence: 1.Impacts how system settings applications interact with input hardware.
deepin pr auto review你好!我是CodeGeeX。我已经仔细审查了你提供的 Wayland 协议 XML 文件及 CMakeLists.txt 的变更。 总体而言,这份协议设计得相当专业,采用了目前 Wayland 社区推荐的“双缓冲/状态累积+apply”模式来配置输入设备,避免了状态闪烁,并且通过 1. 逻辑与协议语义
2. 安全性
3. 代码质量与一致性
4. 性能
修改建议示例(针对 XML 关键部分)<!-- 1. 修正 acceleration_profile 枚举 -->
<enum name="acceleration_profile"> <!-- 移除 bitfield="true" -->
<entry name="flat" value="1" summary="use a flat acceleration profile"/>
<entry name="adaptive" value="2" summary="use an adaptive acceleration profile"/>
<entry name="custom" value="3" summary="use a custom acceleration profile"/>
</enum>
<!-- 2. 为 set_scroll_factor 增加安全约束描述 -->
<request name="set_scroll_factor">
<description summary="set the scroll delta scaling factor">
...
The factor must be strictly greater than zero. If a non-positive factor
is provided, the compositor will raise the invalid_scroll_factor protocol error.
</description>
<arg name="factor" type="fixed" summary="scaling factor applied to scroll axis deltas"/>
</request>
<!-- 3. 为 keyboard 的 done 事件补充 serial -->
<event name="done">
<description summary="end of a keyboard state batch">
...
</description>
<arg name="serial" type="uint" summary="current configuration serial"/>
</event>
<!-- 4. 修正版权年份 -->
<copyright><![CDATA[
SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd.
SPDX-License-Identifier: MIT
]]></copyright>综上所述,协议整体架构优秀,但在边界条件限制、枚举类型定义的严谨性上需要进一步打磨,以防止恶意或编写不佳的客户端利用协议漏洞攻击合成器。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zccrs, zzxyb The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Introduce a new Wayland protocol, treeland_input_manager_v1, to allow
privileged clients to configure compositor-managed input devices.