feat: add PositionFixer component for pixel-perfect positioning#1552
Open
wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
Open
feat: add PositionFixer component for pixel-perfect positioning#1552wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wjyrich The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideIntroduces a reusable C++/QML PositionFixer helper that centralizes device-pixel-ratio‑aware positioning logic, then replaces ad‑hoc QML timers and mapping code in dock/taskmanager and tray surfaces with this component, and wires it into applet buttons; CMake is updated to build the new helper. Sequence diagram for PositionFixer pixel-perfect positioningsequenceDiagram
participant QmlCaller as QmlCaller
participant PositionFixer as PositionFixer
participant QTimer as FixTimer
participant QQuickWindow as Window
participant ContentItem as ContentItem
participant ItemParent as ItemParent
QmlCaller->>PositionFixer: fix()
PositionFixer->>QTimer: start(100ms)
QTimer-->>PositionFixer: timeout()
PositionFixer->>PositionFixer: forceFix()
PositionFixer->>PositionFixer: validate m_item, m_container, window
PositionFixer->>Window: contentItem()
Window-->>PositionFixer: ContentItem
PositionFixer->>Window: devicePixelRatio()
Window-->>PositionFixer: dpr
PositionFixer->>ContentItem: mapToItem(targetX, targetY)
ContentItem-->>PositionFixer: scenePos
PositionFixer->>PositionFixer: physicalX = round(scenePos.x * dpr)
PositionFixer->>PositionFixer: physicalY = round(scenePos.y * dpr)
alt useZeroTarget == true
PositionFixer->>ItemParent: mapFromItem(physicalX / dpr, scenePos.y)
ItemParent-->>PositionFixer: localPosX
PositionFixer->>ItemParent: mapFromItem(scenePos.x, physicalY / dpr)
ItemParent-->>PositionFixer: localPosY
PositionFixer->>m_item: setX(localPosX.x)
PositionFixer->>m_item: setY(localPosY.y)
else useZeroTarget == false
PositionFixer->>ItemParent: mapFromItem(physicalX / dpr, physicalY / dpr)
ItemParent-->>PositionFixer: localPos
PositionFixer->>m_item: setX(localPos.x)
PositionFixer->>m_item: setY(localPos.y)
end
Class diagram for the new PositionFixer helperclassDiagram
class QQuickItem
class QTimer
class QQuickWindow
class PositionFixer {
QQuickItem *m_item
QQuickItem *m_container
bool m_useZeroTarget
QTimer *m_timer
+PositionFixer(parent: QQuickItem*)
+QQuickItem* item()
+void setItem(newItem: QQuickItem*)
+QQuickItem* container()
+void setContainer(newContainer: QQuickItem*)
+bool useZeroTarget()
+void setUseZeroTarget(newUseZeroTarget: bool)
+void fix()
+void forceFix()
+itemChanged()
+containerChanged()
+useZeroTargetChanged()
}
PositionFixer --|> QQuickItem
PositionFixer --> QTimer : uses
PositionFixer --> QQuickItem : item
PositionFixer --> QQuickItem : container
PositionFixer --> QQuickWindow : window() and devicePixelRatio()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The description mentions a
useCeiloption, but thePositionFixerclass only exposesuseZeroTargetand always usesstd::round; if ceiling behavior is still required (e.g. for the QTBUG case), consider adding a dedicateduseCeilproperty and branching the rounding logic accordingly. - In
ShellSurfaceItemProxy.qmlthe old implementation usedMath.ceilwhile the newPositionFixerusesstd::round, which may subtly change popup positioning compared to the previous QTBUG workaround; it may be safer to preserve the original ceiling behavior via configuration for this use case. - The delayed
fix()implementation relies solely on a 100 ms QTimer and returns early whencontainer->window()is null, so iffix()is called before the item is in a window the position may never be corrected; consider triggering anotherfix()onwindowChangedor usingcomponentComplete/afterRenderinghooks to ensure a final adjustment once the window is ready.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The description mentions a `useCeil` option, but the `PositionFixer` class only exposes `useZeroTarget` and always uses `std::round`; if ceiling behavior is still required (e.g. for the QTBUG case), consider adding a dedicated `useCeil` property and branching the rounding logic accordingly.
- In `ShellSurfaceItemProxy.qml` the old implementation used `Math.ceil` while the new `PositionFixer` uses `std::round`, which may subtly change popup positioning compared to the previous QTBUG workaround; it may be safer to preserve the original ceiling behavior via configuration for this use case.
- The delayed `fix()` implementation relies solely on a 100 ms QTimer and returns early when `container->window()` is null, so if `fix()` is called before the item is in a window the position may never be corrected; consider triggering another `fix()` on `windowChanged` or using `componentComplete`/`afterRendering` hooks to ensure a final adjustment once the window is ready.
## Individual Comments
### Comment 1
<location path="panels/dock/positionfixer.cpp" line_range="94-98" />
<code_context>
+
+ QQuickItem *itemParent = m_item->parentItem() ? m_item->parentItem() : m_container;
+
+ if (m_useZeroTarget) {
+ QPointF localPosX = itemParent->mapFromItem(contentItem, QPointF(physicalX / dpr, scenePos.y()));
+ QPointF localPosY = itemParent->mapFromItem(contentItem, QPointF(scenePos.x(), physicalY / dpr));
+ m_item->setX(localPosX.x());
+ m_item->setY(localPosY.y());
+ } else {
+ QPointF localPos = itemParent->mapFromItem(contentItem, QPointF(physicalX / dpr, physicalY / dpr));
</code_context>
<issue_to_address>
**issue (bug_risk):** The useZeroTarget branch mixes snapped and non-snapped coordinates, which likely deviates from the original axis-independent snapping logic.
In the original QML, X and Y were snapped independently by mapping `(physicalX / dpr, 0)` for X and `(0, physicalY / dpr)` for Y. Here, `useZeroTarget` mixes a snapped axis with the live scene coordinate of the other (`(physicalX / dpr, scenePos.y())`, `(scenePos.x(), physicalY / dpr)`), so the non-snapped axis still affects the mapping and can reintroduce sub-pixel drift.
To keep snapping axis-independent and consistent with the previous behavior, consider mapping with a stable reference on the non-snapped axis, e.g.:
- X: `mapFromItem(contentItem, QPointF(physicalX / dpr, 0))`
- Y: `mapFromItem(contentItem, QPointF(0, physicalY / dpr))`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Added a new PositionFixer QML component to handle pixel-perfect positioning of items within the dock panel. This component replaces multiple ad-hoc position fixing implementations in AppItem.qml and ShellSurfaceItemProxy.qml with a unified solution. The fixer calculates the correct physical pixel position considering device pixel ratio and applies rounding or ceiling to prevent sub-pixel rendering issues that cause blurriness. Key changes: 1. Created PositionFixer C++ class with QML integration 2. Added PositionFixer to AppletItemButton.qml for applet positioning 3. Replaced manual position fixing in AppItem.qml with PositionFixer 4. Replaced manual position fixing in ShellSurfaceItemProxy.qml with PositionFixer 5. Added CMakeLists.txt entries for new PositionFixer files The PositionFixer component provides configurable options including useCeil for ceiling operations and useZeroTarget for positioning at origin. It uses a delayed timer to ensure proper timing for position calculations. Log: Improved icon and surface positioning accuracy in dock panel Influence: 1. Test applet icons in dock panel for proper positioning without blurriness 2. Verify application icons in task manager maintain crisp edges 3. Check shell surface items (like system tray popups) render at correct pixel boundaries 4. Test with different display scaling factors (100%, 150%, 200%) 5. Verify drag-and-drop operations still work correctly 6. Test launch animations don't interfere with positioning feat: 添加 PositionFixer 组件实现像素级精确定位 新增 PositionFixer QML 组件，用于处理任务栏面板内项目的像素级精确定位。 该组件取代了 AppItem.qml 和 ShellSurfaceItemProxy.qml 中多个临时位置修 复实现，提供了统一的解决方案。修复器会计算考虑设备像素比的正确物理像素位 置，并应用四舍五入或向上取整操作，防止导致模糊的子像素渲染问题。 主要变更： 1. 创建了带有 QML 集成的 PositionFixer C++ 类 2. 在 AppletItemButton.qml 中添加 PositionFixer 用于小程序定位 3. 在 AppItem.qml 中用 PositionFixer 替换手动位置修复 4. 在 ShellSurfaceItemProxy.qml 中用 PositionFixer 替换手动位置修复 5. 在 CMakeLists.txt 中添加新 PositionFixer 文件的条目 PositionFixer 组件提供可配置选项，包括用于向上取整操作的 useCeil 和用于 原点定位的 useZeroTarget。它使用延迟计时器确保位置计算的正确时机。 Log: 提升任务栏面板中图标和表面定位的准确性 Influence: 1. 测试任务栏面板中的小程序图标定位是否正确且无模糊 2. 验证任务管理器中的应用图标是否保持清晰边缘 3. 检查外壳表面项目（如系统托盘弹出窗口）是否在正确的像素边界渲染 4. 使用不同的显示缩放比例测试（100%、150%、200%） 5. 验证拖放操作是否仍能正常工作 6. 测试启动动画是否不会干扰定位
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added a new PositionFixer QML component to handle pixel-perfect positioning of items within the dock panel. This component replaces multiple ad-hoc position fixing implementations in AppItem.qml and ShellSurfaceItemProxy.qml with a unified solution. The fixer calculates the correct physical pixel position considering device pixel ratio and applies rounding or ceiling to prevent sub-pixel rendering issues that cause blurriness.
Key changes:
The PositionFixer component provides configurable options including useCeil for ceiling operations and useZeroTarget for positioning at origin. It uses a delayed timer to ensure proper timing for position calculations.
Log: Improved icon and surface positioning accuracy in dock panel
Influence:
feat: 添加 PositionFixer 组件实现像素级精确定位
新增 PositionFixer QML 组件，用于处理任务栏面板内项目的像素级精确定位。
该组件取代了 AppItem.qml 和 ShellSurfaceItemProxy.qml 中多个临时位置修 复实现，提供了统一的解决方案。修复器会计算考虑设备像素比的正确物理像素位
置，并应用四舍五入或向上取整操作，防止导致模糊的子像素渲染问题。
主要变更：
PositionFixer 组件提供可配置选项，包括用于向上取整操作的 useCeil 和用于
原点定位的 useZeroTarget。它使用延迟计时器确保位置计算的正确时机。
Log: 提升任务栏面板中图标和表面定位的准确性
Influence:
Summary by Sourcery
Introduce a reusable PositionFixer component to centralize pixel-perfect positioning logic for dock items and tray surfaces.
New Features:
Enhancements:
Build: