Issue metadata
Sign in to add a comment
|
Investigate binary size impact of Harfbuzz on Chrome for Android |
||||||||||||||||||||
Issue descriptionThe Harfbuzz roll below caused a 46 KB size increase on Chrome for Android. https://chromium.googlesource.com/chromium/src/+/c67f53b0b70f33c47159d37f7e59bb44399b0d09 This spawns two questions: 1. Are the AAT changes (which sound Mac related) necessary on Android, and if not, could they be conditionally be disabled? 2. How much of Harfbuzz (and RenderText in general) is used on Android? VR uses it for basic text rendering, but what else uses it?
,
Nov 27
Assigning to drott@chromium.org because this is the only CL in range: Roll HarfBuzz to 2.1.1 plus AAT fixes https://chromium.googlesource.com/external/github.com/harfbuzz/harfbuzz/+log/54d332dd9b0263..ae96c98dfaef3a7 Enables native AAT support in HarfBuzz without relying on CoreText. Performance improvements compared to Mac OS CoreText implementation ranging between 3.2x speedup for Hebrew using Lucida Grande up to 12.2x speedup for Latin with the San Francisco system font. This should unblock issue 862773 and other AAT performance issues we have seen. For performance data, see: https://goo.gl/tZJU2y The following HarfBuzz issues on AAT issues were reported to HarfBuzz and have been ironed out in the HarfBuzz commit that we're rolling to: https://github.com/harfbuzz/harfbuzz/issues?q=1263+1264+1278+1303+1305+1331+1342+1348+1343+1393+1405+1406 The rebaselines affect: * Kerning differences, especially improved kerning with punctuation on Windows, unrelated to AAT implementation * Slight difference in glyph coordinate rounding for AAT fonts due to internal computation with subpixel values, but screen placement at rounded pixels * Combining mark placement differences between HarfBuzz AAT and CoreText AAT, compare https://github.com/harfbuzz/harfbuzz/issues/1264 * Changes in vertical origin computation in HarfBuzz, unrelated to AAT IMPORTANT: If HarfBuzz AAT needs to be reverted, consider changing if (false) in BUILD.gn to if (is_mac) to re-enable the hb-coretext backend for processing AAT instead of reverting the whole CL. Big kudos and a big thank you to behdad@ for the excellent work on implementing AAT in HarfBuzz and this smooth collaboration in maturing the implementation. Binary-Size: Intended functionality add in HarfBuzz to support AAT fonts natively without using CoreText Bug: 894354 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: Iab082fbd47cd1df9e1fd9558a12ddf597db77f7b Reviewed-on: https://chromium-review.googlesource.com/c/1275945 Commit-Queue: Dominik Röttsches <drott@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#610734}
,
Nov 27
Claiming this for initial investigation.
,
Nov 27
From a VR perspective, it looks like (at least) GFX' skia canvas and related code uses RenderText as well, so we wouldn't gain anything if VR cut its dependency. Over to drott@ for input on Harfbuzz's utilization of AAT on Android. Dominik, could you point out whether AAT benefits Android in any way, and if not, whether there's a switch we can flip to compile it out?
,
Nov 27
HarfBuzz is used to shape any text you see. As upstream HarfBuzz maintainer, we don't support building without AAT, since the code is quite entangled with OpenType code, plus, we like to have all HarfBuzz-using clients using a certain version or later to support the same features. That's one of the biggest advantages to HarfBuzz (fidelity of shaping across Android, Firefox, Chrome), and I suggest thinking twice before changing that.
,
Nov 28
HarfBuzz added AAT shaping to vastly improve our performance for text shaping on Mac (3.2 to 12.2x speedup), unblocking issues such as issue 862773. Also, introducing AAT type font support for non Mac platforms enables additional typographic fidelity for example by using the trak AAT table for improved font-size specific letter-spacing. We have previously attempted to build a custom configuration without AAT support. This has repeatedly lead to painful to debug failures with rolling to a new HarfBuzz version, and - as Behdad explains - is generally not supported by HarfBuzz upstream. We would also much prefer not to introduce differences in what font technologies we support on our supported platforms. So, for the additional maintenance burden in maintaining an unsupported build configuration, and for reasons of cross-platform consistency, we'd much prefer keeping the functionality enabled in Android.
,
Nov 28
Thanks for the thoughts! Your points make sense, and it's fine. Just wanted to be sure. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Nov 27