fix: suppress splash for singleton apps with existing instances#329
Open
wineee wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: suppress splash for singleton apps with existing instances#329wineee wants to merge 1 commit intolinuxdeepin:masterfrom
wineee wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
The change modifies the splash screen logic to skip showing the splash screen for singleton applications that already have running instances. Previously, splash screens were only suppressed for system autostart launches. Now, when a singleton application (marked with X-Deepin- Singleton=true) is launched while another instance is already running, the splash screen will be skipped to prevent redundant visual feedback. Technical details: 1. Added detection for singleton applications using the X-Deepin- Singleton desktop entry key 2. Added check for existing application instances using m_Instances.isEmpty() 3. Combined both conditions to determine when to skip splash screen 4. Added appropriate logging for the new skip condition Log: Skip splash screen when launching singleton applications with existing instances Influence: 1. Test launching singleton applications when no instance is running - should show splash 2. Test launching singleton applications when instance is already running - should skip splash 3. Verify system autostart launches still skip splash as before 4. Test non-singleton applications behavior remains unchanged 5. Check logging output for proper splash skip messages fix: 为已有实例的单例应用跳过启动闪屏 修改了启动闪屏逻辑，对于已有运行实例的单例应用跳过显示启动闪屏。之前 仅对系统自启动的应用程序跳过闪屏显示。现在当单例应用（标记为X-Deepin- Singleton=true）在已有实例运行时再次启动，将跳过闪屏以避免冗余的视觉 反馈。 技术细节： 1. 添加了对单例应用的检测，使用X-Deepin-Singleton桌面条目键 2. 添加了对现有应用实例的检查，使用m_Instances.isEmpty() 3. 结合两个条件来确定何时跳过闪屏显示 4. 为新的跳过条件添加了适当的日志记录 Log: 当启动已有实例的单例应用时跳过启动闪屏 Influence: 1. 测试在无实例运行时启动单例应用 - 应显示闪屏 2. 测试在已有实例运行时启动单例应用 - 应跳过闪屏 3. 验证系统自启动应用仍按原有方式跳过闪屏 4. 测试非单例应用的行为保持不变 5. 检查日志输出中正确的闪屏跳过消息
|
[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 guide (collapsed on small PRs)Reviewer's GuideAdjusts prelaunch splash behavior so that singleton applications with existing instances skip the splash, extending the previous autostart-only suppression and adding logging for the new condition. Sequence diagram for prelaunch splash decision in ApplicationService::LaunchsequenceDiagram
actor User
participant Launcher
participant ApplicationService
participant SplashHelper
User->>Launcher: request app launch
Launcher->>ApplicationService: Launch(action, fields, options)
ApplicationService->>ApplicationService: isAutostartLaunch = optionsMap.contains(_autostart)
ApplicationService->>ApplicationService: isSingleton = findEntryValue(X_Deepin_Singleton)
ApplicationService->>ApplicationService: singletonWithInstance = isSingleton && !m_Instances.isEmpty()
alt isAutostartLaunch
ApplicationService->>ApplicationService: log Skip prelaunch splash (autostart)
ApplicationService-->>Launcher: continue launch without splash
else singletonWithInstance
ApplicationService->>ApplicationService: log Skip prelaunch splash (singleton with existing instance)
ApplicationService-->>Launcher: continue launch without splash
else normal_launch
ApplicationService->>ApplicationService: am = parent()
ApplicationService->>SplashHelper: splashHelper() and show splash with icon
SplashHelper-->>ApplicationService: splash shown
ApplicationService-->>Launcher: continue launch with splash
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码修改主要针对在 Wayland 会话下启动应用时的启动画面(splash screen)显示逻辑进行了优化。以下是对该 1. 语法逻辑审查结果：通过
2. 代码质量审查结果：良好，有优化建议
3. 代码性能审查结果：良好
4. 代码安全审查结果：通过
总结与建议这段代码修改是正确且必要的，它解决了单例应用在已有实例运行时仍显示启动画面的逻辑冗余问题。 建议优化后的代码片段（主要针对魔法字符串的改进）： // 建议在类头文件中定义常量，例如：
// static const char kDeepinSingletonKey[] = "X-Deepin-Singleton";
// Notify the compositor to show a splash screen if in a Wayland session.
// Suppress splash for system autostart launches or singleton apps with existing instances.
const bool isAutostartLaunch = optionsMap.contains("_autostart") && optionsMap.value("_autostart").toBool();
// 使用常量替代字符串字面量
const bool isSingleton = findEntryValue(DesktopFileEntryKey, kDeepinSingletonKey, EntryValueType::Boolean).toBool();
const bool singletonWithInstance = isSingleton && !m_Instances.isEmpty();
if (isAutostartLaunch) {
qCInfo(amPrelaunchSplash) << "Skip prelaunch splash (autostart)" << id();
} else if (singletonWithInstance) {
qCInfo(amPrelaunchSplash) << "Skip prelaunch splash (singleton with existing instance)" << id();
} else if (auto *am = parent()) {
// ... 后续逻辑 |
![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 left some high level feedback:
- Consider whether
m_Instances.isEmpty()is safe from races in this context (e.g., if a new instance can be registered concurrently with this Launch call) and, if needed, document or guard against any potential timing issues. - The
isSingletonlookup viafindEntryValueis now on the hot path for every launch; if this value is static per desktop file, consider caching it at a higher level to avoid repeated lookups.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider whether `m_Instances.isEmpty()` is safe from races in this context (e.g., if a new instance can be registered concurrently with this Launch call) and, if needed, document or guard against any potential timing issues.
- The `isSingleton` lookup via `findEntryValue` is now on the hot path for every launch; if this value is static per desktop file, consider caching it at a higher level to avoid repeated lookups.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR enhances the splash screen logic to suppress splash screens for singleton applications that already have running instances. Previously, splash suppression only applied to system autostart launches.
Changes:
- Added detection of singleton applications via the
X-Deepin-Singletondesktop entry key - Added logic to skip splash screens when a singleton app with existing instances is launched
- Updated comments and logging to reflect the new behavior
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Suppress splash for system autostart launches. | ||
| // Suppress splash for system autostart launches or singleton apps with existing instances. | ||
| const bool isAutostartLaunch = optionsMap.contains("_autostart") && optionsMap.value("_autostart").toBool(); | ||
| const bool isSingleton = findEntryValue(DesktopFileEntryKey, "X-Deepin-Singleton", EntryValueType::Boolean).toBool(); |
There was a problem hiding this comment.
Consider adding a null check before calling toBool() for consistency with the existing pattern used in noDisplay() (line 541-547). While calling toBool() on a null QVariant safely returns false, the codebase convention appears to explicitly check for null first. For example:
auto val = findEntryValue(DesktopFileEntryKey, "X-Deepin-Singleton", EntryValueType::Boolean);
const bool isSingleton = val.isNull() ? false : val.toBool();This matches the pattern at line 541 where noDisplay() checks val.isNull() before calling val.toBool().
|
TAG Bot New tag: 1.2.45 |
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.
The change modifies the splash screen logic to skip showing the splash screen for singleton applications that already have running instances. Previously, splash screens were only suppressed for system autostart launches. Now, when a singleton application (marked with X-Deepin- Singleton=true) is launched while another instance is already running, the splash screen will be skipped to prevent redundant visual feedback.
Technical details:
Log: Skip splash screen when launching singleton applications with existing instances
Influence:
fix: 为已有实例的单例应用跳过启动闪屏
修改了启动闪屏逻辑，对于已有运行实例的单例应用跳过显示启动闪屏。之前
仅对系统自启动的应用程序跳过闪屏显示。现在当单例应用（标记为X-Deepin-
Singleton=true）在已有实例运行时再次启动，将跳过闪屏以避免冗余的视觉
反馈。
技术细节：
Log: 当启动已有实例的单例应用时跳过启动闪屏
Influence:
Summary by Sourcery
Bug Fixes: