fix(surface): make minimized an orthogonal flag separate - #1396
Conversation
Reviewer's GuideSeparates minimization from the surface-state enum, preserving normal/maximized/fullscreen/tiling state while independently managing minimize transitions, proxy visibility, animations, protocol metadata, and diagnostics. Sequence diagram for minimizing and restoring a surfacesequenceDiagram
participant Caller
participant SurfaceWrapper
participant Parent as ParentSurface
participant ShellSurface
participant Child as SubSurface
participant WorkspaceProxy
Caller->>SurfaceWrapper: minimize(onAnimation)
SurfaceWrapper->>SurfaceWrapper: isMinimized()
SurfaceWrapper->>SurfaceWrapper: minimizedChanged()
opt modal parent is not minimized
SurfaceWrapper->>Parent: minimize(false)
end
SurfaceWrapper->>ShellSurface: setMinimize(true)
SurfaceWrapper->>WorkspaceProxy: minimized becomes true
loop non-modal sub-surfaces
SurfaceWrapper->>Child: minimize(false)
end
Caller->>SurfaceWrapper: restoreFromMinimized(onAnimation)
SurfaceWrapper->>SurfaceWrapper: minimizedChanged()
opt modal parent is minimized
SurfaceWrapper->>Parent: restoreFromMinimized(false)
end
SurfaceWrapper->>ShellSurface: setMinimize(false)
SurfaceWrapper->>WorkspaceProxy: minimized becomes false
loop minimized sub-surfaces
SurfaceWrapper->>Child: restoreFromMinimized(false)
end
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/surface/surfacewrapper.cpp" line_range="85" />
<code_context>
, m_engine(qmlEngine)
, m_shellSurface(shellSurface)
, m_type(type)
+ , m_minimized(false)
, m_positionAutomatic(true)
, m_visibleDecoration(true)
</code_context>
<issue_to_address>
**issue (broader_impact):** Workspace proxy wrappers constructed from an existing SurfaceWrapper always initialize their independent `m_minimized` bit to false and do not copy or synchronize the original wrapper's minimized state. Consequently, a minimized window can remain active and rendered in workspace proxies because the new QML checks the proxy's `surface.minimized` property.
**Triggers:** When a minimized window has a workspace proxy created or refreshed.
**Suggested fix:** Initialize proxy minimized state from `original->isMinimized()` and propagate subsequent `minimizedChanged` updates from the original wrapper, or bind the proxy's minimized property directly to the source wrapper.
</issue_to_address>There was a problem hiding this comment.
🟡 Changes recommended
Unresolved findings affect minimized-state updates, public API consistency, event ordering, and schema documentation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Separates minimized status from the underlying surface state and exposes it across QML, remote metadata, and debugging tools.
Changes:
- Adds independent minimized-state handling.
- Updates workspace proxies and minimize animations.
- Extends window-tree and debug output metadata.
File summaries
| File | Description |
|---|---|
tools/treeland-debug/main.cpp |
Displays and watches minimized status. |
tools/treeland-debug/debugsession.cpp |
Adds minimized status to JSON output. |
src/surface/surfacewrapper.h |
Declares the minimized property and signal. |
src/surface/surfacewrapper.cpp |
Implements independent minimize/restore behavior. |
src/modules/resource/treelandwindowtree.rep |
Extends WindowInfo with minimized metadata. |
src/modules/resource/treelandremotesource.cpp |
Publishes minimized status remotely. |
src/core/qml/WorkspaceProxy.qml |
Hides minimized surfaces. |
src/core/qml/Animations/MinimizeAnimation.qml |
Uses the underlying surface state for shadows. |
Review details
Suppressed comments (4)
src/modules/resource/treelandwindowtree.rep:21
- English: This adds
minimizedto the publicWindowInfoschema, but the existing English and Chinese debug documentation still describesstate == 2as Minimized and does not document the new field. That makes the documented API contradict the values now emitted bybuildWindowInfo()and can cause clients to keep reading the old state encoding. Update both WindowInfo tables to describeminimizedand remove Minimized from the surface-state values.
中文:这里向公开的WindowInfoschema 添加了minimized,但现有中英文调试文档仍将state == 2描述为 Minimized,且没有记录新字段。这使文档 API 与buildWindowInfo()现在输出的值不一致,客户端可能继续按旧编码解析。请同时更新中英文WindowInfo表,说明minimized并从 surface state 值中移除 Minimized。
bool minimized,
src/surface/surfacewrapper.cpp:1884
setMinimize(true)emitsWToplevelSurface::minimizeChangedsynchronously, and foreign-toplevel listeners publish that state immediately. This call currently precedes clearing the wrapper's focus and active capabilities, so clients can observe a minimized window as still activated before the later inactivation event; the previousdoSetSurfaceStateordering cleared those capabilities first. Move the two capability updates beforesetMinimize(true).
setMinimize(true) 会同步发出 WToplevelSurface::minimizeChanged,foreign-toplevel 监听器会立即发布该状态。当前调用发生在清除 wrapper 的焦点和激活能力之前,因此客户端可能先观察到“已最小化但仍激活”的窗口,之后才收到失活事件;原来的 doSetSurfaceState 顺序是先清除能力。请将两个能力更新移到 setMinimize(true) 之前。
m_shellSurface->setMinimize(true);
updateFocusControlState(FocusControlState::UnMinimized, false);
updateHasActiveCapability(ActiveControlState::UnMinimized, false);
src/surface/surfacewrapper.cpp:1596
State::Minimizedis still a public enum value andsetSurfaceState/setSurfaceStateDirectlyremain public, but the removedMinimizedcases mean this path now changes onlym_surfaceState; it never setsm_minimizedor hides the item. An existing caller that passesState::Minimizedcan therefore producesurfaceState() == MinimizedwhileisMinimized() == false, andrestoreFromMinimized()will not repair it. Remove/deprecate that state from this API or translate it through the dedicated minimize/restore methods.
State::Minimized 仍是公开枚举值,setSurfaceState/setSurfaceStateDirectly 也仍是公开接口,但移除 Minimized 分支后,这条路径只会修改 m_surfaceState,不会设置 m_minimized 或隐藏 item。现有调用者传入 State::Minimized 后可能得到 surfaceState() == Minimized 但 isMinimized() == false 的不一致状态,restoreFromMinimized() 也无法修复。请从该 API 中移除/弃用该状态,或将其转换为专用的最小化/恢复方法。
setVisibleDecoration(newSurfaceState == State::Normal);
setNoCornerRadius(newSurfaceState == State::Maximized || newSurfaceState == State::Fullscreen
|| newSurfaceState == State::Tiling);
src/surface/surfacewrapper.h:358
MultitaskviewSurfaceModelstill connectshandleSurfaceStateChanged()only tosurfaceStateChanged(src/plugins/multitaskview/multitaskview.cpp:161-165and561-565), and that handler is where itsMinimizedRoleis refreshed. Since minimizing now changes this new flag without changingsurfaceState,WindowSelectionGridwill retain stale minimized values and fail to hide/show windows after minimize or restore. Connect and disconnect the model usingminimizedChangedas well.
MultitaskviewSurfaceModel 仍然只将 handleSurfaceStateChanged() 连接到 surfaceStateChanged(见 src/plugins/multitaskview/multitaskview.cpp:161-165 和 561-565),而该处理函数负责刷新 MinimizedRole。现在最小化只改变这个新标志而不改变 surfaceState,因此 WindowSelectionGrid 会保留过期的最小化状态,最小化或恢复后无法正确隐藏/显示窗口。请同时使用 minimizedChanged 连接和断开该模型。
void minimizedChanged();
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Four moderate findings and one documentation nit remain unresolved.
Review details
Suppressed comments (5)
src/modules/resource/treelandwindowtree.rep:21
- This adds a public
WindowInfofield and changes the documented state contract, butREADME.mdandREADME.zh_CN.mdstill omitminimizedand still describe state2as Minimized,3as Fullscreen, and4as Tiling. Users following either CLI/JSON schema will miss the new flag and misinterpret the output. Update both API tables with the new field and final numeric mapping. / 这里新增了公开的WindowInfo字段并改变了 state contract,但README.md和README.zh_CN.md仍未记录minimized,且仍把 state2写成 Minimized、3写成 Fullscreen、4写成 Tiling。使用 CLI/JSON schema 的用户会遗漏新标志并误读输出,请同步更新两份 API 表格。
bool minimized,
src/plugins/multitaskview/multitaskview.cpp:164
- The new orthogonal state flow now allows a minimized window to enter fullscreen, which changes its wrapper geometry while it remains hidden. This connection no longer observes
surfaceStateChanged, andhandleMinimizedChanged()only updatesMinimizedRole; ready surfaces have also disconnectedgeometryChanged, so the multitask model keeps the oldGeometryRole/layout and shows stale dimensions after the window is restored. Keep a state/geometry update path for state changes while minimized (and emit the affected geometry/layout roles). / 现在允许最小化窗口进入全屏,窗口隐藏期间其 geometry 仍会改变;但这里不再监听surfaceStateChanged,handleMinimizedChanged()只更新MinimizedRole,而 ready surface 又断开了geometryChanged,因此 multitask model 会保留旧的GeometryRole/布局,恢复后显示过期尺寸。请保留最小化期间的 state/geometry 更新路径,并发出相应的 geometry/layout roles。
&SurfaceWrapper::minimizedChanged,
this,
&MultitaskviewSurfaceModel::handleMinimizedChanged,
src/surface/surfacewrapper.cpp:1974
- With minimization now stored independently from
m_surfaceState, this guard still makesmaximize()a no-op for every minimized Normal window. A window minimized in Normal therefore cannot transition to Maximized, while a minimized Maximized window can be unmaximized and fullscreen/tiling transitions are otherwise allowed; that makes the new orthogonal state asymmetric and causes remote/QML maximize requests to be silently ignored. Remove them_minimizedcondition (or apply a consistent policy to all surface-state transitions). / 由于最小化现在与m_surfaceState独立保存,这个判断仍会让所有处于 Normal 状态的最小化窗口无法执行maximize()。因此最小化的 Normal 窗口不能转为 Maximized,而最小化的 Maximized 窗口却可以取消最大化,且全屏/平铺转换没有同样限制;这使新的正交状态不对称,并会静默忽略远程或 QML 的最大化请求。请移除m_minimized条件,或为所有 surface-state 转换采用一致策略。
if (m_minimized || m_surfaceState == State::Fullscreen
|| !isMaximizable())
src/surface/surfacewrapper.cpp:1917
m_shellSurface->setMinimize(true)emitsWToplevelSurface::minimizeChangedsynchronously, and the foreign-toplevel implementation forwards that state immediately. Because the focus/activation capability flags are cleared only afterward, clients can observe a transient minimized-but-activated window and focus fallback runs too late; the previousdoSetSurfaceStateorder cleared the flags first. Move the two capability updates beforesetMinimize(true). /m_shellSurface->setMinimize(true)会同步发出minimizeChanged,foreign-toplevel 会立即转发该状态;当前先发送最小化再清除焦点/激活能力,客户端可能观察到短暂的“已最小化但仍激活”窗口,焦点回退也会延后。之前的顺序是先清除能力再设置最小化,请将两个能力更新移到setMinimize(true)之前。
m_shellSurface->setMinimize(true);
updateFocusControlState(FocusControlState::UnMinimized, false);
updateHasActiveCapability(ActiveControlState::UnMinimized, false);
tools/treeland-debug/debughelpers.cpp:17
WindowInfo.stateis exposed as a serialized integer in the Remote Objects/JSON API, but removing the middle enum member renumbers existing values: oldFullscreen=3andTiling=4now become2and3. Existing JSON consumers will mislabel those states, especially since the documented schema has no versioning. Preserve the existing numeric assignments (for example, keep value 2 reserved and assign explicit values) or version/migrate this API. /WindowInfo.state以整数形式暴露在 Remote Objects/JSON API 中,但删除中间枚举成员会重新编号:旧的Fullscreen=3和Tiling=4现在变成2和3。现有 JSON 客户端会误判这些状态,且文档中的 schema 没有版本号。请保留原有数值(例如保留值 2 并显式指定枚举值),或为该 API 做版本迁移。
case 2: return QStringLiteral("Fullscreen");
case 3: return QStringLiteral("Tiling");
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical API compatibility and moderate minimization-transition and regression-test issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
src/modules/resource/treelandwindowtree.rep:21
- English: The new
WindowInfo.minimizedfield and the changed state numbering are not reflected inREADME.mdorREADME.zh_CN.md; both still document state 2 as Minimized and omit this field. That makes the documented JSON contract incorrect for every debug client, so update both tables with this schema change. 中文:新增的WindowInfo.minimized字段和状态编号变化没有同步到README.md与README.zh_CN.md;两者仍将状态 2 描述为 Minimized,且遗漏了该字段。这会使所有调试客户端依据文档解析错误,请随本次 schema 变更更新两份表格。
bool minimized,
- Files reviewed: 12/12 changed files
- Comments generated: 3
- Review effort level: Lite
… state - Add independent `m_minimized` bitfield to SurfaceWrapper, decoupling minimized state from `m_surfaceState` so a window can be both minimized and fullscreen/maximized/tiling simultaneously. - `minimize()` / `restoreFromMinimized()` no longer go through `doSetSurfaceState()`; they only toggle `m_minimized` and call `shellSurface->setMinimize()` with focus/active-control updates. - `doSetSurfaceState()` removes the Minimized branch; `m_surfaceState` now only represents layout state (Normal/Maximized/Fullscreen/Tiling). - `enterFullscreen()` / `leaveFullscreen()` skip animation when the window is not visible (`!isVisible()`), using `setSurfaceStateDirectly()` instead. - QML: `WorkspaceProxy.qml` uses `!surface.minimized` instead of `surfaceState !== State.Minimized`; `MinimizeAnimation.qml` unifies `showShadow` on `surfaceState === State.Normal`. Log: 修复全屏→最小化→恢复→取消全屏后窗口卡在最小化状态 PMS: BUG-376727 Influence: SurfaceWrapper 全屏与最小化状态恢复逻辑,不影响其他模块 Signed-off-by: glyvut <guolin@uniontech.com>
- Add `bool minimized` field to `WindowInfo` POD in `treelandwindowtree.rep` and populate it in `treelandremotesource.cpp`. - `treeland-debug`: show minimized column in table/tree/top output, include it in JSON serialization, and report minimize/unminimize transitions in watch mode. Log: treeland-debug 补上 minimized 状态显示 PMS: BUG-376727 Influence: treeland-debug 调试工具及 WindowInfo 序列化,不影响窗口管理逻辑 Signed-off-by: glyvut <guolin@uniontech.com>
- Extend treeland-foreign-toplevel-manager-v2 tests with minimize_keeps_maximized/fullscreen_state cases: minimizing under a layout state keeps surfaceState and hides the window, restoring keeps the layout state, and removing the layout returns to Normal. - Expose wrapper_self_minimized/wrapper_state/wrapper_visible in the server-state fixture and refresh INDEX/README coverage notes. Log: 补充最小化与布局正交的协议测试 PMS: BUG-376727 Influence: 仅新增协议测试与文档,不影响 compositor 运行时行为 Signed-off-by: guolin <guolin@uniontech.com>
|
[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 |
Summary by Sourcery
Make minimization an independent window flag so layout states remain intact while windows are minimized and restored.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: