Fix "Draw intense colors in bold font" feature
ClosedPublic

Authored by mglb on Feb 23 2019, 11:16 PM.

Details

Summary

Make the feature work and improve it slightly - the weight used as bold
is dependent on selected font's weight. "Regular" will use "Bold", but
e.g. "Thin" will use "Light".

styleName is almost always redundant - all font properties like
weight, stretch, etc, are stored separately.

Source Code Pro 12pt with font weight set to: Extra Light, Light,
Regular, Medium, Semibold, Bold:

Test Plan

In every case: turn on "Draw intense colors in bold font" in Edit
Profile → Appearance.

The feature:

  • Use some font which has "Bold" style available and set it to "Regular" style.
  • Run: printf '\033[1mBold|\033[0m|Normal\n'
  • "Bold|" text should use bold font.

Different weights:

  • Use some font which has multiple weights available (e.g. Thin, Bold, Heavy, ...), e.g. Source Code Pro.
  • Select lightest style.
  • Run: printf '\033[1mBold|\033[0m|Normal\n'
  • Select "regular" font style.
  • Bold text for "regular" style should be bolder than the text for lightest style should be.

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mglb requested review of this revision.Feb 23 2019, 11:16 PM
mglb created this revision.
mglb retitled this revision from For MacOS portion, use nullptr instead of NULL to Fix "Draw intense colors in bold font" feature.Feb 23 2019, 11:17 PM
mglb updated this revision to Diff 52417.Feb 23 2019, 11:21 PM
mglb edited the summary of this revision. (Show Details)
mglb edited the test plan for this revision. (Show Details)

Fix diff

Neat! But is this essentially a workaround for https://bugs.kde.org/show_bug.cgi?id=378523?

mglb added a comment.Feb 24 2019, 8:29 AM

Depends on your opinion. As for me:

  • It is nice that we get all possible information about what an user selected in the font dialog - we can remove/change things we don't need/support.
  • The style name allows for most precise (afaik) style specification. Technically, there can exist two styles, with the same weight, stretch, etc, but different glyphs. However, Qt should unset styleName after manual change of particular style properties. It doesn't, so we need to do it manually (this can be considered the problem to which the patch is a workaround).

If you just mean "does this fix that bug" - yes.

Well +1 for fixing it in any case! But it might be nice to try to improve the situation in Qt or KFontRequester too. Those issues keep getting stalled. :(

hindenburg accepted this revision.Feb 24 2019, 10:37 PM
hindenburg added a subscriber: hindenburg.

Thanks, looks fine. A minor nitpick, if you're going to format the comments, try for < 80 column width - typically 70ish is better.

This revision is now accepted and ready to land.Feb 24 2019, 10:37 PM
This revision was automatically updated to reflect the committed changes.
mglb added a comment.Feb 25 2019, 5:09 AM

A minor nitpick, if you're going to format the comments, try for < 80 column width - typically 70ish is better.

Ok, will remember

Should this bug fix be applied to Applications/18.12 as well?

Yes, I missed the tagging date unfortunately.

Does not seem to be working with a "Text" font style, such as IBM Plex Mono Text. Bold does not appear bold!

mglb added a comment.Mar 12 2019, 9:08 AM

https://fonts.google.com/specimen/IBM+Plex+Mono this one?

It works with base font (IBM Plex Mono). IBM Plex Mono Light/Medium/... do not work because they are separate fonts, each with only one weight.

In D19266#429513, @mglb wrote:

https://fonts.google.com/specimen/IBM+Plex+Mono this one?

It works with base font (IBM Plex Mono). IBM Plex Mono Light/Medium/... do not work because they are separate fonts, each with only one weight.

Correct, that is the font family, but I am using the ttf-ibm-plex package from official Arch Linux repo which also include the "Text" variant. Can be found here: IBM GitHub

I prefer the Text style because it is slightly thicker than the regular. Is this QT related issue then? For testing purposes, I tested with Gnome Terminal and Bold would work if I choose IBM Plex Sans Text as my font!

mglb added a comment.Mar 15 2019, 11:27 AM

Is this QT related issue then?

Most likely - when you open font selector in any Qt app, Plex Mono font has only 4 basic styles. To get "text" style you'll have to use Plex Mono Text font (and there is no bold).

However, this is the case only with ttf fonts. I've tried OTF fonts (available on github) and they work as they should - Plex Mono has all available styles, so "bold" characters are bolder than normal text (for all styles except boldest one).

mglb added a comment.Mar 15 2019, 11:10 PM

@ngraham this is something else (styleName is handled in the patch). The fonts and their styles are not linked correctly, so we have fonts/styles:

  • IBM Plex Mono
    • Regular
    • Regular Italic
    • Bold
    • Bold Italic
  • IBM Plex Mono Text
    • Regular
    • Regular Italic

To get the font with "Text" style, you have to select "IBM Plex Mono Text" font, which has only one style - "Regular". Since there is nothing bolder in this specific font, "bold" does not work. The correct behavior is when the "IBM Plex Mono" font has all possible styles (including text and bold), which is the case with OTF fonts described above. Note that "IBM Plex Mono Text" will still exist even when "IBM Plex Mono" has its styles.
However, I did the first test (not working) with Qt 5.11 + TTF probably from different places (Google Fonts and Github). "Working" test uses Qt 5.9 + all OTF from github. Now I am testing it with Qt 5.11 and TTF fonts from github work correctly too. Gnome Terminal did work in all cases.