Skip to content

Conversation

@yixinshark
Copy link
Contributor

@yixinshark yixinshark commented Sep 29, 2025

…ss of system scaling

as title

Log: as title
Pms: BUG-302035

Summary by Sourcery

Ensure the show desktop indicator line remains one physical pixel regardless of system scaling by using the device pixel ratio to compute its dimensions.

Bug Fixes:

  • Fix the line thickness changing with system scaling by adjusting implicitWidth and implicitHeight using Screen.devicePixelRatio

Enhancements:

  • Import QtQuick.Window to access Screen.devicePixelRatio

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 29, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Ensure the show desktop line remains exactly one physical pixel thick under any system scaling by introducing Screen.devicePixelRatio and updating the line’s implicit dimensions accordingly.

Class diagram for updated Rectangle in showdesktop.qml

classDiagram
    class Rectangle {
        D.Palette lineColor
        real devicePixelRatio
        implicitWidth
        implicitHeight
        color
    }
    Rectangle : devicePixelRatio = Screen.devicePixelRatio
    Rectangle : implicitWidth = showdesktop.useColumnLayout ? showdesktop.implicitWidth : (1 / devicePixelRatio)
    Rectangle : implicitHeight = showdesktop.useColumnLayout ? (1 / devicePixelRatio) : showdesktop.implicitHeight
Loading

File-Level Changes

Change Details Files
Use devicePixelRatio to enforce a 1 physical pixel line thickness
  • Import QtQuick.Window to access Screen.devicePixelRatio
  • Add devicePixelRatio property bound to Screen.devicePixelRatio
  • Adjust implicitWidth and implicitHeight to divide by devicePixelRatio based on layout
panels/dock/showdesktop/package/showdesktop.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `panels/dock/showdesktop/package/showdesktop.qml:38-39` </location>
<code_context>
-            implicitHeight: useColumnLayout ? 1 : showdesktop.implicitHeight
+            // Use device pixel ratio to ensure the line is always 1 physical pixel regardless of system scaling
+            property real devicePixelRatio: Screen.devicePixelRatio
+            implicitWidth: showdesktop.useColumnLayout ? showdesktop.implicitWidth : (1 / devicePixelRatio)
+            implicitHeight: showdesktop.useColumnLayout ? (1 / devicePixelRatio) : showdesktop.implicitHeight

             color: D.ColorSelector.lineColor
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Division by devicePixelRatio could result in non-integer sizes, which may cause rendering artifacts.

Rounding the calculated size or using Math.max(1, Math.round(1 / devicePixelRatio)) can help prevent blurry or inconsistent rendering due to subpixel values.

```suggestion
            implicitWidth: showdesktop.useColumnLayout ? showdesktop.implicitWidth : Math.max(1, Math.round(1 / devicePixelRatio))
            implicitHeight: showdesktop.useColumnLayout ? Math.max(1, Math.round(1 / devicePixelRatio)) : showdesktop.implicitHeight
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@yixinshark yixinshark force-pushed the fix-1px branch 2 times, most recently from 311f611 to 9c31205 Compare September 29, 2025 08:59
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码是一个QML文件中的Rectangle组件的修改,主要涉及线条宽度的计算。我来分析一下代码的变更和潜在问题:

代码变更分析

  1. 原代码中,无论是否使用列布局(useColumnLayout),线条的宽度或高度都是固定的1个逻辑像素。
  2. 新代码添加了设备像素比(devicePixelRatio)的考虑,将线条的宽度或高度调整为1/设备像素比,以确保在系统缩放情况下线条始终保持1个物理像素的宽度。

潜在问题和改进建议

  1. 设备像素比处理

    • 优点:新代码考虑了高DPI显示器的情况,确保线条在所有设备上显示为1个物理像素宽,这是一个很好的改进。
    • 潜在问题:如果设备像素比为0(虽然理论上不应该发生),会导致除零错误。建议添加保护:
      property real devicePixelRatio: Math.max(1, Screen.devicePixelRatio)
  2. 代码可读性

    • 当前的注释已经说明了修改的目的,但可以更详细地解释为什么需要除以设备像素比。
    • 建议将计算逻辑提取为一个函数,提高可读性和可维护性:
      function calculateLineWidth() {
          return 1 / Math.max(1, Screen.devicePixelRatio)
      }
      implicitWidth: useColumnLayout ? showdesktop.implicitWidth : calculateLineWidth()
      implicitHeight: useColumnLayout ? calculateLineWidth() : showdesktop.implicitHeight
  3. 性能考虑

    • Screen.devicePixelRatio是一个属性,可能会变化,但在这个场景下,它通常只在启动时设置一次。如果担心性能影响,可以考虑在Component.onCompleted时缓存这个值。
  4. 兼容性

    • 确保在所有目标平台上都能正确获取Screen.devicePixelRatio。如果需要支持较旧的Qt版本,可能需要添加兼容性检查。
  5. 样式一致性

    • 考虑这种1像素线条的样式是否与项目中其他类似元素的样式保持一致。如果不一致,可能需要统一处理方式。

总结建议

这个修改本身是一个很好的改进,解决了高DPI显示器上的显示问题。建议添加一些保护措施来防止潜在的除零错误,并考虑代码的可读性和可维护性。如果项目中还有其他类似的UI元素,应该采用相同的处理方式来保持一致性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

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

…ss of system scaling

as title

Log: as title
Pms: BUG-302035
@yixinshark
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Oct 9, 2025

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit a5466cd into linuxdeepin:master Oct 9, 2025
8 of 11 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