New issue
Advanced search Search tips
Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment
link

Issue 901611: Fonts look too thin on 10.14 Mojave still

Reported by rsesek@chromium.org, Nov 3 Project Member

Issue description

Chrome Version: 71.0.3578.30
OS: 10.14.1

What steps will reproduce the problem?
(1) Look at fonts on crbug
(2) Look at fonts in the app/three-dot menu

What is the expected result?
Fonts should have more weight.

What happens instead?
Fonts look thin and washed-out, see attached screen shot. This version does include the fix from  issue 858861 .

This is on a non-retina iMac 13,2. AppleFontSmoothing is set to 1 (enabled).
 
Screen Shot 2018-11-03 at 9.43.56 AM.png
531 KB View Download

Comment 1 by meh...@chromium.org, Nov 3

Labels: -Type-Bug -Pri-2 Pri-1 Type-Bug-Regression
This looks like a recent regression.

In Chrome Stable Version 70.0.3538.77 it looks better.

I am using a non-retina MacBook Air 2012.

A screenshot is attached.

Left Canary, right Stable.
Bildschirmfoto 2018-11-03 um 21.54.43.png
335 KB View Download

Comment 2 by meh...@chromium.org, Nov 3

Hmm, I'am trying to find the regression range, but unfortunately I can't :(

Could this be caused by an experiment that I/we are in?

Comment 3 by meh...@chromium.org, Nov 3

Okay, I have done some further testing.

I tested build 595178, which doesn't include the fix from  issue 858861  and I tested the build 595198 which includes the fix.

The strange thing is that without the fix the font looks fine and with the fix not.

Could it be that Apple changed something in 10.14.1, so that the fix from  issue 858861  is no longer necessary?

Regression range: https://chromium.googlesource.com/chromium/src/+log/ce145c44392b06566e731c1b71d50b88ef21cf65..404c2004ebc8a5ebca2782078b473a4b28551960?pretty=fuller&n=10000

Comment 4 by meh...@chromium.org, Nov 3

+screenshots comparing the font in build #595178 and #595198 under macOS 10.14.1
#595178.png
93.3 KB View Download
#595198.png
96.3 KB View Download

Comment 5 by rsesek@chromium.org, Nov 4

Thanks for investigating. Another possibility is that the fix from  issue 858861  works well on retina, but not on non-retina.

I can also confirm that build 595178 (pre-fix) looks better on my machine.

Comment 6 by rsesek@chromium.org, Nov 4

I extracted the new smooth_behavior() function from the fix in https://skia-review.googlesource.com/c/skia/+/157566/ into a standalone file so I could run it on my Mac (attached). I'm getting SmoothBehavior::some when I run it locally.
smooth_behavior.cc
15.8 KB View Download

Comment 7 by lgrey@google.com, Nov 8

Cc: bunge...@chromium.org
Components: Internals>Skia

Comment 8 by rsesek@chromium.org, Nov 20

bungeman: Are there more tweaks we can make when a Mac is reporting SmoothBehavior::some ? My screenshots match mehmet@'s and I think Chrome's text looks considerably worse on 10.14 versus 10.13 on the same Mac.

Comment 9 by meh...@chromium.org, Nov 25

My bad eyesight has really problems with this washed-out font on the non-retina devices :( I would really appreciate a fix here. Thanks in advance :)

Comment 10 by ellyjo...@chromium.org, Nov 26

Cc: -bunge...@chromium.org
Labels: Target-73 M-73
Owner: bunge...@chromium.org
Status: Assigned (was: Untriaged)
Mac triage: to bungeman@ with labels - is there something more we can do here?

Comment 11 by kon...@gmail.com, Nov 26

Yet somehow Safari seems to be totally immune to this. I guess if Safari is immune (or appears to be immune), then Chrome could somehow do the same...?

Native apps seem to be unaffected though...

Comment 12 by terry27...@gmail.com, Dec 13

I agree; the Safari font implementation even with the change from  issue 858861  is bolder and at a more readable weight than Chrome's on Retina screens.

Comment 13 by reed@google.com, Dec 13

Cc: hcm@google.com djsollen@google.com

Comment 14 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/16d00eeef7d18dcfd28725bcb4c15dad633ab7eb

commit 16d00eeef7d18dcfd28725bcb4c15dad633ab7eb
Author: Ben Wagner <bungeman@google.com>
Date: Fri Dec 14 21:32:24 2018

CG smoothing implies gamma blit.

There are three distinct attributes controlled by font smoothing,
subpixel converage, fake bolding, and gamma 2 blitting. In macOS 14 the
subpixel coverage was removed but the other two apparently remain. Prior
to this change the code had assumed that the gamma 2 blitting had also
been removed, but this appears not to be the case.

Bug:  chromium:901611 
Change-Id: Ie0623267fa20c86d953e8b6c1fb3ead7be51b930
Reviewed-on: https://skia-review.googlesource.com/c/177880
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>

[modify] https://crrev.com/16d00eeef7d18dcfd28725bcb4c15dad633ab7eb/src/ports/SkFontHost_mac.cpp

Comment 15 by bugdroid1@chromium.org, Dec 15

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df641df1ddbd97c134c3249d20cec5534b31c08e

commit df641df1ddbd97c134c3249d20cec5534b31c08e
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Sat Dec 15 03:23:05 2018

Roll src/third_party/skia af67819ee1d9..a1bded9a4f28 (9 commits)

https://skia.googlesource.com/skia.git/+log/af67819ee1d9..a1bded9a4f28


git log af67819ee1d9..a1bded9a4f28 --date=short --no-merges --format='%ad %ae %s'
2018-12-14 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/angle2 dfaccbc08abb..a7af56be7a6c (3 commits)
2018-12-14 bsalomon@google.com Fix stroked round capped circular arc batched with filled circle.
2018-12-14 bungeman@google.com CG smoothing implies gamma blit.
2018-12-14 kjlubick@google.com [canvaskit] Canvas API for loading fonts
2018-12-14 kjlubick@google.com [canvaskit] Expose setVolatile (especially for animations)
2018-12-14 benjaminwagner@google.com Add another NVIDIA LSAN suppression.
2018-12-14 bsalomon@google.com Reduce the number of backend->pixelconfig GrCaps virtuals.
2018-12-14 kjlubick@google.com Avoid system fonts when fuzzing
2018-12-14 caryclark@skia.org protect against fuzz generated fLastMoveToIndex


Created with:
  gclient setdep -r src/third_party/skia@a1bded9a4f28

The AutoRoll server is located here: https://autoroll.skia.org/r/skia-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG=chromium:b/119394958,chromium:901611,chromium:818769
TBR=ethannicholas@chromium.org

Change-Id: I47033164a4df7fc2b617c556f07232bc1ee033b0
Reviewed-on: https://chromium-review.googlesource.com/c/1378836
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#616939}
[modify] https://crrev.com/df641df1ddbd97c134c3249d20cec5534b31c08e/DEPS

Comment 16 by meh...@chromium.org, Dec 16

Great! After your changes in c#14 and c#15, the fonts in the Chrome UI and in the web contents look crisp and clear again in latest Canary Version 73.0.3642.0 on my Non-Retina Device(s) with 10.14.2 Mojave. Thanks you very much :)

Comment 17 by meh...@chromium.org, Dec 16

+ Screenshot: Chrome Stable vs Chrome Canary vs Safari
Bildschirmfoto 2018-12-16 um 17.46.32.png
378 KB View Download

Comment 18 by bunge...@chromium.org, Dec 17

Labels: Merge-Request-72
Requesting merge of Skia change https://skia.googlesource.com/skia/+/16d00eeef7d18dcfd28725bcb4c15dad633ab7eb%5E%21/#F0 to Skia branch https://skia.googlesource.com/skia/+/chrome/m72 to fix this on 72 as well.

Comment 19 by sheriffbot@chromium.org, Dec 17

Project Member
Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 20 by srinivassista@google.com, Dec 17

bungeman@ how safe is this change to take into M72. The change looks simple but want to make sure it would not create any other regressions.

Comment 21 by bunge...@chromium.org, Dec 17

This is a super safe change. This is both small and targeted and exactly describes the difference between how things where thought to work and how they actually work.

Comment 22 by srinivassista@google.com, Dec 17

Labels: -Merge-Review-72 Merge-Approved-72
approving for M72 , branch: 3626

Comment 23 by bugdroid1@chromium.org, Dec 17

Project Member
Labels: merge-merged-m72
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/6e486700380b3b4f352b40f8ec33888e48b66516

commit 6e486700380b3b4f352b40f8ec33888e48b66516
Author: Ben Wagner <bungeman@google.com>
Date: Mon Dec 17 20:39:15 2018

CG smoothing implies gamma blit.

There are three distinct attributes controlled by font smoothing,
subpixel converage, fake bolding, and gamma 2 blitting. In macOS 14 the
subpixel coverage was removed but the other two apparently remain. Prior
to this change the code had assumed that the gamma 2 blitting had also
been removed, but this appears not to be the case.

Bug:  chromium:901611 
Reviewed-on: https://skia-review.googlesource.com/c/177880
Reviewed-by: Herb Derby <herb@google.com>

Change-Id: I4da500e19066efefb27825f537bc5359375e7ebe
Cherry-pick: 16d00eeef7d18dcfd28725bcb4c15dad633ab7eb
Approval:  https://crbug.com/901611#c22 
Reviewed-on: https://skia-review.googlesource.com/c/178266
Reviewed-by: Ben Wagner <bungeman@google.com>

[modify] https://crrev.com/6e486700380b3b4f352b40f8ec33888e48b66516/src/ports/SkFontHost_mac.cpp

Comment 24 by bunge...@chromium.org, Dec 17

Status: Fixed (was: Assigned)

Comment 25 by abdulsyed@google.com, Dec 18

Labels: -Merge-Approved-72

Sign in to add a comment