Skip to content

fix(toolbar): improve toolbar positioning on HiDPI displays#825

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-4-16
Apr 16, 2026
Merged

fix(toolbar): improve toolbar positioning on HiDPI displays#825
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-4-16

Conversation

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor

Adjusted the toolbar positioning logic to account for pixel ratio differences in Qt6+XCB environments. The changes ensure that the toolbar remains within screen bounds by applying consistent boundary checks using logical coordinates. This fix addresses potential offscreen positioning issues when the toolbar is displayed on high-DPI screens.

bug: https://pms.uniontech.com/bug-view-354679.html

Adjusted the toolbar positioning logic to account for pixel ratio differences in Qt6+XCB environments. The changes ensure that the toolbar remains within screen bounds by applying consistent boundary checks using logical coordinates. This fix addresses potential offscreen positioning issues when the toolbar is displayed on high-DPI screens.

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

deepin pr auto review

代码审查意见

1. 总体评价

这段代码主要处理了多屏幕环境下工具栏位置计算和倒计时显示位置的问题,特别是针对Qt6+XCB环境下的坐标系统差异进行了处理。代码整体逻辑清晰,但存在一些可以优化的地方。

2. 语法与逻辑问题

2.1 坐标系统转换的重复代码

在多个地方(updateToolBarPosgetTwoScreenIntersectPos函数)重复了相同的坐标转换逻辑:

const int screenX = Utils::isQt6XcbEnv ? static_cast<int>(m_screenInfo[i].x / m_pixelRatio) : m_screenInfo[i].x;
const int screenY = Utils::isQt6XcbEnv ? static_cast<int>(m_screenInfo[i].y / m_pixelRatio) : m_screenInfo[i].y;
const int screenW = static_cast<int>(m_screenInfo[i].width / m_pixelRatio);
const int screenH = static_cast<int>(m_screenInfo[i].height / m_pixelRatio);

建议:将这些转换逻辑提取为私有辅助函数或lambda表达式,减少代码重复。

2.2 getTwoScreenIntersectPos函数中的lambda表达式

auto toLogicalPos = [&](int v) -> int {
    return Utils::isQt6XcbEnv ? static_cast<int>(v / ratio) : v;
};
auto toLogicalSize = [&](int v) -> int {
    return static_cast<int>(v / ratio);
};

问题

  1. toLogicalSize函数没有考虑Utils::isQt6XcbEnv条件,与toLogicalPos不一致
  2. 注释说明m_screenInfo.width/height为物理像素存储,但代码中未明确验证这一点

建议:统一处理逻辑,确保一致性。

3. 代码质量问题

3.1 魔法数字

代码中存在多个未定义的常量:

  • TOOLBAR_Y_SPACING
  • SIDEBAR_X_SPACING

建议:确保这些常量在类或命名空间中正确定义,并添加注释说明其用途。

3.2 复杂的条件判断

startCountdown函数中的跨屏检测逻辑较为复杂:

if (Utils::isQt6XcbEnv) {
    bool found = false;
    for (QScreen *screen : allScreens) {
        // ...
        if (screenPhysical.contains(recordRect)) {
            found = true;
            break;
        }
    }
    isRecordAreaCrossScreen = !found;
}

建议:将跨屏检测逻辑提取为单独的函数isRectCrossingScreens,提高可读性和可维护性。

3.3 调试日志

代码中包含多处调试日志:

qCDebug(dsrApp) << "isRecordAreaCrossScreen:" << isRecordAreaCrossScreen;
qCDebug(dsrApp) << "Countdown on screen" << targetScreen->name() << "position:" << countdownX << countdownY;

建议:考虑使用条件编译或日志级别控制,确保发布版本中不会输出过多调试信息。

4. 代码性能问题

4.1 重复计算

updateToolBarPos函数中,每次循环都重新计算屏幕坐标:

for (int i = 0; i < m_screenInfo.size(); ++i) {
    const int screenX = Utils::isQt6XcbEnv ? static_cast<int>(m_screenInfo[i].x / m_pixelRatio) : m_screenInfo[i].x;
    // ...
}

建议:如果屏幕信息在函数执行期间不会改变,可以在循环外预先计算所有屏幕的逻辑坐标。

4.2 频繁的屏幕查询

startCountdown函数中:

QList<QScreen *> allScreens = QGuiApplication::screens();

建议:如果屏幕配置不常变化,可以考虑缓存屏幕信息。

5. 代码安全问题

5.1 类型转换

代码中多处使用static_cast<int>进行类型转换:

const int screenX = Utils::isQt6XcbEnv ? static_cast<int>(m_screenInfo[i].x / m_pixelRatio) : m_screenInfo[i].x;

建议:确保转换不会导致精度丢失或溢出,特别是处理高DPI屏幕时。

5.2 空指针检查

代码中对QScreen*指针进行了部分检查:

if (recordAreaScreen) {
    // ...
} else {
    isRecordAreaCrossScreen = true;
}

建议:确保所有可能为空的指针都进行了检查,避免潜在崩溃。

6. 改进建议

6.1 提取坐标转换逻辑

// 在类中添加私有辅助函数
private:
    struct LogicalScreenInfo {
        int x, y, width, height;
    };
    
    LogicalScreenInfo toLogicalScreenInfo(const ScreenInfo& physicalInfo) const {
        LogicalScreenInfo logical;
        if (Utils::isQt6XcbEnv) {
            logical.x = static_cast<int>(physicalInfo.x / m_pixelRatio);
            logical.y = static_cast<int>(physicalInfo.y / m_pixelRatio);
            logical.width = static_cast<int>(physicalInfo.width / m_pixelRatio);
            logical.height = static_cast<int>(physicalInfo.height / m_pixelRatio);
        } else {
            logical.x = physicalInfo.x;
            logical.y = physicalInfo.y;
            logical.width = physicalInfo.width;
            logical.height = physicalInfo.height;
        }
        return logical;
    }

6.2 提取跨屏检测逻辑

bool MainWindow::isRectCrossingScreens(const QRect& rect) const {
    if (Utils::isQt6XcbEnv) {
        QList<QScreen*> allScreens = QGuiApplication::screens();
        for (QScreen* screen : allScreens) {
            qreal dpr = screen->devicePixelRatio();
            QRect screenPhysical(
                screen->geometry().x(),
                screen->geometry().y(),
                static_cast<int>(screen->geometry().width() * dpr),
                static_cast<int>(screen->geometry().height() * dpr));
            if (screenPhysical.contains(rect)) {
                return false;
            }
        }
        return true;
    } else {
        QRect rectLogical(
            static_cast<int>(rect.x() / m_pixelRatio),
            static_cast<int>(rect.y() / m_pixelRatio),
            static_cast<int>(rect.width() / m_pixelRatio),
            static_cast<int>(rect.height() / m_pixelRatio));
        
        QScreen* screen = QGuiApplication::screenAt(rectLogical.center());
        return !screen || !screen->geometry().contains(rectLogical);
    }
}

6.3 改进倒计时位置计算

QPoint MainWindow::calculateCountdownPosition(const QRect& recordRect) {
    if (isRectCrossingScreens(recordRect)) {
        QScreen* targetScreen = getTargetScreenForCountdown();
        QRect targetLocal = getLocalScreenGeometry(targetScreen);
        
        return QPoint(
            targetLocal.x() + (targetLocal.width() - countdownTooltip->width()) / 2,
            targetLocal.y() + (targetLocal.height() - countdownTooltip->height()) / 2
        );
    } else {
        return QPoint(
            recordX + (recordWidth - countdownTooltip->width()) / 2,
            recordY + (recordHeight - countdownTooltip->height()) / 2
        );
    }
}

7. 总结

这段代码主要解决了多屏幕环境下的坐标系统问题,特别是Qt6+XCB环境下的特殊处理。主要改进方向包括:

  1. 减少代码重复,提取公共逻辑
  2. 改进条件判断的可读性
  3. 确保类型转换的安全性
  4. 优化性能,减少重复计算
  5. 增强代码的可维护性

通过以上改进,代码将更加健壮、高效且易于维护。

@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

/forcemerge

@deepin-bot deepin-bot bot merged commit dc76388 into linuxdeepin:master Apr 16, 2026
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