Fix note list rendering at fractional display scaling#769
Fix note list rendering at fractional display scaling#769Shriram-Vasudevan wants to merge 2 commits into
Conversation
|
@nuttyartist Do we have an AI-policy? Personally I'm not a big fan of these fully AI-generated PRs, I mean the OP didn't even bother to let the AI install cmake:
I recently added one in a personal repo if you want an example: https://github.com/zjeffer/split-monitor-workspaces/blob/main/CONTRIBUTING.md |
|
I installed the missing local build tooling and pushed follow-up commit 404e7cf with the clang-format adjustments found by the repo format gate. Verification now run locally:
The build produced only existing/project-wide Qt deprecation and macOS deployment-target linker warnings, not errors in the touched note-list files. |
nuttyartist
left a comment
There was a problem hiding this comment.
Thanks for the fix. I left a few inline suggestions for the remaining DPR source-rect issue. The helper approach looks good; the main thing is making the later drawPixmap source rectangles use the same coordinate space as the DPR pixmaps.
| #include <cmath> | ||
| #include <QString> | ||
| #include <QDateTime> | ||
| #include <QPixmap> |
There was a problem hiding this comment.
This helper will also need QRect for converting logical source rects to pixmap/device-pixel source rects.
| #include <QPixmap> | |
| #include <QPixmap> | |
| #include <QRect> |
| inline QPixmap makeDevicePixelRatioPixmap(const QSize &logicalSize, const QWidget *widget) | ||
| { | ||
| const qreal scale = devicePixelRatioForWidget(widget); | ||
| QPixmap pixmap(logicalSize * scale); | ||
| pixmap.setDevicePixelRatio(scale); | ||
| return pixmap; | ||
| } |
There was a problem hiding this comment.
Could we pair the DPR pixmap allocator with a small source-rect conversion helper? Painting into the pixmap stays in logical coordinates, but QPainter::drawPixmap(..., sourceRect) reads sourceRect in pixmap/device pixels.
| inline QPixmap makeDevicePixelRatioPixmap(const QSize &logicalSize, const QWidget *widget) | |
| { | |
| const qreal scale = devicePixelRatioForWidget(widget); | |
| QPixmap pixmap(logicalSize * scale); | |
| pixmap.setDevicePixelRatio(scale); | |
| return pixmap; | |
| } | |
| inline QPixmap makeDevicePixelRatioPixmap(const QSize &logicalSize, const QWidget *widget) | |
| { | |
| const qreal scale = devicePixelRatioForWidget(widget); | |
| QPixmap pixmap(logicalSize * scale); | |
| pixmap.setDevicePixelRatio(scale); | |
| return pixmap; | |
| } | |
| inline QRect devicePixelRatioPixmapSourceRect(const QRect &logicalRect, const QPixmap &pixmap) | |
| { | |
| const qreal scale = pixmap.devicePixelRatio(); | |
| const QRectF scaledRect(logicalRect.x() * scale, logicalRect.y() * scale, logicalRect.width() * scale, logicalRect.height() * scale); | |
| return scaledRect.toAlignedRect(); | |
| } |
| { | ||
| auto bufferSize = bufferSizeHint(option, index); | ||
| QPixmap buffer{ bufferSize }; | ||
| QPixmap buffer = utils::makeDevicePixelRatioPixmap(bufferSize, m_view); |
There was a problem hiding this comment.
Once this buffer is DPR-aware, the drawPixmap(..., buffer, QRect{...}) calls lower in paintBackground() still need to convert their source rects before drawing. One compact way is to add this near the draw section and route those calls through it:
auto drawBuffer = [&buffer, painter](const QRect &targetRect, const QRect &sourceRect) {
painter->drawPixmap(targetRect, buffer, utils::devicePixelRatioPixmapSourceRect(sourceRect, buffer));
};Then replace each painter->drawPixmap(..., buffer, QRect{...}) in this function with drawBuffer(..., QRect{...}). The same pattern applies to the animated label buffer in paintLabels().
| { | ||
| auto bufferSize = rect().size(); | ||
| QPixmap buffer{ bufferSize }; | ||
| QPixmap buffer = utils::makeDevicePixelRatioPixmap(bufferSize, this); |
There was a problem hiding this comment.
Same coordinate-space issue here: the buffer is now DPR-aware, but the later source rect passed to drawPixmap() is still logical. The final draw should convert that source rect first, for example:
auto rowHeight = rect().height();
const QRect sourceRect{ 0, bufferSize.height() - rowHeight, rect().width(), rowHeight };
painter->drawPixmap(rect(), buffer, utils::devicePixelRatioPixmapSourceRect(sourceRect, buffer));|
@zjeffer Not really yet, and I agree, we should have. Yours seems reasonable enough, I'll add them to the repo. @Shriram-Vasudevan Thanks for your PR. |
Summary
Fixes #164.
Verification
git diff --checkc++ -std=c++17 -fsyntax-only -include arm_acle.h ... src/notelistdelegate.cpp src/notelistdelegateeditor.cppagainst locally installed Qt 6.9.2 headersQGuiApplication::setHighDpiScaleFactorRoundingPolicy(...)against Qt 6.9.2 headersI could not run the full CMake build locally because
cmakeis not installed in this environment, andclang-formatis also unavailable locally.