Also disable automatic scaling on Qt >= 5.14
ClosedPublic

Authored by ahartmetz on Dec 23 2019, 11:56 AM.

Details

Summary

Applications which set Qt::AA_EnableHighDpiScaling use the DPI of the monitor the window is on to calculate an additional scaling factor.
Plasma sets QT_SCREEN_SCALE_FACTORS in the environment for manual specification of per-monitor scale, which combined with the automatic scaling may result in double scaling.
To disable the automatic scaling, QT_AUTO_SCREEN_SCALE_FACTOR=0 has to be set, even on Qt 5.14.

As a side effect, this works around QTBUG-80967 as well.

BUG: 415421

Test Plan

On X11, with a >144dpi monitor and a scaling factor of 2 set manually, scaling was doubled previously. Now it works as expected.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahartmetz created this revision.Dec 23 2019, 11:56 AM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 23 2019, 11:56 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ahartmetz requested review of this revision.Dec 23 2019, 11:56 AM

Compiled against Qt 5.14 before this change:

Compiled against Qt 5.14 after this change:

ahartmetz edited the summary of this revision. (Show Details)Dec 23 2019, 12:05 PM
ahartmetz edited the test plan for this revision. (Show Details)Dec 23 2019, 12:25 PM

Test with qputenv(QT_ENABLE_HIGHDPI_SCALING, "1") it's especially for 5.14+

Test with qputenv(QT_ENABLE_HIGHDPI_SCALING, "1") it's especially for 5.14+

That unfortunately does not seem to help. I set the environment in a shell inside konsole from which I launched the konsole in which I took the screenshot.

I'm not all that sure. Not it's coming from D24255 by @dfaure, approved by @davidedmundson. Maybe they have some insight.

davidedmundson requested changes to this revision.Dec 24 2019, 2:56 PM

After 900f2cb6f7070 in QtBase auto dpi changed it's meaning somewhat. The comment written no longer applies and #ifdef'ing is the correct thing to get the consistent behaviour.

With QT_AUTO_SCREEN_SCALE_FACTOR=0 set, we no longer scale the logical DPI by the screen scale factor which is something we need to deliver high DPI correctly.
i.e on a low res screen with a physical of 96, running QT_SCREEN_SCALE_FACTORS=2 should double the fonts. This patch breaks that which means we've broken the config UI.

There may well be some changes needed, but this on its own isn't it.
Relevant code to look through is qhighdpi.cpp in QtBase

This revision now requires changes to proceed.Dec 24 2019, 2:56 PM
ahartmetz added a comment.EditedDec 24 2019, 9:58 PM

Just to make this extra clear, I have nothing against auto-scaling. There is probably a KCM to set it however I want if I don't like the default. My problem is with blurry and in some cases ("-=") disfigured font rendering. That should never happen regardless of physical and logical DPI (though no subpixel rendering and maybe no hinting might be fine at >= 2.0 scaling). With this patch, it looks like with Qt 5.13 on not very high DPI screens, i.e. it looks nice. I don't own a really high DPI screen so far.

I would guess that a full solution is somewhere in the guts of the Qt FreeType font engine, not in how DPI is determined. The scaling factor is always just one number (correct me?), and regardless of its value, the rendering should look good. I have some experience in Qt font engines, but very little with high DPI. This patch is what I could come up with without days of research. Can somebody with serious knowledge about high DPI help out here?

I think I see the problem. You pay for the possibility to have scaling with unhinted fonts. I am pretty sure that that isn't an acceptable way of handling it. Most Linux desktop fonts are intended to be used with some hinting (slight hinting usually?). QHighDpiScaling::isActive() returns true even when the scale factor is 1.0 as in my case. Looks like a Qt bug to me.

diff --git a/src/platformsupport/fontdatabases/fontconfig/qfontconfigdatabase.cpp b/src/platformsupport/fontdatabases/fontconfig/qfontconfigdatabase.cpp
index e28b40c240..05bd01f158 100644
--- a/src/platformsupport/fontdatabases/fontconfig/qfontconfigdatabase.cpp
+++ b/src/platformsupport/fontdatabases/fontconfig/qfontconfigdatabase.cpp
@@ -611,7 +611,7 @@ QFontEngine::HintStyle defaultHintStyleFromMatch(QFont::HintingPreference hintin
     }
 
     if (QHighDpiScaling::isActive())
-        return QFontEngine::HintNone;
+        return QFontEngine::HintNone; // NNNNOOOOOOO
 
     int hint_style = 0;
     if (FcPatternGetInteger (match, FC_HINT_STYLE, 0, &hint_style) == FcResultMatch) {
@@ -908,7 +908,7 @@ QFont QFontconfigDatabase::defaultFont() const
 void QFontconfigDatabase::setupFontEngine(QFontEngineFT *engine, const QFontDef &fontDef) const
 {
     bool antialias = !(fontDef.styleStrategy & QFont::NoAntialias);
-    bool forcedAntialiasSetting = !antialias || QHighDpiScaling::isActive();
+    bool forcedAntialiasSetting = !antialias || QHighDpiScaling::isActive(); // NNNNOOOOOOO
 
     const QPlatformServices *services = QGuiApplicationPrivate::platformIntegration()->services();
     bool useXftConf = false;

It's probably okay to still say that scaling is enabled "in general" but for scaling <2, and why not, maybe even greater, hinting should not be inhibited. The screen is still made of pixels. Qt is not going to be able to throw out all the hinting and subpixel code like Apple did, because it still needs to be supported for non-Retina screens anyway, for a long time. I'll open a Qt bug and see how it goes.

Can forcing bigger font DPI make things better?

Can forcing bigger font DPI make things better?

AFAICS no. If scaling is conceptually enabled, i.e. if an application would be scaled if you attached a high DPI screen and moved the application to it, you get no hinting. That's all.

Thanks for following up!

I'll open a Qt bug and see how it goes.

Can you link it please.

Thanks for following up!

I'll open a Qt bug and see how it goes.

Can you link it please.

Somebody was faster. I added a comment and a link to this discussion.
https://bugreports.qt.io/browse/QTBUG-80967

fvogt added a subscriber: fvogt.Jan 8 2020, 3:34 PM
acooligan added a subscriber: acooligan.EditedJan 11 2020, 9:49 AM

I think you may leave ifdef here but set:

qputenv("QT_ENABLE_HIGHDPI_SCALING", "0");

if it's false as QT_AUTO_SCREEN_SCALE_FACTOR is deprecated since qt 5.14.

After 900f2cb6f7070 in QtBase auto dpi changed it's meaning somewhat. The comment written no longer applies and #ifdef'ing is the correct thing to get the consistent behaviour.

With QT_AUTO_SCREEN_SCALE_FACTOR=0 set, we no longer scale the logical DPI by the screen scale factor which is something we need to deliver high DPI correctly.
i.e on a low res screen with a physical of 96, running QT_SCREEN_SCALE_FACTORS=2 should double the fonts. This patch breaks that which means we've broken the config UI.

On my system, when QT_AUTO_SCREEN_SCALE_FACTOR/QT_ENABLE_HIGHDPI_SCALING isn't disabled and both display scaling & font dpi scaling is enabled it results in double scaling - all qt apps are unproportionately huge which makes ui broken without this patch.

https://old.reddit.com/r/kde/comments/elj2oj/newest_kde_update_on_arch_just_disables/

https://old.reddit.com/r/archlinux/comments/elysp6/hidpi_and_scale_factor_oversized_kde_apps_after/

So what do we do here? The Qt bug has been fixed but still I'm not sure what should be done about the environment variables.

fvogt added a comment.Jan 17 2020, 8:15 AM

The change itself is correct, but not in relation to the title and summary which are about https://bugreports.qt.io/browse/QTBUG-80967, which was a genuine bug in Qt, fixed in 5.14.1.
What this change fixes is double scaling in applications which enable Qt::AA_EnableHighDpiScaling.

So please change those accordingly, add BUG: 415421 and it can go in.

Ack. Thanks for chasing up on indentifying causes.

fvogt accepted this revision.Tue, Jan 21, 10:46 AM
fvogt retitled this revision from Fix font rendering when compiled against Qt >= 5.14 to Also disable automatic scaling on Qt >= 5.14.
fvogt edited the summary of this revision. (Show Details)
fvogt edited the test plan for this revision. (Show Details)
davidedmundson accepted this revision.Tue, Jan 21, 10:53 AM
This revision is now accepted and ready to land.Tue, Jan 21, 10:53 AM
This revision was automatically updated to reflect the committed changes.

@fvogt Yeah thanks, that made more sense than you spoon-feeding me the information for the commit message ;)