Bug 97261

Summary: Incorrect punctuation position/orientation in vertical CJK text on chromium-android
Product: WebKit Reporter: Xianzhu Wang <wangxianzhu>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bashi, hyatt, koansin.tan, kojii, peter, tkent
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 67587, 97824    
Bug Blocks: 96398    
Attachments:
Description Flags
test case
none
The Japanese font used on Android
none
actual result
none
expected result none

Xianzhu Wang
Reported 2012-09-20 14:23:21 PDT
Created attachment 164980 [details] test case On platforms using Source/WebCore/platform/graphics/skia/SimpleFontDataSkia.cpp and GlyphPageTreeNodeSkia.cpp, when vertical CJK text is displayed using a font without vhea and VORG tables, the punctuations are not displayed in correct position and/or orientation. It occurs at least in the following scenarios: - Layout test of chromium-linux. For example, the puctuations are not in correct position/orientation in the current *-expected.png of the following tests: fast/writing-mode/Kusa-Makura-background-canvas.html fast/writing-mode/japanese-lr-selection.html fast/writing-mode/japanese-lr-text.html fast/writing-mode/japanese-rl-selection.html fast/writing-mode/japanese-rl-text.html - On chromium-android I tried to comment out the checking of vhea and VORG tables in SimpleFontDataSkia.cpp, and both chromium-linux layout tests and chromium-android can display the test case correctly. I checked the font on chromium-android, and it has 'vert', but neither vhea nor VORG table has a non-zero length. I'm wondering if we can just remove the checking of vhea and VORG tables.
Attachments
test case (252 bytes, text/html)
2012-09-20 14:23 PDT, Xianzhu Wang 		no flags
Details
The Japanese font used on Android (2.74 MB, application/x-font-ttf)
2012-09-20 14:26 PDT, Xianzhu Wang 		no flags
Details
actual result (8.78 KB, image/png)
2012-09-20 14:29 PDT, Xianzhu Wang 		no flags
Details
expected result (8.68 KB, image/png)
2012-09-20 14:30 PDT, Xianzhu Wang 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Xianzhu Wang
Comment 1 2012-09-20 14:26:39 PDT
Created attachment 164982 [details] The Japanese font used on Android
Xianzhu Wang
Comment 2 2012-09-20 14:29:44 PDT
Created attachment 164984 [details] actual result
Xianzhu Wang
Comment 3 2012-09-20 14:30:25 PDT
Created attachment 164985 [details] expected result
Xianzhu Wang
Comment 4 2012-09-20 14:53:56 PDT
In fact the expected result was generated on chromium-linux after the code of checking of vhea and VORG tables in SimpleFontDataSkia.cpp commented out.
Kenichi Ishibashi
Comment 5 2012-09-20 16:12:26 PDT
It seems that the attached font has vhea table. Why removing the check fixed the problem? header dump of the font (by showttf): version=1, numtables=16, searchRange=256 entrySel=4 rangeshift=0 File Checksum =b1b0afba (should be 0xb1b0afba), diff=0 GSUB checksum=71595868 actual=71595868 diff=0 offset=268 len=364 OS/2 checksum=db6fe209 actual=db6fe209 diff=0 offset=632 len=86 cmap checksum=676ea984 actual=676ea984 diff=0 offset=720 len=34282 gasp checksum=00170009 actual=00170009 diff=0 offset=35004 len=16 glyf checksum=b9165229 actual=b9165229 diff=0 offset=35020 len=2757878 head checksum=f495ec78 actual=31f3a228 diff=c5664e50 offset=2792900 len=54 hhea checksum=07872136 actual=07872136 diff=0 offset=2792956 len=36 hmtx checksum=85d7b13d actual=85d7b13d diff=0 offset=2792992 len=30444 loca checksum=38022bde actual=38022bde diff=0 offset=2823436 len=30492 maxp checksum=25df0195 actual=25df0195 diff=0 offset=2853928 len=32 mort checksum=02b5ee92 actual=02b5ee92 diff=0 offset=2853960 len=372 name checksum=5b4d9bed actual=5b4d9bed diff=0 offset=2854332 len=1358 post checksum=ffad0026 actual=ffad0026 diff=0 offset=2855692 len=32 prep checksum=3f71193d actual=3f71193d diff=0 offset=2855724 len=10 vhea checksum=0787047c actual=0787047c diff=0 offset=2855736 len=36 vmtx checksum=847b8faa actual=847b8faa diff=0 offset=2855772 len=15246
Xianzhu Wang
Comment 6 2012-09-20 17:08:48 PDT
(In reply to comment #5) > It seems that the attached font has vhea table. Why removing the check fixed the problem? > Thanks for the info. I only checked the font with fontmatrix which seems not to show vhea. I will check why SkFontHost::GetTableSize(fontID, vheaTag) return 0 in the cases.
Kenichi Ishibashi
Comment 7 2012-09-20 17:22:34 PDT
(In reply to comment #6) > (In reply to comment #5) > > It seems that the attached font has vhea table. Why removing the check fixed the problem? > > > > Thanks for the info. I only checked the font with fontmatrix which seems not to show vhea. I will check why SkFontHost::GetTableSize(fontID, vheaTag) return 0 in the cases. FYI, I'm preparing a patch that removes substituteWithVerticalGlyphs() in GlyphPageTreeNodeSkia.cpp and uses Koji's implementation for chromium linux and android. This might fix the problem.
Xianzhu Wang
Comment 8 2012-09-20 17:24:53 PDT
(In reply to comment #7) > > FYI, I'm preparing a patch that removes substituteWithVerticalGlyphs() in GlyphPageTreeNodeSkia.cpp and uses Koji's implementation for chromium linux and android. This might fix the problem. Cool! Thanks!
Xianzhu Wang
Comment 9 2012-09-25 14:18:25 PDT
Verified that the patch to bug 97277 resolved the issue on chromium-linux, but not on chromium-android. On Android SkFontHost::GetTableSize(fontID, vheaTag) returns zero because the fontID is not always the font used to render the specific characters. Font fallback of individual characters is done differently in Skia for Android. Skipping the check of vhea/VORG table is skipped on Android. However I'm wondering what the drawback is.
Xianzhu Wang
Comment 10 2012-09-25 14:46:53 PDT
(In reply to comment #9) > > Skipping the check of vhea/VORG table is skipped on Android. However I'm wondering what the drawback is. Sorry, please ignore the above sentence.
Xianzhu Wang
Comment 11 2012-09-26 15:57:43 PDT
Filed Skia bug: http://code.google.com/p/skia/issues/detail?id=913. Uploaded Skia change: https://codereview.appspot.com/6572059/. WebKit needs a small change after the Skia change.
Xianzhu Wang
Comment 12 2012-10-09 10:12:37 PDT
*** Bug 98768 has been marked as a duplicate of this bug. ***
Xianzhu Wang
Comment 13 2012-10-16 13:46:46 PDT
The patch to bug 67587 should fix this bug.
Note You need to log in before you can comment on or make changes to this bug.