Repository navigation
Conversation
Reviewer's GuideImplements activation-token-based window transitions end to end: clients attach persistent source rectangles and optional images, the compositor associates them with activated windows, and QML-rendered open/close animations use live source geometry with lifecycle-safe fallback and cleanup. Sequence diagram for activation-token window transitionsequenceDiagram
participant Client
participant ActivationManager
participant TransitionManager
participant SurfaceWrapper
participant WindowTransition
Client->>TransitionManager: get_window_transition_rect(token)
Client->>TransitionManager: set_geometry(x,y,width,height)
Client->>TransitionManager: set_source_buffer(buffer)
Client->>ActivationManager: commit()
ActivationManager->>TransitionManager: takeCommittedRect(token, tokenResource)
Client->>ActivationManager: activate(token, targetSurface)
ActivationManager->>SurfaceWrapper: activateRequested(token, targetSurface, originatingSurface)
TransitionManager->>SurfaceWrapper: associatePendingRect(token, target, origin)
SurfaceWrapper->>SurfaceWrapper: setPendingActivation(seat)
SurfaceWrapper->>WindowTransition: createWindowTransition(fromRect, toRect, sourceBuffer)
WindowTransition-->>SurfaceWrapper: finished()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
c610a59 to
033c530
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: glyvut 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 |
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="src/modules/activation/activationmanagerinterfacev1.cpp" line_range="231-251" />
<code_context>
auto it = std::find_if(m_tokens.begin(), m_tokens.end(),
</code_context>
<issue_to_address>
**issue (bug_risk):** Expired activation tokens are accepted because the new single-pass lookup computes disposition without checking `it->expiry.hasExpired()`. A token can therefore still activate a window after the intended 60-second lifetime.
**Triggers:** When a client waits until the activation token has expired before calling activate.
**Suggested fix:** Reject the token and use `Invalid` disposition when `it->expiry.hasExpired()` is true.
</issue_to_address>
### Comment 2
<location path="examples/test-window-transition/CMakeLists.txt" line_range="21-28" />
<code_context>
+ Qt6::Gui
+ Qt6::Widgets
+ Qt6::WaylandClient
+ Qt6::GuiPrivate
+ Qt6::WaylandClientPrivate
+)
+
+install(TARGETS ${BIN_NAME} RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
</code_context>
<issue_to_address>
**issue (bug_risk):** The example links `Qt6::GuiPrivate` and `Qt6::WaylandClientPrivate` unconditionally, although those components are only found when Qt is at least 6.10. With an older supported Qt version, CMake cannot resolve these imported targets and configuration fails.
**Triggers:** When building the examples with Qt older than 6.10.
**Suggested fix:** Only link the private Qt targets in the same Qt-version conditional, or require Qt 6.10 for this example.
```suggestion
target_link_libraries(${BIN_NAME}
PRIVATE
Qt6::Gui
Qt6::Widgets
Qt6::WaylandClient
)
if(Qt6_VERSION VERSION_GREATER_EQUAL 6.10)
target_link_libraries(${BIN_NAME}
PRIVATE
Qt6::GuiPrivate
Qt6::WaylandClientPrivate
)
endif()
```
</issue_to_address>59102fe to
e0fb127
Compare
Remove unnecessary #include <QtCore> and fix QML opacity binding to prevent first-frame flicker on close animations. 移除不必要的 #include <QtCore>,修复 QML opacity 绑定以防止 关闭动画首帧闪烁。 Log: 应用代码审查修复,移除宽泛头文件并修复 opacity 绑定 Issue: Fixes linuxdeepin#1394 Influence: 移除 surfacewrapper.cpp 中过于宽泛的 QtCore 头文件引用, WindowTransition.qml 中 BufferItem 的 opacity 改为直接绑定, 避免关闭动画时首帧闪烁。
0fc7fdb to
32310a5
Compare
32310a5 to
5da0db1
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved protocol-state, activation, rendering geometry, cleanup, and test-coverage issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Implements activation-token-based window open/close transitions with persistent source rectangles, optional images, QML animations, and a sample client.
Changes:
- Adds the server-side window-transition protocol and activation integration.
- Adds transition lifecycle, geometry, buffer handling, and animation support.
- Adds logging, build integration, and a demonstration application.
File summaries
| File | Summary |
|---|---|
src/surface/surfacewrapper.h |
Transition and pending-activation state. |
src/surface/surfacewrapper.cpp |
Transition lifecycle and animation handling. |
src/seat/helper.h |
Stores the transition manager. |
src/seat/helper.cpp |
Registers the protocol and integrates activation. |
src/modules/window-transition/windowtransitionmanagerinterfacev1.h |
Protocol manager API. |
src/modules/window-transition/windowtransitionmanagerinterfacev1.cpp |
Protocol resources, buffers, and associations. |
src/modules/window-transition/CMakeLists.txt |
Protocol generation and module build. |
src/modules/CMakeLists.txt |
Registers the module. |
src/modules/activation/activationmanagerinterfacev1.h |
Extended activation API. |
src/modules/activation/activationmanagerinterfacev1.cpp |
Token and originating-surface handling. |
src/core/qmlengine.h |
Declares the transition component factory. |
src/core/qmlengine.cpp |
Registers and creates the animation. |
src/core/qml/Animations/WindowTransition.qml |
Implements the transition animation. |
src/common/treelandlogging.h |
Declares transition logging. |
src/common/treelandlogging.cpp |
Defines transition logging. |
src/CMakeLists.txt |
Integrates QML resources. |
examples/test-window-transition/main.cpp |
Demonstrates protocol usage. |
examples/test-window-transition/CMakeLists.txt |
Builds the sample client. |
examples/CMakeLists.txt |
Registers the example. |
Review details
Suppressed comments (5)
src/modules/window-transition/windowtransitionmanagerinterfacev1.cpp:184
- EN: The manager accepts multiple rectangle resources for one token, and each
set_geometry()overwrites the token'sm_committedRectsentry. If an earlier rectangle is then destroyed, this unconditional removal erases the newer rectangle's entry as well, so the commit consumer can no longer find it. Remove the map entry only when it still points tothis(or reject duplicate rectangles). 中文:当前 manager 允许同一个 token 创建多个矩形,而每次set_geometry()都会覆盖m_committedRects中的条目;如果旧矩形随后销毁,这里的无条件删除会连新矩形的条目一起删除,导致 commit 时无法找到新矩形。只有当 map 当前仍指向this时才应删除,或直接拒绝重复矩形。
if (m_manager && m_geometrySet && !m_consumed)
m_manager->m_committedRects.remove(m_tokenResource);
}
src/surface/surfacewrapper.cpp:1696
- The close-transition path has the same stacking omission as the open path: it bypasses the common
restackWindowAnimationAbove()call, so the newly-created close item is appended above unrelated windows. This makes a closing window's animation cover other windows while it shrinks. Restack the item immediately after creation.\n\n关闭转场路径存在与打开路径相同的堆叠遗漏:它绕过了公共的restackWindowAnimationAbove()调用,新建的关闭动画项会被追加到无关窗口之上。这样窗口缩小时可能覆盖其他窗口;请在创建后立即调整堆叠顺序。
m_windowAnimation->setProperty("enableBlur", m_blur);
src/surface/surfacewrapper.cpp:1629
- When
readyChangedis used to deferfinishWindowTransitionOpen(), an unmap can happen first. The subsequentonMappedChanged(false)callscreateNewOrClose(CLOSE_ANIMATION), but the new guard returns whilem_windowAnimation/m_windowTransitionPendingare still set; if the surface never becomes ready, those fields remain stuck and later mappings cannot start another transition. Cancel and release the deferred open animation on unmap before entering close handling.\n\n通过readyChanged延迟finishWindowTransitionOpen()时,窗口可能先发生取消映射。随后onMappedChanged(false)调用createNewOrClose(CLOSE_ANIMATION),但新增的保护条件会因为m_windowAnimation/m_windowTransitionPending仍被设置而直接返回;如果该 surface 之后不再 ready,这些状态会永久卡住,后续映射也无法启动转场。请在进入关闭处理前取消并释放延迟的打开动画。
if (!m_surfaceItem->isReady()) {
connect(m_surfaceItem,
&WSurfaceItem::readyChanged,
this,
&SurfaceWrapper::startWindowTransition,
Qt::SingleShotConnection);
src/surface/surfacewrapper.cpp:2830
- EN: This calculation is not global:
originWrapper->position()is relative to the origin wrapper's parent, while the transition item is created under the target'scontainer(). If the source and target are in different containers or under a transformed parent, the open/close rectangle is offset and the transition starts or ends at the wrong place. Map the source item point into the transition item's parent (or use a global point and map it back) before constructing this rectangle.
中文:这里的计算并不是全局坐标:originWrapper->position()是相对于源窗口父项的坐标,而转场项被创建在目标窗口的container()下。当源窗口和目标窗口位于不同容器或父项带有变换时,矩形会产生偏移,导致开关窗口转场从错误位置开始或结束。构造矩形前应将源项坐标映射到转场项父项(或先取全局坐标再映射回来)。
const QPointF mappedTopLeft = surfItem->mapFromSurface(m_windowTransitionLocalRect.topLeft());
return QRectF(m_windowTransitionOriginWrapper->position() + mappedTopLeft,
m_windowTransitionLocalRect.size());
src/surface/surfacewrapper.cpp:213
- EN: If the target is destroyed before
readyChangedfires, this cleanup resets the pending flag and disconnects the surface-item listener, but leaves them_windowAnimationitem created instartWindowTransition()alive.destroy()sees that pointer and defers deleting the wrapper, yet nofinished()signal is connected or started for this pending item, so the wrapper can remain indefinitely. Cancel/delete the pending QML animation and release its buffer during invalidation, or route this case through a cleanup path that also completes wrapper deletion.
中文:如果目标窗口在readyChanged发出前被销毁,这里的清理会重置待处理标志并断开 surface item 监听,但不会释放startWindowTransition()创建的m_windowAnimation。destroy()看到该指针后会延迟删除 wrapper,但这个尚未启动的项既没有连接也不会发出finished(),因此 wrapper 可能永久残留。应在失效处理时取消/删除待处理的 QML 动画并释放 buffer,或让该路径完成 wrapper 的删除清理。
m_windowTransitionPending = false;
m_windowTransitionLocalRect = QRectF();
m_windowTransitionOriginWrapper = nullptr;
m_hasWindowTransitionRect = false;
m_pendingActivation = false;
- Files reviewed: 19/19 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
5da0db1 to
531bfa3
Compare
299bbf7 to
396f7f9
Compare
| m_targetWrapper->setWindowTransitionRect(m_rect, originWrapper); | ||
| m_targetWrapper->setWindowTransitionSourceImage(m_sourceImage); | ||
|
|
||
| m_targetInvalidatedConnection = |
396f7f9 to
3fa090a
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical compatibility, resource-validation, and transition-lifecycle issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (3)
Resolved since last review (3)
| wlr_surface *surface = wlr_surface_from_resource(surfaceResource); | ||
| if (!surface | ||
| || wl_resource_get_client(surfaceResource) != wl_resource_get_client(resource->handle)) { | ||
| wl_resource_post_error(resource->handle, | ||
| error_invalid_surface, | ||
| "set_source_surface: surface is not a wl_surface of this client"); | ||
| return; |
40db361 to
f2b4c3e
Compare
Add the treeland-window-transition-unstable-v1 server protocol and its manager, associate client rects with xdg-activation tokens, and drive surface open/close transitions via WindowTransition.qml. Support activation requests arriving before the target surface is mapped. 新增 treeland-window-transition-unstable-v1 服务端协议及管理器, 将客户端矩形与 xdg-activation 令牌关联,通过 WindowTransition.qml 驱动窗口开关过渡动画,并支持目标表面映射前到达的激活请求。 Log: 添加窗口过渡协议v1及窗口开关动画支持 PMS: TASK-395857 Influence: 新增窗口过渡协议全局对象;SurfaceWrapper 增加过渡与待激活状态,激活与窗口开关动画行为有变化。
Add test-window-transition example using xdg-activation-v1 and the treeland-window-transition-unstable-v1 client protocol to verify open and close transition animations. 新增 test-window-transition 示例,使用 xdg-activation-v1 与 treeland-window-transition-unstable-v1 客户端协议验证窗口开关过渡动画。 Log: 添加窗口过渡测试示例 PMS: TASK-395857 Influence: 仅新增示例程序,不影响合成器行为。
f2b4c3e to
03385f9
Compare


Add the server-side implementation of treeland-window-transition-unstable-v1, which plays a window open/close transition relative to a source rectangle attached to an xdg-activation token.
A client attaches a persistent transition rectangle (geometry plus an optional source image) to an xdg_activation_token_v1 before committing the token. At activation the compositor associates the rectangle with the target window, animating from the rectangle's global position on open and back to it on close. The rectangle stays alive, so set_geometry and set_source_buffer update it immediately.
新增 treeland-window-transition-unstable-v1 的服务端实现,基于关联到 xdg-activation token 的源矩形播放窗口打开/关闭转场。
客户端在提交 token 前挂载一个持久的转场矩形(几何信息及可选源图像)。
激活时合成器将矩形关联到目标窗口,打开时从矩形的全局位置播放动画,
关闭时过渡回该矩形。矩形持续存活,set_geometry / set_source_buffer
可立即更新。
Log: 实现窗口转场协议,基于激活 token 的源矩形播放开/关转场
Influence: 新增窗口转场模块、QML 动画组件及 test-window-transition 样例; 激活流程支持矩形关联并播放开/关动画。
Summary by Sourcery
Implement activation-token-based window open and close transitions using persistent source rectangles and optional source images.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests: