Skip to content

fix(topWindow): filter out dde-shell windows by correct WM_CLASS values#841

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-5-21
May 19, 2026
Merged

fix(topWindow): filter out dde-shell windows by correct WM_CLASS values#841
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-5-21

Conversation

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor

Add "dde-shell/dock" and "org.deepin.dde-shell" to the exclusion list
for topWindow screenshot, matching actual WM_CLASS values returned by
dde-shell.

补充 "dde-shell/dock" 和 "org.deepin.dde-shell" 到窗口截图排除列表,
匹配 dde-shell 实际返回的 WM_CLASS 值。

Log: 修复Alt+PrintScreen截图误捕获dde-shell窗口的问题
PMS: BUG-361527
Influence: 修复后Alt+PrintScreen截图将正确跳过桌面环境窗口,不再误截dde-shell/dock

  Add "dde-shell/dock" and "org.deepin.dde-shell" to the exclusion list
  for topWindow screenshot, matching actual WM_CLASS values returned by
  dde-shell.

  补充 "dde-shell/dock" 和 "org.deepin.dde-shell" 到窗口截图排除列表,
  匹配 dde-shell 实际返回的 WM_CLASS 值。

  Log: 修复Alt+PrintScreen截图误捕获dde-shell窗口的问题
  PMS: BUG-361527
  Influence: 修复后Alt+PrintScreen截图将正确跳过桌面环境窗口,不再误截dde-shell/dock
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。这次修改的主要目的是将硬编码的wmClass字符串比较重构为基于容器的查找,并扩展了排除列表。整体思路是正确的,提高了代码的可扩展性。

但在语法逻辑、代码质量、性能和安全方面,我有以下改进意见:

1. 语法与逻辑

  • 逻辑正确性:修改前后的逻辑等价性是正确的。将||条件转换为集合的contains查找,逻辑清晰,没有问题。

2. 代码性能

  • QVector vs QSet:当前使用的是 QVector<QString>QVector::contains 的时间复杂度是 O(N)。虽然目前列表中只有4个元素,性能损耗可以忽略不计,但从数据结构语义和最优实践来看,进行存在性测试应该使用 QSet,其 contains 时间复杂度为 O(1)
  • Qt::CaseSensitivitywmClass 在Linux窗口系统中(如X11的WM_CLASS)通常是区分大小写的。QVector::contains 默认区分大小写,这与原本的 == 行为一致,这是正确的。如果未来需要不区分大小写的匹配,QSet 结合自定义哈希或转小写处理会更灵活。

3. 代码质量

  • 现代C++容器初始化static const QVector<QString> excludedWmClasses = { ... }; 使用的是C++11的初始化列表。每次程序启动到该作用域时,会构造一个临时的初始化列表,再拷贝/移动给 QVector。更高效、更现代的写法是直接使用 QSet 的构造函数。
  • 字符串构造优化window->wmClass() 返回的可能是 QStringconst QString&。将其赋值给 QString wmClass 时,如果是返回值,可能会触发一次不必要的拷贝构造(尽管Qt有隐式共享机制,但使用 const auto& 是零成本的最佳实践)。

4. 代码安全

  • 空指针解引用风险:代码中使用了 window->wmClass(),但并未检查 window 指针是否为空。如果在并发环境下窗口被销毁,或者传入的 window 本身就是 nullptr,这里会发生崩溃。建议增加空指针检查。
  • 潜在的字符串截断或编码问题:X11 的 WM_CLASS 确实可能包含空字符或非UTF-8编码,但既然原代码已经信任了 wmClass() 的封装,这里保持一致即可,风险较低。

改进后的代码建议

// 建议增加 window 的空指针检查,防止崩溃
if (!window) {
    continue;
}

// 使用 const auto& 避免不必要的拷贝,配合 Qt 隐式共享更安全高效
const auto& wmClass = window->wmClass();

// 使用 QSet 替代 QVector,O(1) 查找性能,更符合语义
// 使用构造函数直接初始化,避免中间临时变量的开销
static const QSet<QString> excludedWmClasses = {
    "dde-dock", 
    "dde-shell", 
    "dde-shell/dock", 
    "org.deepin.dde-shell"
};

if (excludedWmClasses.contains(wmClass)) {
    qCDebug(dsrApp) << "topWindow excluded by wmClass:" << wmClass << ", continue";
    continue;
}

改进点总结:

  1. 增加安全性:添加了 if (!window) 检查,防止空指针解引用导致的Segmentation Fault。
  2. 提升性能:将 QVector 替换为 QSet,将查找复杂度从 O(N) 降至 O(1)。
  3. 消除冗余拷贝:将 QString wmClass = ... 改为 const auto& wmClass = ...,利用常量引用避免对象拷贝。
  4. 优化初始化:使用 QSet 直接构造初始化列表,代码更简洁高效。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

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

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 19, 2026

This pr cannot be merged! (status: unstable)

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 19, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit 4a134f5 into linuxdeepin:master May 19, 2026
9 of 10 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