Awkward letter spacing on BlinkMacSystemFont |
|||||||||||||
Issue descriptionGithub.com just rolled out a change to their fonts, placing -apple-system and BlinkMacSystemFont at the top of their font stack. On Mac, things look awkward. The attached screenshot clearly shows the text looking sensible in Safari and difficult in Chrome. In addition to being used on Github, DevTools uses the systemfont internally, and we've noticed some odd spacing/kerning/tracking in our text as well.
,
Jul 12 2016
I am, alas, not a layout or text expert. Adding some others.
,
Jul 12 2016
LayoutTextControlMultiLine::getAvgCharWidth should be unrelated. It is only for <input> and <textarea>.
,
Jul 12 2016
Aye, looking some more, it appears LayoutTextControlMultiLine::getAvgCharWidth is indeed unrelated. Apologies. Any other ideas on why our rendering is so wonky in comparison to Safari's?
,
Jul 13 2016
BTW, WebKit still has the hack, they just moved it down to Font (which looks sensible to me): http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/FontCascade.cpp#L420 It looks like either kerning is turned off, or subpixel is disabled, probably the latter. Are you on non-Retina? I'm seeing on my Retina mac, see a slight difference but not as much as the screenshot.
,
Jul 13 2016
I posted the original screenshot on Twitter. I'm on a 2013 MacBook Air, so non-Retina. I have no idea about the Blink or Webkit internals, but my initial thoughts were a problem with both kerning and subpixel rendering. Still seems likely (at least kerning is definitely different, and I'd argue wrong).
,
Jul 13 2016
Do you have a URL that demonstrates the problem? Ideally the one from your screenshot? https://github.com/ uses the webfont Roboto exclusively for me on both Mac, Windows and Linux. Regardless, LayoutTextControlMultiLine::getAvgCharWidth is unrelated as it's only used for form controls.
,
Jul 13 2016
Hi, I work at Github! You'll want to visit a repository page to see the font stack in full effect. We use Roboto in our marketing pages. See https://github.com/github/linguist as an example.
,
Jul 13 2016
Re: #8, thanks Otto. On a retina mac the rendering looks almost identical between Safari and Chrome. Both have kerning enable and respect the fonts kerning information. I suspect that Safari uses optimzeSpeed (which disables kerning) on non-retina but don't have a non-retina mac to test on at the moment. Could you please try to either turn on kerning on Safari (using text-rendering: optimizeLegability) or disable kerning on Chrome (using font-kerning: none)?
,
Jul 14 2016
I believe Safari (9.1.1 on macOS 10.11.5) is already using "text-rendering: optimizeLegability". Setting it has no effect. Setting "text-rendering: optimizeSpeed" spaces out the letters (increases the kerning) very slightly, but not to the same extent that Chrome is doing by default. None of the other "text-rendering" options has any effect. On Chrome (51.0.2704.106), setting "font-kerning: none" increases the kerning more than it already is (though only slightly more), making it even worse. Setting any "text-rendering" options has no effect. Neither did adjust the "LCD text antialiasing" flag.
,
Jul 14 2016
I made a short screen recording to demonstrate the differences I see on my end (a retina 13" MBP in max screen resolution). See http://f.cl.ly/items/3Z2W1M0c0j0i1b1v201D/safari-chrome-sf-rendering.mov (or let me know if that doesn't work). Interestingly enough, I noticed that the sidebar labels (font-weight:600;) aren't as affected as the normal weight headings and comment content. I don't see it mentioned here, but for reference, the San Francisco automatically switches between SF Text and SF Display variants at 19px. I hesitate to say that's the cause because all font-sizes appear affected here, but wanted to at least mention it.
,
Jul 14 2016
Thanks joshua.jabbour and otto.
,
Jul 15 2016
I'm fairly sure it's the fact that hb-coretext currently always renders at the same point size and linearly scales the results, whereas San Francisco uses the 'trak' table to adjust spacing at various sizes. IIRC Skia also has such an assumption baked in... But we should be able to fix this in HarfBuzz without changing in Skia. I'll think about how to fix this. We probably need new API in harfbuzz.
,
Jul 18 2016
Ben, is Behdad's assumption about the linear scaling in Skia still correct? I seem to remember there were some changes in Skia around this. Thanks for commenting.
,
Jul 18 2016
Way down inside, Skia calls CTFontGetAdvancesForGlyphs to get the glyph advance. When Skia sets up the CTFont it does so with the exact size (extracted from the current matrix). However, it does so by making an 'exact' copy; it avoids the automatic switch between 'SF Text' and 'SF Display'. So if Blink calls SkCreateTypefaceFromCTFont to create a typeface, that exact font is always used (the font will not change because of size / matrix changes). If we didn't do this then Blink goes crazy, if text scales all of a sudden the glyphs are wrong (because a different font with different glyph mappings is suddenly used just because the matrix changed). I even wrote a comment about the 'trak' table at one point... the issue with 'trak' was probably working with https://codereview.chromium.org/752183002/ but then broken with https://codereview.chromium.org/770383002/ because asking CG for the requested size (and supplying the matrix for the rest) doesn't work with color bitmaps.
,
Aug 31 2016
This is something we (drott@, bungeman@, and I) should look into together, perhaps at the next Chrome text work week.
,
Sep 2 2016
Sounds good to me. Happy to come out. Ben, would you be up for meeting in US-CHAP for example and work on this together? Behdad@, I have collected another couple of things for such a week that we could look into.
,
Sep 6 2016
Would love to join if you all are ok ;-)
,
Sep 6 2016
Of course!
,
Sep 22 2016
BlinkMacSystemFont looks a lot worse since updating to macOS 10.12 Sierra. The letter spacing got a lot wider, and it seems that [contextual kerning](https://en.wikipedia.org/wiki/Kerning#Contextual_kerning) isn't working anymore. Here are screenshots demonstrating the difference: macOS 10.11 El Capitan: https://imgur.com/a/bdQ2q macOS 10.12 Sierra: https://imgur.com/a/MdG52 Is this change in macOS 10.12 related to this bug, or something different?
,
Sep 22 2016
feross@, if I identify your screenshots correctly, they affect the title bar and would be a Chrome UI issue. Do you have any screenshots of where this affects the content?
,
Sep 22 2016
drott@, My screenshots are from a web page, not the title bar. They do look a bit misleading ;-). Here is a clearer example: macOS 10.11 El Capitan: https://imgur.com/a/61xPj macOS 10.12 Sierra: https://imgur.com/a/pZd5K Source code: https://codepen.io/anon/pen/WGRaka You can see on 10.11 that the letters around the 'T' in 'Torrent' are a lot tighter and legible. On 10.12, the space around the 'T' is too wide.
,
Sep 23 2016
Behdad, we should prioritize the trak table metrics on Mac.
,
Sep 23 2016
Issue 578799 has been merged into this issue.
,
Oct 3 2016
Has there been any progress on this? Does anyone known of a workaround?
,
Oct 5 2016
No progress. Dominik, should we look into this together end of month?
,
Oct 6 2016
Yes, gladly.
,
Oct 17 2016
Further analysis on FontPlatformData/SimpleFontData indicated that my concern on it being one of the causes for this bug was wrong, so hopefully supporting trak alone can fix this. Sorry for the noise.
,
Nov 2 2016
Rather as notes to self, there are several aspects to this: 1) Our Mac system font matching would need to simulate the switching between SF Text and SF Display, the result of this is an NSFont/CTFont that we use to instantiate a new FontPlatformData and inside this, an SkTypeface, using SkCreateTypefaceFromCTFont. 2) Trak table support is needed for the case where we stay inside the same font face, i.e. as long as we stay within SF Display and SF Text, we need trak support. For this, HarfBuzz needs to shape the request at the requested point size, and we need to way/API to specify this to HarfBuzz. 3) We either need to synchronize Skia and HarfBuzz to both switch the face for one CTFont on different sizes, or keep the exact same one. If the exact same one is kept, we need to keep two different SkTypefaces for this case in Blink.
,
Nov 2 2016
Looking at the github project readme pages, Chrome 55 already gets different fonts for the body text and the headlines. SF Text for the text, and SF Display for the headlines, this part preliminarily seems working. Behdad suggests diagnostics: Inside hb-coretext, create a new CTFont each time, with an approximated point size, based on the scale factor that came in. The assumption would be, that if we create a new CTFont each time, this would bring default support for trak at the right approximated PT size. If this approach works, then we need an API on HarfBuzz to set the pt size for what we want to get shaped, and HarfBuzz would internally create a new CTFont at requested point size.
,
Nov 2 2016
Yeah, I should have written #28 better. As far as I checked, we create one SKTypeface (and thus CTFont) for each size, only for Mac. The SKTypeface sharing is enabled only on other platforms.
,
Nov 2 2016
and checked with tzik@ we do so.
,
Nov 2 2016
Oh I see, so we don't use the CTFont in HB. Understood the problem.
,
Jan 23 2017
Any update on this?
,
Feb 4 2017
Depends on https://github.com/behdad/harfbuzz/issues/360 and a brief update in that bug.
,
Feb 22 2017
I've asked Behdad for an update in the github issue. We would need HarfBuzz to pass the correct pt size to CoreText and instantiate a CTFont at this size for shaping so that the CoreText backend can take 'trak' table data into account.
,
Feb 23 2017
Will finish it this quarter for sure; hopefully next week.
,
Mar 2 2017
,
Oct 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/32f8cd34973f66a76c6b33d501609d5e73ef1765 commit 32f8cd34973f66a76c6b33d501609d5e73ef1765 Author: Dominik Röttsches <drott@chromium.org> Date: Wed Oct 11 12:40:19 2017 Move fixed resolution units to platform/ And rename CSSHelper to CSSResolutionUnits. Preparation for fixing issue 627609 since HarfBuzz will need to know a points size at which the hb_font is going to be rasterized. No functional change. Bug: 627609 Change-Id: I46431b7195346de7528fc44f59541d431a0ddf5d Reviewed-on: https://chromium-review.googlesource.com/712154 Reviewed-by: Morten Stenshorne <mstensho@opera.com> Commit-Queue: Dominik Röttsches <drott@chromium.org> Cr-Commit-Position: refs/heads/master@{#507959} [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/animation/SVGLengthInterpolationType.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/BUILD.gn [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp [rename] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/CSSResolutionUnits.h [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/CSSToLengthConversionData.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/MediaValues.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/MediaValuesDynamic.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/cssom/CSSUnitValue.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/svg/SVGLengthContext.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/core/svg/SVGSVGElement.cpp [modify] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/platform/BUILD.gn [copy] https://crrev.com/32f8cd34973f66a76c6b33d501609d5e73ef1765/third_party/WebKit/Source/platform/ResolutionUnits.h
,
Oct 12 2017
Behdad and I have a local fix for this issue, based on HarfBuzz' fix to https://github.com/behdad/harfbuzz/issues/360 . It turns out that tracking for the '.SF NS Display' and '.SF NS Text' fonts can only be activated through CTFontCreateUIFontForSize. Other than this call there is no CoreText API or FontDescriptor attributes that would allow us to control tracking. So the nature of the HarfBuzz fix is detecting when we come into HarfBuzz with one of those system fonts, and recreate a CTFont at the right size using this API call, in order to activate tracking. It is important to note that a CoreText "point" unit is a device independent size, defined similar to a CSS pixel, it is not a typographic point size unit. At the moment, the API for telling HarfBuzz about the actual visual font size to be used on the screen is to convert from CSS Pixel to typographic points. HarfBuzz then converts this to what CoreText expects, so converts its to CoreText "points". We're going to roll HarfBuzz after the Chrome branch and upload a small change on the Blink side to pass the point size to HarfBuzz.
,
Oct 13 2017
,
Nov 3 2017
Fixed in https://chromium-review.googlesource.com/c/chromium/src/+/718860 🎉
,
Jan 3 2018
Unfortunately this is not fully fixed if the font is in italics. It still shows the wide letter spacing. Chrome vs Safari https://i.gyazo.com/0fa111dcd4c638d7ae3a93e0b14c6595.png
,
Jan 3 2018
Right. We don't match the Italic. |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by paulir...@chromium.org
, Jul 12 2016214 KB
214 KB View Download