Bug 350039 - krdb throws away all previous settings if no specific font DPI value is given
Summary: krdb throws away all previous settings if no specific font DPI value is given
Status: RESOLVED FIXED
Alias: None
Product: systemsettings
Classification: Applications
Component: krdb (show other bugs)
Version: 5.3.2
Platform: Fedora RPMs Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: kdelibs bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-09 00:37 UTC by Adam Williamson
Modified: 2021-10-19 18:45 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.23.1


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Williamson 2015-07-09 00:37:37 UTC
Back in 2005, this bug was filed:

https://bugs.kde.org/show_bug.cgi?id=111754

as part of the fix for that, this code was added to krdb.cpp :

if( cfgfonts.readNumEntry( "fontDPI", 0 ) != 0 )
    contents += "Xft.dpi: " + cfgfonts.readEntry( "fontDPI" ) + '\n';
else
{
    KProcIO proc;
    proc << "xrdb" << "-quiet" << "-remove" << "-nocpp";
    proc.writeStdin( QByteArray( "Xft.dpi" ), true );
    proc.closeWhenDone();
    proc.start( KProcess::Block );
}

It's since been ported to KProcess, but basically remains the same:

    if( cfgfonts.readEntry( "forceFontDPI", 0 ) != 0 )
      contents += "Xft.dpi: " + cfgfonts.readEntry( "forceFontDPI" ) + '\n';
    else
    {
      KProcess proc;
      proc << "xrdb" << "-quiet" << "-remove" << "-nocpp";
      proc.start();
      if (proc.waitForStarted())
      {
        proc.write( QByteArray( "Xft.dpi\n" ) );
        proc.closeWriteChannel();
        proc.waitForFinished();
      }
    }

What that's obviously *trying* to do in the case where there's no 'forceFontDPI' value is to unset the 'Xft.dpi' value and leave all other settings unchanged. It's basically doing this:

echo "Xft.dpi" | xrdb -quiet -remove -nocpp

I can see how the author arrived at it, if we look at the xrdb manpage. It says that it reads from stdin if no filename is given (as the final argument to the command), so OK. And the section on -remove reads:

       -remove This option indicates that the specified properties  should  be removed from the server.

so fine, we're telling it to remove the 'Xft.dpi' property, right? That makes sense; the idea is that when that's not set to some specific value, the 'correct' DPI for the screen (based on resolution and the reported physical size of the display) will be used, which is what KDE thinks is a good idea (I'm not gonna dive down the rabbit hole of whether that's actually true or not here).

Unfortunately, no, that's not right. The problem is what it means by 'property'. The individual settings like Xft.dpi are not 'properties'. The 'property' is the entire RESOURCE_MANAGER content. You can't use -remove to strip out a single setting from the RESOURCE_MANAGER property - you can only use it to remove *the entirety of the RESOURCE_MANAGER property* from a particular screen. Basically, any time you run 'xrdb -remove', you're going to delete the entire RESOURCE_MANAGER property for some screen or another.

This means that any time exportXftSettings is true (not sure exactly how that gets decided) and there's no forceFontDPI value, *all* RESOURCE_MANAGER settings that got set before krdb ran (e.g. via xinitrc) will be thrown away - not just the Xft.dpi value, which is what the code is actually trying to do, but all the others too.

AFAIK there isn't a completely straightforward way to do what the code really wants to do. You can do it with a one-liner in shell script:

xrdb -query | grep -v "^Xft\.dpi:" | xrdb -nocpp

(thanks Kevin Kofler for that one) - i.e. read the current settings, filter out any line that starts 'Xft.dpi:' , and load the result back in again. You'd want to do a concise version of the same thing in C++, I guess.

Reproducible: Always

Steps to Reproduce:
1. Boot a Fedora KDE live image
2. Run 'xrdb -query'


Actual Results:  
Xft.hinting and Xft.hintstyle do not appear in the results.

Expected Results:  
Those settings should appear in the results, because they are specified by /etc/X11/Xresources , which is loaded by a command in /etc/X11/xinit/xinitrc-common :

sysresources=/etc/X11/Xresources
...
# merge in defaults
[ -r "$sysresources" ] && xrdb -nocpp -merge "$sysresources"

Note this is possibly subject to a race - if xinitrc-common somehow runs after krdb, its settings will be in the xrdb -query output. That shouldn't usually happen, though.
Comment 1 Kevin Kofler 2015-07-09 01:26:31 UTC
It also looks like deleting Xft.dpi will no longer have the intended effect in upcoming GTK+ releases, unless we can get:
https://git.gnome.org/browse/gtk+/commit/?id=bdf0820c501437a2150d8ff0d5340246e713f73f
reverted. :-(
Comment 2 Adam Williamson 2015-07-09 01:42:59 UTC
Just for a bit of context, I started this epic yak shave by noticing that the Fedora installer rendered slightly differently on our GNOME live image from how it rendered on our KDE live image and our traditional installer images. The difference boiled down to being in the DPI and hintstyle settings. GNOME (via gnome-settings-daemon) has a default of 96dpi and hintstyle 'medium'; it does not try and calculate a 'correct' DPI. GTK+, however - before the commit KK mentioned - defaulted to a calculated DPI and hintstyle 'full'.

Fedora ships an /etc/X11/Xresources which also specifies 96dpi and hintstyle 'medium', but because krdb intentionally throws away Xft.dpi (because of this bug it also throws away everything else, but the throwing away of Xft.dpi is really intended), anaconda running in KDE winds up using the GTK+ default, i.e. the calculated 'correct' DPI. KVM virtual machines (which I was using to test), I think, try to specify a display size in mm which will lead to a DPI as *close as possible* to 96, but don't quite make it precise. The upshot was that the DPI used by anaconda running in KDE in a VM at 1024x768 was very slightly higher than the DPI used by anaconda running in GNOME in a VM at 1024x768 (like, 96.09 vs. 96), which made it render upper-case letters one pixel taller, which messed up our openQA tests...

After Matthias helped me figure out that's what was going on, he decided to make GTK+ follow the same defaults as GNOME, i.e. just unconditionally use 96dpi (well, with an integer scaling factor for hidpi displays) if no specific Xft.dpi value is set (and default to 'medium' hinting rather than 'full'). Hence the commit above. I can see why KDE might not agree with the change, though, as it may be difficult for KDE to make GTK+ apps running in KDE use the same DPI as Qt apps; presumably you'd need to explicitly set Xft.dpi to the calculated DPI, but I don't know how straightforward it would be to have krdb find that calculated value from whatever calculates it.
Comment 3 Adam Williamson 2015-07-11 01:16:00 UTC
One more note for the record - after writing comment #2 I found out that X is written to simply lie about the physical display size in order to cause things that calculate a resolution from it to get 96dpi in any case:

https://bugs.freedesktop.org/show_bug.cgi?id=41115
https://www.happyassassin.net/2015/07/09/of-dpis-desktops-and-toolkits/

the only cases where that won't happen is where the user explicitly specifies a DPI on the X command line, or where they specify a DisplaySize in their Xorg config. So in fact in most cases, Qt is kinda wasting its time doing the calculation anyway, as X.org has nerfed the numbers so it'll result in 96dpi (or *nearly* 96dpi - I couldn't quite figure out if Qt rounds the calculation or not. If it doesn't, it'll wind up with a bit more or less than 96dpi in many cases).
Comment 4 Nate Graham 2020-06-16 05:19:29 UTC
In Plasma 5.19, I see:

Xft.hinting:    1
Xft.hintstyle:  hintslight

...in the output of `xrdb -query`. Is this still a problem for you with the latest Plasma version?
Comment 5 Adam Williamson 2020-06-16 15:06:51 UTC
Well, the code that wipes the whole rdb is still there:

https://github.com/KDE/plasma-desktop/blame/master/kcms/krdb/krdb.cpp#L476

One thing has changed around it, though. This commit:
https://github.com/KDE/plasma-desktop/commit/873cbc1da8f3f2b589f9f2722a4e15a8fcbfe4e5

adds default values for anti-aliasing, which I think are what you're seeing. I don't really think the problem is 'solved', though.
Comment 6 Nate Graham 2020-06-16 16:05:03 UTC
Ah, ok. I notice that you set the ticket to ASSIGNED. Are you planning to work on this?
Comment 7 Adam Williamson 2020-06-16 16:11:46 UTC
no, sorry, I just figured that was the appropriate state to set it back to from NEEDINFO.
Comment 8 Bug Janitor Service 2021-10-15 12:14:32 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/1119
Comment 9 Alexander Volkov 2021-10-18 10:28:19 UTC
Git commit d4dd7478893e9ac72b0c89772a086b213bb33292 by Alexander Volkov.
Committed on 18/10/2021 at 10:21.
Pushed by volkov into branch 'master'.

krdb: Fix removal of Xft.dpi from Xresources

'xrdb -remove' can't remove a specific entry, it removes all entries only.
So query all entries, remove "Xft.dpi" from them, and replace Xresources
with the result.
Related: bug 376406

M  +28   -7    kcms/krdb/krdb.cpp

https://invent.kde.org/plasma/plasma-workspace/commit/d4dd7478893e9ac72b0c89772a086b213bb33292
Comment 10 Alexander Volkov 2021-10-18 10:36:05 UTC
Git commit 85399471b2aad37e4524bde20b70fe2cb61354ba by Alexander Volkov.
Committed on 18/10/2021 at 10:34.
Pushed by volkov into branch 'Plasma/5.23'.

krdb: Fix removal of Xft.dpi from Xresources

'xrdb -remove' can't remove a specific entry, it removes all entries only.
So query all entries, remove "Xft.dpi" from them, and replace Xresources
with the result.
Related: bug 376406

(cherry picked from commit d4dd7478893e9ac72b0c89772a086b213bb33292)

M  +28   -7    kcms/krdb/krdb.cpp

https://invent.kde.org/plasma/plasma-workspace/commit/85399471b2aad37e4524bde20b70fe2cb61354ba