Repository navigation
feat: add output-mirror protocol, deprecate virtual-output-v1 - #109
Conversation
Reviewer's GuideIntroduces the experimental output-mirror manager/group protocol with wl_output-based identity, push-based group enumeration, explicit lifecycle and membership semantics, while moving the old virtual-output protocol to deprecated installation and documenting the 0.7.0 migration in English and Chinese. Sequence diagram for output mirror group discovery and configurationsequenceDiagram
participant Client
participant Manager as treeland_output_mirror_manager_v1
participant Group as treeland_output_mirror_group_v1
participant Output as wl_output
Client->>Manager: bind
Manager-->>Client: group_added(id, name)
Manager-->>Group: source(output or null)
Manager-->>Group: output_added(mirror)
Client->>Manager: create_group(id, name)
Manager-->>Client: group object
Client->>Group: set_source(Output)
Group-->>Client: source(Output)
Client->>Group: add_output(Output)
Group-->>Client: output_added(Output)
State diagram for output mirror group lifecyclestateDiagram-v2
[*] --> Empty: create_group
Empty --> Configured: set_source / add_output
Configured --> Mirroring: source and mirror present
Mirroring --> Configured: remove_output
Empty --> Dissolved: dissolve
Configured --> Dissolved: dissolve
Mirroring --> Dissolved: dissolve
Dissolved --> [*]: destroy
state Configured {
[*] --> SourceOrMirrors
SourceOrMirrors --> SourceOrMirrors: set_source / add_output
SourceOrMirrors --> SourceOrMirrors: remove_output
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
3dfa329 to
f5253f7
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The unchanged v2 wire identity, undefined unknown-UUID initialization, stale package version, and incorrect documented API name must be addressed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds UUID-based output identity and mirroring while retaining the legacy virtual-output protocol for migration.
Changes:
- Adds UUID and output-mirror protocols.
- Migrates output-manager v2 from
wl_outputobjects to UUIDs. - Updates installation and bilingual migration documentation.
File summaries
| File | Description |
|---|---|
CMakeLists.txt |
Registers new and deprecated protocols. |
deprecated/treeland-virtual-output-manager-v1.xml |
Preserves the legacy protocol. |
dde/treeland-output-uuid-unstable-v1.xml |
Defines UUID lookup for wl_output. |
dde/treeland-output-uuid-head-unstable-v1.xml |
Adds UUID lookup for wlr heads. |
dde/treeland-output-mirror-manager-unstable-v1.xml |
Defines UUID-based mirror groups. |
dde/treeland-output-manager-unstable-v2.xml |
Replaces object arguments with UUID strings. |
dde/README.md |
Documents protocols and migration. |
dde/README.zh_CN.md |
Adds synchronized Chinese documentation. |
Review details
- Files reviewed: 7/8 changed files
- Comments generated: 5
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
a0cb707 to
28b2b8b
Compare
|
TAG Bot New tag: 0.7.0 |
28b2b8b to
9c952d0
Compare
ca9bb3b to
85ecbb4
Compare
ca02988 to
4fe98e3
Compare
deepin-wm
left a comment
deepin-wm
left a comment
There was a problem hiding this comment.
代码审查报告
审查范围：4fe98e3（ca02988 之后的描述性整理也在本次范围内），diff = 5 文件 +554/−4。
结论：方向正确，实现质量中上。建议在修复 4 个 P1 前不合入——P1 全部是协议语义/线缆格式的歧义，改动量都是文档级。
已实测（PR 描述 Influence 三条全部跑过）：
./check-protocol-xml.sh全仓通过；新文件client-header/server-header/public-code/private-code生成无告警（wayland-scanner 1.26.0）。cmake -S . -B … && cmake --install：新版treeland-output-group-manager-unstable-v1.xml与旧版treeland-virtual-output-manager-v1.xml同时落到share/treeland-protocols，.pcVersion: 0.8.0。- 命名约定（AGENTS.md）、SPDX 头、EN/ZH 两份 README 的 0.8.0 子节（各 9 条、标题层级一致）均无问题。旧协议
git mv保留 rename 检测 100%，未夹带修改。
P1（阻塞）
P1-1 state 的「定长 uuid 拼接数组」依赖一个 uuid 协议从未承诺的保证
dde/treeland-output-group-manager-unstable-v1.xml:451：
Output uuids assigned through
treeland-output-uuid-unstable-v1are fixed-length and never contain the NUL byte; the compositor must preserve this property for every uuid it assigns, and clients may rely on it to split the array.
但 public/treeland-output-uuid-unstable-v1.xml 的原文是：
The uuid is an opaque, non-empty UTF-8 string … Clients must not parse it, derive meaning from its contents …
两者直接冲突：让客户端按「总字节数 ÷ count」切分数组，本身就是从内容推导结构；而 uuid 协议只保证「非空 UTF-8」，没有任何长度保证。
触发条件：只要有一个 uuid 是 37 字节（例如未来改用带花括号的 QUuid、改用 EDID 序列号、或 DConfig 里残留历史格式的值），客户端按固定长度切分就会把后续所有镜像 uuid 全部错位，且无法检测——count 只校验数量，不校验长度。
我核对了 compositor 侧实现（treeland/src/output/outputmanager.cpp:259）：当前是 QUuid::createUuid().toString(QUuid::WithoutBraces)，即 36 字节定长，所以今天能跑。但把「今天恰好成立」写成对另一个已发布协议的规范性约束（compositor must preserve），而后者文档未同步修改，等于给未来所有实现（含非 treeland 实现）埋了一个静默数据损坏点。
建议（二选一，代价都很小）：
- 在本 PR 内给
public/treeland-output-uuid-unstable-v1.xml补一段：uuid 恒为 36 字节8-4-4-4-12小写十六进制、不含 NUL，并消除与「must not derive structure from contents」的冲突；或 state改用 wayland 惯用的 NUL 分隔字符串数组（旧 v1 的outputs事件就是这么做的），不再需要长度保证，count也可以去掉。
P1-2 镜像列表的定义自相矛盾，compositor 实现必然分叉
同文件两处对「mirror list 里放什么」给出互斥定义：
:436-437「The mirror list holds the configured mirrors that are currently connected and enabled, in list order」:271-275「The source uuid is empty when the group has no effective source … In that case the mirror list is empty too」
触发条件：create_group 传空 source → add_output(B)（B 已连接、已启用）。按 :437 应报 source="" count=1 uuids=B；按 :274 应报 source="" count=0。两种实现都「符合文档」，而客户端只能赌一个——这是本协议唯一的全量状态通道。
建议：二选一并在两处对齐。要么把 :274 明确成「mirror list 仅在组有有效源时才有意义，无有效源时 compositor 发送 count=0」，要么从 :437 删掉「configured」改成「effective mirrors（且仅当组有有效源时非空）」。
P1-3 create_group 未定义「初始 source 已被其它组占用」的行为
:229（接口描述）和 :24（协议描述）都写了「An output may belong to at most one group at a time」，set_source（:332）与 add_output（:353）也各自给出了 output_in_use 处理，但 create_group（:97-125）只定义了 invalid_source，对 source 已在别的组里一字未提：返回 output_in_use？建一个 inert 组？静默接管？
触发场景很常见：显示设置面板的两条路径（或两个客户端）同时把同一块屏设为源。按当前文档，各 compositor 作者只能自由发挥，客户端无法预测。
建议：在 create_group 描述里补一句，与其它请求对齐（返回 output_in_use 失败并保持组不创建，或明确接管语义）。
P1-4 瞬时/竞态状态用致命错误，违背协议自己声明的原则，并把旧 v1 的健壮性改回去了
三处互相矛盾：
:70「concurrent edits never terminate a connection and clients may retry」:233-236「Requests that cannot be applied because of transient output state … are reported via a non-fataloperation_failed… Onlyalready_dissolved… is a fatal protocol error」- 但
create_group的invalid_source（:112，manager 级error枚举:78）和get_group的invalid_name（:140，:80）都是 fatal——wl_resource_post_error按定义直接断连。
复现 1：客户端 get_uuid 拿到 A 的 uuid → 用户拔线 → 客户端发 create_group(source=A) → invalid_source → 整个连接被 kill。旧 v1 对完全相同的竞态用的是组级非致命 error 事件（invalid_output），所以这是相对 v1 的回退。
复现 2：客户端在 group_removed 到达前发 get_group(name) → invalid_name → 断连。XML :138-140 自己承认了这个窗口（「note that a group may have been dissolved between the group_removed event and this request」），而 dde/README.md:36 第 5 条却宣称新模型「removes the list-then-lookup TOCTOU window」——同一份文档一边消除 TOCTOU，一边新增一个致命 TOCTOU。
仓库内已有正确先例：treeland-output-manager-unstable-v2 的 set_primary_output 对「uuid 无对应已连接输出」用的是非致命 primary_output_failed（见 dde/README.md:75）。同一份 README 相邻两节，对同类情况处理相反。
建议：给 manager 增加一个非致命事件（可复用 operation_failed 命名，带 invalid_source / invalid_name 码）；或 create_group 在 source 无效时建一个 source 为空的 inert 组，由 state 反映真实状态；get_group 失败时返回一个立即收到 removed 的对象。若坚持致命，必须给 :70 的「never terminate a connection」加上限定，否则文档内部自相矛盾。
P2（非阻塞，建议本轮或紧随其后的 PR 解决）
P2-5 配置态不可观测，陈旧配置永远清不掉。 state（:424）只报有效态；remove_output（:364）需要 uuid，而 uuid 只能从 wl_output 或 zwlr_output_head_v1 取，uuid-head 只对已连接输出有 head。推论：一块永久拆掉的显示器留下的 configured mirror，客户端永远拿不到它的 uuid，也就永远无法 remove_output，只能 dissolve 整组再重建；同时显示设置面板无法展示「本组还配着一块未接入的屏」。对一个主打「配置持久化」的协议，这是功能性缺口。建议：给 state 增加配置成员（或另设 configured 事件），或提供 reset / clear 请求。
P2-6 自动提升的镜像仍是 configured mirror，set_source 无法确认。 源掉线后 compositor 把镜像 B 提升为有效源，但 B 仍是 configured mirror（:260-268 把「temporary / permanent」列为实现细节）。此时客户端想「就用 B 当源」只能 set_source(B) → duplicate_output；按文档得先 remove_output(B) 再 set_source(B)，而 remove_output(B)（:384）本身会触发一次重新解析，可能把源又抢走。对主要消费者（显示设置面板）是易踩的坑。建议：允许 set_source 对「当前有效源」幂等成功，或明确规定「提升为永久时 compositor 必须同时把该镜像移出 configured mirror」，即禁止 temporary 语义。
P2-7 旧协议「仍安装」是有条件的。 移入 deprecated/ 后受 TREELAND_PROTOCOLS_INSTALL_DEPRECATED（默认 ON）控制；发行版若设 OFF，迁移期旧客户端直接拿不到 XML。建议在 dde/README.md:30 的迁移说明里补「仅在 -DTREELAND_PROTOCOLS_INSTALL_DEPRECATED=ON 时安装」。
P3（细节）
P3-8 迁移指南与旧协议事实不符（dde/README.md:33、:37）：第 2 条称移除 invalid_group_name / name_exists manager 错误——旧 manager 根本没有 error 枚举，name_exists 在全部 11 个历史版本中都不存在；第 6 条称移除 output_added / output_removed 增量事件——旧协议从来只有 outputs 全量快照事件。第 8 条对 invalid_group_name（组级）的描述才是对的，与第 2 条互相打脸；第 3 条漏了 outputs 事件的 name 参数。读者会照着去旧文件里找不存在的东西。
P3-9 版本 bump 方式偏离仓库惯例：git log -- CMakeLists.txt 显示历次版本都是独立 chore: bump version to X commit 且同时更新 debian/changelog（如 49e985e）。本 PR 把 0.8.0 塞进 feature commit，debian/changelog 仍是 0.7.0。建议拆出去或补 changelog。
P3-10 commit message 未遵循 AGENTS.md：废弃那一半应使用 deprecate: move <protocol> to deprecated/ 形式（参见 c0a729c、393da93），当前把两种性质混在一个 commit。
P3-11 operation_failed（:479）缺请求标识：只带 code + uuid。流水线发送 set_source(X) + add_output(X) 时，duplicate_output + X 无法区分是哪一个失败。建议加请求标识，或明确要求客户端串行等待 state。
P3-12 state 的 count（:465）与 uuids（:467）冗余：若采纳 P1-1 的方案 2，count 可直接去掉；若保留，需补一句「数组长度必须等于 count × 定长」的显式校验要求（目前只说「always matches」，没有可操作的校验方式）。
P3-13 PR 标题/描述与实际不一致：标题与描述称「output-mirror」，实际协议名是 output-**group**；描述称新增「0.7.0 破坏性变更子节」，实际是 0.8.0。同一功能三个名字会拖累后续 grep 与 issue 检索。
值得肯定
- 用
group_added/groups_done/group_removed取代「取列表再查」方向正确；groups_done明确区分初次 dump 与后续增量，比多数 registry 模式协议严谨。 destroy与dissolve分离、多 handle 语义、already_dissolved边界写得很清楚。- 全篇 RFC 2119 措辞、EXPERIMENTAL 警告、
name事件「在任何其它事件之前」的排序约束都到位。 - 旧协议原样移动、未夹带无关改动，审查成本低。
需要澄清的问题（架构层面）
- 镜像输出的 uuid 必须走
uuid-head（zwlr_output_head_v1），意味着本协议强依赖 wlr-output-management 全局。若某个 Treeland 构建不导出该全局（裁剪会话），协议对镜像输出完全不可用。这是有意的硬依赖，还是应该允许镜像输出通过其它入口取 uuid？ - 组名由 compositor 分配且跨重启持久化，但命名规则未写进协议（只说「unique among existing groups」，且允许解散后重用）。客户端按名字缓存状态时，旧名重用是否会混淆？是否需要规定命名形式以便多实现一致？
- 绑定策略与
treeland-output-manager-unstable-v2不一致：后者写「Only one client should bind this interface at a time」，本协议写「Multiple privileged clients … may bind concurrently」。这是有意放开，还是 v2 措辞需要同步？
4fe98e3 to
a550611
Compare
1. Add dde/treeland-output-group-manager-unstable-v1.xml with a manager/group split (treeland_output_group_manager_v1 / treeland_output_group_v1). Outputs are addressed by opaque uuid strings from treeland-output-uuid-unstable-v1 (public/); mirror uuids are queried through treeland-output-uuid-head-unstable-v1. 2. Groups are created with create_group(source) so observers never see an empty-source group; group names are assigned by the compositor and announced via the name event, with get_group available on the manager. 3. Enumeration uses group_added/groups_done/group_removed events; the group outputs event carries a count plus concatenated fixed-length uuids, replacing incremental added/removed events. 4. Distinguished configured vs effective state: unplugged or disabled outputs stay configured and resume automatically, there is no automatic source promotion, and groups persist across compositor restarts. 5. Split group failures by severity: invalid_output, duplicate_output, output_in_use and not_in_group are reported non-fatally through operation_failed (code + uuid), so transient output state never terminates the client connection; only already_dissolved and a bad source uuid at creation remain fatal protocol errors. 6. Move treeland-virtual-output-manager-v1.xml to deprecated/ unchanged, still installed by default for migration; register the new protocol in TREELAND_PROTOCOL_DDE_XML_FILES and the old one in TREELAND_PROTOCOL_DEPRECATED_XML_FILES. 7. Update dde README.md and README.zh_CN.md: new protocol table row and a 0.8.0 breaking-changes subsection describing the wire-level differences from virtual-output-v1. 8. Bump the project version to 0.8.0 for the new breaking release. Log: Replace the output-name/string-array based virtual output protocol with an output group protocol built on stable per-output uuids, explicit source/mirror separation, compositor-assigned names, full-list enumeration, persistent groups, and non-fatal transient failures. Influence: 1. Verify wayland-scanner generates client/server headers and code for the new XML without errors. 2. Verify cmake configure/build/install places the new file in share/treeland-protocols and the deprecated file still installs from deprecated/. 3. Verify an add_output failing because the output was unplugged emits operation_failed without terminating the connection. 4. Verify dde README.md and README.zh_CN.md table rows and the 0.8.0 breaking-changes subsection are present and in sync. 5. Verify the generated treeland-protocols.pc reports Version: 0.8.0. feat: 新增输出分组协议，废弃 virtual-output-v1 1. 新增 dde/treeland-output-group-manager-unstable-v1.xml，采用 manager/group 分离（treeland_output_group_manager_v1 / treeland_output_group_v1）。输出以来自 public/ 中 treeland-output-uuid-unstable-v1 的不透明 uuid 字符串寻址；镜像 输出的 uuid 经 treeland-output-uuid-head-unstable-v1 获取。 2. 组通过 create_group(source) 创建，观察者不会看到空源组；组名由 合成器分配并通过 name 事件告知，manager 上提供 get_group。 3. 枚举通过 group_added/groups_done/group_removed 事件完成；组的 outputs 事件携带数量与定长 uuid 拼接数组，替代增量式 added/removed 事件。 4. 区分配置态与运行时有效态：拔出或禁用的输出保留配置并在恢复后 自动生效，不再自动提升源，组配置跨合成器重启持久化。 5. 按严重程度划分组错误：invalid_output、duplicate_output、 output_in_use 与 not_in_group 通过 operation_failed（code + uuid） 非致命上报，瞬态输出状态不再终止客户端连接；仅 already_dissolved 与创建时非法的 source uuid 仍为致命协议错误。 6. 将 treeland-virtual-output-manager-v1.xml 原样移至 deprecated/， 默认仍安装以供迁移；新协议注册到 TREELAND_PROTOCOL_DDE_XML_FILES，旧协议注册到 TREELAND_PROTOCOL_DEPRECATED_XML_FILES。 7. 更新 dde README.md 与 README.zh_CN.md：新增协议表行，并在双语 README 中新增 0.8.0 破坏性变更子节，描述相对 virtual-output-v1 的线缆级差异。 8. 项目版本升至 0.8.0，对应该破坏性变更版本。 Log: 以基于稳定输出 uuid 的输出分组协议取代基于输出名称/字符串数组 的虚拟输出协议，显式区分源与镜像、组名由合成器分配、全量枚举、 组配置持久化，并采用非致命的瞬态失败上报。 Influence: 1. 验证 wayland-scanner 对新 XML 生成 client/server 头文件与代码无 报错。 2. 验证 cmake 配置/构建/安装将新文件安装至 share/treeland-protocols， 废弃文件仍从 deprecated/ 安装。 3. 验证 add_output 因输出拔出失败时发送 operation_failed 且不终止 连接。 4. 验证 dde README.md 与 README.zh_CN.md 表行及 0.8.0 破坏性变更 子节存在且内容同步。 5. 验证生成的 treeland-protocols.pc 中 Version 为 0.8.0。 PMS: TASK-395091
a550611 to
acdd6b5
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 语法正确，逻辑清晰 ✅ 通过 潜在问题： 建议： [] 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 潜在问题： 建议： [] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： [] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： [] 💡 改进建议代码示例本次提交无需修复，代码质量优秀。建议后续实现该协议的合成器端代码时， 注意以下几点以保持协议设计的安全性： 1. 确保 uuid 生成满足固定 36 字节且不含 NUL 字节的规范； 2. commit 队列处理时正确跳过失败操作并继续应用剩余操作； 3. 持久化配置的恢复逻辑需处理输出不可用时的降级场景。本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Groveer, wineee, 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 |
Log: Redesign the output copy/mirror protocol around wl_output object identity instead of output-name strings, with explicit source/mirror separation, registry-push enumeration, and fully specified object lifecycles; the old string-array and positional semantics are removed.
Influence:
feat: 新增 output-mirror 协议，废弃 virtual-output-v1
Log: 围绕 wl_output 对象身份重新设计输出复制/镜像协议，显式
区分源与镜像、采用 registry 推送枚举并完整定义对象生命周期；
旧的字符串数组与位置语义已移除。
Influence:
Summary by Sourcery
Introduce the output-group mirroring protocol and deprecate the legacy virtual-output interface while preserving it for migration.
New Features:
Enhancements:
Build:
Documentation: