Created attachment 372231 [details] test case [WinCairo] incorrect font height for 'Google Sans Display' font Google News: https://news.google.com/?hl=en-US&gl=US&ceid=US:en
Created attachment 372232 [details] [Screenshot] WinCairo port
Created attachment 372233 [details] WIP patch
It seems that FreeType 2.10 fixed the issue. [sfnt] Use typo metrics if OS/2 fsSelection USE_TYPO_METRICS bit is set. http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=f72b00746c64e13625cf8371f031411fbd0d6161 Line height of a font is disregarded (#1626) · Issues · GNOME / gtk · GitLab https://gitlab.gnome.org/GNOME/gtk/issues/1626
Can you please write (or find) a WPT test similar to: https://github.com/web-platform-tests/wpt/blob/master/mathml/tools/use-typo-lineheight.py https://w3c-test.org/mathml/relations/text-and-math/use-typo-metrics-1.html (and check whether that one is working on your port)
Created attachment 372624 [details] WIP patch
Created attachment 372821 [details] Patch
Created attachment 372822 [details] Patch
Created attachment 372823 [details] Patch
Comment on attachment 372822 [details] Patch It would help to have the script to generate UseTypo.ttf or at least explanation on its metrics.
Created attachment 372825 [details] UseTypo.ttx (In reply to Frédéric Wang (:fredw) from comment #9) > It would help to have the script to generate UseTypo.ttf or at least > explanation on its metrics. Here is the step how I created the UseTypo.ttf. 1. Open Ahem.ttf by FontForge 2. Clear all glyphs but 'O'. 3. Save as UseTypo.ttf 4. ttx UseTypo.ttf 5. Edit UseTypo.ttf manually Turn USE_TYPO_METRICS flag on Change ascent of hhea to 3000 Change descent of hhea to 3000 6. ttx UseTypo.ttx https://github.com/fonttools/fonttools Should I add a comment in the test case?
(In reply to Fujii Hironori from comment #10) > Created attachment 372825 [details] > UseTypo.ttx > > (In reply to Frédéric Wang (:fredw) from comment #9) > > It would help to have the script to generate UseTypo.ttf or at least > > explanation on its metrics. > > Here is the step how I created the UseTypo.ttf. > > 1. Open Ahem.ttf by FontForge > 2. Clear all glyphs but 'O'. > 3. Save as UseTypo.ttf > 4. ttx UseTypo.ttf > 5. Edit UseTypo.ttf manually > Turn USE_TYPO_METRICS flag on > Change ascent of hhea to 3000 > Change descent of hhea to 3000 > 6. ttx UseTypo.ttx > > https://github.com/fonttools/fonttools > > Should I add a comment in the test case? I think it would be good to have some a script similar to https://github.com/web-platform-tests/wpt/blob/master/mathml/tools/use-typo-lineheight.py so that people know the font metrics and may even modify it in the future. Right now it's not obvious to me what are the ascent/descent/linegap and win/hhea/typo values for this font. Is this test passing before/after your patch: https://w3c-test.org/mathml/relations/text-and-math/use-typo-metrics-1.html (I would have expected it fails before and passes after... hence you would actually not need to write a new test) Note: Regarding failures on Apple platforms, see bug 150394 comment 12. I'll let Myles comment about this.
Comment on attachment 372823 [details] Patch Attachment 372823 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/12569355 New failing tests: fonts/use-typo-metrics-2.html
Created attachment 372828 [details] Archive of layout-test-results from ews101 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 372823 [details] Patch Attachment 372823 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12569396 New failing tests: fonts/use-typo-metrics-2.html
Created attachment 372829 [details] Archive of layout-test-results from ews106 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
(In reply to Frédéric Wang (:fredw) from comment #11) > I think it would be good to have some a script similar to > > https://github.com/web-platform-tests/wpt/blob/master/mathml/tools/use-typo- > lineheight.py > > so that people know the font metrics and may even modify it in the future. Got it. Will do. Should I check the script in the repository? Then, which directory? > Right now it's not obvious to me what are the ascent/descent/linegap and > win/hhea/typo values for this font. You can see them in the ttx file. > Is this test passing before/after your patch: > https://w3c-test.org/mathml/relations/text-and-math/use-typo-metrics-1.html > (I would have expected it fails before and passes after... hence you would > actually not need to write a new test) Unfortunately, this patch doesn't make the test pass. It is filed in Bug 199186. > Note: Regarding failures on Apple platforms, see bug 150394 comment 12. I'll > let Myles comment about this. Good to know. Thanks.
Comment on attachment 372823 [details] Patch Attachment 372823 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12569446 New failing tests: fonts/use-typo-metrics-2.html
Created attachment 372831 [details] Archive of layout-test-results from ews116 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-highsierra Platform: Mac OS X 10.13.6
(In reply to Fujii Hironori from comment #16) > Got it. Will do. > Should I check the script in the repository? Then, which directory? I would put it in the same directory as the generated font file, unless there is another place where we have such scripts. > > Right now it's not obvious to me what are the ascent/descent/linegap and > > win/hhea/typo values for this font. > > You can see them in the ttx file. Right but my point is that people shouldn't need to execute programs to review a patch.
Created attachment 372833 [details] Patch
Comment on attachment 372833 [details] Patch Clearing flags on attachment: 372833 Committed r246831: <https://trac.webkit.org/changeset/246831>
All reviewed patches have been landed. Closing bug.
<rdar://problem/52218772>