zwsp is missing in ShapeResult on some Macs |
||
Issue description
Shape the following string with kRtl:
UChar string[] = {kZeroWidthSpaceCharacter,
kZeroWidthSpaceCharacter,
0x0627,
0x0631,
0x062F,
0x0648,
kZeroWidthSpaceCharacter,
kZeroWidthSpaceCharacter};
1. mac_chromium_rel_ng (10.9, it seems)
#chars=8, #glyphs=4, dir=1,
runs[1]{
0:{start=2, #chars=6, dir=5, glyphs[4]{
0:{char=3, glyph=356}
1:{char=2, glyph=268}
2:{char=1, glyph=272}
3:{char=0, glyph=240}}}}
2. my MacBook (10.12.6)
#chars=8, #glyphs=5, dir=1,
runs[2]{
0:{start=2, #chars=6, dir=5, glyphs[4]{
0:{char=3, glyph=356}
1:{char=2, glyph=268}
2:{char=1, glyph=272}
3:{char=0, glyph=240}}}
1:{start=0, #chars=2, dir=5, glyphs[1]{
0:{char=0, glyph=65535}}}}
3. Linux
#chars=8, #glyphs=8, dir=1,
runs[1]{
0:{start=0, #chars=8, dir=5, glyphs[8]{
0:{char=7, glyph=3}
1:{char=6, glyph=3}
2:{char=5, glyph=1392}
3:{char=4, glyph=1372}
4:{char=3, glyph=1374}
5:{char=2, glyph=1364}
6:{char=1, glyph=3}
7:{char=0, glyph=3}}}}
Differences:
A. On mac bots, we're missing glyph_data for the zwsp. We rely on start_index of the last run (in RTL) to know the start range, and this is blocking issue 722462 (and will affect LayoutNG line breaker.)
B. On my local MacBook, start_index is 0, but character index 1 is missing.
,
Oct 12 2017
Forgot the test CL: https://chromium-review.googlesource.com/c/chromium/src/+/714739
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ad22ea599918d0cec377261b385374e65bb02d5 commit 7ad22ea599918d0cec377261b385374e65bb02d5 Author: Koji Ishii <kojii@chromium.org> Date: Thu Oct 12 20:18:16 2017 Fix ShapeResult::StartIndexForResult on Mac On Mac, ShapeResult may not contain RunInfo/HarfBuzzGlyphData for Zero Width Space at the logical start in RTL. This patch changes StartIndexForResult to compute from the first RunInfo, so that it is not affected by the logical start character in RTL. Bug: 722462, 774034 Change-Id: I10051467e9915cfe28d0cd8ebb474263dd775a00 Reviewed-on: https://chromium-review.googlesource.com/714739 Reviewed-by: Emil A Eklund <eae@chromium.org> Commit-Queue: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#508436} [modify] https://crrev.com/7ad22ea599918d0cec377261b385374e65bb02d5/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp [modify] https://crrev.com/7ad22ea599918d0cec377261b385374e65bb02d5/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp [modify] https://crrev.com/7ad22ea599918d0cec377261b385374e65bb02d5/third_party/WebKit/Source/platform/fonts/shaping/ShapeResult.cpp [modify] https://crrev.com/7ad22ea599918d0cec377261b385374e65bb02d5/third_party/WebKit/Source/platform/fonts/shaping/ShapeResult.h
,
Oct 12 2017
,
Oct 13 2017
Thanks for fixing this, Koji. I am assuming that the results on Mac and Linux are different due to different fonts, and only the font on Mac exposes this problem.
,
Oct 13 2017
Yeah, or by the underlying APIs. Glyph id 3 is usually .notdef, according to Adobe guideline, so Linux handles as notdef glyph, while Core Graphics may return the glyph is missing, or Apple fonts don't have them in CMAP. The CL fixes StartIndexForResult but does not change how we build GlyphData. I guess you're ok to have the differences?
,
Oct 13 2017
Yes, LGTM.
,
Nov 26
The still disabled tests should now work after issue 894354 has been addressed. |
||
►
Sign in to add a comment |
||
Comment 1 by kojii@chromium.org
, Oct 12 2017Components: Blink>Fonts