protocols: add treeland-cross-subsurface-unstable-v1 - #56
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wineee 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 |
|
TAG Bot New tag: 0.5.7 |
|
TAG Bot New tag: 0.5.8 |
|
TAG Bot New tag: 0.5.9 |
b9942be to
7939e38
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new Treeland Wayland protocol XML specification to support cross-process subsurface relationships, and wires it into the build/install list so it ships with the rest of the protocol set.
Changes:
- Introduces
treeland_cross_subsurface_unstable_v1protocol with manager/export/remote-subsurface interfaces and token-based parenting/sibling references. - Adds the new XML to the top-level
CMakeLists.txtXMLinstall list.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| xml/treeland-cross-subsurface-unstable-v1.xml | New protocol specification for exporting surfaces and creating cross-process subsurfaces, including sibling restacking and parent geometry events. |
| CMakeLists.txt | Installs the new protocol XML alongside existing Treeland protocols. |
Suppressed comments (3)
xml/treeland-cross-subsurface-unstable-v1.xml:339
- The place_above description contradicts itself: it says using an empty string causes bad_sibling, but later lists "" as a valid sentinel for "top". This should be consistent so clients/implementations don’t diverge.
of the sub-surfaces. The reference surface must be
identified by a valid sibling_token, or the parent's
surface_token. Using an empty string or any
unrecognized token will cause a bad_sibling protocol
error.
xml/treeland-cross-subsurface-unstable-v1.xml:223
- surface_destroyed does not explicitly say what happens to surface_token / subsurface_token validity. Since create_remote_subsurface requires a live exported surface, it should be explicit that tokens are revoked once the underlying wl_surface is destroyed.
The underlying wl_surface was destroyed. This
treeland_exported_surface_v1 object is now inert.
No further events will be emitted. The client
should destroy this object.
</description>
xml/treeland-cross-subsurface-unstable-v1.xml:40
- PR description explicitly includes set_sync/set_desync behavior aligned with wl_subcompositor, but the XML states synchronized mode is not supported and treeland_remote_subsurface_v1 has no set_sync/set_desync requests. Please reconcile the design doc and the protocol surface so clients know which behavior is actually supported.
sub-surfaces are always in desynchronized mode; synchronized
mode is not supported.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f16f0b3 to
285d462
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 no new comments.
Suppressed comments (3)
xml/treeland-cross-subsurface-unstable-v1.xml:25
- The protocol description and API use UUID string tokens (surface_token/subsurface_token, empty-string sentinel for top), but the PR description/design doc describes a uint32 ID namespace (including reserved values 0 and 0xFFFFFFFF) plus automatic discovery of standard wl_subsurface children (child_entered/child_left). Please reconcile the spec and the PR description so clients have one authoritative contract (either update this XML to match the uint32-based design, or update the PR description to reflect the token-based protocol actually being added).
client. This protocol removes that restriction by using
UUID-based tokens exchanged out-of-band:
1. A client exports a wl_surface and receives a surface_token.
2. Another client provides that surface_token with its own
local wl_surface to create a cross-process sub-surface.
3. Each cross-process sub-surface receives a subsurface_token
that can be used as a sibling reference in place_above
and place_below.
xml/treeland-cross-subsurface-unstable-v1.xml:131
- This text makes destruction ordering a hard "must" but does not define what happens if the wl_surface is destroyed first (and there is no specific protocol error for this case). Other Treeland protocols either enforce ordering with an explicit protocol error or document inert behavior. Consider relaxing this to "should" and documenting compositor behavior when the wl_surface is destroyed first.
The client must destroy this object before
destroying the associated wl_surface.
xml/treeland-cross-subsurface-unstable-v1.xml:215
- place_above's description currently says that using an empty string or an unrecognized token in place_below is a bad_sibling error, which is both confusing (wrong request name) and contradicts the earlier statement that empty string is a valid sentinel for place_above. This should describe place_above's own error behavior (unrecognized sibling_token) and leave place_below rules to the place_below section.
- An empty string (""), a sentinel meaning
"place at the very top of the sibling stack".
Using an empty string or any unrecognized token in
place_below results in a bad_sibling protocol error.
Reviewer's GuideAdds a new Wayland protocol XML definition for cross-process sub-surface compositing (treeland-cross-subsurface-unstable-v1) and wires it into the build/install, defining manager, exported-surface, and remote-subsurface interfaces with token-based cross-client parenting and stacking semantics. Sequence diagram for cross-process remote subsurface creation and stackingsequenceDiagram
participant Client_A
participant Client_B
participant Compositor
participant treeland_subsurface_manager_v1
Client_A->>treeland_subsurface_manager_v1: export_surface(id, surface)
treeland_subsurface_manager_v1->>Client_A: surface_token(token)
Client_B->>treeland_subsurface_manager_v1: export_surface(id, surface)
treeland_subsurface_manager_v1->>Client_B: surface_token(child_token)
Client_B->>Compositor: create_remote_subsurface(id, parent_token)
Compositor->>Client_B: parent_geometry(x, y, scale, transform)
Client_B->>Compositor: set_position(x, y)
Client_B->>Compositor: place_above(sibling_token)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The protocol XML diverges significantly from the accompanying design document (UUID tokens vs uint32 IDs, no automatic tracking of standard wl_subsurface children, no set_sync/set_desync, different place_above semantics); consider aligning the implementation with the documented design or updating the design doc to match the actual protocol.
- Error handling and role constraints around export_surface/create_remote_subsurface are a bit unclear (two different bad_surface meanings, parent_token errors placed on treeland_exported_surface rather than the manager); tightening and documenting the exact role compatibility and where each error is raised would make the protocol semantics easier to implement correctly.
- The sibling reference model only allows siblings identified via shared UUID tokens and does not integrate standard wl_subcompositor subsurfaces as siblings under the same parent; if you intend to support mixed standard and cross-process subsurface trees as in the design doc, you may want to introduce events or IDs that expose compositor-tracked standard subsurfaces.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The protocol XML diverges significantly from the accompanying design document (UUID tokens vs uint32 IDs, no automatic tracking of standard wl_subsurface children, no set_sync/set_desync, different place_above semantics); consider aligning the implementation with the documented design or updating the design doc to match the actual protocol.
- Error handling and role constraints around export_surface/create_remote_subsurface are a bit unclear (two different bad_surface meanings, parent_token errors placed on treeland_exported_surface rather than the manager); tightening and documenting the exact role compatibility and where each error is raised would make the protocol semantics easier to implement correctly.
- The sibling reference model only allows siblings identified via shared UUID tokens and does not integrate standard wl_subcompositor subsurfaces as siblings under the same parent; if you intend to support mixed standard and cross-process subsurface trees as in the design doc, you may want to introduce events or IDs that expose compositor-tracked standard subsurfaces.
## Individual Comments
### Comment 1
<location path="xml/treeland-cross-subsurface-unstable-v1.xml" line_range="112-121" />
<code_context>
+ <request name="create_remote_subsurface">
</code_context>
<issue_to_address>
**question:** Specify whether a surface can be its own parent via parent_token to avoid undefined self-parenting.
The spec says the exported surface "may act as a parent" and that `create_remote_subsurface` attaches "this surface" to the `parent_token`, but it’s unclear whether passing this surface’s own `surface_token` as `parent_token` is allowed. Self-parenting could lead to ambiguous stacking and mapping behavior. Please either explicitly forbid using the same exported surface as both parent and child, or clearly define the semantics if self-parenting is supported.
</issue_to_address>
### Comment 2
<location path="xml/treeland-cross-subsurface-unstable-v1.xml" line_range="207-216" />
<code_context>
+ <request name="place_above">
</code_context>
<issue_to_address>
**suggestion:** Define behavior when sibling_token refers to this sub-surface itself to avoid ambiguous restacking.
The protocol should define what happens if `sibling_token` refers to this sub-surface itself, since valid values include the parent and sibling cross-process sub-surfaces. Without a specified behavior (e.g., explicit no-op vs. `bad_sibling`), implementations may diverge, leading to inconsistent restacking semantics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dae4078 to
da27f83
Compare
Add a new protocol and its design document for cross-process sub-surface compositing. treeland_cross_subsurface_unstable_v1 extends wl_subcompositor semantics to surfaces belonging to different client connections. Surfaces are exported via treeland_exported_surface_v1, receiving a numeric surface_id that can be shared out-of-band. A remote client attaches its own wl_surface as a sub-surface by passing the parent's surface_id to create_remote_subsurface, obtaining a treeland_remote_subsurface_v1 that mirrors the full wl_subsurface API (set_position, place_above/below, set_sync/desync). Standard wl_subcompositor sub-surfaces created under an exported surface are automatically tracked by the compositor and reported via child_entered / child_left, assigning them a subsurface_id that participates in the same numeric namespace. The primary motivation is Wine's winewayland.drv, where GPU-rendered child windows belonging to a different Wine process cannot attach their wl_surface as a sub-surface of the parent's surface today. docs/cross-subsurface-design.md documents the background, design rationale, compositor implementation notes, Wine integration plan, and a comparison with xdg-foreign-v2.
deepin pr auto review★ 总体评分:100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 <?xml version="1.0" encoding="UTF-8"?>
<protocol name="treeland_cross_subsurface_unstable_v1">
<copyright><![CDATA[
SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd.
SPDX-License-Identifier: MIT
]]></copyright>
<description summary="cross-process sub-surface compositing">
Extends wl_subcompositor to support sub-surface relationships
between surfaces belonging to different client connections.
<!-- 其余协议内容保持不变 -->
</description>
</protocol> |
treeland_cross_subsurface_unstable_v1 设计文档
1. 背景与问题
1.1 Wayland 的进程隔离限制
标准
wl_subcompositor要求 parent 和 child 的wl_surface来自同一个wl_compositor(即同一个客户端连接)。这意味着跨进程的 subsurface 关系在标准协议下是不可能的。1.2 X11 的做法
X11 中
XReparentWindow()是服务端操作,任何 X 客户端都可以把窗口 reparent 到另一个 X 窗口下,没有进程隔离限制。所有 Wine 进程共享同一个gdi_display,跨进程父子窗口关系天然支持。X11 驱动中
attach_client_window()(winex11.drv/window.c:2309)通过XReparentWindow()将 client window 嵌入 parent 的whole_window,即使 parent 属于另一个进程也能正常工作。当跨进程 parent 查找失败时(get_win_data(toplevel)返回 NULL),X11 只是跳过坐标偏移修正,client window 仍然正常存在和渲染。1.3 Wine Wayland 后端的现状
Wine 的
winewayland.drv中有两类 subsurface:wayland_surface.c:305):WS_CHILD 窗口作为 toplevel 的 subsurfacewayland_surface.c:1179):OpenGL/Vulkan 渲染区域作为 toplevel 的 subsurface两者都通过
wl_subcompositor_get_subsurface()创建,必须在同一进程内。wayland_client_surface_attach()(wayland_surface.c:1150)中,当 toplevel 属于另一个进程时:wayland_win_data_get_nolock()只能查找当前进程的红黑树,跨进程查找返回 NULL,导致 client_surface 被 detach。1.4 触发场景
实际触发跨进程失败需要同时满足三个条件:
wayland_client_surface)!managed(无 caption/thickframe/sysmenu/WS_EX_APPWINDOW)owner_hint(来自GW_OWNER或NtUserWindowFromPoint)指向另一个进程这在真实应用中极为罕见,因为使用 GPU 渲染的窗口几乎总是有 caption 或 thickframe,会被判为 managed。但协议设计应覆盖这类场景以保证完整性。
1.5 place_above 的实际使用模式
Wine 中
place_above只引用两种目标(wayland_surface.c:611-694):wl_surface(toplevel 窗口表面本身)client_surface->wl_surface(OpenGL/Vulkan 渲染区域)并且只在 if/else 分支中使用,不需要引用任意中间 subsurface。
2. 设计目标
wl_subcompositor,减少客户端适配成本wl_subcompositor创建的 subsurface 也能被自动纳入 ID 管理3. 协议设计
3.1 整体架构
3.2 ID 命名空间
所有 ID(surface_id 和 subsurface_id)在同一个 uint32 命名空间中:
ID 来源有三个:
treeland_exported_surface_v1.surface_idtreeland_exported_surface_v1.child_enteredtreeland_remote_subsurface_v1.subsurface_idcompositor 内部维护一个全局 ID 分配器,保证三个来源不重复。
3.3 核心机制:标准 subsurface 的自动发现
compositor 在以下时机自动追踪标准
wl_subcompositorsubsurface:wl_subcompositor.get_subsurface(surface, parent)请求child_entered事件通知 export ownerchild_left事件通知 export owner(wl_subsurface 销毁时)这个机制不需要改动标准
wl_subcompositor协议,完全在 compositor 侧实现。3.4 与 wl_subcompositor 的对应关系
get_subsurface(id, surface, parent)create_remote_subsurface(id, surface, parent_id)set_position(x, y)set_position(x, y)place_above(sibling: wl_surface)place_above(sibling_ref: uint32)place_below(sibling: wl_surface)place_below(sibling_ref: uint32)set_sync()set_sync()set_desync()set_desync()subsurface_id事件parent_geometry事件parent_destroyed事件export_surface+surface_idchild_entered/child_left3.5 place_above / place_below 的 sibling_ref 语义
示例:
place_above(sibling_ref = parent_id)等价于标准协议中的place_above(parent_wl_surface)。3.6 parent_geometry 事件
跨进程后,子进程没有 parent 的
wl_surfaceproxy,无法直接获取 parent 的buffer_scale 和 buffer_transform。
parent_geometry事件提供这些信息:x, y:parent 在 compositor 全局逻辑坐标系中的原点位置scale:parent 的wl_surface.buffer_scaletransform:parent 的wl_surface.buffer_transform(遵循wl_output.transform语义)子进程使用这些值将自身逻辑坐标转换为 parent 的 surface-local 坐标后传给
set_position()。4. Compositor 侧实现要点
4.1 ID 管理
所有 ID(exported surface、standard subsurface、remote subsurface)通过同一个分配器管理。
4.2 标准 subsurface 监听
compositor 在处理
wl_subcompositor.get_subsurface时:4.3 place_above / place_below 处理
4.4 安全策略
compositor 应通过以下机制限制访问:
treeland_subsurface_manager_v1(通过 app-id 或 PID 白名单)create_remote_subsurface时验证 parent_id 对应的 export 属于受信任的客户端wl_surface参数只在 export owner 的连接中有效,其他客户端无法通过这个 proxy 操作 surface5. Wine 集成方案
5.1 与现有 wine_window_management 的关系
treeland_wine_window_management_v1的window_id和本协议的surface_id是两个独立系统,各自有独立的命名空间。Wine driver 需要在 wineserver 共享内存
中同时维护两套映射:
5.2 集成流程
5.3 修改点(winewayland.drv)
需要修改的函数:
wayland_client_surface_attach()wayland_surface_make_subsurface()wayland_surface_reconfigure_subsurface()wayland_surface_reconfigure_client()6. 与 xdg-foreign-v2 的对比
xdg-foreign-v2 只解决了 "把我的窗口放在另一个窗口上面" 的问题,而
treeland_cross_subsurface 解决的是 "把我的 surface 作为另一个 surface 的子 surface"
——包括定位、同步、排序在内的完整 subsurface 语义。
Summary by Sourcery
Introduce a new treeland cross-process subsurface Wayland protocol and register it in the build system.
New Features: