feat: make splash screens visible and closable via foreign-toplevel#745
Open
wineee wants to merge 1 commit intolinuxdeepin:masterfrom
Open
feat: make splash screens visible and closable via foreign-toplevel#745wineee wants to merge 1 commit intolinuxdeepin:masterfrom
wineee wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
protocol Added foreign-toplevel protocol integration for splash screens to make them visible and manageable in the dock. Splash screens now appear in the dock with proper identification and can be closed directly from the dock. When a splash screen is closed before the actual application window appears, the matching application window will be automatically closed to maintain consistency. Key changes: 1. Register splash screens with foreign-toplevel protocol to appear in dock 2. Implement splash screen closure handling via dock requests 3. Track closed splash appIds to close matching application windows 4. Move foreign-toplevel management from Helper to ShellHandler for better integration 5. Add proper splash screen identification and state management Log: Splash screens now appear in dock and can be closed via dock interface Influence: 1. Test splash screen visibility in dock for various applications 2. Verify splash screen closure from dock works correctly 3. Test that closing splash screen prevents matching application window from opening 4. Verify normal window behavior remains unchanged after splash integration 5. Check that skipDockPreView property still works correctly for all surface types 6. Test application launch with and without splash screen closure feat: 使闪屏通过foreign-toplevel协议在dock栏可见可关闭 为闪屏添加foreign-toplevel协议集成，使其在dock栏中可见并可管理。闪屏现在 会出现在dock栏中并带有适当标识，可以直接从dock栏关闭。如果在实际应用程序 窗口出现之前关闭闪屏，匹配的应用程序窗口将自动关闭以保持一致性。 主要更改： 1. 将闪屏注册到foreign-toplevel协议以在dock栏显示 2. 实现通过dock请求处理闪屏关闭功能 3. 跟踪已关闭闪屏的appId以关闭匹配的应用程序窗口 4. 将foreign-toplevel管理从Helper移动到ShellHandler以实现更好集成 5. 添加适当的闪屏标识和状态管理 Log: 闪屏现在可在dock栏显示并可通过dock界面关闭 Influence: 1. 测试各种应用程序的闪屏在dock栏中的可见性 2. 验证从dock栏关闭闪屏功能正常工作 3. 测试关闭闪屏后是否阻止匹配应用程序窗口打开 4. 验证闪屏集成后正常窗口行为保持不变 5. 检查skipDockPreView属性对所有表面类型是否仍正常工作 6. 测试在有和没有关闭闪屏的情况下应用程序启动行为
|
[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 |
Reviewer's GuideIntegrates splash screens with the Treeland foreign-toplevel protocol by moving Treeland-specific toplevel registration into ShellHandler, adding proper splash identification and close handling, and tracking closed splash appIds so that subsequent real windows are auto-closed, while ensuring skipDockPreView still governs dock visibility for all surface types. Sequence diagram for dock-driven splash close and auto-closing real windowsequenceDiagram
actor User
participant DockUI
participant ForeignToplevelV1
participant ToplevelHandle as treeland_foreign_toplevel_handle_v1
participant ShellHandler
participant SplashWrapper as SurfaceWrapper_splash
participant AppResolver as AppIdResolver
participant RealSurface as WXdgToplevelSurface
User->>DockUI: Click close on splash icon
DockUI->>ToplevelHandle: requestClose
ToplevelHandle->>SplashWrapper: close()
SplashWrapper->>SplashWrapper: emit requestCloseSplash
SplashWrapper-->>ShellHandler: requestCloseSplash()
ShellHandler->>ShellHandler: m_closedSplashAppIds.insert(appId)
ShellHandler->>ShellHandler: m_prelaunchWrappers.removeOne(SplashWrapper)
ShellHandler->>ShellHandler: m_rootSurfaceContainer->destroyForSurface(SplashWrapper)
note over RealSurface,AppResolver: Later, real app window appears
RealSurface-->>ShellHandler: onXdgToplevelSurfaceAdded(RealSurface)
ShellHandler->>AppResolver: resolveAppId(pidfd)
AppResolver-->>ShellHandler: resolved appId
ShellHandler->>ShellHandler: ensureXdgWrapper(RealSurface, appId)
alt appId in m_closedSplashAppIds
ShellHandler->>ShellHandler: m_closedSplashAppIds.remove(appId)
ShellHandler->>RealSurface: close()
else appId not in m_closedSplashAppIds
ShellHandler->>ShellHandler: create normal wrapper
ShellHandler->>ForeignToplevelV1: addSurface(wrapper)
end
Class diagram for updated ShellHandler, SurfaceWrapper and ForeignToplevelV1classDiagram
class ShellHandler {
- RootSurfaceContainer* m_rootSurfaceContainer
- LayerSurfaceContainer* m_backgroundContainer
- LayerSurfaceContainer* m_bottomContainer
- Workspace* m_workspace
- SurfaceContainer* m_normalContainer
- SurfaceContainer* m_popupContainer
- WindowConfigStore* m_windowConfigStore
- QList~SurfaceWrapper*~ m_prelaunchWrappers
- QSet~QString~ m_pendingPrelaunchAppIds
- QSet~QString~ m_closedSplashAppIds
- QList~WToplevelSurface*~ m_pendingAppIdResolveToplevels
- ForeignToplevelV1* m_treelandForeignToplevel
+ ShellHandler(RootSurfaceContainer* rootContainer, WServer* server)
+ Workspace* workspace()
+ void createPrelaunchSplash(QString appId, QString iconName, QString title)
+ void onXdgToplevelSurfaceAdded(WXdgToplevelSurface* surface)
+ void onXWaylandSurfaceAdded(WXWaylandSurface* surface)
- void ensureXdgWrapper(WXdgToplevelSurface* surface, QString targetAppId)
- void ensureXwaylandWrapper(WXWaylandSurface* surface, QString targetAppId)
- void setupSurfaceActiveWatcher(SurfaceWrapper* wrapper)
- void registerSurfaceToForeignToplevel(SurfaceWrapper* wrapper)
}
class ForeignToplevelV1 {
- treeland_foreign_toplevel_manager_v1* m_manager
- map~SurfaceWrapper*, treeland_foreign_toplevel_handle_v1*~ m_surfaces
+ wl_global* global() const
+ void addSurface(SurfaceWrapper* wrapper)
+ void removeSurface(SurfaceWrapper* wrapper)
- void initializeToplevelHandle(SurfaceWrapper* wrapper, treeland_foreign_toplevel_handle_v1* handle)
}
class SurfaceWrapper {
<<QQuickItem>>
+ Type: XdgToplevel, XWayland, SplashScreen, Other
- Type m_type
- bool m_skipDockPreView
- WToplevelSurface* m_shellSurface
- QString m_appId
+ ~SurfaceWrapper()
+ Type type() const
+ QString appId() const
+ bool skipDockPreView() const
+ void setSkipDockPreView(bool skip)
+ void close()
+ void markWrapperToRemoved()
+ void requestCloseSplash()
+ void skipDockPreViewChanged()
+ Q_SIGNAL void aboutToBeInvalidated()
+ Q_SIGNAL void surfaceItemCreated()
+ Q_SIGNAL void requestCloseSplash()
}
class Helper {
- RootSurfaceContainer* m_rootSurfaceContainer
- ShellHandler* m_shellHandler
- WServer* m_server
- WForeignToplevel* m_foreignToplevel
- WExtForeignToplevelListV1* m_extForeignToplevelListV1
+ Helper(QObject* parent)
- void init(Treeland* treeland)
- void onSurfaceWrapperAdded(SurfaceWrapper* wrapper)
- void onSurfaceWrapperAboutToRemove(SurfaceWrapper* wrapper)
}
class DockPreview {
+ void close()
}
ShellHandler --> RootSurfaceContainer : uses
ShellHandler --> ForeignToplevelV1 : owns m_treelandForeignToplevel
ShellHandler --> SurfaceWrapper : manages wrappers
ShellHandler --> WServer : constructed with
ForeignToplevelV1 --> SurfaceWrapper : manages via addSurface/removeSurface
Helper --> ShellHandler : creates and connects
Helper --> WServer : uses
Helper --> DockPreview : controls via dock requests
SurfaceWrapper ..> DockPreview : represented in dock via ForeignToplevelV1
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new
SurfaceWrapper::requestCloseSplashis declared as a slot but used as a signal (e.g.connect(wrapper, &SurfaceWrapper::requestCloseSplash, ...)andQ_EMIT requestCloseSplash()), which will not compile with the new connect syntax; move it to thesignalssection instead ofpublic Q_SLOTS. - In
ForeignToplevelV1::addSurface, theset_identifiernow uses astatic int counterinstead of something tied to the underlying surface/handle; consider whether this needs to be stable/unique across the compositor lifetime and make the identifier generation more robust (and thread‑safe) if clients rely on it. - The
m_closedSplashAppIdsset is only pruned when a matching window appears; if an app never creates a window after its splash is closed, the appId will remain in the set indefinitely—consider adding a timeout or cleanup path to avoid unbounded growth over long sessions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `SurfaceWrapper::requestCloseSplash` is declared as a slot but used as a signal (e.g. `connect(wrapper, &SurfaceWrapper::requestCloseSplash, ...)` and `Q_EMIT requestCloseSplash()`), which will not compile with the new connect syntax; move it to the `signals` section instead of `public Q_SLOTS`.
- In `ForeignToplevelV1::addSurface`, the `set_identifier` now uses a `static int counter` instead of something tied to the underlying surface/handle; consider whether this needs to be stable/unique across the compositor lifetime and make the identifier generation more robust (and thread‑safe) if clients rely on it.
- The `m_closedSplashAppIds` set is only pruned when a matching window appears; if an app never creates a window after its splash is closed, the appId will remain in the set indefinitely—consider adding a timeout or cleanup path to avoid unbounded growth over long sessions.
## Individual Comments
### Comment 1
<location> `src/surface/surfacewrapper.h:330` </location>
<code_context>
void showOnAllWorkspaceChanged();
void requestActive();
void requestInactive();
+ void requestCloseSplash();
void skipSwitcherChanged();
void skipDockPreViewChanged();
</code_context>
<issue_to_address>
**issue (bug_risk):** requestCloseSplash should be declared as a signal, not a slot, to match its usage.
`requestCloseSplash` is currently under `public Q_SLOTS`, but in `surfacewrapper.cpp` you use `Q_EMIT requestCloseSplash();` and in `ShellHandler::createPrelaunchSplash` you connect with `&SurfaceWrapper::requestCloseSplash` as if it were a signal. Because it’s not declared under `signals:`, there will be no meta-object entry, so this pointer-to-member connection will fail at runtime (usually with `QObject::connect: No such signal ...`) and the handler won’t be invoked. Please move `requestCloseSplash` to the `signals:` section (or add one) and keep the implementation the same.
</issue_to_address>
### Comment 2
<location> `src/core/shellhandler.h:137` </location>
<code_context>
WAYLIB_SERVER_NAMESPACE::WLayerShell *m_layerShell = nullptr;
WAYLIB_SERVER_NAMESPACE::WInputMethodHelper *m_inputMethodHelper = nullptr;
QList<WAYLIB_SERVER_NAMESPACE::WXWayland *> m_xwaylands;
+ ForeignToplevelV1 *m_treelandForeignToplevel = nullptr;
QPointer<RootSurfaceContainer> m_rootSurfaceContainer;
</code_context>
<issue_to_address>
**issue (bug_risk):** m_treelandForeignToplevel is used from Helper but declared private, which will not compile.
In `Helper::init` you connect directly to `m_shellHandler->m_treelandForeignToplevel`, but this member is private in `ShellHandler` and `Helper` is not a friend, so this will not compile. Please either add a public accessor (e.g. `ForeignToplevelV1 *treelandForeignToplevel() const`) or route these connections via `ShellHandler` signals instead of accessing its internals directly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| void showOnAllWorkspaceChanged(); | ||
| void requestActive(); | ||
| void requestInactive(); | ||
| void requestCloseSplash(); |
There was a problem hiding this comment.
issue (bug_risk): requestCloseSplash should be declared as a signal, not a slot, to match its usage.
requestCloseSplash is currently under public Q_SLOTS, but in surfacewrapper.cpp you use Q_EMIT requestCloseSplash(); and in ShellHandler::createPrelaunchSplash you connect with &SurfaceWrapper::requestCloseSplash as if it were a signal. Because it’s not declared under signals:, there will be no meta-object entry, so this pointer-to-member connection will fail at runtime (usually with QObject::connect: No such signal ...) and the handler won’t be invoked. Please move requestCloseSplash to the signals: section (or add one) and keep the implementation the same.
| WAYLIB_SERVER_NAMESPACE::WLayerShell *m_layerShell = nullptr; | ||
| WAYLIB_SERVER_NAMESPACE::WInputMethodHelper *m_inputMethodHelper = nullptr; | ||
| QList<WAYLIB_SERVER_NAMESPACE::WXWayland *> m_xwaylands; | ||
| ForeignToplevelV1 *m_treelandForeignToplevel = nullptr; |
There was a problem hiding this comment.
issue (bug_risk): m_treelandForeignToplevel is used from Helper but declared private, which will not compile.
In Helper::init you connect directly to m_shellHandler->m_treelandForeignToplevel, but this member is private in ShellHandler and Helper is not a friend, so this will not compile. Please either add a public accessor (e.g. ForeignToplevelV1 *treelandForeignToplevel() const) or route these connections via ShellHandler signals instead of accessing its internals directly.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR integrates prelaunch splash screens into Treeland’s ForeignToplevelV1 (dock/preview) so they appear in the dock and can be closed from it, and moves Treeland foreign-toplevel registration responsibilities from Helper into ShellHandler for tighter lifecycle integration.
Changes:
- Register splash
SurfaceWrapperinstances withForeignToplevelV1, including basic metadata and close handling. - Centralize Treeland foreign-toplevel surface registration in
ShellHandler, including reacting toskipDockPreViewchanges. - Track appIds for splash screens closed from the dock and close the matching real toplevel when it appears.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/surface/surfacewrapper.h |
Adds a unified close() API and a splash-close request signal hook. |
src/surface/surfacewrapper.cpp |
Implements SurfaceWrapper::close() and adjusts skipDockPreView teardown behavior and validation. |
src/seat/helper.h |
Removes stored Treeland foreign-toplevel manager pointer from Helper. |
src/seat/helper.cpp |
Constructs ShellHandler with WServer and reroutes dock-preview connections to the manager owned by ShellHandler. |
src/modules/foreign-toplevel/foreigntoplevelmanagerv1.h |
Extracts toplevel-handle initialization into a helper method. |
src/modules/foreign-toplevel/foreigntoplevelmanagerv1.cpp |
Extends Treeland foreign-toplevel handling for splash screens and refactors handle initialization. |
src/core/shellhandler.h |
Stores Treeland foreign-toplevel manager and adds tracking for dock-closed splash appIds. |
src/core/shellhandler.cpp |
Registers wrappers (including splash) with Treeland foreign-toplevel and enforces “closed splash cancels real window” behavior. |
（中文）
本 PR 将预启动闪屏接入 Treeland 的 ForeignToplevelV1（dock/preview 协议）以便在 dock 中可见并可关闭，并将 Treeland foreign-toplevel 的管理从 Helper 迁移到 ShellHandler，以获得更一致的生命周期与状态联动。
主要改动：
- 将闪屏
SurfaceWrapper注册到ForeignToplevelV1，并支持从 dock 发起关闭。 - 在
ShellHandler中集中处理 Treeland foreign-toplevel 的注册/反注册（随skipDockPreView动态变化）。 - 记录从 dock 关闭的闪屏 appId，并在真实窗口出现时自动关闭匹配窗口。
| connect(handle, &treeland_foreign_toplevel_handle_v1::requestClose, surface, [surface] { | ||
| surface->close(); | ||
| }); | ||
| connect(handle, |
There was a problem hiding this comment.
treeland_foreign_toplevel_handle_v1::requestClose is only connected for splash wrappers; for normal XdgToplevel/XWayland handles there is no close handler anymore, so dock-initiated close requests won’t close the window. Please connect requestClose to the unified SurfaceWrapper::close() (or directly to shellSurface()->close()) during normal handle initialization.
treeland_foreign_toplevel_handle_v1::requestClose 目前只对闪屏 wrapper 做了连接；对普通 XdgToplevel/XWayland handle 已没有关闭处理，导致 dock 发起的关闭请求无法关闭窗口。请在普通窗口 handle 初始化时将 requestClose 连接到统一的 SurfaceWrapper::close()（或直接连接到 shellSurface()->close()）。
Suggested change
| connect(handle, | |
| connect(handle, | |
| &treeland_foreign_toplevel_handle_v1::requestClose, | |
| wrapper, | |
| [wrapper]() { | |
| wrapper->close(); | |
| }); | |
| connect(handle, |
Comment on lines
+96
to
98
| qInfo() << "Register surface to ForeignToplevelV1, appId=" << wrapper->appId() | ||
| << wrapper->type() << wrapper->skipDockPreView(); | ||
|
|
There was a problem hiding this comment.
This file uses qInfo() for the new register/unregister logs, which bypasses the project’s categorized logging (see docs/logging-guidelines.md). Please switch these to qCInfo(treelandProtocol) (or another appropriate Treeland category) so log filtering works consistently.
这里新增的注册/反注册日志使用了 qInfo()，会绕过项目的分类日志规范（见 docs/logging-guidelines.md）。建议改为 qCInfo(treelandProtocol)（或其它合适的 Treeland category），以便统一过滤与排查。
Comment on lines
+61
to
+65
| static int counter = 1; | ||
| handle->set_identifier( | ||
| // *reinterpret_cast<const uint32_t *>(surface->surface()->handle()->handle())); | ||
| //*reinterpret_cast<const uint32_t *>(wrapper)); | ||
| counter++); |
There was a problem hiding this comment.
The set_identifier block contains commented-out pointer/reinterpret_cast experiments and uses a function-local static int counter. Please remove the commented code and consider using an explicit uint32_t monotonic member (or another clearly documented identifier source) to keep this maintainable.
set_identifier 这段还保留了注释掉的指针/reinterpret_cast 试验代码，并且用函数内的 static int counter 生成 id。建议删除这些注释代码，并考虑改为显式的 uint32_t 单调递增成员变量（或其它有明确来源/文档的标识符方案），以提升可维护性。
| qCWarning(treelandSurface) << "Only xdgtoplevel and x11 surface can `setSkipDockPreView`"; | ||
| if (!skip && (m_type != Type::XdgToplevel && m_type != Type::XWayland)) { | ||
| qCWarning(treelandSurface) | ||
| << "Only xdgtoplevel, x11 surface or can `setSkipDockPreView` to false"; |
There was a problem hiding this comment.
The warning string here is grammatically incorrect and misleading ("or can ... to false"). Please reword it to clearly state that only XdgToplevel/XWayland can set skipDockPreView to false.
这里的 warning 文案语法不通且容易误解（"or can ... to false"）。建议改成更清晰的表述：只有 XdgToplevel/XWayland 才允许把 skipDockPreView 设置为 false。
Suggested change
| << "Only xdgtoplevel, x11 surface or can `setSkipDockPreView` to false"; | |
| << "Only XdgToplevel or XWayland surfaces are allowed to set `skipDockPreView` to false. " | |
| << "仅允许 XdgToplevel/XWayland 将 `skipDockPreView` 设置为 false。"; |
Comment on lines
+254
to
+256
| qInfo() << "Unregister surface from ForeignToplevelV1, appId=" << wrapper->appId() | ||
| << wrapper->type() << wrapper->skipDockPreView(); | ||
|
|
There was a problem hiding this comment.
This unregister log also uses qInfo() instead of categorized logging. Please use qCInfo(treelandProtocol) (or a more specific category) to follow docs/logging-guidelines.md.
这条反注册日志同样使用了 qInfo() 而不是分类日志。请改用 qCInfo(treelandProtocol)（或更具体的 category），以符合 docs/logging-guidelines.md。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
protocol
Added foreign-toplevel protocol integration for splash screens to make them visible and manageable in the dock. Splash screens now appear in the dock with proper identification and can be closed directly from the dock. When a splash screen is closed before the actual application window appears, the matching application window will be automatically closed to maintain consistency.
Key changes:
Log: Splash screens now appear in dock and can be closed via dock interface
Influence:
feat: 使闪屏通过foreign-toplevel协议在dock栏可见可关闭
为闪屏添加foreign-toplevel协议集成，使其在dock栏中可见并可管理。闪屏现在
会出现在dock栏中并带有适当标识，可以直接从dock栏关闭。如果在实际应用程序
窗口出现之前关闭闪屏，匹配的应用程序窗口将自动关闭以保持一致性。
主要更改：
Log: 闪屏现在可在dock栏显示并可通过dock界面关闭
Influence:
Summary by Sourcery
Integrate splash screens with the foreign-toplevel protocol so they appear and can be managed from the dock, and ensure subsequent app windows respect splash-close actions.
New Features:
Bug Fixes:
Enhancements: