Issue metadata
Sign in to add a comment
|
Bad kerning in M68 (Roboto)
Reported by
jonat...@titanous.com,
May 3 2018
|
|||||||||||||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; CrOS x86_64 10635.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3416.0 Safari/537.36 Platform: 10635.0.0 (Official Build) dev-channel eve Steps to reproduce the problem: 1. Look at text. What is the expected behavior? What went wrong? After updating from M67 to M68, the text kerning is noticeably terrible. Did this work before? N/A Chrome version: 68.0.3416.0 Channel: dev OS Version: 10635.0.0 Flash Version:
,
May 7 2018
tapted@, you're been messing with fonts recently. Should you own this?
,
May 8 2018
I think this is WAI, but +UX for opinions. The only change is to increase the omnibox font size from 14pt Roboto to 15pt Roboto. The larger font size is going to align to pixels differently, but it looks correct to me. E.g. the kerning in webcontents of Roboto at the same size looks identical. To change this, we'd probably need a new Roboto with different kerning at 15pt.
,
May 8 2018
Ah, actually this does seem to be better in m65. So perhaps this is a recent Skia change that affected webcontents similarly. (i.e. not omnibox-specific) Maybe related to https://skia-review.googlesource.com/12492 ?
,
May 8 2018
To be clear: In #5 do you mean https://skia-review.googlesource.com/c/skia/+/125300 instead of 12492? In #1, the complaint is that the spacing between the x and the t in text is too tight? And, also the spacing of the r and the n are too tight? In #4, 14pt example, is the complaint that the t and the e of text are too far apart?
,
May 8 2018
Yeah the "text" is what stood out to me (but maybe the original reporter noticed other things). In m68 (at both point sizes), the 'e' seems shifted to the right, whereas in earlier milestones the 'e' looks centered between the first 't' and the 'x'. And yeah 125300 is the CL (I was trying to link to 124921 but lost a digit). Note I'm not sure whether that CL actually impacts this stuff - I just grepped for 'kerning' changes in the right timeframe.
,
May 8 2018
Currently, I suspect that Skia is not involved even though the CL seems very alluring. Skia only kerns when the DrawText call is used. I doubt that you are using that call because it does not handle text shaping and and parts of unicode correctly, and it is not called by any product we know of. I suspect you are looking at a FreeType library change or a Roboto change.
,
May 8 2018
Kerning seems definitely correct in #6.
,
May 8 2018
(removing Blink>Fonts as the omnibox isn't rendered using the blink text stack)
,
May 8 2018
jonathan@titanous.com: can you go to https://typekit.com/fonts/roboto and do a side-by-side comparison? (i.e. set point size to 15pt, set some text, and scroll down to Roboto Regular) None of my attempts show the level of badness in your screengrab -- all the changes I see are identical between blink and omnibox in each revision.
,
May 9 2018
In another thread about bad kerning in 68 on ChromeOS, someone pointed out https://crrev.com/c/1027005.
,
May 9 2018
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 9 2018
It looks like if I specifically toggle chrome://flags#enable-pixel-canvas-recording to disabled, the kerning improves quite a bit.
,
May 9 2018
This is due to https://chromium-review.googlesource.com/c/chromium/src/+/1027005
,
May 9 2018
#16 - One of the primary reasons to introduce pixel canvas was to solve text kerning at dsf > 1. Its strange that disabling it helps. @oshima - thoughts?
,
May 9 2018
I think there are still kerning problems with it disabled, but it definitely stops the kerning from jumping around in the omnibox dropdown while typing.
,
May 9 2018
Adding more screenshots for comparison.
,
May 9 2018
The above screen shots are for device scale factor of 2x. - Toggling Pixel canvas seems to have no effect. - Disabling subpixel positioning gives a much sharper result.
,
May 9 2018
For device scale factors other than an integer we can see the difference in Toggling Pixel Canvas. However, enabling subpixel positioning seems to help.
,
May 9 2018
More screenshots for 1.6x scale factor. Looking at the screenshots, I think we still need Subpixel positioning as it provides improvement on fractional scale factors when combined with pixel canvas.
,
May 9 2018
#24: Thanks for the comparison screenshots! Yeah, it looks to me like we need to keep subpixel positioning enabled, at least in those cases.
,
May 9 2018
Uploading a patch to enable subpixel positioning for non integral dsf.
,
May 10 2018
Looks like a bug in rendertext? Jumping in the candidate window looks different bug. Can we re-enable it for the native ui for the time being?
,
May 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f00044b1973636900b1e65818e162dbb93efdeb9 commit f00044b1973636900b1e65818e162dbb93efdeb9 Author: Malay Keshav <malaykeshav@chromium.org> Date: Tue May 15 17:30:30 2018 Enable subpixel positioning for chrome os for fractional dsf This patch enables the font subpixel positioning for the browser ui and blink for fractional device scale factors. It also adds a switch to force disable subpixel positioning for easier debugging and testing on devices. Bug: 839214 Change-Id: If8fd7dbc39ac3950bc27b5b7435b482fb4931065 Component: Font, subpixel positioning, kerning Reviewed-on: https://chromium-review.googlesource.com/1053289 Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Cr-Commit-Position: refs/heads/master@{#558764} [modify] https://crrev.com/f00044b1973636900b1e65818e162dbb93efdeb9/ash/display/display_manager_unittest.cc [modify] https://crrev.com/f00044b1973636900b1e65818e162dbb93efdeb9/ui/gfx/font_render_params_linux.cc [modify] https://crrev.com/f00044b1973636900b1e65818e162dbb93efdeb9/ui/gfx/font_render_params_linux_unittest.cc [modify] https://crrev.com/f00044b1973636900b1e65818e162dbb93efdeb9/ui/gfx/switches.cc [modify] https://crrev.com/f00044b1973636900b1e65818e162dbb93efdeb9/ui/gfx/switches.h
,
May 15 2018
|
||||||||||||||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||||||||||||||
Comment 1 by weifangsun@chromium.org
, May 4 2018