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

[deepin-ci-robot]

deepin pr auto review

这份代码修改主要涉及 appearance.cpp 文件中的 Appearance 类，主要对日志输出、代码结构、屏幕查找逻辑以及错误处理进行了优化。以下是详细的审查意见：

1. 代码逻辑与功能变更审查

• 屏幕查找逻辑重构 (updateCurrentWallpaperBlurhash)：

  • 变更：引入了 screenByName lambda 函数，并改变了屏幕查找的回退逻辑。
  • 审查：
    • 优点：新的逻辑更加清晰且健壮。原代码中，如果 LauncherController 提供的屏幕名称无效，会尝试根据 Dock 位置计算屏幕。新代码保留了这一逻辑，但代码结构更紧凑。它增加了一个最终的回退链：Dock位置 -> 鼠标位置 -> 主屏幕。这确保了在几乎所有情况下都能获取到一个有效的 QScreen 指针，避免了后续使用空指针的风险。
    • 注意：原代码注释中提到优先级是 "Dock position, 3. Cursor position"，但原实现似乎只做了 Dock 的判断。新代码显式地实现了这个完整的回退链，这是一个改进。

• DBus 异步调用处理 (updateCurrentWallpaperBlurhash)：

  • 变更：简化了 QDBusPendingCallWatcher 的回调 lambda 表达式，将 call->deleteLater() 移到了函数开头，并移除了大量的调试日志。
  • 审查：
    • 优点：将 call->deleteLater() 移至开头是一个好的习惯（RAII 风格），防止在中间 return 时忘记释放资源。
    • 优点：移除 reply.isError() 的日志分支，直接 return，减少了代码嵌套层级，提高了可读性。

• JSON 解析 (updateAllWallpaper)：

  • 变更：合并了 JSON 解析错误的检查条件。
  • 审查：将 if (err.error ...) 和 if (!doc.isObject()) 合并为 if (err.error ... || !doc.isObject())，逻辑上等价，但代码更简洁。

• 属性设置 (setOpacity)：

  • 变更：移除了日志输出，简化了判断逻辑。
  • 审查：对于简单的 setter 函数，移除详细的调试日志是合理的，可以减少运行时的开销。

2. 代码质量与可读性

• 移除冗余注释和日志：

  • 审查：代码移除了大量如 // Use a delayed call... 或 qCDebug(...) << "Opacity changed..." 的非关键性注释和日志。
  • 意见：正面。这减少了代码噪音，使核心逻辑更突出。但在调试复杂问题时，关键路径（如屏幕回退选择、DBus 错误）的日志缺失可能会增加排查难度。建议保留最关键的错误日志（如 DBus 调用失败），或者仅在 Debug 模式下编译这些日志。

• Lambda 表达式使用：

  • 审查：screenByName lambda 的使用封装了重复的屏幕查找逻辑，提高了代码的模块化程度。
  • 意见：正面。这使得 updateCurrentWallpaperBlurhash 函数的主流程更清晰。

3. 代码性能

• 字符串与容器操作：
  • 审查：screenByName 中遍历了 qApp->screens()。虽然屏幕数量通常很少（1-4个），性能影响微乎其微，但如果此函数被高频调用，可能会有轻微开销。
  • 意见：可接受。考虑到上下文（壁纸模糊哈希更新），这通常不是高频操作。
• DBus 调用：
  • 审查：保持了异步调用机制，没有阻塞 UI 线程。
  • 意见：正面。

4. 代码安全

• 空指针检查：
  • 审查：在获取 targetScreen 后，代码检查了 targetScreen 是否为空，并在使用 screenName 前进行了非空检查。
  • 意见：正面。新代码通过增加回退链（Cursor -> Primary Screen）显著降低了 targetScreen 为空的可能性，提高了鲁棒性。
• 资源释放：
  • 审查：如前所述，call->deleteLater() 的位置调整确保了 QDBusPendingCallWatcher 对象一定会被删除，防止了潜在的内存泄漏。
  • 意见：正面。

5. 改进建议

尽管代码已经有所优化，但仍有以下几点可以进一步完善：

1. 关键错误日志保留：
   在 updateCurrentWallpaperBlurhash 的 DBus 回调中：

   if (reply.isError()) {
       // 建议保留一行警告日志，否则静默失败会让问题难以排查
       // qCWarning(logDdeIntegration) << "Failed to get wallpaper:" << reply.error().message();
       return;
   }

   同样在 updateAllWallpaper 中：

   if (err.error != QJsonParseError::NoError || !doc.isObject()) {
       // 建议保留解析错误的日志
       // qCWarning(logDdeIntegration) << "Invalid wallpaper data format";
       return;
   }

2. screenByName 的逻辑优化：
   目前的实现是遍历所有屏幕。Qt 的 QGuiApplication::screens() 返回的是列表。如果屏幕名称唯一性有保证，逻辑没问题。如果考虑极端情况，可以增加对 s 指针本身的非空检查（虽然 qApp->screens() 通常不返回空指针，但防御性编程总是好的）：

   auto screenByName = [](const QString &name) -> QScreen * {
       if (name.isEmpty()) return nullptr;
       const auto screens = qApp->screens();
       for (QScreen *s : screens) {
           if (s && s->name() == name) return s;
       }
       return nullptr;
   };

3. dockGeometry 的有效性检查：
   在判断 dockGeometry 时，原代码检查了 x() >= 0，新代码改为了 width() > 0 && height() > 0。

   • 建议：通常检查 isValid() (即 !isEmpty()) 就足够了。如果 width 和 height 都大于 0，isValid 必定为真。目前的改动是安全的，但 isValid() 语义更清晰。

总结

这份 Diff 展示了一次高质量的代码清理工作。它成功地移除了冗余的日志和注释，简化了控制流，并增强了屏幕选择的健壮性。代码变得更整洁、更安全。唯一的潜在风险是完全移除错误日志可能会在后续维护中增加调试难度，建议适度保留关键路径的警告信息。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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:

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