feat(window-transition): use a live source surface for the transition - #115
Conversation
|
Skipping CI for Draft Pull Request. |
Reviewer's GuideUpdates the window-transition protocol to use a live, compositor-exclusive Sequence diagram for the live window transition source surfacesequenceDiagram
participant A as SourceClient
participant R as TransitionRect
participant S as SourceSurface
participant C as Compositor
participant B as TargetWindow
A->>R: set_geometry(x, y, width, height)
A->>R: set_source_surface(S)
A->>S: Commit live content
A->>C: Pass activation token to B
C->>B: Start open transition
loop While transition is playing
C->>S: Render latest surface content
C->>B: Animate between source rect and window
end
B->>C: Close window
C->>S: Render latest surface content
C->>B: Animate close transition to source rect
C-->>R: closed
A->>R: destroy()
State diagram for the transition rectangle lifecyclestateDiagram-v2
[*] --> Configured: get_window_transition_rect
Configured --> LiveSource: set_source_surface(surface)
Configured --> Configured: set_geometry(...)
LiveSource --> LiveSource: Surface content committed
LiveSource --> Configured: set_source_surface(null)
LiveSource --> Configured: set_source_surface(other_surface)
Configured --> Transitioning: Target window starts transition
LiveSource --> Transitioning: Target window starts transition
Transitioning --> Transitioning: Render live source surface
Transitioning --> Closed: Compositor done
Closed --> [*]: destroy()
LiveSource --> Configured: Source surface destroyed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
38ea67e to
8dbc3d6
Compare
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="public/treeland-window-transition-unstable-v1.xml" line_range="169-190" />
<code_context>
+ <request name="set_source_surface">
</code_context>
<issue_to_address>
**issue (bug_risk):** This changes the wire contract of the existing v1 interface by removing `set_source_buffer` and changing its argument from `wl_buffer` to `wl_surface`, while retaining the same protocol and interface version. Clients using the published v1 XML send the old request and buffer object, which is incompatible with the new compositor-side request and can result in an invalid-object protocol error.
**Triggers:** When a client built against the previous v1 protocol communicates with a compositor built from this XML.
**Suggested fix:** Publish this incompatible contract as a new major protocol file/interface and retain the old v1 definition, or explicitly coordinate and document a breaking v1 replacement if this repository intentionally permits it.
</issue_to_address>
### Comment 2
<location path="public/treeland-window-transition-unstable-v1.xml" line_range="181-183" />
<code_context>
+ displayed anywhere else, and the client must not use it for any
+ other purpose.
+
+ The client must keep the surface alive, and keep valid content
+ on it, until the closed event is delivered, or until it clears
+ or replaces the source.
+
+ Passing null clears the source surface. The client may replace
</code_context>
<issue_to_address>
**issue (bug_risk):** The source-surface lifetime rule requires the client to keep the surface alive until `closed`, but destroying the rectangle before the target closes explicitly produces no `closed` event. The client therefore has no protocol notification that the compositor has released the source surface in this path and must either retain it indefinitely or risk destroying it while it is still referenced.
**Triggers:** When the client destroys the rectangle, disconnects, or its originating surface is destroyed before the target window closes.
**Suggested fix:** State that destroying the rectangle, disconnecting, or destroying the originating surface immediately releases the source surface, and define that the client may destroy the source surface after that operation; alternatively provide a release event/acknowledgement.
</issue_to_address>
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
📝 变更目的本次 PR 的目的是将窗口转场协议中的源数据从一次性
🔍 详细分析1. 语法逻辑 ✓评价: 语法正确,逻辑清晰 ✓ 分析内容: 修改文件
潜在问题: 建议: 无需修改 2. 代码质量 ✓评价: 代码结构清晰,注释完整 ✓ 分析内容:
潜在问题: 建议: 无需修改 3. 代码性能 ✓评价: 性能良好,资源使用合理 ✓ 分析内容: 本次变更从一次性 buffer 采样改为实时 surface 渲染,实际上是性能改进:
潜在问题: 建议: 无需修改 4. 代码安全 ✓评价: 存在0个安全漏洞 ✓ 分析内容: 本次变更为 Wayland 协议定义文件修改,不涉及可执行代码,不存在安全漏洞。协议层面安全性良好:
漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个 安全漏洞详情: 建议: 无需修改 📋 审查总结
审查结论: 代码审查通过。本次 PR 将 Wayland 窗口转场协议从一次性 buffer 采样改为实时 surface 渲染,变更合理且符合 commit message 所述目的。协议定义语法正确、逻辑清晰,文档注释详尽完善,中英文 README 同步更新一致。未发现安全漏洞、语法错误、逻辑缺陷或性能问题。 本报告由 AI 代码审查工具自动生成 |
8dbc3d6 to
1c7475b
Compare
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="public/treeland-window-transition-unstable-v1.xml" line_range="176-179" />
<code_context>
+ <request name="set_source_surface">
</code_context>
<issue_to_address>
**issue (bug_risk):** The request requires the source `wl_surface` to have no existing role, but does not define what happens when the surface has an existing role or how that condition maps to the new `invalid_surface` error. Clients cannot determine whether the request raises a protocol error, is ignored, or leaves the previous source active.
**Triggers:** When a client passes a role-assigned `wl_surface`.
**Suggested fix:** Explicitly state that this condition raises `invalid_surface` and define whether the previous source remains active or is cleared.
```suggestion
The surface must not already have a role. If it does, this
request raises the invalid_surface error, and any previously
set source surface remains unchanged. The compositor reserves
it for use as a transition source only: it is not displayed
anywhere else, and the client must not use it for any other
purpose.
```
</issue_to_address>1. Replace set_source_buffer with set_source_surface taking a wl_surface 2. Render the source surface live instead of sampling a one-off buffer 3. Rename the invalid_buffer error to invalid_surface 4. Clarify the closed event and the source surface lifetime contract 1. 将 set_source_buffer 替换为接收 wl_surface 的 set_source_surface 2. 源 surface 在转场中实时渲染,不再一次性采样 buffer 3. 将 invalid_buffer 错误更名为 invalid_surface 4. 澄清 closed 事件语义与源 surface 生命周期约定 PMS: TASK-395857 Log: 转场源改用实时 surface,替代一次性 buffer Influence: 1. 用 wayland-scanner 重建协议头文件并确认无警告 2. 将调用 set_source_buffer 的合成器/客户端改为 set_source_surface 3. 验证源 surface 在打开/关闭转场中实时渲染且不重复显示
1c7475b to
64e8db4
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: glyvut, zccrs 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 |
Replace set_source_buffer with set_source_surface taking a wl_surface
Render the source surface live instead of sampling a one-off buffer
Rename the invalid_buffer error to invalid_surface
Clarify the closed event and the source surface lifetime contract
将 set_source_buffer 替换为接收 wl_surface 的 set_source_surface
源 surface 在转场中实时渲染,不再一次性采样 buffer
将 invalid_buffer 错误更名为 invalid_surface
澄清 closed 事件语义与源 surface 生命周期约定
Log: 转场源改用实时 surface,替代一次性 buffer
Influence:
Summary by Sourcery
Replace one-off transition source buffers with live source surfaces and define their lifecycle and closure behavior.
New Features:
Enhancements:
Documentation: