Bug 198909 – [WinCairo] incorrect font height for 'Google Sans Display' font

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Fujii Hironori]

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

[Fujii Hironori]

Created attachment 372232 [details]
[Screenshot] WinCairo port

[Fujii Hironori]

Created attachment 372233 [details]
WIP patch

[Fujii Hironori]

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

[Frédéric Wang (:fredw)]

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)

[Fujii Hironori]

Created attachment 372624 [details]
WIP patch

[Fujii Hironori]

Created attachment 372821 [details]
Patch

[Fujii Hironori]

Created attachment 372822 [details]
Patch

[Fujii Hironori]

Created attachment 372823 [details]
Patch

[Frédéric Wang (:fredw)]

Comment on attachment 372822 [details]
Patch

It would help to have the script to generate UseTypo.ttf or at least explanation on its metrics.

[Fujii Hironori]

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?

[Frédéric Wang (:fredw)]

(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.

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[EWS Watchlist]

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

[Fujii Hironori]

(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.

[EWS Watchlist]

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

[EWS Watchlist]

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

[Frédéric Wang (:fredw)]

(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.

[Fujii Hironori]

Created attachment 372833 [details]
Patch

[Fujii Hironori]

Comment on attachment 372833 [details]
Patch

Clearing flags on attachment: 372833

Committed r246831: <https://trac.webkit.org/changeset/246831>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/52218772>