D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Anthony Fieroni]

anthonyfieroni added a reviewer: dfaure.
anthonyfieroni added a comment.


  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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Nathaniel Graham]

ngraham added a comment.


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

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  >   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).

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Nathaniel Graham]

ngraham added a comment.


  Does anyone have strong objections to landing this?

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Andres Betts]

abetts added a comment.


  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?

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  >   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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Nathaniel Graham]

ngraham added a comment.


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

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  >   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).

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Nathaniel Graham]

ngraham added a comment.


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

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[David Faure]

dfaure added a comment.


  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)

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  >   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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[David Faure]

dfaure added a comment.


  In https://phabricator.kde.org/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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: rdieter, abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  >   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. :)

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: rdieter, abetts, anthonyfieroni, ngraham, cfeck, fvogt, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[David Faure]

dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  I think this patch makes sense, even though I'm sad that it doesn't actually 
fix the bug I was seeing (you call it broken config file, in that case that 
would mean that the code writing out that config file is -- or was -- broken, 
if it shouldn't write out ",Regular").

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: ahmadsamir, rdieter, abetts, anthonyfieroni, ngraham, cfeck, fvogt, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: ahmadsamir, rdieter, abetts, anthonyfieroni, ngraham, cfeck, fvogt, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Bastian Köcher]

bkchr added a comment.


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

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: bkchr, ahmadsamir, rdieter, abetts, anthonyfieroni, ngraham, cfeck, fvogt, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Fabian Vogt]

This revision was automatically updated to reflect the committed changes.
Closed by commit R135:2e971be576d2: KDE platform plugin: don't force 
default stylename on user-specified fonts (authored by rjvbb, committed by 
fvogt).

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9070?vs=23175&id=36055

REVISION DETAIL
  https://phabricator.kde.org/D9070

AFFECTED FILES
  src/platformtheme/kfontsettingsdata.cpp

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: bkchr, ahmadsamir, rdieter, abetts, anthonyfieroni, ngraham, cfeck, fvogt, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Fabian Vogt]

fvogt added a comment.


  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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck, dfaure
Cc: bkchr, ahmadsamir, rdieter, abetts, anthonyfieroni, ngraham, cfeck, fvogt, 
plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Anthony Fieroni]

anthonyfieroni added reviewers: Frameworks, davidedmundson, graesslin.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin
Cc: fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  >   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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin
Cc: fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Christoph Feck]

cfeck added a reviewer: cfeck.
cfeck added a comment.


  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).

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck
Cc: cfeck, fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Fabian Vogt]

fvogt added a comment.


  In https://phabricator.kde.org/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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck
Cc: cfeck, fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[Christoph Feck]

cfeck added a comment.


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

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck
Cc: cfeck, fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  > 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.

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck
Cc: cfeck, fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D9070: KDE platform plugin: don't force default stylename on user-specified fonts

[René J . V . Bertin]

rjvbb added a comment.


  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).

REPOSITORY
  R135 Integration for Qt applications in Plasma

REVISION DETAIL
  https://phabricator.kde.org/D9070

To: rjvbb, #frameworks, davidedmundson, graesslin, cfeck
Cc: ngraham, cfeck, fvogt, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart