Skip to content

fix: refresh window icons on icon theme change#302

Merged
mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy:icon
Feb 3, 2026
Merged

fix: refresh window icons on icon theme change#302
mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy:icon

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Feb 3, 2026

  1. Added code to refresh window icons when icon theme changes
  2. Iterates through all top-level windows and re-sets their icons
  3. This ensures window icons are properly updated after theme switch
  4. Fixes issue where window icons might not refresh after theme change

fix: 修复图标主题变更时窗口图标刷新问题

  1. 添加代码在图标主题变更时刷新窗口图标
  2. 遍历所有顶层窗口并重新设置其图标
  3. 确保主题切换后窗口图标能正确更新
  4. 修复主题变更后窗口图标可能不刷新的问题

PMS: BUG-271403
PMS: BUG-326433
PMS: BUG-321407

1. Added code to refresh window icons when icon theme changes
2. Iterates through all top-level windows and re-sets their icons
3. This ensures window icons are properly updated after theme switch
4. Fixes issue where window icons might not refresh after theme change

fix: 修复图标主题变更时窗口图标刷新问题

1. 添加代码在图标主题变更时刷新窗口图标
2. 遍历所有顶层窗口并重新设置其图标
3. 确保主题切换后窗口图标能正确更新
4. 修复主题变更后窗口图标可能不刷新的问题

PMS: BUG-271403
PMS: BUG-326433
PMS: BUG-321407
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这段代码的目的是在图标主题变更时(onIconThemeSetCallback 函数中),刷新所有顶层窗口的图标。虽然逻辑上看似可行,但在代码质量、性能和安全性方面存在一些改进空间。以下是详细的审查意见和改进建议:

1. 代码逻辑与功能

  • 潜在无效操作window->setIcon(window->icon()) 这行代码试图通过将图标设置为它自己来触发刷新。
    • 问题:在 Qt 的 QWindow 实现中,如果传入的 QIcon 与当前持有的 QIcon 完全相同(共享数据),setIcon 内部通常会直接返回,不执行任何更新操作。这意味着这段代码可能完全无法达到刷新图标的目的
  • 建议:如果目标是强制刷新图标以应用新主题,通常需要触发窗口的重新绘制或属性更新。如果这是 DTK/Deepin 特有的 hack,请确保底层实现支持这种“自设”刷新。否则,可能需要寻找其他 API(例如 update() 或触发窗口属性变化)。

2. 代码质量 (C++ & Qt 规范)

  • 变量声明位置const QWindowList list = ... 的声明位置符合 C++ 规范,但在 Qt 习惯中,通常直接在 for 循环中使用基于范围的 for 循环(C++11)或 foreach(Qt 风格),这样代码更简洁且不易出错。
  • 不必要的索引访问:使用 list.at(i) 并结合索引 i 是旧式 C++ 的写法。
  • 建议:使用基于范围的 for 循环(Range-based for loop)。

3. 代码性能

  • 拷贝开销const QWindowList list = ... 会触发一次 QList 的拷贝构造。虽然 QList 是隐式共享的,拷贝开销很小,但在高频调用的场景下(虽然主题切换通常不是高频),直接使用引用或 const 引用是更好的习惯。
  • 建议:使用 const QWindowList &list 避免不必要的引用计数操作。

4. 代码安全

  • 空指针检查冗余QWindowList::at() 返回的是指针,且 QList 本身不存储 nullptr(除非显式插入)。虽然 if (QWindow *window = list.at(i)) 检查是安全的,但在 topLevelWindows() 返回的列表中通常不需要这么严格的防御性编程,除非有插件在列表中插入了空指针。
  • 建议:可以简化检查逻辑,除非上下文环境非常复杂。

5. 改进后的代码建议

假设 window->setIcon(window->icon()) 确实是预期的逻辑(尽管可能无效),建议修改如下:

    // 刷新窗口图标
    // 使用 const 引用避免列表拷贝
    const QWindowList &list = QGuiApplication::topLevelWindows();
    
    // 使用基于范围的 for 循环 (C++11),更简洁安全
    for (QWindow *window : list) {
        if (window) {
            // 注意:如果底层实现比较指针地址,这行代码可能无效。
            // 如果需要强制刷新,可能需要先 setIcon(QIcon()) 再 setIcon(originalIcon)
            window->setIcon(window->icon());
        }
    }

6. 进一步的思考(关于逻辑有效性)

如果上述代码在实际运行中没有效果(因为 setIcon 检测到图标没变),建议采用以下强制刷新策略:

    // 保存原始图标
    QIcon originalIcon = window->icon();
    // 先设置为空图标,强制清除状态
    window->setIcon(QIcon());
    // 重新设置原始图标,强制触发更新
    window->setIcon(originalIcon);

总结
主要问题在于 window->setIcon(window->icon()) 可能是一个无效操作。建议先验证该操作是否能生效。如果无效,请采用“置空再重设”的策略。代码风格上,建议使用 C++11 的范围 for 循环和 const 引用来提升代码可读性和性能。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

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

@mhduiy mhduiy merged commit f12a2f1 into linuxdeepin:master Feb 3, 2026
20 of 21 checks passed
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.

3 participants