Skip to content

feat: support treeland prelaunch splash manager v2#332

Merged
zccrs merged 1 commit intolinuxdeepin:masterfrom
wineee:splash-v2
Feb 27, 2026
Merged

feat: support treeland prelaunch splash manager v2#332
zccrs merged 1 commit intolinuxdeepin:masterfrom
wineee:splash-v2

Conversation

@wineee
Copy link
Member

@wineee wineee commented Feb 10, 2026

  1. Upgrade from treeland-prelaunch-splash-v1 to v2 protocol
  2. Generate instance UUID early for splash association
  3. Track splash objects per instance ID for proper lifecycle management
  4. Add closeSplashForInstance and closeAllSplashes methods
  5. Implement proper cleanup of splash objects in destructor

Log: Added support for treeland prelaunch splash manager v2 with instance-based splash management

Influence:

  1. Test application launch with prelaunch splash display
  2. Verify splash closes when instance is removed normally
  3. Test multiple instances of same application with separate splashes
  4. Verify splash cleanup during application manager shutdown
  5. Test client crash scenarios to ensure orphaned splashes are cleaned up
  6. Validate Wayland protocol compatibility with updated v2 interface

feat: 支持 treeland 预启动闪屏管理器 v2

  1. 从 treeland-prelaunch-splash-v1 升级到 v2 协议
  2. 提前生成实例 UUID 用于闪屏关联
  3. 按实例 ID 跟踪闪屏对象以实现正确的生命周期管理
  4. 添加 closeSplashForInstance 和 closeAllSplashes 方法
  5. 在析构函数中实现闪屏对象的正确清理

Log: 新增支持 treeland 预启动闪屏管理器 v2,支持基于实例的闪屏管理

Influence:

  1. 测试应用程序启动时的预启动闪屏显示
  2. 验证实例正常移除时闪屏正确关闭
  3. 测试同一应用程序多个实例的独立闪屏管理
  4. 验证应用程序管理器关闭时的闪屏清理
  5. 测试客户端崩溃场景确保孤儿闪屏被清理
  6. 验证与更新 v2 接口的 Wayland 协议兼容性

Summary by Sourcery

Upgrade prelaunch splash handling to the treeland v2 protocol with instance-scoped lifecycle management.

New Features:

  • Support the treeland-prelaunch-splash-v2 Wayland protocol with per-instance splash objects.
  • Associate prelaunch splashes with a stable instance UUID generated prior to job creation.
  • Add APIs to close splashes per application instance and to close all active splashes on teardown.

Bug Fixes:

  • Ensure prelaunch splash surfaces are destroyed when instances are removed, all instances detach, or the helper is destroyed, preventing orphaned splashes.

Enhancements:

  • Track active splash objects and instance IDs internally to avoid duplicate splashes for the same instance and improve logging around splash operations.

Build:

  • Switch Wayland client code generation to use the treeland-prelaunch-splash-v2 XML protocol description.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @wineee, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for the treeland prelaunch splash manager v2 protocol by associating splashes with application instance IDs and managing their lifecycle alongside instance creation/removal.

Changes:

  • Switch Wayland protocol generation and helper implementation from prelaunch-splash v1 to v2.
  • Generate and propagate an instance UUID early so the compositor can associate a splash with a specific instance.
  • Track and close splash objects per instance, including bulk cleanup on instance detachment and helper destruction.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/dbus/prelaunchsplashhelper.h Updates helper API to v2 and adds per-instance splash object tracking.
src/dbus/prelaunchsplashhelper.cpp Implements v2 create/close logic and destructor cleanup for tracked splash objects.
src/dbus/applicationservice.h Adds tracking for splash instance IDs and declares new cleanup helpers.
src/dbus/applicationservice.cpp Generates instance UUID earlier, shows splash with instance association, and closes splashes on instance lifecycle events.
src/dbus/CMakeLists.txt Switches generated Wayland protocol sources from v1 XML to v2 XML.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wineee wineee force-pushed the splash-v2 branch 2 times, most recently from a701323 to ade9878 Compare February 10, 2026 09:52
@wineee wineee marked this pull request as ready for review February 26, 2026 09:42
@sourcery-ai
Copy link

sourcery-ai bot commented Feb 26, 2026

Reviewer's Guide

Migrates the prelaunch splash Wayland integration from the v1 manager to treeland-prelaunch-splash-manager v2, introduces instance-scoped splash tracking with UUIDs, and ensures proper lifecycle management and cleanup of splash surfaces when instances terminate or the application manager shuts down.

Sequence diagram for instance-scoped prelaunch splash lifecycle

sequenceDiagram
    actor User
    participant ApplicationService
    participant ApplicationManager1Service as ApplicationManager
    participant PrelaunchSplashHelper as SplashHelper
    participant WaylandCompositor as Compositor

    User->>ApplicationService: Launch(action, fields, options)
    ApplicationService->>ApplicationService: validate options
    ApplicationService->>ApplicationService: generate instanceRandomUUID
    ApplicationService->>ApplicationManager: parent()
    ApplicationManager-->>ApplicationService: pointer
    ApplicationService->>ApplicationManager: splashHelper()
    ApplicationManager-->>ApplicationService: SplashHelper

    alt isAutostartLaunch or singletonWithInstance
        ApplicationService-->>User: skip splash
    else show splash
        ApplicationService->>SplashHelper: show(appId, instanceRandomUUID, iconName)
        SplashHelper->>SplashHelper: check isActive
        SplashHelper->>SplashHelper: ensure no existing splash for instanceRandomUUID
        SplashHelper->>SplashHelper: buildIconBuffer(icon)
        SplashHelper->>Compositor: create_splash(appId, instanceRandomUUID, appManagerId, buffer)
        Compositor-->>SplashHelper: treeland_prelaunch_splash_v2 object
        SplashHelper->>SplashHelper: store splash in m_splashObjects[instanceRandomUUID]
        ApplicationService->>ApplicationService: m_splashInstanceIds.insert(instanceRandomUUID)
    end

    ApplicationService->>ApplicationManager: jobManager.addJob(path, lambda(instanceRandomUUID))
    ApplicationManager-->>ApplicationService: job created

    rect rgb(230,230,250)
        Note over ApplicationService,ApplicationManager: Later, when an instance is removed
        ApplicationManager->>ApplicationService: removeOneInstance(instancePath)
        ApplicationService->>ApplicationService: lookup InstanceService
        ApplicationService->>ApplicationService: closeSplashForInstance(instanceId)
        ApplicationService->>ApplicationService: m_splashInstanceIds.remove(instanceId)
        ApplicationService->>ApplicationManager: splashHelper()
        ApplicationManager-->>ApplicationService: SplashHelper
        ApplicationService->>SplashHelper: closeSplash(instanceId)
        SplashHelper->>SplashHelper: splash = m_splashObjects.take(instanceId)
        alt splash exists
            SplashHelper->>SplashHelper: splash.destroy()
            SplashHelper->>SplashHelper: delete splash
            SplashHelper-->>ApplicationService: splash closed
        else no splash
            SplashHelper-->>ApplicationService: no active splash
        end
    end

    rect rgb(220,245,220)
        Note over SplashHelper: Application manager shutdown
        ApplicationManager->>ApplicationService: detachAllInstance()
        ApplicationService->>ApplicationService: m_Instances.clear()
        ApplicationService->>ApplicationService: closeAllSplashes()
        loop for each instanceId in m_splashInstanceIds
            ApplicationService->>ApplicationService: closeSplashForInstance(instanceId)
            ApplicationService->>SplashHelper: closeSplash(instanceId)
        end
        SplashHelper->>SplashHelper: ~PrelaunchSplashHelper()
        SplashHelper->>SplashHelper: destroy all remaining splash objects
    end
Loading

Updated class diagram for ApplicationService and PrelaunchSplashHelper

classDiagram
    class ApplicationService {
        +QHash~QDBusObjectPath, QSharedPointer~InstanceService~~ m_Instances
        +QSet~QString~ m_splashInstanceIds
        +Launch(action: QString, fields: QStringList, options: QVariantMap) : QDBusObjectPath
        +removeOneInstance(instance: QDBusObjectPath) void
        +detachAllInstance() void
        +closeSplashForInstance(instanceId: QString) void
        +closeAllSplashes() void
        +parent() ApplicationManager1Service*
        +parent() const ApplicationManager1Service*
        -updateAfterLaunch(isLaunch: bool) void
        -shouldBeShown(entry: unique_ptr~DesktopEntry~) bool
        -autostartCheck(filePath: QString) const bool
        -setAutostartSource(source: AutostartSource&&) void
        -appendExtraEnvironments(runtimeOptions: QVariantMap&) const void
        -processCompatibility(action: QString, options: QVariantMap&, execStr: QString&) void
    }

    class PrelaunchSplashHelper {
        +m_iconBuffers: vector~unique_ptr~QWaylandShmBuffer~~
        +m_splashObjects: QHash~QString, QtWayland::treeland_prelaunch_splash_v2*~
        +PrelaunchSplashHelper()
        +~PrelaunchSplashHelper()
        +show(appId: QString, instanceId: QString, iconName: QString) void
        +closeSplash(instanceId: QString) void
        +buildIconBuffer(icon: QIcon) wl_buffer*
        +bufferRelease(data: void*, buffer: wl_buffer*) void
        +handleBufferRelease(buffer: wl_buffer*) void
    }

    class ApplicationManager1Service {
        +splashHelper() PrelaunchSplashHelper*
        +jobManager() JobManager&
    }

    class InstanceService {
        +instanceId() const QString
    }

    class JobManager {
        +addJob(path: QString, callback: function) QVariant
    }

    class QWaylandClientExtensionTemplate_PrelaunchSplashHelper {
    }

    class treeland_prelaunch_splash_manager_v2 {
    }

    class treeland_prelaunch_splash_v2 {
        +destroy() void
    }

    PrelaunchSplashHelper --|> QWaylandClientExtensionTemplate_PrelaunchSplashHelper
    PrelaunchSplashHelper --|> treeland_prelaunch_splash_manager_v2
    ApplicationService --> ApplicationManager1Service : parent
    ApplicationManager1Service --> PrelaunchSplashHelper : owns
    ApplicationService --> InstanceService : manages instances
    ApplicationService --> JobManager : uses
    PrelaunchSplashHelper --> treeland_prelaunch_splash_v2 : creates and stores
    ApplicationService --> PrelaunchSplashHelper : uses for show and closeSplash
Loading

File-Level Changes

Change Details Files
Generate a per-launch instance UUID earlier and use it consistently for instance DBus object paths and splash association, with per-instance splash closing on removal or detach.
  • Move instance UUID generation out of the job lambda and into Launch() before job creation so it can be reused both for DBus instance path and splash association.
  • Reorder prelaunch splash decision logic to run after autostart/singleton validation while using the pre-generated instance UUID in logging and helper calls.
  • Track active splash instance IDs in a QSet member and insert IDs when a splash is shown.
  • On instance removal, call a new closeSplashForInstance helper before removing the instance and on detach-all, call closeAllSplashes to clean up any remaining splashes.
src/dbus/applicationservice.cpp
src/dbus/applicationservice.h
Introduce explicit splash lifecycle control APIs on ApplicationService tied to instance IDs.
  • Add closeSplashForInstance(const QString &instanceId) to conditionally close a splash only if the instance ID is known, delegating to PrelaunchSplashHelper::closeSplash.
  • Add closeAllSplashes() to iterate over the tracked splash instance IDs and close each splash in a safe, re-entrant manner.
  • Wire closeSplashForInstance into removeOneInstance and closeAllSplashes into detachAllInstance to ensure clean splash teardown when instances or the whole application are removed.
src/dbus/applicationservice.cpp
src/dbus/applicationservice.h
Upgrade the Wayland prelaunch splash protocol client from v1 to v2 and manage per-instance splash objects with proper destruction.
  • Switch generated Wayland client sources from treeland-prelaunch-splash-v1.xml to treeland-prelaunch-splash-v2.xml in the dbus CMake configuration.
  • Update PrelaunchSplashHelper to inherit from treeland_prelaunch_splash_manager_v2 and include the v2 generated header.
  • Change the show() helper to take an instanceId argument, prevent duplicate splashes per instance, call the v2 create_splash(appId, instanceId, role, buffer), and track returned treeland_prelaunch_splash_v2 objects in a QHash keyed by instance ID.
  • Add closeSplash(const QString &instanceId) that looks up, destroys, and deletes the corresponding splash object and logs the action.
  • Implement a non-trivial destructor for PrelaunchSplashHelper that explicitly destroys and deletes all remaining splash objects to avoid orphaned splashes on shutdown.
src/dbus/prelaunchsplashhelper.h
src/dbus/prelaunchsplashhelper.cpp
src/dbus/CMakeLists.txt
Update licensing years to cover 2026 for modified dbus-related sources.
  • Extend SPDX-FileCopyrightText year ranges to 2026 in ApplicationService and PrelaunchSplashHelper source and header files.
src/dbus/applicationservice.cpp
src/dbus/applicationservice.h
src/dbus/prelaunchsplashhelper.cpp
src/dbus/prelaunchsplashhelper.h

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/dbus/applicationservice.cpp" line_range="375" />
<code_context>
+            const auto iconVar = findEntryValue(DesktopFileEntryKey, "Icon", EntryValueType::IconString);
+            const QString iconName = iconVar.isNull() ? QString{} : iconVar.toString();
+            qCInfo(amPrelaunchSplash) << "Show prelaunch splash request" << id() << "instance" << instanceRandomUUID << "icon" << iconName;
+            helper->show(id(), instanceRandomUUID, iconName);
+            m_splashInstanceIds.insert(instanceRandomUUID);
+        } else {
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing `m_splashInstanceIds` from `ApplicationService` and delegating all splash lifecycle management to `PrelaunchSplashHelper` to avoid duplicated state and control flow.

You can drop `m_splashInstanceIds` and the extra close indirection and let `PrelaunchSplashHelper` be the single source of truth for splash state.

Instead of tracking instance IDs in `ApplicationService` and guarding every close with `m_splashInstanceIds.remove(...)`, you can:

1. Stop inserting into `m_splashInstanceIds` when showing the splash.
2. Remove `closeSplashForInstance` and `closeAllSplashes`.
3. Directly call the helper’s `closeSplash(instanceId)` wherever you tear down instances.
4. Optionally add a `closeAll()` to the helper if global teardown is needed.

This keeps all behavior (per-instance splash + teardown) but removes duplicated state and control flow.

### 1. Show splash without `m_splashInstanceIds`

```cpp
// Before
helper->show(id(), instanceRandomUUID, iconName);
m_splashInstanceIds.insert(instanceRandomUUID);

// After
helper->show(id(), instanceRandomUUID, iconName);
```

### 2. Simplify `removeOneInstance`

```cpp
void ApplicationService::removeOneInstance(const QDBusObjectPath &instance) noexcept
{
    if (auto it = m_Instances.constFind(instance); it != m_Instances.cend()) {
        const auto instanceId = it.value()->instanceId();

        if (auto *am = parent()) {
            if (auto *helper = am->splashHelper()) {
                helper->closeSplash(instanceId);   // ignore unknown ids internally
            }
        }

        emit InterfacesRemoved(instance, getChildInterfacesFromObject(it->data()));
        unregisterObjectFromDBus(instance.path());
        m_Instances.remove(instance);
    }
}
```

### 3. Simplify `detachAllInstance`

If you only need to close splashes for currently tracked instances:

```cpp
void ApplicationService::detachAllInstance() noexcept
{
    for (auto it = m_Instances.constBegin(); it != m_Instances.constEnd(); ++it) {
        const auto &instance = it.value();
        orphanedInstances.append(instance);
        instance->setProperty("Orphaned", true);

        if (auto *am = parent()) {
            if (auto *helper = am->splashHelper()) {
                helper->closeSplash(instance->instanceId());
            }
        }
    }

    m_Instances.clear();
}
```

### 4. Optional: centralize a global close in the helper

If you truly need a “close every splash” path (e.g. on compositor restart), add it to `PrelaunchSplashHelper`:

```cpp
// In PrelaunchSplashHelper
void PrelaunchSplashHelper::closeAll()
{
    for (auto it = m_splashObjects.begin(); it != m_splashObjects.end(); ++it) {
        // existing teardown for a single splash
        closeSplash(it.key());
    }
}
```

Then call it instead of `closeAllSplashes()`:

```cpp
if (auto *am = parent()) {
    if (auto *helper = am->splashHelper()) {
        helper->closeAll();
    }
}
```

This removes `m_splashInstanceIds`, `closeSplashForInstance`, and `closeAllSplashes` from `ApplicationService`, cuts the extra loops/guards, and keeps all functionality while reducing cross-component state and complexity.
</issue_to_address>

### Comment 2
<location path="src/dbus/applicationservice.h" line_range="203" />
<code_context>
     DesktopFile m_desktopSource;
     QSharedPointer<DesktopEntry> m_entry{nullptr};
     QHash<QDBusObjectPath, QSharedPointer<InstanceService>> m_Instances;
+    QSet<QString> m_splashInstanceIds;
     void updateAfterLaunch(bool isLaunch) noexcept;
     static bool shouldBeShown(const std::unique_ptr<DesktopEntry> &entry) noexcept;
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing splash-related state from ApplicationService and delegating splash closing directly to the helper to avoid duplicated state and tighter coupling.

You’ve added `m_splashInstanceIds` plus `closeSplashForInstance` / `closeAllSplashes`, which introduces new state in `ApplicationService` solely to mirror splash state that should already be owned by the splash helper. That does increase complexity and coupling.

If `PrelaunchSplashHelper` (or equivalent) already tracks splash objects per instance, you can simplify by:

* Removing the extra container from `ApplicationService`.
* Letting `ApplicationService` delegate directly to the helper for per-instance and “close all” behavior.
* Keeping any close helpers private and stateless.

For example, instead of:

```cpp
// applicationservice.h
QHash<QDBusObjectPath, QSharedPointer<InstanceService>> m_Instances;
QSet<QString> m_splashInstanceIds;

void closeSplashForInstance(const QString &instanceId) noexcept;
void closeAllSplashes() noexcept;
```

you can reduce state to:

```cpp
// applicationservice.h
QHash<QDBusObjectPath, QSharedPointer<InstanceService>> m_Instances;

// No splash ID container here; helper owns splash state
void closeSplashForInstance(const QString &instanceId) noexcept;
void closeAllSplashes() noexcept;
```

and implement the methods as thin delegates in the `.cpp`:

```cpp
// applicationservice.cpp
void ApplicationService::closeSplashForInstance(const QString &instanceId) noexcept
{
    if (auto helper = parent()->prelaunchSplashHelper()) {
        helper->closeSplash(instanceId);
    }
}

void ApplicationService::closeAllSplashes() noexcept
{
    if (auto helper = parent()->prelaunchSplashHelper()) {
        helper->closeAllSplashesForApp(m_desktopId);
    }
}
```

If `closeSplashesForApp` doesn’t exist yet, it can be added to the helper:

```cpp
// prelaunchsplashhelper.h
void closeAllSplashesForApp(const QString &desktopId) noexcept;
```

```cpp
// prelaunchsplashhelper.cpp
void PrelaunchSplashHelper::closeAllSplashesForApp(const QString &desktopId) noexcept
{
    for (const auto &id : m_splashObjects.keys()) {
        if (/* id belongs to desktopId */) {
            closeSplash(id);
        }
    }
}
```

This keeps all splash-related state (`instanceId` tracking, lifetime, etc.) in one place (the helper), and `ApplicationService` becomes a simple client rather than a second source of truth.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@wineee wineee force-pushed the splash-v2 branch 3 times, most recently from 43955c7 to 2699235 Compare February 26, 2026 13:28
1. Upgrade from treeland-prelaunch-splash-v1 to v2 protocol
2. Generate instance UUID early for splash association
3. Track splash objects per instance ID for proper lifecycle management
4. Add closeSplashForInstance and closeAllSplashes methods
5. Implement proper cleanup of splash objects in destructor

Log: Added support for treeland prelaunch splash manager v2 with
instance-based splash management

Influence:
1. Test application launch with prelaunch splash display
2. Verify splash closes when instance is removed normally
3. Test multiple instances of same application with separate splashes
4. Verify splash cleanup during application manager shutdown
5. Test client crash scenarios to ensure orphaned splashes are cleaned
up
6. Validate Wayland protocol compatibility with updated v2 interface

feat: 支持 treeland 预启动闪屏管理器 v2

1. 从 treeland-prelaunch-splash-v1 升级到 v2 协议
2. 提前生成实例 UUID 用于闪屏关联
3. 按实例 ID 跟踪闪屏对象以实现正确的生命周期管理
4. 添加 closeSplashForInstance 和 closeAllSplashes 方法
5. 在析构函数中实现闪屏对象的正确清理

Log: 新增支持 treeland 预启动闪屏管理器 v2,支持基于实例的闪屏管理

Influence:
1. 测试应用程序启动时的预启动闪屏显示
2. 验证实例正常移除时闪屏正确关闭
3. 测试同一应用程序多个实例的独立闪屏管理
4. 验证应用程序管理器关闭时的闪屏清理
5. 测试客户端崩溃场景确保孤儿闪屏被清理
6. 验证与更新 v2 接口的 Wayland 协议兼容性
@deepin-ci-robot
Copy link

deepin pr auto review

这份代码审查主要针对 git diff 提供的变更内容,涉及 Wayland 协议更新、启动画面(Splash Screen)的生命周期管理以及代码风格的微调。

以下是详细的审查意见:

1. 语法与逻辑审查

1.1 Wayland 协议升级与接口变更

  • 修改点CMakeLists.txt 中协议文件从 treeland-prelaunch-splash-v1.xml 更新至 v2PrelaunchSplashHelper 类继承自 treeland_prelaunch_splash_manager_v2
  • 审查意见
    • 逻辑正确性:协议升级通常意味着功能增强。代码中 show() 方法增加了 instanceId 参数,且新增了 closeSplash() 方法,这与 v2 协议通常引入的"针对实例的精确控制"逻辑相符。
    • 对象生命周期管理:在 PrelaunchSplashHelper 中引入了 QHash<QString, QtWayland::treeland_prelaunch_splash_v2 *> m_splashObjects 来追踪活跃的 splash 对象。
      • 析构函数~PrelaunchSplashHelper() 中遍历销毁所有 splash 对象是非常必要的,防止应用退出时造成 Wayland 协议资源泄漏或 compositor 端的悬空指针。
      • 移除逻辑closeSplash 中使用了 take() 从哈希表中移除指针并销毁对象,逻辑严密,防止了野指针访问。

1.2 实例 ID 的生成与传递

  • 修改点:在 ApplicationService::Launch 中,instanceRandomUUID 的生成位置被移到了 jobManager.addJob 的 lambda 捕获之前。
  • 审查意见
    • 逻辑正确性:这是一个关键改进。旧代码在 lambda 内部生成 UUID,导致无法在外部(即显示 splash 时)获知即将启动的实例 ID。现在提前生成并在 lambda 中捕获,使得 helper->show() 和后续的 job 执行能够共享同一个 ID,实现了 splash 与特定实例的绑定。
    • 一致性objectPath 的构建逻辑 m_applicationPath.path() + "/" + instanceRandomUUID 保持了实例路径的唯一性。

1.3 Splash 的关闭时机

  • 修改点ApplicationService::removeOneInstancedetachAllInstance 中增加了关闭 splash 的调用。
  • 审查意见
    • 逻辑正确性
      • 当实例被移除时,调用 closeSplashForInstance 是合理的,因为应用窗口已经显示或销毁,启动画面应当消失。
      • detachAllInstance 中调用 closeAllSplashes 处理了应用服务分离时的清理工作,逻辑完善。
    • 潜在风险:注意 closeSplashForInstancem_splashInstanceIds.remove(instanceId) 的调用。如果应用启动极快,在 show 之后、insert 之前(尽管代码顺序上是先 insert 后 show,但异步操作可能有差异)或者 remove 之前实例 ID 未正确匹配,可能导致 splash 未关闭。目前的实现看起来是同步的,风险较低。

2. 代码质量

2.1 Lambda 捕获与可变性

  • 修改点jobManager.addJob 中的 lambda 从 [this, task = task, cmds = std::move(cmds)] 变更为 [this, task, instanceRandomUUID, cmds = std::move(cmds)] 并添加了注释 do not change it to mutable lambda
  • 审查意见
    • 改进:添加注释解释了为什么不需要 mutable,这有助于后续维护者理解意图。由于 instanceRandomUUID 现在是外部传入的 const 值,lambda 内部不需要修改它,因此确实不需要 mutable。这提高了代码的清晰度和安全性。

2.2 代码风格与格式

  • 修改点:多处空格调整,如 QString& 改为 QString &std::as_const(newRemoved) 前添加空格。
  • 审查意见
    • 规范性:这些改动符合 Qt/KDE/Deepin 常见的代码风格指南(引用符号与类型靠左或靠右取决于具体规范,这里统一为靠右或加空格),提高了代码的可读性和一致性。

3. 代码性能

  • UUID 生成QUuid::createUuid().toString(QUuid::Id128) 涉及字符串操作和系统调用(如果涉及随机数生成),但在应用启动流程中,这个开销通常可以忽略不计,且这是保证唯一性的必要开销。
  • 容器查找m_splashInstanceIdsQSetm_splashObjectsQHash,查找和删除操作的时间复杂度均为平均 O(1),性能良好。
  • 深拷贝风险:在 closeAllSplashes 中,代码使用了 const auto ids = m_splashInstanceIds; 进行了容器的拷贝。这是为了防止在遍历容器时调用 closeSplashForInstance 修改容器导致迭代器失效。考虑到 QString 是隐式共享的,且通常实例数量不多(个位数或几十个),这个拷贝开销是可以接受的,且保证了线程安全(如果涉及多线程)和迭代器安全。

4. 代码安全

  • 空指针检查:在 PrelaunchSplashHelper::closeSplashApplicationService::closeSplashForInstance 中,都检查了 splashhelper 是否存在。这避免了空指针解引用崩溃。
  • 协议版本兼容:代码显式依赖 v2 协议。如果运行环境(Compositor)仅支持 v1,此代码将无法工作。建议在 PrelaunchSplashHelper 初始化或 isActive() 检查中增加对协议版本的支持检测,或者确保部署环境已同步更新。
  • 资源泄漏防护:如前所述,析构函数中的清理逻辑极大地降低了资源泄漏的风险。

总结与改进建议

总体评价:这是一次高质量的代码提交,主要完成了 Wayland 启动画面协议从 v1 到 v2 的迁移,并修复了实例 ID 管理相关的逻辑问题。代码逻辑严密,资源管理得当。

改进建议

  1. 协议版本检测
    虽然代码升级到了 v2,但建议在 PrelaunchSplashHelper::isActive() 或初始化阶段增加对协议绑定器版本或全局接口版本的检查。如果 compositor 不支持 v2,应优雅降级或输出明确的错误日志,而不是直接失败或行为未定义。

  2. 日志完善
    closeSplashForInstance 中,如果 m_splashInstanceIds.remove(instanceId) 返回 false(即找不到该 ID),目前是直接 return。建议在此处添加 qCDebug 日志,记录尝试关闭不存在的 splash ID 的情况,这在调试多实例启动问题时会非常有帮助。

  3. 异常处理
    QUuid::createUuid() 通常不会失败,但在极端系统资源受限情况下可能存在问题。虽然概率极低,但作为关键路径代码,可以考虑增加对 UUID 生成结果的非空检查(尽管 QString 默认构造为空,toString 很难返回空,除非实现有问题)。

  4. 注释一致性
    applicationservice.cpp 中有一行注释:// Command lines that contain a field code that is not listed in this specification are invalid and MUST NOT be processed,被拆分成了两行。建议保持长注释在一行,或者使用块注释 /* ... */,以避免版本控制时产生不必要的 diff 噪音(虽然这次 diff 显示是代码格式化工具自动处理的)。

@deepin-bot
Copy link

deepin-bot bot commented Feb 27, 2026

TAG Bot

New tag: 1.2.46
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #335

@deepin-ci-robot
Copy link

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zccrs zccrs merged commit dd62876 into linuxdeepin:master Feb 27, 2026
17 checks passed
@wineee wineee deleted the splash-v2 branch February 27, 2026 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants