Skip to content

Conversation

@dengzhongyuan365-dev
Copy link

feat: Improve countdown tooltip positioning for multi-screen scenarios

  • Enhanced the logic for positioning the countdown tooltip when the recording area spans multiple screens. The tooltip now appears centered on the relevant screen, improving visibility and user experience.
  • Added detailed comments in the code to clarify the positioning logic based on the toolbar's screen context.

This update addresses potential confusion during screen recording across multiple displays.

优化了录制区域跨屏时倒计时提示框的定位逻辑。

bug: https://pms.uniontech.com/story-view-39101.html

- Enhanced the logic for positioning the countdown tooltip when the recording area spans multiple screens. The tooltip now appears centered on the relevant screen, improving visibility and user experience.
- Added detailed comments in the code to clarify the positioning logic based on the toolbar's screen context.

This update addresses potential confusion during screen recording across multiple displays.

优化了录制区域跨屏时倒计时提示框的定位逻辑。

bug: https://pms.uniontech.com/story-view-39101.html
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码进行审查:

  1. 代码逻辑审查:
  • 代码实现了跨屏录制时倒计时位置的智能调整,逻辑清晰完整
  • 首先判断录制区域是否跨屏,然后根据工具栏位置决定倒计时显示位置
  • 处理了各种边界情况,如找不到屏幕、工具栏跨屏等场景
  • 代码注释清晰,包含了需求链接和实现说明
  1. 代码质量建议:
  • 可以将跨屏判断逻辑抽取为独立函数,提高代码复用性
  • 建议将坐标计算相关的常量定义为宏或常量,提高可维护性
  • 可以考虑使用QRect的center()方法来简化中心点计算
  1. 性能优化建议:
  • 多次调用QGuiApplication::screenAt()可以考虑缓存结果
  • 可以将频繁使用的几何计算结果缓存起来,避免重复计算
  1. 安全性建议:
  • 建议对m_pixelRatio进行有效性检查,防止除零错误
  • 对targetScreenRect.isEmpty()的判断可以更严格,应该同时检查width和height
  • 坐标计算时应该进行边界检查,防止计算结果超出屏幕范围
  1. 改进建议代码:
// 添加辅助函数判断是否跨屏
bool isAreaCrossScreen(const QRect& area, QScreen* screen) {
    if (!screen) return true;
    QRect screenGeom = screen->geometry();
    return !screenGeom.contains(area);
}

// 在类中添加成员变量缓存计算结果
QScreen* m_cachedScreen = nullptr;
QRect m_cachedScreenRect;

void MainWindow::startCountdown() {
    // ... 其他代码 ...
    
    // 使用缓存的屏幕信息
    if (!m_cachedScreen) {
        m_cachedScreen = QGuiApplication::screenAt(recordRect.center());
    }
    
    bool isRecordAreaCrossScreen = isAreaCrossScreen(
        QRect(static_cast<int>(recordRect.x() / m_pixelRatio),
              static_cast<int>(recordRect.y() / m_pixelRatio),
              static_cast<int>(recordRect.width() / m_pixelRatio),
              static_cast<int>(recordRect.height() / m_pixelRatio)),
        m_cachedScreen);

    // ... 其他代码 ...

    // 使用QRect的center()方法简化计算
    if (!isRecordAreaCrossScreen) {
        QPoint center = recordRect.center();
        countdownX = center.x() / m_pixelRatio - countdownTooltip->width() / 2;
        countdownY = center.y() / m_pixelRatio - countdownTooltip->height() / 2;
    }

    // 添加边界检查
    countdownX = qBound(targetScreenRect.left(), countdownX, 
                       targetScreenRect.right() - countdownTooltip->width());
    countdownY = qBound(targetScreenRect.top(), countdownY, 
                       targetScreenRect.bottom() - countdownTooltip->height());

    countdownTooltip->move(countdownX, countdownY);
}

这些改进可以提高代码的可维护性、性能和安全性,同时保持原有功能不变。

@deepin-ci-robot
Copy link

[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
Author

/forcemerge

@deepin-bot deepin-bot bot merged commit 4f99447 into linuxdeepin:develop/snipe Dec 25, 2025
9 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