Skip to content

fix(record): fix selection area and toolbar offset offscreen when drawing small region on HiDPI #824

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

fix(record): fix selection area and toolbar offset offscreen when drawing small region on HiDPI #824
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-4-16

Conversation

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor

fix(toolbar): improve toolbar positioning on HiDPI displays

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

fix(record): fix selection area and toolbar offset offscreen when drawing small region on HiDPI

On HiDPI displays, boundary checks in mouseReleaseEF used rootWindowRect
(physical pixels) while recordX/Y/Width/Height are in logical coordinates,
causing the minimum-size-adjusted selection to overflow the screen bounds.

  • Change boundary checks from rootWindowRect to m_backgroundRect (logical size)
  • Add lower bound protection for recordX/Y < 0 when adjustment pushes origin negative
  • Reorder: perform min-size adjustment and bounds clamping before updating
    sizeTips and toolbar position, avoiding transient incorrect positioning
  • Apply the same fix to both record and shot mode for consistency

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

…wing small region on HiDPI

On HiDPI displays, boundary checks in mouseReleaseEF used rootWindowRect
(physical pixels) while recordX/Y/Width/Height are in logical coordinates,
causing the minimum-size-adjusted selection to overflow the screen bounds.

- Change boundary checks from rootWindowRect to m_backgroundRect (logical size)
- Add lower bound protection for recordX/Y < 0 when adjustment pushes origin negative
- Reorder: perform min-size adjustment and bounds clamping before updating
  sizeTips and toolbar position, avoiding transient incorrect positioning
- Apply the same fix to both record and shot mode for consistency

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

deepin pr auto review

代码审查报告

总体评价

这段代码主要针对截图/录屏工具的界面绘制、工具栏位置更新和选区边界处理进行了优化。主要改进点包括:

  1. 修复了高DPI下的坐标系统不一致问题
  2. 优化了工具栏和侧边栏的背景模糊效果绘制
  3. 改进了选区边界检查逻辑
  4. 增加了布局激活机制

详细审查意见

1. 语法逻辑

优点

  • 坐标系统统一:将所有基于rootWindowRect的边界检查改为使用m_backgroundRect,解决了高DPI下物理像素和逻辑像素不一致的问题
  • 代码结构清晰:将选区尺寸调整、边界检查和工具栏位置更新的逻辑分块处理,提高了可读性
  • 新增函数getInnerWidgetRect()实现正确,能够正确获取内部控件的相对坐标

问题与建议

  1. resizeLeft函数中的硬编码问题

    } else if (m_functionType == 1) {  // 应该使用 status::shot

    建议改为:

    } else if (status::shot == m_functionType) {
  2. 边界检查逻辑重复
    mouseReleaseEF中,recordshot模式的边界检查代码几乎完全相同,建议提取为公共函数:

    void MainWindow::adjustRecordBounds(int &x, int &y, int &w, int &h, int minW, int minH) {
        w = std::max(w, minW);
        h = std::max(h, minH);
        x = std::max(0, std::min(x, m_backgroundRect.width() - w));
        y = std::max(0, std::min(y, m_backgroundRect.height() - h));
    }

2. 代码质量

优点

  • 注释详细,特别是paintEvent中对模糊效果绘制逻辑的说明
  • 函数命名清晰,getInnerWidgetRect准确表达了函数用途
  • 代码组织良好,相关功能集中处理

问题与建议

  1. updateToolBarPos中的布局激活

    if (m_toolBar->layout())
        m_toolBar->layout()->activate();

    建议添加注释说明为什么需要手动激活布局,例如:

    // 手动激活布局以确保在动态调整工具栏位置时正确计算几何尺寸
    if (m_toolBar->layout())
        m_toolBar->layout()->activate();
  2. getInnerWidgetRect中的const_cast

    return QRect(m_toolbarWidget->mapTo(const_cast<ToolBar *>(this), cr.topLeft()), cr.size());

    虽然Qt的API设计导致这里需要const_cast,但建议添加注释说明这是必要的,并非设计缺陷。

3. 代码性能

优点

  • 使用QPainterPath替代QRegion进行裁剪,在复杂形状下性能更好
  • 避免了不必要的重复计算和绘制

问题与建议

  1. paintEvent中的重复计算

    if (m_toolBar && m_toolBar->isVisible()) {
        QPainterPath clipPath;
        QRect innerRect = m_toolBar->getInnerWidgetRect();
        innerRect.translate(m_toolBar->pos());
        clipPath.addRoundedRect(innerRect, 18, 18);

    如果工具栏位置不频繁变化,可以考虑缓存clipPath,避免每次绘制都重新计算。

  2. 边界检查中的重复计算
    resizeTop/Bottom/Left/Right函数中,多次计算相同的边界值,可以考虑在类中缓存这些值。

4. 代码安全

优点

  • 增加了空指针检查:if (!m_sidebarWidget)if (!m_toolbarWidget)
  • 边界检查更加严格,防止选区超出屏幕范围

问题与建议

  1. 坐标转换的安全性

    QRect cr = m_sidebarWidget->contentsRect();
    return QRect(m_sidebarWidget->mapTo(const_cast<SideBar *>(this), cr.topLeft()), cr.size());

    建议添加对mapTo返回值的验证,确保坐标转换成功:

    QPoint topLeft = m_sidebarWidget->mapTo(const_cast<SideBar *>(this), cr.topLeft());
    if (topLeft.isNull() && !cr.topLeft().isNull()) {
        qCWarning(dsrApp) << "Coordinate mapping failed in getInnerWidgetRect";
        return rect();
    }
    return QRect(topLeft, cr.size());
  2. 边界检查的完整性
    mouseReleaseEF中,虽然增加了对负坐标的检查,但建议使用统一的边界检查函数,确保所有边界情况都被正确处理。

总结

这段代码整体质量较高,主要改进了高DPI支持和界面绘制效果。建议的主要改进点包括:

  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
Copy link
Copy Markdown
Contributor

deepin-bot bot commented Apr 16, 2026

This pr force merged! (status: unstable)

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