Implement pattern/order editor DPI scaling, fix pixellation on Qt 6 HiDPI

[nyanpasu64]

This PR was mostly tested on Windows 10 x64 at various DPI settings. It fails on Qt 5 and 6 on KDE, due to KDE/Qt bugs rather than my code or BambooTracker's code. I did not test on Mac.

Honestly I did this mostly out of personal interest and for learning, since I'm not sure if other people use high DPI displays or Qt 6. However it serves as a demonstration that pixel-perfect fractional DPI scaling is possible on Qt 6 (which I thought was impossible until recently) through HighDpiScaleFactorRoundingPolicy. And this may become more important as Qt 5 transitions into a legacy program over time.

Overview

Qt 6 forces applications to use virtual pixels rather than physical pixels. This makes it difficult to perform pixel-level paint operations on widgets, or to create a QPixmap/QImage the same size as a widget. Worse yet, at fractional scale factors (physical pixels per virtual pixel), it's impossible to convert a widget size in virtual pixels to an image size in physical pixels, and have them always match up entirely.

I use HighDpiScaleFactorRoundingPolicy::RoundPreferFloor so Qt make each virtual pixel an integer number of physical pixels. Then when calling functions that take physical pixels, I multiply the size in virtual pixels by the integer scale factor. The only functions I've found so far that take physical pixels are QPixmap's constructor (taking a size in physical pixels), calling QPainter::drawPixmap() (taking a source rect in physical pixels), or calling QPixmap::scroll() for quick draw (both delta and region in physical pixels). If I've missed any, then it would result in redrawing defects. The current version of the PR has all rendering defects I've seen so far fixed.

(All QPainter operations acting on a QPixmap/QWidget, both of which implement QPaintDevice, multiply all coordinates by the target's QPaintDevice::devicePixelRatio[F]().)

Results

I rewrote the pattern and order editors to look sharp at high DPIs. I didn't touch the oscilloscope (which for some reason draws individual points rather than lines, meaning it's not antialiased) or the FM/PSG/ADPCM instrument editors.

The resulting app looks sharp, and lines are appropriately thick, on both 125% and 200% scaling, on both Qt 5 and 6. However, this approach does have some flaws: At 125-150% scaling, elements defined in terms of pixel count rather than font size (I don't think BambooTracker's pattern editor does this, unsure about instrument editor) do not scale compared to 100% (though this can be fixed with more work). At 200% scaling, positions are quantized to multiples of 2 pixels (though this isn't a deal-breaker). Nonetheless this is the best approach achievable on Qt (to my knowledge) if you want sharp and uniformly thick lines.

What are the alternative approaches?

Eliminating virtual pixels entirely, and positioning all widgets in physical pixels (increasing line thicknesses at high DPI unless you want to upset 200%/300% users), is not possible on Qt 6+ (not sure about 5, not sure about AA_DisableHighDpiScaling).

Another approach option is virtual pixels or vector graphics, then performing DPI scaling by rendering at a higher resolution. (If no intrinsic grid, will be fuzzy at all resolutions, if intrinsic grid, will be fuzzy at non-integer scale, unless UI elements are grid-snapped to integer physical pixels). This is the approach taken by many VST GUIs, and HighDpiScaleFactorRoundingPolicy::PassThrough kinda works like this. However the usual approach of using alpha-blended antialiasing (rather than multisampling/etc.) means coinciding edges at non-integer pixel boundaries are drawn incorrectly ("conflation artifacts"). Additionally BambooTracker's use of multiple off-screen QPixmap rather than painting directly to the widget isn't how vector graphics works, and makes it difficult to impossible (not sure which) to even size the QPixmap to the same resolution as the widgets being painted on (which can be seen as a special case of non-integer edges).

One possible concern with this PR that rendering at full pixel resolution will slow down pattern redrawing at high resolutions. I'm not sure if it's worth adding an application-level setting to control whether to render at virtual resolution rather than physical resolution (at 200% DPI, only render 1/2 of pixels horizontally and vertically, and upscale using nearest or linear interpolation).

Bugs

Unfortunately HighDpiScaleFactorRoundingPolicy has no effect on Linux KDE due to QTBUG-95930 (which I reported but has not been fixed), so this PR won't fix inconsistent pixel sizes there (I suggest setting display scaling to 100% and boosting font DPI instead).

Additionally, switching DPI after launching the app causes broken pattern/order editor layout, until you open the settings and click Apply to recompute font metrics. This is because BambooTracker isn't designed to switch DPIs. (Switching the DPI of running apps is easy on Windows, but probably not possible on Linux KDE X11 without manually the Xft/DPI XSettings variable, which I think KDE doesn't touch or Qt doesn't pick up. No clue about Wayland.)

Additionally switching DPI without switching QScreen objects (changing screen DPI, or switching between "laptop only" and "monitor only") is broken in Qt itself with HighDpiScaleFactorRoundingPolicy set, due to QTBUG-95925 and related bugs. I have a workaround to fix this bug, but I think it belongs in a separate PR if it's even approved at all.

Additionally at 2x integer scaling and up (175% and higher OS scale factor), the "bottom line" of the pattern/order editor's headers is misplaced too high, creating a gray line at the bottom of the header. This is a BambooTracker bug exposed by high scale factors, and I didn't fix it because it's minor and shifting a line is not strictly related to the rest of the PR (which adds/fixes scaling factors) I added a surgical fix. To avoid this kind of bug, in exotracker, I avoided QRect's lines and rectangle outlines entirely in favor of my own rectangle left/right border functions.

Code Style

I was rather sloppy with the code style and didn't match the project (#pragma once, removing trailing whitespace from files I touched, placing open braces on the same line as the function, not sure how to break long lines, etc.) So I marked the PR as a draft.

Screenshots on Qt 6

Before (125%):

After (125%):

After (200%):

Qt 5 looks uglier since the window font doesn't scale with DPI, even at 200%. Maybe Qt::AA_EnableHighDpiScaling would help, or the QApplication::setFont(QApplication::font("QMessageBox")); trick.

[nyanpasu64]

sigh, Qt::HighDpiScaleFactorRoundingPolicy doesn't exist on ubuntu 16.04... this pr fails to fix fractional scaling on linux anyway, regardless of new/old qt, and idk how qt 5 handles 200% scaling on linux.

[nyanpasu64]

Linux/KDE status

2x DPI scaling pixelation is fixed on Qt 5 and 6 by scaledQPixmap() (which calls QPixmap::setDevicePixelRatio()).

Fractional DPI scaling pixelation is unfixed on Qt 5 and 6. There's not much that can be done about it, since KDE sets QT_SCREEN_SCALE_FACTORS, which overrides HighDpiScaleFactorRoundingPolicy (likely by design, but I'm not sure).

I'm not sure if setHighDpiScaleFactorRoundingPolicy(RoundPreferFloor) is helpful on Qt 5.14+. On Windows, all it does is make 150% use 1x integer scaling rather than 2x. On KDE, it does nothing. Perhaps on other Linux desktops where the platform plugin supplies a DPI value (I'm not sure if it ever does on X11), this will be useful.

Version check?

Additionally, the AppVeyor Windows 7 builds are being made on 5.10.1 rather than the latest 5.15.2 (should this be changed?), meaning that they won't call setHighDpiScaleFactorRoundingPolicy even though GitHub Actions is building on 5.15.2. I don't think it's really a good idea to introduce changes in behavior dependent on the version of Qt, and fail to be incorporated into dev builds, unless they clearly fix a bug (which setHighDpiScaleFactorRoundingPolicy on Qt 5 doesn't). So I might ifdef for Qt 6.0.0 instead. Thoughts?

[OPNA2608]

I don't think it's really a good idea to introduce changes in behavior dependent on the version of Qt

We already do this to differentiate WinXP from Win7+ builds & enable newer Windows features for the latter (Qt::AA_EnableHighDpiScaling, some RtAudio API support stuff). We could consider locking that stuff behind a CONFIG option instead, but I'm worried about what to call it without confusing users into thinking that the option alone controls Windows XP support (which it doesn't, Qt dictates that).

the AppVeyor Windows 7 builds are being made on 5.10.1 rather than the latest 5.15.2 (should this be changed?)
and fail to be incorporated into dev builds

AppVeyor build stuff wasn't really touched in awhile. The dissonance in Qt versions between GHA & AV is due to negligence of the latter and not intentional. We can make a PR bumping the builder image to Visual Studio 2019 and pinning the Qt version to MinGW 5.15.2 to address this.

[Windows] Qt 5 looks uglier since the window font doesn't scale with DPI, even at 200%.
On Windows, all it does is make 150% use 1x integer scaling rather than 2x.
So I might ifdef for Qt 6.0.0 instead. Thoughts?

If it looks worse on Qt5 (I'm taking your word on it here) then it's fine if it's only enabled for Qt6, maybe with a short code comment explaining the fact.

[nyanpasu64]

Windows Qt5 is uglier (especially at 200% DPI) than Windows Qt6 (though this can be fixed the same way as I did in exotracker, by switching to Segoe UI, are you interested in this?). However setting the rounding mode doesn't harm Windows Qt5 compared to not setting it (and in my subjective opinion improves 150% slightly).

I think I'll wait for the AppVeyor update to Qt 5.15.2 before working further on this PR.

[OPNA2608]

Oh, this is about the default font choice instead of the setting.

What I'm reading from this is that Segoe UI is only available since Vista? If your suggested change doesn't cause problems on XP then I'd be interested, for the sake of consistency across versions.

Although… different Windows locales seem to select different fonts, see #402. Qt5's MS UI Gothic (apparently the default shell font for Japanese Windows) vs Qt6's Segoe UI + Yu Gothic UI. I assume your method would cause weird font mixes in such scenarios, in which case maybe it's better to just leave it to Qt & the system to figure out the preferred font & "deal with it".

[nyanpasu64]

The exact code is QApplication::setFont(QApplication::font("QMessageBox"));. It shouldn't cause any problems for any version of Windows since XP or earlier, since it's based on lfMessageFont, an ancient API which (to this day) returns the default font and size for the current language, or (on Classic theme) the font picked by the user. Note that I haven't tested it on XP or other languages, but I think it's a good idea to try (does anyone still have Windows XP to test on?).

My guess is that Win32 message box fonts can be configured by the user (at least on Windows Classic) because they won't break message boxes which reflow dynamically, whereas Win32 app fonts are hard-coded so they take up a known amount of space, which is necessary because apps build hard-coded layouts around them. BambooTracker uses dynamic layout, so it can use system fonts that vary between OSes and languages.

[OPNA2608]

I bumped the Qt version of Win7 development builds to 5.15.2.

[nyanpasu64]

Argh... devicePixelRatioF() was introduced in Qt 5.6 (2016, 5 years old by now), but Ubuntu 16.04 is older, and the Windows XP build doesn't use it. Do I switch to devicePixelRatio() (which is int on Qt5 and qreal on Qt6) which shouldn't change behavior too much (fractional scaling is janky on Linux anyway, and probably nobody uses DPI scaling on XP)?

• Should I make the DPI rounding policy Qt 6 only (minimize change in behavior) or Qt 5.14+ (arguably better behavior at 150% scaling)? It's currently set to Qt 6 only, but I'm leaning towards switching to 5.14+ (because 2x pixelated bitmaps is ugly, and too-small icons is probably better than too-large icons and padding).
• I removed changes to trailing whitespace. However I still didn't match the project's code style (#pragma once, placing open braces on the same line as the function, not sure how to break long lines, etc.) Does this need to be fixed?

[OPNA2608]

The exact code is QApplication::setFont(QApplication::font("QMessageBox"));.

Should prolly be done in another PR? It affects more than DPI scaling & should be tested individually.

Do I switch to devicePixelRatio() (which is int on Qt5 and qreal on Qt6) which shouldn't change behavior too much (fractional scaling is janky on Linux anyway, and probably nobody uses DPI scaling on XP)?

Should I make the DPI rounding policy Qt 6 only (minimize change in behavior) or Qt 5.14+ (arguably better behavior at 150% scaling)? It's currently set to Qt 6 only, but I'm leaning towards switching to 5.14+ (because 2x pixelated bitmaps is ugly, and too-small icons is probably better than too-large icons and padding).

Sounds fine to me.

I removed changes to trailing whitespace. However I still didn't match the project's code style (#pragma once, placing open braces on the same line as the function, not sure how to break long lines, etc.) Does this need to be fixed?

• #pragma once looks fulfilled to me in dpi.hpp?

• Afaict the braces on functions should be on the next line instead of the same. Same for namespaces, though it's not perfectly consistent in the codebase.

• Additions don't look like they need additional line breaks to me, the existing code has slightly longer lines without line breaks.

[nyanpasu64]

• bambootracker has an inconsistent mix of #pragma once and header guards... sigh. I left #pragma once in place rather than switching to header guards.
• I moved the opening braces to new lines.

Is the "Remove unnecessary unique_ptr from main()" commit OK?

(lol github is complaining changes are requested, even though the review is marked as resolved) (oops i requested review again... what is github doing)

[OPNA2608]

I'll take your word on this working properly on Windows, since I can't test it.