fix: Multi-screen launcher display anomaly

[JWWTSL]

log: When retrieving wallpaper, the function GetCurrentWorkspaceBackgroundForMonitor was called using only the main screen or an incorrect screen name instead of the screen where the launcher currently resides. If this fails to return a valid result, the wallpaper won't update. Consequently, the appearance service consistently returns the main screen's wallpaper.

pms: bug-343521

[sourcery-ai]

Sorry @JWWTSL, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[BLumia]

看这里写了反复多次检查，看上去意思是之前 LauncherController::instance().currentScreen() 返回的结果可能是个无效的屏幕名称？如果是的话，是不是应该看为什么 LauncherController::instance().currentScreen() 会返回无效的名称？

可以问问 @pengfeixx

[deepin-ci-robot]

deepin pr auto review

这段代码修改主要是为了优化屏幕选择逻辑，增强代码的健壮性，并添加了更完善的回退机制。以下是对这段 diff 的详细审查意见：

1. 语法与逻辑审查

• 逻辑清晰度（优）：
  • 修改后的代码引入了一个 targetScreen 指针，并建立了一个清晰的优先级链条：LauncherController -> Dock Geometry -> Cursor Position -> Primary Screen。这比原代码中多次检查 screenName 字符串是否为空的结构更加线性，易于阅读和维护。
• Lambda 表达式使用（优）：
  • 引入 screenByName lambda 函数用于通过名称查找 QScreen 指针，这避免了在多处重复遍历 qApp->screens()，代码复用性更好。
• 边界条件处理（优）：
  • 修改后的代码在获取 targetScreen 指针后，统一在最后检查 screenName.isEmpty() 以及 m_dbusAppearanceIface->isValid()，确保了后续 DBus 调用的安全性。
  • 对 dockGeometry 的检查增加了 width() > 0 && height() > 0，比原代码仅检查 isValid() 和 x() >= 0 更为严谨，防止宽或高为 0 的无效矩形导致 screenAt 返回错误结果。

2. 代码质量审查

• 注释与文档（优）：
  • 添加的 TODO 注释非常有价值，解释了为什么需要回退逻辑（QML 设置的时序问题），并指出了潜在的简化方向。这有助于后续维护者理解业务背景。
• 变量命名（优）：
  • requestedScreenName 和 targetScreen 命名准确，清晰表达了数据的来源和用途。
• 防御性编程（优）：
  • 所有可能返回空指针的操作（如 screenAt）都紧接着进行了判空处理，符合 C++ Qt 开发的最佳实践。

3. 代码性能审查

• 屏幕遍历（建议）：
  • screenByName lambda 函数在最坏情况下（屏幕未找到或名称为空）会遍历所有屏幕。虽然系统中屏幕数量通常很少（通常 < 10），性能影响微乎其微，但如果此函数被高频调用，建议考虑使用 QMap 缓存屏幕名称到指针的映射，或者监听屏幕添加/移除信号来维护映射。
  • 当前实现对于壁纸更新这种低频场景是完全可以接受的。
• DBus 调用（优）：
  • 使用了 QDBusPendingReply 和 Watcher 进行异步调用，不会阻塞主线程，这是正确的做法。

4. 代码安全审查

• 空指针解引用风险（已解决）：
  • 代码逻辑确保了 targetScreen 在被使用前一定经过了有效性检查，且最后通过 targetScreen ? targetScreen->name() : QString() 安全地获取了字符串，消除了空指针解引用的风险。
• DBus 接口有效性（优）：
  • 在发起 DBus 请求前检查了 m_dbusAppearanceIface->isValid()，防止向无效接口发送请求导致崩溃或警告。

5. 改进建议

虽然这段代码质量已经很高，但仍有以下微小的优化空间：

1. 逻辑简化（基于 TODO）：

   • 正如 TODO 中所言，如果 LauncherController::instance().currentScreen() 在大多数情况下总是空的，且 Dock 和 Launcher 总是在同一屏幕，可以考虑直接优先使用 dockGeometry。不过目前的逻辑已经非常健壮，保留现状也是一种保守且安全的策略。

2. 代码风格（微调）：

   • if (!targetScreen) targetScreen = ... 这种写法虽然紧凑，但在某些代码规范中建议即使只有一行语句也加上大括号 {}，或者保持一致的缩进风格，以降低未来添加日志或断言时的出错风险。

3. 屏幕查找的效率（针对极端情况）：

   • screenByName 中的 s->name() 涉及字符串比较。虽然 Qt 内部实现通常很快，但为了极致性能，可以比较指针地址（如果 LauncherController 存储的是 QScreen 指针）。不过鉴于当前接口返回的是 QString，目前的实现是合理的。

总结：
这段 diff 是一个高质量的代码改进。它修复了潜在的时序问题，增强了屏幕选择的鲁棒性，代码结构清晰，逻辑严密，且没有引入明显的性能或安全隐患。建议合并。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, JWWTSL

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment