fix(videowidget): fix hi-dpi display scaling in video widget#493
Merged
deepin-bot[bot] merged 1 commit intoJul 13, 2026
Merged
Conversation
Use devicePixelRatioF() for proper HiDPI scaling, prevent integer overflow in aspect ratio comparison, and fix scene rect calculation with logical coordinates. 修复视频预览控件在高DPI屏幕下的显示缩放问题，使用 devicePixelRatioF() 处理像素比例，防止整数溢出，并修复场景矩形计算。 Log: 修复 videowidget 高DPI显示缩放问题 PMS: BUG-369935 Influence: 修复后视频预览在高DPI屏幕上正常显示，画面比例和场景矩形坐标正确。
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts video widget scaling logic to be HiDPI‑aware, prevents overflow in aspect ratio comparison, and aligns scene rectangle setup with logical coordinates. Flow diagram for updated HiDPI-aware video widget scalingflowchart TD
A[ReceiveMajorImage] --> B[compute dpr using devicePixelRatioF]
B --> C[compute widgetwidth and widgetheight in physical pixels]
C --> D{image width * widgetheight > widgetwidth * image height}
D -->|true| E[set targetWidth = widgetwidth and targetHeight = min of scaled height and widgetheight]
D -->|false| F[set targetHeight = widgetheight and targetWidth = min of scaled width and widgetwidth]
E --> G[scale image to targetWidth,targetHeight]
F --> G
G --> H[create m_framePixmap from scaled image]
H --> I[setDevicePixelRatio on m_framePixmap]
I --> J[compute logical sceneRect using m_framePixmap width,height / dpr]
J --> K[setSceneRect on m_pNormalScene]
K --> L[setPixmap on m_pNormalItem]
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:
- Given the added HiDPI handling, consider extracting the device-pixel scaling and target size computation into a small helper function to make the aspect-ratio logic clearer and easier to reuse or adjust in other code paths.
- The use of Qt::IgnoreAspectRatio with explicit aspect-ratio–based targetWidth/targetHeight is slightly contradictory; if the goal is to preserve the video’s aspect ratio, it may be clearer to use Qt::KeepAspectRatio and let Qt handle the scaling instead of manual ratio calculations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Given the added HiDPI handling, consider extracting the device-pixel scaling and target size computation into a small helper function to make the aspect-ratio logic clearer and easier to reuse or adjust in other code paths.
- The use of Qt::IgnoreAspectRatio with explicit aspect-ratio–based targetWidth/targetHeight is slightly contradictory; if the goal is to preserve the video’s aspect ratio, it may be clearer to use Qt::KeepAspectRatio and let Qt handle the scaling instead of manual ratio calculations.
## Individual Comments
### Comment 1
<location path="src/src/videowidget.cpp" line_range="600-601" />
<code_context>
+ m_framePixmap = QPixmap::fromImage(image->scaled(targetWidth, targetHeight, Qt::IgnoreAspectRatio, Qt::SmoothTransformation));
+ m_framePixmap.setDevicePixelRatio(dpr);
+
+ const QRectF sceneRect(0, 0, qreal(m_framePixmap.width()) / dpr, qreal(m_framePixmap.height()) / dpr);
+ m_pNormalScene->setSceneRect(sceneRect);
m_pNormalItem->setPixmap(m_framePixmap);
</code_context>
<issue_to_address>
**question (bug_risk):** Scene rect now depends on devicePixelRatio; ensure this aligns with how the view interprets logical vs device coordinates.
This change correctly expresses the scene rect in logical coordinates, but it may diverge from any code that still treats the pixmap size as device pixels. Please verify that `QGraphicsView` configuration and any custom painting/transformations are consistent with logical coordinates to avoid subtle offset or zoom issues.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+600
to
+601
| const QRectF sceneRect(0, 0, qreal(m_framePixmap.width()) / dpr, qreal(m_framePixmap.height()) / dpr); | ||
| m_pNormalScene->setSceneRect(sceneRect); |
There was a problem hiding this comment.
question (bug_risk): Scene rect now depends on devicePixelRatio; ensure this aligns with how the view interprets logical vs device coordinates.
This change correctly expresses the scene rect in logical coordinates, but it may diverge from any code that still treats the pixmap size as device pixels. Please verify that QGraphicsView configuration and any custom painting/transformations are consistent with logical coordinates to avoid subtle offset or zoom issues.
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 当前代码已是最优实现，无需额外修改
// 以下为当前代码片段确认
const qreal dpr = this->devicePixelRatioF();
const int widgetwidth = qRound(this->width() * dpr);
const int widgetheight = qRound(this->height() * dpr);
int targetWidth, targetHeight;
if (qint64(image->width()) * widgetheight > qint64(widgetwidth) * image->height()) {
targetWidth = widgetwidth;
targetHeight = qMin(qRound(qreal(widgetwidth) * image->height() / image->width()), widgetheight);
} else {
targetHeight = widgetheight;
targetWidth = qMin(qRound(qreal(widgetheight) * image->width() / image->height()), widgetwidth);
}
m_framePixmap = QPixmap::fromImage(image->scaled(targetWidth, targetHeight, Qt::IgnoreAspectRatio, Qt::SmoothTransformation));
m_framePixmap.setDevicePixelRatio(dpr);
const QRectF sceneRect(0, 0, qreal(m_framePixmap.width()) / dpr, qreal(m_framePixmap.height()) / dpr);
m_pNormalScene->setSceneRect(sceneRect);
m_pNormalItem->setPixmap(m_framePixmap); |
lzwind
approved these changes
Jul 13, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, lzwind 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 |
Contributor
Author
|
/merge |
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
3 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.
Use devicePixelRatioF() for proper HiDPI scaling, prevent integer overflow in aspect ratio comparison, and fix scene rect calculation with logical coordinates.
修复视频预览控件在高DPI屏幕下的显示缩放问题，使用 devicePixelRatioF() 处理像素比例，防止整数溢出，并修复场景矩形计算。
Log: 修复 videowidget 高DPI显示缩放问题
PMS: BUG-369935
Influence: 修复后视频预览在高DPI屏幕上正常显示，画面比例和场景矩形坐标正确。
Summary by Sourcery
Fix HiDPI video preview scaling and scene rectangle calculation in the video widget.
Bug Fixes:
Enhancements: