-
Notifications
You must be signed in to change notification settings - Fork 41
Support Splash #300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Splash #300
Conversation
|
Skipping CI for Draft Pull Request. |
Reviewer's GuideAdds a Wayland-based prelaunch splash mechanism wired into application launch, introduces a PrelaunchSplashHelper Wayland client extension, switches the application manager to QGuiApplication, and updates CMake/test configuration to generate/link Wayland client protocol code and expose needed headers. Sequence diagram for application launch with prelaunch splash on WaylandsequenceDiagram
actor User
participant ClientApp
participant DBus as ApplicationManager1DBus
participant AM1 as ApplicationManager1Service
participant AS as ApplicationService
participant Splash as PrelaunchSplashHelper
participant Compositor as WaylandCompositor
User->>ClientApp: Request application launch
ClientApp->>DBus: org.desktopspec.ApplicationManager1.Launch
DBus->>AM1: Launch call dispatched
AM1->>AS: Create or reuse ApplicationService for appId
AM1-->>DBus: Return pending call handle
AS->>AS: Parse launch options and desktop entry
AS->>AM1: parent()
AS->>AM1: splashHelper()
AM1-->>AS: PrelaunchSplashHelper pointer or null
alt Helper available (Wayland session)
AS->>AS: findEntryValue(Icon)
AS->>Splash: show(appId, iconName)
Splash->>Splash: buildIconBuffer(icon)
Splash->>Compositor: create_splash(appId, dde-application-manager, wl_buffer)
Compositor-->>Splash: Ack splash creation
else No helper (non-Wayland or init failed)
AS->>AS: Log skip prelaunch splash
end
AS->>AS: Complete launch preparation
AS->>System: Spawn application process
System-->>AS: Application started
AS-->>DBus: Return launch result
Class diagram for PrelaunchSplashHelper and application manager integrationclassDiagram
class ApplicationManager1Service {
- std::unique_ptr~Identifier~ m_identifier
- std::unique_ptr~Storage~ m_storage
- QTimer m_reloadTimer
- QHash~QString, QSharedPointer~ApplicationService~~ m_applicationList
- QSharedPointer~CompatibilityManager~ m_compatibilityManager
- QScopedPointer~PrelaunchSplashHelper~ m_splashHelper
+ ApplicationManager1Service(std::unique_ptr~Identifier~ ptr, std::unique_ptr~Storage~ storage)
+ const MimeManager1Service & mimeManager() const
+ const QStringList & systemdPathEnv() const
+ QSharedPointer~CompatibilityManager~ getCompatibilityManager() const
+ PrelaunchSplashHelper * splashHelper() const
}
class ApplicationService {
+ QString Launch(QString action, QStringList fields, QVariantMap optionsMap)
+ QString id() const
+ ApplicationManager1Service * parent() const
}
class PrelaunchSplashHelper {
<<QObject>>
<<QWaylandClientExtensionTemplate_PrelaunchSplashHelper>>
<<treeland_prelaunch_splash_manager_v1>>
- std::unique_ptr~Impl~ m_impl
+ PrelaunchSplashHelper()
+ ~PrelaunchSplashHelper()
+ void show(QString appId, QString iconName)
- wl_buffer * buildIconBuffer(QIcon icon)
- wl_buffer * createBufferFromPixmap(QPixmap pixmap)
}
class PrelaunchSplashHelper_Impl {
+ std::vector~std::unique_ptr~QtWaylandClient_QWaylandShmBuffer~~ iconBuffers
}
class QtWaylandClient_QWaylandShmBuffer {
+ QWaylandClient_QWaylandShmBuffer(QWaylandDisplay display, QSize size, QImage_Format format, qreal devicePixelRatio)
+ QImage * image()
+ wl_buffer * buffer()
}
class QGuiApplication {
+ static QCoreApplication * instance()
+ template QNativeInterface_QWaylandApplication * nativeInterface()
}
class QWaylandIntegration {
+ QWaylandDisplay * display()
}
class QWaylandDisplay
class treeland_prelaunch_splash_manager_v1 {
+ void create_splash(QString appId, QString launcherId, wl_buffer * buffer)
}
ApplicationManager1Service --> ApplicationService : owns
ApplicationManager1Service --> PrelaunchSplashHelper : m_splashHelper
ApplicationService --> ApplicationManager1Service : parent()
ApplicationService --> PrelaunchSplashHelper : show(appId, iconName)
PrelaunchSplashHelper --> PrelaunchSplashHelper_Impl : uses
PrelaunchSplashHelper_Impl --> QtWaylandClient_QWaylandShmBuffer : iconBuffers
PrelaunchSplashHelper --> QtWaylandClient_QWaylandShmBuffer : createBufferFromPixmap
PrelaunchSplashHelper --> treeland_prelaunch_splash_manager_v1 : create_splash
PrelaunchSplashHelper --> QGuiApplication : uses instance
PrelaunchSplashHelper --> QWaylandIntegration : integration()
QWaylandIntegration --> QWaylandDisplay : display()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
TAG Bot New tag: 1.2.38 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- In
src/dbus/CMakeLists.txt, consider making the WaylandClientPrivate include directories target-private instead of adding$<TARGET_PROPERTY:Qt${QT_VERSION_MAJOR}::WaylandClientPrivate,INTERFACE_INCLUDE_DIRECTORIES>todde_am_dbus's PUBLIC include dirs, to avoid leaking Qt private headers into dependents and tightening the coupling to Qt internals.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `src/dbus/CMakeLists.txt`, consider making the WaylandClientPrivate include directories target-private instead of adding `$<TARGET_PROPERTY:Qt${QT_VERSION_MAJOR}::WaylandClientPrivate,INTERFACE_INCLUDE_DIRECTORIES>` to `dde_am_dbus`'s PUBLIC include dirs, to avoid leaking Qt private headers into dependents and tightening the coupling to Qt internals.
## Individual Comments
### Comment 1
<location> `src/dbus/prelaunchsplashhelper.h:36` </location>
<code_context>
+private:
+ wl_buffer *buildIconBuffer(const QIcon &icon);
+ wl_buffer *createBufferFromPixmap(const QPixmap &pixmap);
+ struct Impl;
+ std::unique_ptr<Impl> m_impl;
+};
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing the PIMPL indirection and store the icon buffers directly in PrelaunchSplashHelper to simplify the class structure and reduce overhead.
The PIMPL here adds an extra allocation/indirection without a clear benefit given this helper is already tied to Wayland internals and (per the other review) only stores the icon buffers. You can simplify and reduce complexity/overhead by inlining the state directly in the class.
For example, instead of:
```cpp
class PrelaunchSplashHelper
: public QWaylandClientExtensionTemplate<PrelaunchSplashHelper>
, public QtWayland::treeland_prelaunch_splash_manager_v1
{
Q_OBJECT
public:
explicit PrelaunchSplashHelper();
~PrelaunchSplashHelper();
void show(const QString &appId, const QString &iconName);
private:
wl_buffer *buildIconBuffer(const QIcon &icon);
wl_buffer *createBufferFromPixmap(const QPixmap &pixmap);
struct Impl;
std::unique_ptr<Impl> m_impl;
};
```
you can hold the buffers directly:
```cpp
class PrelaunchSplashHelper
: public QWaylandClientExtensionTemplate<PrelaunchSplashHelper>
, public QtWayland::treeland_prelaunch_splash_manager_v1
{
Q_OBJECT
public:
explicit PrelaunchSplashHelper();
~PrelaunchSplashHelper();
void show(const QString &appId, const QString &iconName);
private:
wl_buffer *buildIconBuffer(const QIcon &icon);
wl_buffer *createBufferFromPixmap(const QPixmap &pixmap);
std::vector<std::unique_ptr<QtWaylandClient::QWaylandShmBuffer>> m_iconBuffers;
};
```
And in the `.cpp`, move any logic from `Impl` methods into `PrelaunchSplashHelper` methods, and replace `m_impl->buffers` (or similar) with `m_iconBuffers`. This keeps the same functionality while removing boilerplate, one heap allocation, and an extra pointer indirection.
</issue_to_address>
### Comment 2
<location> `src/dbus/prelaunchsplashhelper.cpp:19` </location>
<code_context>
+
+Q_LOGGING_CATEGORY(amPrelaunchSplash, "dde.am.prelaunch.splash")
+
+struct PrelaunchSplashHelper::Impl {
+ std::vector<std::unique_ptr<QtWaylandClient::QWaylandShmBuffer>> iconBuffers;
+};
</code_context>
<issue_to_address>
**issue (complexity):** Consider removing the Impl indirection and storing the icon buffers directly as a member to simplify the class’s state management and lifetime handling.
You can simplify the state management by dropping the `Impl` indirection and storing the buffers directly as a member. Since `Impl` only contains `iconBuffers` and there’s no indication of a binary-compatibility requirement here, the extra `std::unique_ptr<Impl>` just adds mental overhead and nullability concerns without a payoff.
### Suggested changes
In the header (`prelaunchsplashhelper.h`), replace the PIMPL:
```cpp
class PrelaunchSplashHelper
: public QWaylandClientExtensionTemplate<PrelaunchSplashHelper>
{
Q_OBJECT
public:
PrelaunchSplashHelper();
~PrelaunchSplashHelper() override;
wl_buffer *buildIconBuffer(const QIcon &icon);
wl_buffer *createBufferFromPixmap(const QPixmap &pixmap);
void show(const QString &appId, const QString &iconName);
private:
struct Impl;
std::unique_ptr<Impl> m_impl;
};
```
with a direct member:
```cpp
class PrelaunchSplashHelper
: public QWaylandClientExtensionTemplate<PrelaunchSplashHelper>
{
Q_OBJECT
public:
PrelaunchSplashHelper();
~PrelaunchSplashHelper() override;
wl_buffer *buildIconBuffer(const QIcon &icon);
wl_buffer *createBufferFromPixmap(const QPixmap &pixmap);
void show(const QString &appId, const QString &iconName);
private:
std::vector<std::unique_ptr<QtWaylandClient::QWaylandShmBuffer>> m_iconBuffers;
};
```
In the implementation file, delete the `Impl` struct and update usages:
```cpp
// remove this:
// struct PrelaunchSplashHelper::Impl {
// std::vector<std::unique_ptr<QtWaylandClient::QWaylandShmBuffer>> iconBuffers;
// };
PrelaunchSplashHelper::PrelaunchSplashHelper()
: QWaylandClientExtensionTemplate<PrelaunchSplashHelper>(1)
{ }
PrelaunchSplashHelper::~PrelaunchSplashHelper() = default;
wl_buffer *PrelaunchSplashHelper::createBufferFromPixmap(const QPixmap &pixmap)
{
// ... existing code up to wlBuf
auto *wlBuf = buffer->buffer();
m_iconBuffers.emplace_back(std::move(buffer));
return wlBuf;
}
void PrelaunchSplashHelper::show(const QString &appId, const QString &iconName)
{
if (!isActive()) {
qCWarning(amPrelaunchSplash, "Skip prelaunch splash (extension inactive): %s", qPrintable(appId));
return;
}
m_iconBuffers.clear();
// ... rest unchanged
}
```
This keeps all behavior the same while making the object’s lifetime and state management more straightforward (no heap-allocated `Impl`, no extra level of indirection).
</issue_to_address>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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds Wayland-based prelaunch splash screen support to the DDE application manager. The implementation introduces a new PrelaunchSplashHelper class that communicates with the compositor via the Wayland protocol to display splash screens when applications are launched. The change requires switching from QCoreApplication to QGuiApplication to enable Wayland and GUI functionality.
Key Changes:
- Added PrelaunchSplashHelper Wayland client extension for requesting splash screens from the compositor
- Integrated splash display trigger into application launch flow (only on Wayland sessions)
- Switched application base class from QCoreApplication to QGuiApplication to support Wayland features
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/dbus/prelaunchsplashhelper.h | New header defining the Wayland client extension class for splash screens |
| src/dbus/prelaunchsplashhelper.cpp | Implementation of splash helper with icon buffer management and Wayland protocol calls |
| src/dbus/applicationmanager1service.h | Added splash helper member and accessor method |
| src/dbus/applicationmanager1service.cpp | Wayland session detection and splash helper initialization logic |
| src/dbus/applicationservice.cpp | Integration point to trigger splash display during application launch |
| apps/dde-application-manager/src/main.cpp | Changed from QCoreApplication to QGuiApplication |
| apps/dde-application-manager/src/CMakeLists.txt | Added Qt6::Gui dependency |
| src/dbus/CMakeLists.txt | Added Wayland protocol generation and Qt WaylandClient dependencies |
| CMakeLists.txt | Added Qt6 WaylandClient, Gui components and TreelandProtocols package |
| debian/control | Added treeland-protocols and qt6-wayland-dev build dependencies |
| tests/CMakeLists.txt | Updated include paths for generated DBus headers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/dbus/applicationservice.cpp:342
- Inconsistent null-checking for
parent(). The splash helper code checks ifparent()is null (lines 325-336), but immediately after, the code usesparent()without null-checking at lines 339, 342, and 367.
Either the null check is unnecessary (if parent is guaranteed to be non-null in production), or the subsequent uses of parent() should also include null checks for consistency and safety. Based on the test setup in tests/ut_applicationmanager.cpp:36, parent() can be null, so the defensive checks should be applied consistently throughout this function.
// Notify the compositor to show a splash screen if in a Wayland session.
if (auto *am = parent()) {
if (auto *helper = am->splashHelper()) {
const auto iconVar = findEntryValue(DesktopFileEntryKey, "Icon", EntryValueType::IconString);
const QString iconName = iconVar.isNull() ? QString{} : iconVar.toString();
qCInfo(amPrelaunchSplash) << "Show prelaunch splash request" << id() << "icon" << iconName;
helper->show(id(), iconName);
} else {
qCInfo(amPrelaunchSplash) << "Skip prelaunch splash (no helper instance)" << id();
}
} else {
qCWarning(amPrelaunchSplash) << "Skip prelaunch splash (no parent ApplicationManager1Service)" << id();
}
optionsMap.remove("_hooks"); // this is internal property, user shouldn't pass it to Application Manager
if (const auto &hooks = parent()->applicationHooks(); !hooks.isEmpty()) {
optionsMap.insert("_hooks", hooks);
}
optionsMap.insert("_builtIn_searchExec", parent()->systemdPathEnv());
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
TAG Bot New tag: 1.2.39 |
Added Wayland prelaunch splash screen functionality to improve user experience when launching applications. The changes include: 1. Added Qt6 WaylandClient and Gui dependencies to support Wayland protocols 2. Replaced QCoreApplication with QGuiApplication to enable GUI features 3. Implemented PrelaunchSplashHelper class using treeland-prelaunch- splash-v1 protocol 4. Added automatic Wayland debug mode when running in Wayland sessions 5. Integrated splash screen triggering during application launch process Log: Added prelaunch splash screen for applications in Wayland sessions Influence: 1. Test application launching in Wayland session to verify splash screen appears 2. Verify no impact on X11 session functionality 3. Test with DDE_AM_DISABLE_WAYLAND_SPLASH environment variable to disable feature 4. Check Wayland debug output when WAYLAND_DEBUG is set 5. Verify application manager starts correctly in both session types 6. Test with various application types to ensure splash screen consistency feat: 添加 Wayland 预启动闪屏支持 新增 Wayland 预启动闪屏功能以改善应用程序启动时的用户体验。主要变更 包括: 1. 添加 Qt6 WaylandClient 和 Gui 依赖以支持 Wayland 协议 2. 将 QCoreApplication 替换为 QGuiApplication 以启用 GUI 功能 3. 使用 treeland-prelaunch-splash-v1 协议实现预启动闪屏助手类 4. 在 Wayland 会话中自动启用 Wayland 调试模式 5. 在应用程序启动过程中集成闪屏触发机制 Log: 为 Wayland 会话中的应用程序添加预启动闪屏功能 Influence: 1. 在 Wayland 会话中测试应用程序启动,验证闪屏是否正常显示 2. 验证对 X11 会话功能无影响 3. 使用 DDE_AM_DISABLE_WAYLAND_SPLASH 环境变量测试功能禁用 4. 检查设置 WAYLAND_DEBUG 时的 Wayland 调试输出 5. 验证应用程序管理器在两种会话类型中都能正常启动 6. 测试不同类型的应用程序以确保闪屏显示一致性
Added qt6-wayland-dev to the Build-Depends list in debian/control file. This dependency is required for proper Wayland support during the build process, ensuring compatibility with Qt6 applications running on Wayland display servers. Influence: 1. Verify build system can locate and use qt6-wayland development headers 2. Test application functionality on Wayland displays 3. Ensure no regression on X11 displays 4. Check build process completes successfully with new dependency chore: 在构建依赖中添加 qt6-wayland-dev 在 debian/control 文件的 Build-Depends 列表中添加了 qt6-wayland-dev。此 依赖项是构建过程中正确支持 Wayland 所必需的,确保与在 Wayland 显示服务器 上运行的 Qt6 应用程序的兼容性。 Influence: 1. 验证构建系统能够定位和使用 qt6-wayland 开发头文件 2. 在 Wayland 显示器上测试应用程序功能 3. 确保在 X11 显示器上没有回归问题 4. 检查构建过程在新依赖项下能成功完成
Changed the Wayland platform detection method from checking platform name string to using Qt's native interface API for more reliable detection. Updated the prelaunch splash protocol to include a sandbox engine identifier as required by the new Wayland protocol specification. The previous implementation relied on string matching of the platform name which could be error-prone. The new approach uses QWaylandApplication native interface which provides definitive Wayland detection. Additionally, the splash protocol now includes a stable sandbox engine identifier "dde-application-manager" as the first parameter to comply with updated Wayland compositor requirements. Log: Improved Wayland compatibility for application prelaunch splash screens Influence: 1. Test application launching on Wayland compositors to verify splash screens appear correctly 2. Verify no regression on X11 platforms where splash should be skipped 3. Check logging output to confirm proper Wayland detection and splash initialization 4. Test with various Qt applications to ensure compatibility with new protocol refactor: 改进 Wayland 检测和闪屏协议 将 Wayland 平台检测方法从检查平台名称字符串改为使用 Qt 原生接口 API, 以实现更可靠的检测。更新了预启动闪屏协议,包含沙盒引擎标识符以满足新的 Wayland 协议规范要求。 之前的实现依赖于平台名称的字符串匹配,这可能容易出错。新方法使用 QWaylandApplication 原生接口,提供了确定的 Wayland 检测。此外,闪屏协议 现在包含一个稳定的沙盒引擎标识符 "dde-application-manager" 作为第一个参 数,以符合更新的 Wayland 合成器要求。 Log: 改进了应用程序预启动闪屏的 Wayland 兼容性 Influence: 1. 在 Wayland 合成器上测试应用程序启动,验证闪屏是否正确显示 2. 验证在 X11 平台上没有回归,闪屏应被跳过 3. 检查日志输出以确认正确的 Wayland 检测和闪屏初始化 4. 使用各种 Qt 应用程序测试,确保与新协议的兼容性
Added support for displaying application icons in prelaunch splash screens. The implementation includes loading application icons from desktop files, converting them to Wayland-compatible buffers, and sending them to the compositor via the prelaunch splash protocol. This enhances user experience by showing recognizable application icons during application startup. Key changes: 1. Modified CMakeLists to link against WaylandClientPrivate for buffer creation 2. Updated application service to extract icon name from desktop file 3. Implemented icon buffer creation with proper size selection and rendering 4. Added support for both file paths and theme icons 5. Maintained backward compatibility for icon-less splash screens Log: Added application icon display to prelaunch splash screens Influence: 1. Test application launches with valid icon names from desktop files 2. Verify splash screen behavior with missing or invalid icons 3. Test both file path icons and theme icons 4. Check compatibility with different icon sizes 5. Verify Wayland session functionality 6. Test fallback behavior when icons are unavailable feat: 为预启动闪屏添加应用图标支持 增加了在预启动闪屏中显示应用图标的功能。实现包括从桌面文件加载应用图标, 将其转换为 Wayland 兼容的缓冲区,并通过预启动闪屏协议发送给合成器。这通 过显示可识别的应用图标来增强用户在应用启动时的体验。 主要变更: 1. 修改 CMakeLists 以链接 WaylandClientPrivate 用于缓冲区创建 2. 更新应用服务以从桌面文件提取图标名称 3. 实现带正确尺寸选择和渲染的图标缓冲区创建 4. 添加对文件路径图标和主题图标的支持 5. 保持无图标闪屏的向后兼容性 Log: 在预启动闪屏中新增应用图标显示功能 Influence: 1. 测试从桌面文件加载有效图标名称的应用启动 2. 验证缺少或无效图标时的闪屏行为 3. 测试文件路径图标和主题图标 4. 检查不同图标尺寸的兼容性 5. 验证 Wayland 会话功能 6. 测试图标不可用时的回退行为
1. Remove unnecessary Impl struct and use direct member variable for icon buffers 2. Fix memory management by keeping icon buffers alive until compositor releases them 3. Add safety check for TREELAND_PROTOCOLS_DATA_DIR environment variable 4. Clean up include order and remove unused includes 5. Add detailed class documentation for better code understanding The changes address memory management issues where icon buffers were being cleared prematurely, potentially causing crashes when the compositor tries to use them. The refactoring simplifies the code structure and ensures proper buffer lifecycle management. Log: Fixed prelaunch splash screen memory management issues Influence: 1. Test prelaunch splash screen display with various applications 2. Verify no crashes occur when multiple applications are launched simultaneously 3. Check that splash screens display correctly with different icon types 4. Test with missing TREELAND_PROTOCOLS_DATA_DIR environment variable 5. Verify memory usage remains stable during application launches fix: 改进预启动启动画面助手的内存管理 1. 移除不必要的 Impl 结构体,直接使用成员变量管理图标缓冲区 2. 修复内存管理问题,确保图标缓冲区在合成器释放前保持存活 3. 添加 TREELAND_PROTOCOLS_DATA_DIR 环境变量的安全检查 4. 清理头文件包含顺序并移除未使用的头文件 5. 添加详细的类文档以提高代码可读性 这些更改解决了图标缓冲区被过早清除导致的内存管理问题,可能在使用时导致崩 溃。重构简化了代码结构并确保正确的缓冲区生命周期管理。 Log: 修复预启动启动画面内存管理问题 Influence: 1. 测试各种应用程序的预启动启动画面显示 2. 验证同时启动多个应用程序时不会发生崩溃 3. 检查不同图标类型的启动画面是否正确显示 4. 测试缺少 TREELAND_PROTOCOLS_DATA_DIR 环境变量的情况 5. 验证应用程序启动期间内存使用保持稳定
Added buffer release listener to properly manage wayland buffer lifecycle. The compositor sends release events when it's done using buffers, and we need to clean up our buffer references accordingly. Previously, wayland buffers were kept alive indefinitely in m_iconBuffers vector. Now we register a buffer release listener that removes buffers from the vector when the compositor signals they are no longer needed. This prevents memory leaks and ensures proper resource management for wayland splash screen buffers. Influence: 1. Test splash screen display with various application icons 2. Verify memory usage doesn't grow with repeated splash screen usage 3. Check that buffers are properly cleaned up after compositor release 4. Test with different icon sizes and formats 5. Verify splash screen functionality remains stable fix: 添加 Wayland 缓冲区释放处理 添加了缓冲区释放监听器以正确管理 Wayland 缓冲区生命周期。当合成器完成使 用缓冲区时会发送释放事件,我们需要相应地清理缓冲区引用。 之前,Wayland 缓冲区被无限期保存在 m_iconBuffers 向量中。现在注册了缓冲 区释放监听器,当合成器发出不再需要缓冲区的信号时,从向量中移除缓冲区。 这可以防止内存泄漏,并确保 Wayland 启动闪屏缓冲区的正确资源管理。 Influence: 1. 测试使用不同应用图标的启动闪屏显示 2. 验证重复使用启动闪屏时内存使用不会增长 3. 检查缓冲区在合成器释放后是否正确清理 4. 测试不同图标大小和格式 5. 验证启动闪屏功能保持稳定
Added qt6-wayland-private-dev and libxkbcommon-dev to Build-Depends These dependencies are required for proper Wayland integration and keyboard handling in the dde-application-manager package Log: Updated build dependencies for Wayland compatibility Influence: 1. Verify package builds successfully with new dependencies 2. Test application functionality under Wayland environment 3. Check keyboard input handling in Wayland sessions 4. Ensure no regression in X11 environment 5. Validate Wayland-specific features work correctly chore: 更新 Wayland 支持的构建依赖 添加 qt6-wayland-private-dev 和 libxkbcommon-dev 到构建依赖 这些依赖是 dde-application-manager 包实现正确 Wayland 集成 和键盘处理所必需的 Log: 更新构建依赖以支持 Wayland 兼容性 Influence: 1. 验证包使用新依赖后能成功构建 2. 在 Wayland 环境下测试应用功能 3. 检查 Wayland 会话中的键盘输入处理 4. 确保 X11 环境下没有回归问题 5. 验证 Wayland 特定功能正常工作
Changed QScopedPointer to std::unique_ptr for better C++ standard compliance and consistency Improved icon selection algorithm to prefer larger sizes (128x128) for better display quality Simplified icon loading by using only QIcon::fromTheme instead of manual path handling Enhanced buffer creation with explicit device pixel ratio handling for high-DPI displays Influence: 1. Test application startup splash screen display with various icon sizes 2. Verify icon rendering quality on different display scales 3. Test with missing icons to ensure graceful fallback behavior 4. Verify memory management and resource cleanup 5. Test compatibility with different Wayland compositors refactor: 从 QScopedPointer 迁移到 std::unique_ptr 将 QScopedPointer 改为 std::unique_ptr 以提高 C++ 标准兼容性和一致性 改进图标选择算法,优先选择更大尺寸(128x128)以获得更好的显示质量 简化图标加载逻辑,仅使用 QIcon::fromTheme 而不是手动路径处理 增强缓冲区创建,显式处理设备像素比以支持高 DPI 显示 Influence: 1. 测试不同图标尺寸下的应用启动闪屏显示 2. 验证不同显示比例下的图标渲染质量 3. 测试缺失图标时的优雅降级行为 4. 验证内存管理和资源清理 5. 测试与不同 Wayland 合成器的兼容性
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d07a2fc to
438c3ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. Changed std::unique_ptr to QScopedPointer for better Qt memory management integration 2. Fixed code formatting by adding space in QSize initialization for consistency 3. Added proper error handling for empty icon names and missing theme icons 4. Added documentation comments for bufferRelease method to clarify its purpose Log: Improved error handling for application splash screen icons Influence: 1. Test application launch with empty icon names to verify warning messages 2. Test with non-existent theme icons to ensure proper error handling 3. Verify memory management changes don't affect application stability 4. Check splash screen display with various icon scenarios 5. Test Wayland buffer release functionality to ensure proper resource cleanup fix: 修复代码评审中的多处问题 1. 将 std::unique_ptr 改为 QScopedPointer 以更好地集成 Qt 内存管理 2. 修复代码格式,在 QSize 初始化中添加空格以保持一致性 3. 添加对空图标名称和缺失主题图标的错误处理 4. 为 bufferRelease 方法添加文档注释以明确其用途 Log: 改进应用程序启动画面图标的错误处理 Influence: 1. 测试使用空图标名称启动应用程序,验证警告消息是否正确显示 2. 测试使用不存在的主题图标,确保错误处理正常 3. 验证内存管理更改不影响应用程序稳定性 4. 检查各种图标场景下的启动画面显示 5. 测试 Wayland 缓冲区释放功能以确保资源正确清理
deepin pr auto review这是一个关于添加 Wayland 预启动启动画面的代码更改。我来分析一下各个方面的改进建议:
改进建议:
// 在 PrelaunchSplashHelper::show 中添加更详细的错误检查
if (!isActive()) {
qCWarning(amPrelaunchSplash) << "Prelaunch splash extension is not active";
return;
}
if (appId.isEmpty()) {
qCWarning(amPrelaunchSplash) << "Application ID cannot be empty";
return;
}
// 在 PrelaunchSplashHelper 中添加资源清理
void PrelaunchSplashHelper::cleanup()
{
// 清理所有未释放的缓冲区
m_iconBuffers.clear();
}
// 添加配置选项来控制是否启用预启动画面
static const bool PRELAUNCH_SPLASH_ENABLED = []() {
return QSettings{}.value("Appearance/EnablePrelaunchSplash", true).toBool();
}();
// 添加图标缓存机制
class PrelaunchSplashHelper {
private:
QCache<QString, QIcon> m_iconCache; // 缓存已加载的图标
};
// 添加互斥锁保护共享资源
class PrelaunchSplashHelper {
private:
QMutex m_bufferMutex;
std::vector<std::unique_ptr<QtWaylandClient::QWaylandShmBuffer>> m_iconBuffers;
};
// 添加单元测试
class TestPrelaunchSplashHelper : public QObject {
Q_OBJECT
private slots:
void testShow();
void testBufferManagement();
void testIconSizeSelection();
};
/**
* @brief 构造预启动画面助手
*
* 初始化 Wayland 扩展,设置必要的回调函数。
* 如果 Wayland 显示不可用,对象将保持非活动状态。
*
* @throws std::runtime_error 如果 Wayland 初始化失败
*/
PrelaunchSplashHelper::PrelaunchSplashHelper()
// 添加资源限制检查
static constexpr int MAX_BUFFER_COUNT = 10; // 限制最大缓冲区数量
if (m_iconBuffers.size() >= MAX_BUFFER_COUNT) {
qCWarning(amPrelaunchSplash) << "Too many pending buffers, skipping new splash";
return;
}这些改进建议主要关注:
建议在实现这些改进时,保持代码的简洁性和可读性,避免过度设计。 |
|
[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 |
Summary by Sourcery
Add Wayland-based prelaunch splash support to the application manager and wire it into app launches.
New Features:
Enhancements:
Build: