Closed Bug 1708285 Opened 4 years ago Closed 4 years ago

Non-native theme prevents LCD antialiasing in content processes.

Categories

(Core :: Widget: Gtk, defect, P2)

defect

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox88 --- unaffected
firefox89 --- verified
firefox90 --- verified

People

(Reporter: Oriol, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(6 files)

Attached file test-case.html (deleted) —

Open attached testcase.

Expected: the text has LCD antialiasing
Actual: just greyscale antialiasing

Works fine with widget.non-native-theme.enabled = false.

Attached image magnified-screenshot.png (deleted) —

Can't reproduce. LCD antialiasing works for me with non-native theme.
Obviously there are some more conditions involved.

Emilio, can you reproduce this anti-aliasing problem on Linux?

AFAIK, the non-native theme should not affect text rendering. I don't see any difference in anti-aliasing with or without non-native theme on my Windows laptop or MacBook Pro. (I don't have a Linux machine.)

Flags: needinfo?(emilio)
Regressed by: nnt-webcompat
Has Regression Range: --- → yes

I can reproduce on 2 different computers. Ubuntu 20.04 with KDE plasma.

Priority: -- → P2

Yeah, I can repro on KDE, but interestingly not on GNOME (or at least not on GNOME + Wayland, will test GNOME + X11 later). Oriol, can you confirm a couple things, just to check I'm seeing the same thing as you are:

  • Once the non-native pref has been off in one content process, then fonts start rendering as expected, even if you flip it back on.
  • There is still LCD-text in the parent process (like the urlbar and about:config).

I suspect that some bits of the GTK initialization are not happening anymore in the content process with the non-native theme.

Assignee: nobody → emilio
Flags: needinfo?(oriol-bugzilla)

Yeah, removing this check seems to fix it... But we don't want to initialize gtk on child processes, so we probably need a different solution, somehow.

Attached file fcpattern-good.txt (deleted) —

So this is the fontconfig pattern we get in the child process with non-native theme disabled (or gtk manually initialized). So far so good, but just for reference.

Attached file fcpattern-bad.txt (deleted) —

This is the one we get with the non-native-theme enabled. Note the difference:

diff --git a/good.txt b/bad.txt
index fc81e41fe20e..566ed27831cd 100644
--- a/good.txt
+++ b/bad.txt
@@ -1,4 +1,4 @@
-Pattern has 37 elts (size 48)
+Pattern has 35 elts (size 48)
        family: "Bitstream Vera Serif"(s)
        familylang: "en"(s)
        style: "Regular"(s) "Roman"(s)
@@ -11,7 +11,6 @@ Pattern has 37 elts (size 48)
        size: 30.72(f)(s)
        pixelsize: 32(f)(s)
        foundry: "Bits"(s)
-       antialias: True(s)
        hintstyle: 1(i)(w)
        hinting: True(s)
        verticallayout: False(s)
@@ -21,7 +20,6 @@ Pattern has 37 elts (size 48)
        index: 0(i)(s)
        outline: True(s)
        scalable: True(s)
-       rgba: 1(i)(s)
        charset: 
        0000: 00000000 ffffffff ffffffff 7fffffff 00000000 ffffffff ffffffff ffffffff
        0001: c00230c0 00030000 c00c0006 61000003 00040000 00000000 00000000 00000000

So we don't know how to do LCD anti-aliasing. So I guess I need to dig into why those fields are appearing, and what GTK does to make those appear, or something.

Those options come from here.... now let's see why they're not the same with the non-native theme disabled.

Summary: Non-native theme prevents LCD antialiasing → Non-native theme prevents LCD antialiasing in content processes.

So, this is the stack of the initialization in the good case:

#0  0x00007fe2a45952bb in gdk_screen_set_font_options (screen=0x7fe2a5a52c00, options=<optimized out>) at /usr/src/debug/gtk3-3.24.29-1.fc34.x86_64/gdk/gdkscreen.c:378
#1  0x00007fe2a4932673 in settings_update_font_options (settings=settings@entry=0x7fe2879041a0) at /usr/src/debug/gtk3-3.24.29-1.fc34.x86_64/gtk/gtksettings.c:3161
#2  0x00007fe2a49380d8 in gtk_settings_create_for_display (display=<optimized out>) at /usr/src/debug/gtk3-3.24.29-1.fc34.x86_64/gtk/gtksettings.c:2002
#3  gtk_settings_get_for_display (display=<optimized out>) at /usr/src/debug/gtk3-3.24.29-1.fc34.x86_64/gtk/gtksettings.c:2028
#4  0x00007fe29e84610a in nsLookAndFeel::ConfigureTheme(mozilla::widget::LookAndFeelTheme const&) (aTheme=...) at /home/emilio/src/moz/gecko/widget/gtk/nsLookAndFeel.cpp:962
#5  0x00007fe29e7e7b01 in nsXPLookAndFeel::GetInstance() () at /home/emilio/src/moz/gecko/widget/nsXPLookAndFeel.cpp:345
#6  0x00007fe29e7e8f01 in mozilla::LookAndFeel::GetColor(mozilla::StyleSystemColor, mozilla::LookAndFeel::ColorScheme, mozilla::LookAndFeel::UseStandins)
    (aId=mozilla::StyleSystemColor::WindowBackground, aScheme=mozilla::LookAndFeel::ColorScheme::Light, aUseStandins=mozilla::LookAndFeel::UseStandins::No) at /home/emilio/src/moz/gecko/widget/nsXPLookAndFeel.cpp:961
#7  0x00007fe29e7dcdde in mozilla::LookAndFeel::Color(mozilla::StyleSystemColor, mozilla::LookAndFeel::ColorScheme, mozilla::LookAndFeel::UseStandins, unsigned int)
    (aId=228, aScheme=(unknown: 0x20), aUseStandins=mozilla::LookAndFeel::UseStandins::No, aDefault=4278190080) at /home/emilio/src/moz/gecko/obj-debug/dist/include/mozilla/LookAndFeel.h:467
#8  0x00007fe29fa7b464 in nsWebBrowser::Create(nsIWebBrowserChrome*, nsIWidget*, mozilla::dom::BrowsingContext*, mozilla::dom::WindowGlobalChild*) (aContainerWindow=<optimized out>, aParentWidget=0x7fe288899800, aBrowsingContext=
    0x7fe28880f800, aInitialWindowChild=0x7fe28918b460) at /home/emilio/src/moz/gecko/toolkit/components/browser/nsWebBrowser.cpp:133
#9  0x00007fe29e3ee829 in mozilla::dom::BrowserChild::Init(mozIDOMWindowProxy*, mozilla::dom::WindowGlobalChild*)

Makes sense, I guess.

Instead of relying on GTK to be initialized in the child process.

Flags: needinfo?(oriol-bugzilla)
Flags: needinfo?(emilio)

Oriol, if you can test a build from https://treeherder.mozilla.org/jobs?repo=try&revision=f8421c7db1c7c51a2739a5971e27661de21ca6eb and confirm it fixes the issue for you it'd be great, thanks!

Attached file GTK init on Gnome (deleted) —

On GNOME, gtk_init ends up creating the settings objects sooner than on wayland, which explains why it works.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)

Oriol, if you can test a build from https://treeherder.mozilla.org/jobs?repo=try&revision=f8421c7db1c7c51a2739a5971e27661de21ca6eb and confirm it fixes the issue for you it'd be great, thanks!

I downloaded the target.tar.bz2 of the Linux x64 opt, but it seems an old 88.0 with widget.non-native-theme.enabled=false.

That's odd, it's on top of today's central, and I just tried.. Maybe you had an existing target.tar.bz2 or firefox/ directory?

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

  • Once the non-native pref has been off in one content process, then fonts start rendering as expected, even if you flip it back on.

Seems to be like this:

  1. With the pref on, I get greyscale antialiasing
  2. I set the pref to false and kill the content process in about:processes. Now I get LCD antialiasing
  3. I set the pref to true and kill the content process in about:processes. I still get LCD antialiasing
  4. I kill the content process in about:processes. Now I get greyscale antialiasing
  • There is still LCD-text in the parent process (like the urlbar and about:config).

Yes.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #16)

That's odd, it's on top of today's central, and I just tried.. Maybe you had an existing target.tar.bz2 or firefox/ directory?

I extracted it into a new firefox-tmp folder and ran it from there ¯\(ツ)

Comment on attachment 9219192 [details]
Bug 1708285 - Pass default font settings from parent to child processes. r=jfkthame

Beta/Release Uplift Approval Request

  • User impact if declined: OS antialiasing settings won't be honored on KDE.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: On KDE, set font settings to LCD antialiasing, and open any page on Firefox, and zoom in with a magnifier. If this is too complex maybe Oriol can confirm once this is on Nightly.
  • List of other uplifts needed: none
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): It's a decently-sized patch, but not too complicated.
  • String changes made/needed: none
Attachment #9219192 - Flags: approval-mozilla-beta?
Flags: qe-verify+

I'm a little unclear whether we've confirmed the patch here fixes the issue reliably... Oriol, could you please take another look at Emilio's try build and check how it behaves for you? I tried downloading the target.tar.bz and can confirm that I got a 90.0a1 build dated 2021-04-29, with non-native-theme enabled by default. Not sure what happened for you but maybe worth trying again?

Flags: needinfo?(oriol-bugzilla)

I tried again and I can confirm it fixes the problem. Maybe the other time I ran firefox instead of ./firefox 😅

Flags: needinfo?(oriol-bugzilla)
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7e8d6b124d6f Pass default font settings from parent to child processes. r=jfkthame
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

Emilio, do you think your GTK fix is safe to uplift to 89 Beta? We have a little over three weeks of bake time until 89 Beta rides to Release (on 2021-06-01).

Flags: needinfo?(emilio)

Yes, otherwise I wouldn't have requested the uplift

Flags: needinfo?(emilio)
QA Whiteboard: [qa-triaged]

I tried to reproduce this issue in several ways, on Ubuntu 20.04 with KDE plasma, set font settings to LCD antialiasing (maybe I didn't do this step properly), but I never noticed the color subpixel rendering neither on the bad build and nor on the fixed build.
Oriol can you confirm this fix on the latest nightly build? Here is the link: https://archive.mozilla.org/pub/firefox/nightly/2021/05/2021-05-04-21-49-50-mozilla-central/firefox-90.0a1.en-US.linux-x86_64.tar.bz2. Thanks!

Flags: needinfo?(oriol-bugzilla)

I have verified that Lubuntu 18.04 with LXDE also had the problem, and it works well in the latest nightly.
I used xzoom to make it bigger since otherwise it's hard to notice.

Flags: needinfo?(oriol-bugzilla)

Thanks Oriol, to reproducing the issue and verified it on latest nightly Firefox build. I will change the flags accordingly, mark as verified on Fx 90.0a1.

Flags: qe-verify+

Comment on attachment 9219192 [details]
Bug 1708285 - Pass default font settings from parent to child processes. r=jfkthame

I am taking this one in beta 9 as it is a new 89 regression, was verified on nightly and has a dupe. That said, given that it has a medium risk, we will back it out from beta if it regresses something. Thanks.

Attachment #9219192 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Hi, Oriol can you confirm this fix again on Fx 89.0b9, please? Thanks.

Flags: needinfo?(oriol-bugzilla)

I'm the one who opened the duplicate bug and I can confirm that the fix works on 89b9, thanks!

Flags: needinfo?(oriol-bugzilla)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: