Closed Bug 1732577 Opened 3 years ago Closed 2 years ago

On Linux, with hinting disabled (hintstyle=hintnone), TTF glyph widths are rounded to integers

Categories

(Core :: Graphics: Text, defect)

Firefox 92
defect

Tracking

()

VERIFIED FIXED
96 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox-esr91 --- fixed
firefox92 --- wontfix
firefox93 --- wontfix
firefox94 --- wontfix
firefox95 --- wontfix
firefox96 --- fixed

People

(Reporter: nyanpasu64, Assigned: lsalzman)

References

(Regression)

Details

(Keywords: regression)

Attachments

(5 files)

Attached file Test page

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:92.0) Gecko/20100101 Firefox/92.0

Steps to reproduce:

  • Launch a KDE Plasma X11 or Wayland session.
    • This does not happen on Xfce or Gnome X11.
  • In System Settings's Fonts, ensure Anti-Aliasing is enabled, and set hinting to None. (Sub-pixel rendering can be set to None or RGB.)
  • Restart Firefox.
  • (optional) Go to htmledit.squarefree.com and paste in <span style="font-family: liberation sans">||||||||||||||||||||||||||||||||</span> (or another choice of installed TTF font). If you don't have Liberation Sans installed and | appears too wide (making it hard to identify the width rounding), try repeating l instead.
  • (optional) Open a Google Doc, switch to Arial or Source Sans Pro (or another TTF font), and repeatedly type | or l.

Actual results:

For TTF (not OTF) fonts, glyph advance widths are now integers. Character spacing/kerning is awkward. Changing zoom level can now change the percentage of a line a word takes up. The width of repeating strings like |||||||||||||||||||||||||||||||| (32 characters wide) is always an integer multiple of 32 pixels, even when you change browser zoom or font size smoothly.

Expected results:

Disabling hinting preserves fractional glyph advance/widths.

(I'm currently digging deeper into the root cause and will reply soon™.)

Investigation

I decided to compare the environment of KDE vs. GNOME to find the cause of the bug. (I didn't fully test the behavior of GNOME on Wayland, only Xorg.)

On both KDE and GNOME, xrdb -query -all|grep Xft|sort shows the same output:

Xft.antialias:  1
Xft.dpi:        96 (or 120, same bug either way)
Xft.hinting:    0
Xft.hintstyle:  hintnone
Xft.rgba:       rgb (or none, same bug either way)

I found that on Firefox KDE, I can fix Firefox's behavior (reenable fractional advance while keeping hinting off) by editing ~/.config/fontconfig/fonts.conf and changing "hintstyle" from hintnone to hintslight, but keeping "hinting" at false.

If I start a Xfce session and disable hinting in Appearance (or a GNOME session and disable hinting in Tweaks), Firefox renders with hinting disabled. If I then add a fonts.conf created by KDE with hinting enabled (with hinting=true and hintstyle=hintslight), this changes fc-match -v on Xfce from hinting: True(s) to hinting: True(w). The (w) causes fontconfig to override xrdb's Xft.hinting: 0, and Firefox starts rendering with hinting enabled, ignoring Xfce's Appearance settings. The presence of a fonts.conf created by KDE causes Xfce/GNOME's font settings to be ignored. This is probably a bug caused by mixing DEs (there are so many bugs caused by mixing DEs...)

I noticed that fc-match -v on GNOME and XFCE returns hintstyle: 1(i)(w) which overrides xrdb, even with no ~/.config/fontconfig/fonts.conf present. This is because on my Arch Linux systems, "/etc/fonts/conf.d/10-hinting-slight.conf" sets <edit name="hintstyle" mode="append"><const>hintslight</const></edit>. I think this makes gnome-tweaks and Xfce's Hinting control (which only sets xrdb's Xft.hintstyle) not affect Firefox at all.

If I temporarily remove "/etc/conf.d" entirely, then fc-match -v says hintstyle: 3(i)(s) and hinting: True(s). This indicates that (s) means the setting wasn't explicitly set and this is the default value, whereas (w) settings were explicitly set.

Summary

Changing font settings in KDE writes to ~/.config/fontconfig/fonts.conf (which affects other DEs) and xrdb (only during KDE). Changing font settings in GNOME/Xfce only writes to xrdb (only during GNOME/Xfce), not fonts.conf (maybe they write to GSettings too, but if so, I think Firefox doesn't read it). GNOME and Xfce hinting settings are changed independently.

Fontconfig rules in "~/.config/fontconfig/fonts.conf" with modes "assign" or "append" (I think any mode) cause fc-match -v to display the rule with (w), indicating an explicitly set value. Removing the fontconfig files containing these rules causes the lines in fc-match -v to disappear or say (s), indicating a default value.

Firefox picks its font rendering settings by mixing settings from xrdb Xft and fontconfig queries (unsure about GSettings). In Firefox, fontconfig rules with (w) override the corresponding xrdb Xft setting, whereas fontconfig rules with (s) are overriden by xrdb.

Firefox currently disables fractional advance when the merged/effective hintstyle is hintnone. When GNOME/Xfce or KDE disable hinting, they change hintstyle to hintnone and turn hinting off. But GNOME/Xfce only change xrdb, and hintstyle=hintnone is ignored because fontconfig's "/etc/fonts/conf.d/10-hinting-slight.conf" sets hintstyle to hintslight which overrides xrdb (causing Firefox to ignore GNOME/Xfce's hintstyle). When KDE disables hinting, it writes to "~/.config/fontconfig/fonts.conf" (which overrides "10-hinting-slight.conf"), and its hintstyle=hintnone causes Firefox to switch to integer glyph widths (for TTF fonts, not OTF fonts).

Firefox should only disable fractional advance when antialias is off, and otherwise keep it on regardless of the current hinting settings (i.e. ignore hintstyle).

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Component: Widget: Gtk → Graphics: Text

So a bunch of people have CC'd onto this bug. If you're interested, my (unfinished) investigation is at https://docs.google.com/document/d/1b4qEwuWzz5ycCEVBfd73wQNzhrau18uqCfBx-bcj32o/edit?usp=sharing. Here's my summary so far:

gfxFontconfigFontEntry::CreateFontInstance() determines glyph sizing, but not layout or rendering. First it calls PrepareFontOptions. When FC_HINT_STYLE is slight (regardless if hinting is enabled), PrepareFontOptions sets loadFlags to FT_LOAD_TARGET_LIGHT, which enables fractional glyph widths. Afterwards, CreateFontInstance() uses loadFlags to create a gfxFontconfigFont object (and the base class initializes mFTLoadFlags = loadFlags), which tells FreeType how to compute glyph widths (but not how to layout or render text).

ScaledFontFontconfig.cpp#InstanceData() controls whether the renderer hints glyphs. If FC_HINTING is false, mHinting is NONE, and otherwise FC_HINT_STYLE is used to determine mHinting (which affects on-screen text appearance).

ScaledFontFontconfig::UseSubpixelPosition() determines whether glyphs should be rendered at subpixel positions. If this is forced to return false, then glyphs will be positioned with subpixel advance, but rendered on-screen at rounded integer coordinates (like Office 2003).


Firefox has 3 functions that control fractional glyph widths/positioning, that do subtly different things.

ScaledFontFontconfig::UseSubpixelPosition() (true) determines whether glyphs should be rendered at subpixel positions. If this is forced to return false, then glyphs have fractional advance widths and fractional positions, but they get rounded to integer coordinates when rendering on-screen. This results in uneven glyph spacing, like Office 2003 and today's LibreOffice.

gfxFT2FontBase::ShouldRoundXOffset() (false) determines whether glyphs should have integer layout advance widths. If this is forced to return true, repeating a single character results in integer spacing, regardless of position.

gfxFT2FontBase::ShouldHintMetrics() (true for all non-print gfxFontconfigFont) is a mistake. If ShouldHintMetrics is true, then gfxFT2FontBase::GetFTGlyphExtents() rounds font advance widths to integer pixels (somewhat like setting ShouldRoundXOffset() to false), but only if hinting is disabled via FT_LOAD_NO_HINTING (because if hinting was enabled, then FreeType would handle rounding itself)... or that's the idea…

To be continued. I think gfxFT2FontBase::GetFTGlyphExtents() is wrong, and all usages of ShouldHintMetrics() are suspect. What was the intent of ShouldHintMetrics()? Vertical quantization? Horizontal and vertical quantization?

Correction: If ShouldHintMetrics is true, then gfxFT2FontBase::GetFTGlyphExtents() rounds font advance widths to integer pixels (somewhat like setting ShouldRoundXOffset() to false), but only if hinting is disabled via mFTLoadFlags & FT_LOAD_NO_HINTING (because if hinting was enabled, then FreeType would handle rounding itself… or that's the idea…)

According to Searchfox, ShouldHintMetrics is only referenced in two functions (gfxFont::GetRoundOffsetsToPixels() and gfxFT2FontBase::GetFTGlyphExtents()). The other function gfxFont::GetRoundOffsetsToPixels() seems to treat !ShouldHintMetrics as "don't round horizontal and vertical", and ShouldHintMetrics as "round vertical, and only round horizontal if ShouldRoundXOffset".

(Are ShouldHintMetrics and ShouldRoundXOffset really supposed to be separate methods, overridden in different subclasses with different implementations, but with related meanings? Or did this organically evolve over time with no high-level or greenfield plan?)

I decided to change gfxFT2FontBase::GetFTGlyphExtents() to behave like gfxFont::GetRoundOffsetsToPixels(), by only rounding horizontal advance if both ShouldHintMetrics() and ShouldRoundXOffset() were true (replacing some code checking ShouldHintMetrics and other code checking ShouldHintMetrics). It works in my testing, but is this the right decision?

Diff attached. I don't know right now how to push a Firefox patch; I'm new to Mercurial.

I'm not super familiar with Firefox's font architecture, but why do PrepareFontOptions() (gfxFontconfigFontEntry::CreateFontInstance()) and ScaledFontFontconfig::InstanceData::InstanceData() independently query fontconfig (FcPatternGetBool(...FC_HINTING) and FcPatternGetInteger(...FC_HINT_STYLE)), and have different control flow (and InstanceData() uses "clever" non-obvious logic rather than a consistent pattern)? Is this due to a deliberate module boundary and separation of responsibility, or due to a historical accident of the way Firefox evolved?

Do you plan to rewrite the font loading code in Rust, and write a fontconfig wrapper function returning Result<bool/i32, FontConfig::Error>? It would be easier to understand than InstanceData().

In bug 1547063, a bug was introduced where a logic condition was improperly negated.
!(printing || !hinting) became (!printing || hinting), when the negation is actually
!printing && hinting. This fixes that so that we do the right thing here and actually
ignore the following hint-style check if hinting is false.

Assignee: nobody → lsalzman
Has Regression Range: --- → yes

I don't know whether your patch (if (!printing && hinting &&) fixes a logic error or not, but it does not fix my issue (turning hinting off forces integer glyph widths).

Correction: Your patch causes this bug to appear on XFCE as well, which is good for consistency I guess. (As you said, now PrepareFontOptions() properly detects "hinting off" even if hintstyle is set.)

Is there any more testing or investigation I should do, or should I stop commenting on this bug for now?

Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d4e863a0a523
Fix interpretation of Fontconfig hinting toggle in PrepareFontOptions. r=jfkthame
Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---

This patch does not fix the bug, but causes it to occur on XFCE (and probably GNOME) as well as KDE.

The severity field is not set for this bug.
:lsalzman, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(lsalzman)

(In reply to nyanpasu64 from comment #12)

This patch does not fix the bug, but causes it to occur on XFCE (and probably GNOME) as well as KDE.

I'll investigate this bug further next week. At least for now, I just managed to fix the coincidental finding of the hinting interpretation being buggy. It's okay to leave this bug open.

Flags: needinfo?(lsalzman)

The severity field is not set for this bug.
:lsalzman, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(lsalzman)
Status: UNCONFIRMED → NEW
Ever confirmed: true

This bug originated from bug 1547063 when we started to allow subpixel positioning
for FreeType fonts. It would check if we requested no hinting from FreeType and
always try to round the X extents. However, light hinting disables X hinting as
well (just not Y hinting), so the behavior in this case diverged between light and
no hinting. This should make the two cases now look similar.

Flags: needinfo?(lsalzman)
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/666f41aa52c9
Don't round FT glyph extents when hinting is disabled. r=jfkthame
Status: NEW → RESOLVED
Closed: 3 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: 95 Branch → 96 Branch

Can you verify if my latest patch fixed the issue in nightly? Disabling hinting should now have a similar effect to (s)light hinting, which ironically did not actually round glyph metrics while unhinted did...

Flags: needinfo?(nyanpasu64)

The bug seems fixed. Thanks!

Status: RESOLVED → VERIFIED
Flags: needinfo?(nyanpasu64)

Comment on attachment 9251444 [details]
Bug 1732577 - Don't round FT glyph extents when hinting is disabled. r?jfkthame

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Font layout issues on potentially all content for Linux users
  • User impact if declined: Font layout issues on potentially all content for Linux users. Linux distributions tend to ship ESR, so for such users, it would be an extremely long time before they would ever see this fix if we don't uplift to ESR.
  • Fix Landed on Version: 96
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Impact confined to Linux users who have disabled font hinting in system settings.
  • String or UUID changes made by this patch:
Attachment #9251444 - Flags: approval-mozilla-esr91?
Attachment #9245127 - Flags: approval-mozilla-esr91?

This has landed too recently on mozilla-central to be uplifted in our upcoming ESR. I am also wondering how much of an edge case it is for ESR91 given that there are no duplicates and we have this bug since version 71. Major distros follow our rapid releases cycle, I can only think of RHEL (which defaults to Gnome, so not affected) and Debian/KDE that are popular distros using ESR by default and may have some users experimenting this bug.

(In reply to Pascal Chevrel:pascalc from comment #22)

This has landed too recently on mozilla-central to be uplifted in our upcoming ESR. I am also wondering how much of an edge case it is for ESR91 given that there are no duplicates and we have this bug since version 71. Major distros follow our rapid releases cycle, I can only think of RHEL (which defaults to Gnome, so not affected) and Debian/KDE that are popular distros using ESR by default and may have some users experimenting this bug.

The bug title is incorrect. This is not related to do with KDE, and is merely about whether you change settings in Fontconfig, or any settings panel which lets you change global font settings, which anyone can do on any distribution.

Summary: On Linux KDE, with hinting disabled (hintstyle=hintnone), TTF glyph widths are rounded to integers → On Linux, with hinting disabled (hintstyle=hintnone), TTF glyph widths are rounded to integers
Flags: qe-verify+
Flags: qe-verify+

Comment on attachment 9245127 [details]
Bug 1732577 - Fix interpretation of Fontconfig hinting toggle in PrepareFontOptions. r?jfkthame

Approved for 91.5esr.

Attachment #9245127 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Attachment #9251444 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
QA Whiteboard: [qa-96b-p2]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: