Skip to content

fix: fallback dock theme to system settings on wayland#1605

Open
18202781743 wants to merge 1 commit into
linuxdeepin:masterfrom
18202781743:master
Open

fix: fallback dock theme to system settings on wayland#1605
18202781743 wants to merge 1 commit into
linuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743
Copy link
Copy Markdown
Contributor

@18202781743 18202781743 commented May 21, 2026

  1. Disable WallpaperColorManager initialization in WaylandDockHelper
    constructor
  2. Extract wallpaper color management to a separate
    initWallpaperColorManager() method
  3. Connect dock theme changes to DGuiApplicationHelper themeTypeChanged
    signal on Wayland
  4. This ensures dock follows system theme when wallpaper color is
    unavailable

Log: Dock theme on Wayland now falls back to system color settings

Influence:

  1. Test dock color theme toggling between light and dark on Wayland
  2. Verify dock theme syncs with system settings after change
  3. Ensure no regression in dock color behavior on X11
  4. Check that wallpaper color feature is cleanly disabled without errors
  5. Verify dock screen changes don't trigger wallpaper color manager
    calls

fix: wayland下dock暗亮色回退到系统设置

  1. 禁用WaylandDockHelper构造函数中的WallpaperColorManager初始化
  2. 将壁纸颜色管理提取到独立的initWallpaperColorManager()方法中
  3. 在Wayland上连接dock主题变化到DGuiApplicationHelper的themeTypeChanged
    信号
  4. 确保壁纸颜色不可用时dock遵循系统主题

Log: Wayland下Dock主题现在回退到系统颜色设置

Influence:

  1. 在Wayland上测试dock颜色主题在亮色和暗色间切换
  2. 验证dock主题在系统设置更改后同步
  3. 确保X11上dock颜色行为无回归
  4. 检查壁纸颜色功能被干净禁用,无错误产生
  5. 验证dock屏幕变化不会触发壁纸颜色管理器调用

PMS: BUG-344841

Summary by Sourcery

Make the Wayland dock fall back to system theme settings when wallpaper-based colors are unavailable.

Bug Fixes:

  • Ensure the dock color theme on Wayland tracks system light/dark theme changes when wallpaper color data is not available.

Enhancements:

  • Isolate wallpaper color management into an initWallpaperColorManager() helper for future re-enablement.

1. Disable WallpaperColorManager initialization in WaylandDockHelper
constructor
2. Extract wallpaper color management to a separate
initWallpaperColorManager() method
3. Connect dock theme changes to DGuiApplicationHelper themeTypeChanged
signal on Wayland
4. This ensures dock follows system theme when wallpaper color is
unavailable

Log: Dock theme on Wayland now falls back to system color settings

Influence:
1. Test dock color theme toggling between light and dark on Wayland
2. Verify dock theme syncs with system settings after change
3. Ensure no regression in dock color behavior on X11
4. Check that wallpaper color feature is cleanly disabled without errors
5. Verify dock screen changes don't trigger wallpaper color manager
calls

fix: wayland下dock暗亮色回退到系统设置

1. 禁用WaylandDockHelper构造函数中的WallpaperColorManager初始化
2. 将壁纸颜色管理提取到独立的initWallpaperColorManager()方法中
3. 在Wayland上连接dock主题变化到DGuiApplicationHelper的themeTypeChanged
信号
4. 确保壁纸颜色不可用时dock遵循系统主题

Log: Wayland下Dock主题现在回退到系统颜色设置

Influence:
1. 在Wayland上测试dock颜色主题在亮色和暗色间切换
2. 验证dock主题在系统设置更改后同步
3. 确保X11上dock颜色行为无回归
4. 检查壁纸颜色功能被干净禁用,无错误产生
5. 验证dock屏幕变化不会触发壁纸颜色管理器调用

PMS: BUG-344841
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743

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

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 21, 2026

Reviewer's Guide

Refactors Wayland dock wallpaper color handling by disabling WallpaperColorManager initialization on Wayland, extracting it into a dedicated initializer, and wiring the dock’s color theme to DGuiApplicationHelper’s themeTypeChanged signal so that on Wayland the dock falls back to system theme colors when wallpaper-based colors are unavailable.

Sequence diagram for dock theme fallback to system settings on Wayland

sequenceDiagram
    participant DGuiApplicationHelper
    participant DockPanel
    participant WaylandDockHelper
    participant WallpaperColorManager

    Note over DockPanel,WaylandDockHelper: Wayland platform

    alt [wallpaper color available]
        DockPanel->>WaylandDockHelper: initWallpaperColorManager()
        WaylandDockHelper->>WallpaperColorManager: watchScreen(dockScreenName())
        WallpaperColorManager-->>DockPanel: activeChanged
        DockPanel->>DockPanel: setColorTheme(ColorTheme_from_wallpaper)
    else [wallpaper color unavailable]
        DGuiApplicationHelper-->>DockPanel: themeTypeChanged
        DockPanel->>DockPanel: setColorTheme(ColorTheme_from_themeType)
    end
Loading

File-Level Changes

Change Details Files
Disable WallpaperColorManager usage in WaylandDockHelper and move its setup into a dedicated initializer for future reuse.
  • Remove immediate construction of WallpaperColorManager from WaylandDockHelper constructor.
  • Remove connections from DockPanel and wallpaper color signals that were previously established in the WaylandDockHelper constructor.
  • Introduce initWallpaperColorManager() that encapsulates WallpaperColorManager creation, signal wiring, and initial watchScreen setup without invoking it yet.
panels/dock/waylanddockhelper.cpp
panels/dock/waylanddockhelper.h
Make Wayland dock theme follow DGuiApplicationHelper’s themeTypeChanged signal as a fallback when wallpaper colors are not available.
  • Initialize dock theme from DGuiApplicationHelper::themeType regardless of platform.
  • On Wayland platform, create WaylandDockHelper but additionally connect DGuiApplicationHelper::themeTypeChanged to DockPanel::setColorTheme for fallback theme synchronization.
  • Retain existing X11 (xcb) themeTypeChanged connection for regression safety.
panels/dock/dockpanel.cpp

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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX,你的智能编程助手。我已经仔细审查了你提供的 Git Diff。本次修改主要针对 Wayland 环境下的 Dock 面板主题颜色获取逻辑进行了重构,将 WallpaperColorManager 的初始化延迟提取到了 initWallpaperColorManager() 方法中,并在 Wayland 分支中临时增加了通过 DGuiApplicationHelper 获取主题颜色的 Fallback 逻辑。

以下是针对语法逻辑、代码质量、代码性能和代码安全四个方面的详细审查意见及改进建议:

1. 语法与逻辑

  • 潜在问题:initWallpaperColorManager() 未被调用
    • 现象:Diff 中将原本在 WaylandDockHelper 构造函数里的 WallpaperColorManager 初始化及信号连接代码移到了新方法 initWallpaperColorManager() 中,但没有在构造函数或其它任何地方调用该方法。这会导致 m_wallpaperColorManager 始终为空,相关功能完全失效。
    • 建议:如果当前确实不需要该功能(正如 TODO 注释所言),请确保逻辑自洽;如果未来需要重新启用,请在适当位置(如构造函数末尾,或通过外部条件触发)调用 initWallpaperColorManager()
  • 潜在问题:Fallback 逻辑与原逻辑的互斥性
    • 现象:在 dockpanel.cpp 中,Wayland 分支新增了监听 DGuiApplicationHelper::themeTypeChanged 的 Fallback 逻辑。未来当 initWallpaperColorManager 被重新启用时,Wayland 下会同时存在两套主题变更通知(基于壁纸颜色和基于全局主题)。如果两者触发时机不同,可能会导致主题颜色被频繁覆盖或闪烁。
    • 建议:在重新启用 initWallpaperColorManager 时,必须增加互斥逻辑。例如:当 WallpaperColorManager 成功获取到壁纸颜色时,应断开 Fallback 的连接;或者增加判断,如果壁纸颜色有效则忽略全局主题的信号。
  • 逻辑一致性xcb 分支的信号连接使用了 this 作为 receiver,而新增的 wayland 分支的 Fallback lambda 没有传递 this 作为 receiver(虽然 lambda 捕获了 this,上下文仍在当前对象)。建议保持代码风格一致,加上 this 以确保当 DockPanel 销毁时,信号能被正确断开。

2. 代码质量

  • 代码重复
    • 现象dockpanel.cpp 中 Wayland 和 Xcb 两个分支的 Fallback 逻辑几乎完全一致:
      QObject::connect(Dtk::Gui::DGuiApplicationHelper::instance(), &Dtk::Gui::DGuiApplicationHelper::themeTypeChanged,
                       this, [this]() {
          setColorTheme(static_cast<ColorTheme>(Dtk::Gui::DGuiApplicationHelper::instance()->themeType()));
      });
    • 建议:可以将这段公共逻辑提取到 if-else 语句块的外部,减少代码冗余。只有在两个分支的逻辑产生分化(例如未来 Wayland 需要移除该连接)时,再分别处理。
  • 版权声明修改的合理性
    • 现象:修改了 SPDX 版权声明从 20242024 - 2026
    • 建议:通常版权年份应记录的是代码首次创建或最后一次重大修改的年份,写未来的结束年份(2026)并不符合常规规范。建议确认是否为笔误,通常格式为 20242022 - 2024

3. 代码性能

  • 信号槽连接优化
    • 现象:新增的信号槽连接使用了 Lambda,且在 Lambda 内部调用了 DGuiApplicationHelper::instance()->themeType()
    • 建议:虽然对单次 UI 事件影响微乎其微,但更优的写法是直接将信号连接到 setColorTheme 方法,并利用 Qt 的自动类型转换,避免 Lambda 的开销和额外的函数调用。不过由于 themeTypeChanged 信号可能不带参数,或者参数类型与 ColorTheme 不匹配,如果必须进行 static_cast,当前 Lambda 写法是可接受的。
  • 对象生命周期与内存
    • 现象m_wallpaperColorManager.reset(new WallpaperColorManager(this)); 传入了 this 作为父对象。
    • 建议:这是 Qt 规范的良好实践,确保了内存不泄漏。但需注意,如果未来多次调用 initWallpaperColorManager()reset 会销毁旧对象并创建新对象,此时需确保旧对象的信号槽连接被正确清理(reset 销毁 QObject 时会自动断开信号槽,这点 Qt 已经处理好了,因此是安全的)。

4. 代码安全

  • Lambda 捕获导致的悬垂引用风险
    • 现象:Wayland 分支新增的 Lambda 使用了 [this]() 捕获。如果 DGuiApplicationHelper 的生命周期长于 DockPanel,当 DockPanel 析构时,如果信号触发,就会产生悬垂指针崩溃。
    • 建议:在 QObject::connect 中显式传入上下文对象 this。这样当 DockPanel 析构时,连接会自动断开,确保安全。当前 Diff 中的 Xcb 分支也没有加 this 作为 receiver,这是一个潜在的安全隐患。
  • 类型强转的边界检查
    • 现象static_cast<ColorTheme>(DGuiApplicationHelper::instance()->themeType()) 进行了强转。
    • 建议:如果 themeType() 返回的枚举值超出了 ColorTheme 的定义范围,static_cast 不会报错,但会导致未定义行为。建议在 setColorTheme 方法内部增加对入参的有效性校验(如 Q_ASSERT 或边界判断),增强鲁棒性。

💡 综合改进建议代码示例

基于以上分析,我为你优化了 dockpanel.cpp 中的代码结构,消除了重复,并修复了潜在的内存安全问题:

// panels/dock/dockpanel.cpp

bool DockPanel::init()
{
    m_theme = static_cast<ColorTheme>(Dtk::Gui::DGuiApplicationHelper::instance()->themeType());
    auto platformName = QGuiApplication::platformName();
    
    // 提取公共 Fallback 逻辑,消除代码重复,并增加 this 作为 receiver 保证对象析构时安全断开连接
    // TODO: remove this when initWallpaperColorManager is re-enabled for Wayland
    QObject::connect(Dtk::Gui::DGuiApplicationHelper::instance(), &Dtk::Gui::DGuiApplicationHelper::themeTypeChanged,
                     this, [this]() {
        setColorTheme(static_cast<ColorTheme>(Dtk::Gui::DGuiApplicationHelper::instance()->themeType()));
    });

    if (QStringLiteral("wayland") == platformName) {
        m_helper = new WaylandDockHelper(this);
        // Fallback to DGuiApplicationHelper for theme color when wayland wallpaper color is not available.
        // 注意:未来重新启用 initWallpaperColorManager 时,需在此处断开上述 Fallback 连接
    } else if (QStringLiteral("xcb") == platformName) {
        // xcb 的逻辑已经被上方的公共逻辑取代,如无特殊需求可留空或删除原有重复代码
    }
    // ... 其他逻辑
}

对于 waylanddockhelper.cpp,请务必确认 initWallpaperColorManager() 的调用时机,如果当前阶段确实不需要,建议在头文件和源文件中通过宏或更明确的注释标明该功能当前处于禁用状态,以免后续维护者遗漏调用。

Copy link
Copy Markdown

@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 left some high level feedback:

  • The new initWallpaperColorManager() helper is currently unused; consider either wiring it behind a clear feature flag or removing it for now to avoid dead code and keep the Wayland path easier to reason about.
  • In DockPanel::init() on Wayland, the themeTypeChanged connection is created unconditionally; if init() can be called more than once, consider using Qt::UniqueConnection or moving this connection to the constructor to avoid accumulating duplicate connections.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `initWallpaperColorManager()` helper is currently unused; consider either wiring it behind a clear feature flag or removing it for now to avoid dead code and keep the Wayland path easier to reason about.
- In `DockPanel::init()` on Wayland, the `themeTypeChanged` connection is created unconditionally; if `init()` can be called more than once, consider using `Qt::UniqueConnection` or moving this connection to the constructor to avoid accumulating duplicate connections.

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.

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.

2 participants