So after calling setStyleName on a QFont, it's "tainted" and subsequent calls won't have the same effect?

It depends a little bit what kind of call you make. But yes, certain "conflicting" attributes can no longer be set. A particular example reported by many is setting bold. If you do setStyleName("Regular") on a Qfont, setBold(true) will no longer have the intended effect, or possibly not the same effect (apparently you can also end up with an emboldened version of the font, rather than with the actual bold version).

Have you ever worked with professional typesetting, drawing of page-layout applications like InDesign or Illustrator? There you don't set a font to bold, you select another typeface from among the ones you have installed. This is of course how fonts are designed and implemented but most of expect that "old-fashioned inconvenience" to be hidden. IOW, flip a font to bold by hitting Ctrl-B and you get "Foo Bold" rather than "Foo Regular-or-Whatever-you-had turned into something looking bold". Calling setStyleName() brings back a bit of that old-fashioned "pedantic" nature.

Oh, now I understand: It fixes the issue that the styleName was prefilled with the stylename of the default font and fromString might not reset it.
IMO that's a feature though and is the expected behaviour. For instance, if we change the default window title to be bold, users with "windowTitle=Comic Sans" will also have a bold title.

qtbase's src/gui/text/qfont.cpp says:

setFamily(l[0].toString());
    if (count > 1 && l[1].toDouble() > 0.0)
        setPointSizeF(l[1].toDouble());
    if (count == 9) {
        setStyleHint((StyleHint) l[2].toInt());
        setWeight(qMax(qMin(99, l[3].toInt()), 0));
        setItalic(l[4].toInt());
        setUnderline(l[5].toInt());
        setStrikeOut(l[6].toInt());
        setFixedPitch(l[7].toInt());
    } else if (count >= 10) {
        if (l[2].toInt() > 0)
            setPixelSize(l[2].toInt());
        setStyleHint((StyleHint) l[3].toInt());
        setWeight(qMax(qMin(99, l[4].toInt()), 0));
        setStyle((QFont::Style)l[5].toInt());
        setUnderline(l[6].toInt());
        setStrikeOut(l[7].toInt());
        setFixedPitch(l[8].toInt());
        if (count == 11)
            d->request.styleName = l[10].toString();
        else
            d->request.styleName.clear();
    }

So just specifying exactly 10 parameters as part of the config key should have the exact same effect.

IMO that's a feature though and is the expected behaviour. For instance, if we change the default window title to be bold, users with "windowTitle=Comic Sans" will also have a bold title.

So how would you "change the default window title to be bold" and more importantly, how many people are going to have only a font family name in their configuration files? The vast majority will use the available GUI methods for selecting fonts and will end up with a complete specification.

I'd go one step further: why would changes to defaults override choices already made by users? That's bad practice any way you look at it.

So just specifying exactly 10 parameters as part of the config key should have the exact same effect.

I don't know what Qfont method that comes from, but experience shows that just removing the stylename extension from the config key (a freshly generated one) doesn't do the trick. IOW, the stylename isn't cleared.

The config would work with removed styleName() for the cases where the weight() and style() attributes are sufficient. The intention from Qt's side was to uniquely identify fonts that cannot be selected by those two attributes. Imagine a font that has these faces:

• HandwriteRegular
• HandwriteBold
• HandwriteItalic
• HandwriteBoldItalic
• HandwriteCursive
• HandwriteBoldCursive

Now the user picks "HandwriteCursive", and unless we do not save the styleName(), the font matching would probably select "HandwriteItalic" based on the weight() and style() attributes. On the other hand, calling setBold(true) on a font called "HandwriteCursive" _should_ select "HandwriteBoldCursive", even if the styleName was intended to enforce a specific face. If Qt cannot fix this issue, then we have to clear the styleName() at least for those fonts, where a perfect match is possible using the attributes alone (by comparing with the database again).

In D9070#173835, @rjvbb wrote:

IMO that's a feature though and is the expected behaviour. For instance, if we change the default window title to be bold, users with "windowTitle=Comic Sans" will also have a bold title.

So how would you "change the default window title to be bold"

By editing the files in plasma-integration and the fonts kcm in plasma-desktop, chaning the default stylename and weight.

and more importantly, how many people are going to have only a font family name in their configuration files? The vast majority will use the available GUI methods for selecting fonts and will end up with a complete specification.

All of those will also have a StyleName, so this here doesn't apply either.

I'd go one step further: why would changes to defaults override choices already made by users? That's bad practice any way you look at it.

Nothing gets overridden here. It's just that the user didn't specify it.

So just specifying exactly 10 parameters as part of the config key should have the exact same effect.

I don't know what Qfont method that comes from, but experience shows that just removing the stylename extension from the config key (a freshly generated one) doesn't do the trick. IOW, the stylename isn't cleared.

I just checked, the behaviour I described is part of Qt 5.8.0-rc1 and onwards.

In the end, this is just a matter of preference: Should the default stylename for a font family be the same as the stylename we chose as default or just empty?
The options available in the GUI force a selection of the stylename, so it only applies to migrated and hand-edited configuration anyway.

The patch also addresses the bug only for default fonts, but not per-application fonts that write their settings to the appnamerc file.

On the other hand, calling setBold(true) on a font called "HandwriteCursive" _should_ select "HandwriteBoldCursive", even if the styleName was intended to enforce a specific face. If Qt cannot fix this issue, then we have to clear the styleName() at least for those fonts, where a perfect match is possible using the attributes alone (by comparing with the database again).

Indeed - and I think there are limits to what any automated system can do here. There's a point where you have to accept that fonts may have a design issue in how they're named and what additional information is available in the font file (fortunately the font name isn't the only metadata). And at that point you indeed have to fall back to using databases (maybe FontConfig could help here)?

The patch also addresses the bug only for default fonts, but not per-application fonts that write their settings to the appnamerc file.

True. Yet I only got bold syntax highlighting again after I applied this patch in addition to removing the stylename extension from kateschemarc. I don't yet have an explanation for that ...

All of those will also have a StyleName, so this here doesn't apply either.

Hah, that depends with what Qt version they were generated, and on whether or not the user removed the parameter. I didn't invent that move myself, reading comments from others who did that to restore issues is what got me to write this patch in the end.

Nothing gets overridden here. It's just that the user didn't specify it.

Which probably means he didn't want it, in this case... What if that short name is one of the fonts from Christoph's example (and thus includes an explicit weight specification), are you going to do heuristics on it?

This kind of changing the default is fine for users who never use the configuration utilities and thus may not even have a kdeglobals file.
It can get a bit tricky if s/he has a spec in the file that corresponds to an old default, but when in doubt I always assume the user is right in this sort of thing.

If you read https://bugreports.qt.io/browse/QTBUG-63792?focusedCommentId=381570 the take-home message seems to be that the platform theme plugin (and KDE in general) shouldn't be messing with setStyleName() at all UNLESS asking for a font with properties that cannot be represented in the old Panose system.

Such fonts should probably be rare and a priori mostly encountered in very specific applications (Krita, Karbon and the like).

I'm not sure it's Qt bug, it change behavior and now it doing in *right* way. So i'm patch to be merged +1.

So does this actually resolve https://bugs.kde.org/show_bug.cgi?id=378523, or just lay the groundwork for resolving it?

So this this actually resolve https://bugs.kde.org/show_bug.cgi?id=378523, or just lay the groundwork for resolving it?

No, it's more a change similar to proposed fix for that bug (don't set a stylename in KFontRequester).
This patch implements the principle that you shouldn't set a style name if you cannot guarantee that the side-effects of that change are without consequence. The platform theme plugin loads the desktop fonts and if it imposes a style name on them no application can set them to bold or italic anymore.

So you could say that this patch is necessary for the KFontRequester fix to work as intended (as far as the desktop fonts are concerned).

Does anyone have strong objections to landing this?

Is this change purely a conversation of what developers use in code to call up fonts in their applications? Or does this also include a discussion where regular users have interfaces that allow changes to font naming? Let's say, something like System Settings that would allow users to change the naming for system fonts?

Is this change purely a conversation of what developers use in code to call up fonts in their applications?

I think so.

Or does this also include a discussion where regular users have interfaces that allow changes to font naming? Let's say, something like System Settings that would allow users to change the naming for system fonts?

That already exists as you must have noticed: the Fonts settings panel.

This suggested change removes one source of stylenames being set on fonts beyond the user's control. In the original/current code, the table with the default fonts (those used before the user makes any customisations) contains stylenames. Later on in the code those are set on the fonts being looked up.
The modified version still does that, but only if looked-up font already has a stylename set.

Concretely this means a better guarantee that the trick of removing the stylename part from the font descriptions in settings (rc) files will actually work.

The modification should be transparent to anyone who does NOT remove stylenames and is thus the least invasive change. I still think the platform theme plugin should never have started calling setStyleName in the first place, and certainly not for default fonts that are perfectly described by the PANOSE system and thus don't need a stylename.

Thanks for the info! So if/hen this goes in, what are the next steps?

A diagonally related anecdote that shows this location is a more central point in the font selection process than I thought first:

Applications like Qt's Assistant that call for Helvetica often end up using a font that looks pixelated - because it's actually an embedded bitmap version. I only saw that under X11 and always wrote that off to a missing font though the fact that scaling up the text solved the issue appeared strange.
Then I noticed it too on Mac during my comparisons of the CoreText and Freetype font engines.

One solution to this particular issue (itself unrelated to style names) is to use the ForceOutline style strategy when the Freetype engine is used. I implemented that in my Mac version of the platform integration plugin, right next to where this patch applies. That was mostly to be exhaustive (there is no way to set this as a global strategy applying to all current and future fonts). I was surprised to see that it apparently applies to the result of all font lookups.

Thanks for the info! So if/hen this goes in, what are the next steps?

I'd say

• decide whether this is an issue you want to solve only for plasma environments, or for all platforms where KF5 applications are supported.
• draw up the conditions under which a stylename has to be set and whether to rely on Qt to do this in those cases.
• how to avoid the effects of Qt setting a stylename in the conditions where this is not a necessity, how to get rid of it reliably etc.

Optionally, when those things have become clear and have a well-tested implementation, try to work with the Qt team to merge any appropriate parts into Qt. From what I've seen in the exchanges with them it would be of no use to do this any earlier. Their stance seems to be "just don't rely on being able to setBold(true)" or any similar attribute change.

Not that I see how that could be possible, but the observation I posted in my previous comment may indicate that there might be a central location after all where we could get rid of a stylename (in addition to KFontRequester).

Thanks René. Regarding those questions, what would your preferences be?

I've been hitting a very similar bug where a Qt app was trying to use a bold font, and this had no effect when running in a plasma workspace, because of this code in kfontsettingsdata.cpp.

Testcase (couldn't be simpler...) http://www.davidfaure.fr/2018/qlabel_inherited_font.cpp
with this in ~/.config/kdeglobals:

[General]
font=Noto Sans,7,-1,5,50,0,0,0,0,0,Regular

Unfortunately, this patch doesn't fix it. The text still isn't bold.

Workaround: enabling the line that calls QApplication::setDesktopSettingsAware(false).
Proof that this code is guilty: commenting out the fromString line makes it work, too.

What really confuses me is: why isn't it all set up so that this platform-plugin code provides *default* fonts, and so that what apps do overrides that? (sorry that I didn't dig as far as some of you guys did)

Unfortunately, this patch doesn't fix it. The text still isn't bold.

No, this patch is written to be as transparent as possible. It won't impose a stylename, but will not remove it either.

What happens when you remove the ,Regular, i.e.

[General]
font=Noto Sans,7,-1,5,50,0,0,0,0,0

Normally you should get a bold font again then?

Workaround: enabling the line that calls QApplication::setDesktopSettingsAware(false).
Proof that this code is guilty: commenting out the fromString line makes it work, too.

Which line is that, where?

What really confuses me is: why isn't it all set up so that this platform-plugin code provides *default* fonts, and so that what apps do overrides that?

That is what used to be the case, and that's why I think the commit that added the setStyleName call to kfontsettingsdata.cpp ought to be reverted. (I haven't gone back to re-read the commit message, but IIRC it was mostly made to align with the way fonts are saved, without questioning whether they should always be saved like that.)

I'd be happy to modify the patch so it strips the style names from the initial-defaults table as well as the call to setStyleName(). I could even go so far as to add code that strips the styleName if the font can be represented without it - but this also has a side-effect.

In D9070#197332, @rjvbb wrote:

What happens when you remove the ,Regular

That fixes the problem indeed.

Workaround: enabling the line that calls QApplication::setDesktopSettingsAware(false).
Proof that this code is guilty: commenting out the fromString line makes it work, too.

Which line is that, where?

In the code shown by this patch ;) Line 80.
But the test you suggested that I do above is further proof anyhow.

What really confuses me is: why isn't it all set up so that this platform-plugin code provides *default* fonts, and so that what apps do overrides that?

That is what used to be the case, and that's why I think the commit that added the setStyleName call to kfontsettingsdata.cpp ought to be reverted. (I haven't gone back to re-read the commit message, but IIRC it was mostly made to align with the way fonts are saved, without questioning whether they should always be saved like that.)

But even without setStyleName call (i.e. with your patch) I get the bug, due to call to QFont::fromString.

In the code shown by this patch ;) Line 80.

Right. Doh. The line that causes other side-effects when you comment it out :)

But the test you suggested that I do above is further proof anyhow.

But even without setStyleName call (i.e. with your patch) I get the bug, due to call to QFont::fromString.

That's the problem with the styleName: once you caught one it's almost impossible to get rid of in code. You can set an empty styleName, but that doesn't restore the pre-styleName state completely. The only safe way I know of is to convert to string, remove the stylename and recreate a QFont fromString, or write out the equivalent set of operations like I did for QtCurve.

Still, not setting style names in the platform plugin, AND taking care not to set it in KFontRequester should improve the situation - but it will not take care of your tainted settings files for you, of course. :)

David,

Could you please push it for me? I probably won't be able to do so for at least a week and it'd be a pity if the change just misses a release because of that.

Thanks.

Hi,
this patch is still no applied, would someone do it? :)
Because it already missed 5.13.

In D9070#277428, @bkchr wrote:

Hi,
this patch is still no applied, would someone do it? :)
Because it already missed 5.13.

I pushed it now.