fix(remote-subsurface): reattach child subsurface when ancestor is re… - #1375
Conversation
|
Skipping CI for Draft Pull Request. |
Reviewer's GuideThe fix makes remote subsurface contexts resilient to ancestor WSurface wrapper destruction by retaining positions, recreating missing child WSubsurface objects during mapping checks, and restoring their position and z-order so nested subsurfaces become visible again. Sequence diagram for restoring nested remote subsurfacessequenceDiagram
participant Parent as Ancestor WSurface
participant Context as RemoteSubsurfaceContext
participant Manager as WRemoteSubsurfaceManagerV1Private
participant Child as WSubsurface
Parent->>Context: recheckMapping()
Context->>Context: ensureSubsurface()
alt WSubsurface missing
Context->>Manager: createRemoteSubsurface(parent, child)
Manager-->>Context: WSubsurface
Context->>Child: setPosition(position())
Context->>Manager: syncRemoteSubsurfaceOrder(parent)
end
Context->>Child: recheckMapping()
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
5673905 to
6e99cbc
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="waylib/src/server/protocols/wremotesubsurfacemanagerv1.cpp" line_range="651-653" />
<code_context>
auto *parentWs = wsurfaceFrom(m_parent);
parentMapped = parentWs && parentWs->mapped();
+ // ensureSubsurface re-creates this subsurface when the parent WSurface
+ // wrapper was destroyed and recreated. Fast no-op when it still exists.
+ ensureSubsurface();
+
bool childHasBuffer = childQw && wlr_surface_has_buffer(childQw);
</code_context>
<issue_to_address>
**issue (bug_risk):** Recreating a non-remote ancestor WSurface does not trigger `recheckMapping()` for its remote children because `trackCommits()` remains connected to the destroyed WSurface wrapper and is never reconnected to the replacement wrapper. Consequently, `ensureSubsurface()` is never called and the child remains detached until some unrelated child or parent event happens.
**Triggers:** When the ancestor is an xdg-toplevel or other non-remote surface whose WSurface wrapper is recreated while the child already has a buffer and no subsequent commit occurs.
**Suggested fix:** Reconnect each affected RemoteSubsurfaceContext to the replacement WSurface's `commit` and `mappedChanged` signals, or explicitly recheck all remote children when a WSurface wrapper is recreated.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
6e99cbc to
5197f3f
Compare
5197f3f to
f5b7318
Compare
f5b7318 to
6fb578c
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The change affects low-level Wayland surface lifecycle/mapping behavior and should be validated with runtime scenarios (涉及底层 Wayland 生命周期与映射逻辑,建议人工结合实际场景验证后再合入).
Pull request overview
EN: This PR fixes visibility regressions for nested remote subsurfaces by reattaching (recreating) missing WSubsurface objects when an ancestor WSurface wrapper gets torn down and rebuilt (e.g., role lifecycle changes), while restoring position and z-order so children become visible again.
中文:本 PR 修复了嵌套远程子曲面在祖先 WSurface 包装器被销毁并重建(例如 role 生命周期变化)后不可见的问题:在检测到子曲面对象缺失时重新创建 WSubsurface,并恢复位置与 z 序以确保重新可见。
Changes:
- EN: Listen to parent
wlr_surfacecommit viaWScopedListenerso remapping logic survivesWSurfacewrapper recreation.
中文:通过WScopedListener监听父wlr_surface的 commit,使重建WSurface包装器后仍能触发重映射检查。 - EN: Add
ensureSubsurface()to recreate missingWSubsurface, restore saved position, and resync remote subsurface ordering.
中文:新增ensureSubsurface():在WSubsurface缺失时重建、恢复缓存的位置,并重新同步远程子曲面排序。
File summaries
| File | Description |
|---|---|
waylib/src/server/protocols/wremotesubsurfacemanagerv1.cpp |
EN: Reattach/remap nested remote subsurfaces across WSurface wrapper recreation using wlr_surface commit listeners and on-demand subsurface recreation. 中文:通过监听 wlr_surface commit + 按需重建子曲面,保证 WSurface 包装器重建后嵌套远程子曲面可重新挂载并恢复可见性。 |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
6fb578c to
ac21156
Compare
…created When a remote subsurface B is destroyed and recreated, its child C loses the parent WSubsurface link and never reappears. The root cause is that QPointer<WSubsurface> m_subsurface is cleared when the ancestor WSurface wrapper is destroyed, but nothing re-creates it afterwards. Fix by adding a m_position cache and an ensureSubsurface() member, and by tracking the parent surface state at the wlr_surface level: - Cache the subsurface position in m_position so it survives the WSubsurface being destroyed along with the ancestor WSurface wrapper. - ensureSubsurface() is called on every recheckMapping(); it is a fast no-op when the subsurface still exists, and re-creates it otherwise, restoring the cached position and z-order. - Replace WSurface::mappedChanged Qt connections (which break on WSurface wrapper recreation) with wlr_surface::events.map / events.unmap WScopedListeners. These live at the wlr_surface level and survive role teardown/rebuild, so no reconnection is ever needed. This mirrors the upstream wlroots scene subsurface tree pattern. 重建祖先 WSurface 包装器时重新挂载嵌套的远程子曲面,恢复其位置和 z 序,使其重新可见。 当中间层远程子曲面 B 被销毁后重建时,其子曲面 C 丢失了父 WSubsurface 链接, 导致 C 无法恢复显示。根本原因是祖先 WSurface 包装器被销毁时,持有 WSubsurface 的 QPointer 被清空,但之后没有任何逻辑重建该链接。 修复方案:新增 m_position 位置缓存和 ensureSubsurface() 成员函数, 并在 wlr_surface 层追踪父表面状态: - 在 m_position 中缓存子曲面位置,使位置在 WSubsurface 随祖先 WSurface 包装器一同销毁后仍然保留。 - 每次 recheckMapping() 调用 ensureSubsurface(),当 subsurface 仍存在时 为快速空操作,否则重建 subsurface 并恢复缓存的位置和 z 序。 - 用 wlr_surface::events.map / events.unmap 的 WScopedListener 替代 WSurface::mappedChanged 的 Qt 连接。后者会随 WSurface 包装器重建而断开, 前者在 wlr_surface 层存活,无需任何重连逻辑。 此方案与上游 wlroots scene subsurface tree 模式完全一致。 PMS: TASK-393795
ac21156 to
f2ffc21
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wineee, 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 |
…created
Reattach nested remote subsurfaces when recreating an ancestor WSurface wrapper, restoring their position and z-order so they become visible again.
重建祖先 WSurface 包装器时重新挂载嵌套的远程子曲面,恢复其位置和 z 序,使其重新可见。
PMS: TASK-393795
Summary by Sourcery
Ensure nested remote subsurfaces are recreated and reattached when their ancestor surface wrapper is recreated.
Bug Fixes:
Enhancements: