916048 - SVG-in-OpenType glyphs use the font's unitsPerEm value, not a fixed 1000-unit space

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Jonathan Kew [:jfkthame]]

We missed one issue in bug 906521: the unified spec says that the glyphs "are designed on an em specified in the head table’s unitsPerEm field", rather than the fixed 1000-unit space in our original implementation.

Comment 1

[Jonathan Kew [:jfkthame]]

Oh, and while we're here, it looks like SVG glyphs won't take account of any font-size-adjust scaling that's in effect. We should fix that, too.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: SVG-in-OpenType glyphs should use the font's unitsPerEm value.

This makes us respect the font's unitsPerEm value. Note that this will affect existing examples/testcases that are based on TrueType fonts with upem=2048, so we'll need to update those accordingly.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 804385 [details] [diff] [review]
SVG-in-OpenType glyphs should use the font's unitsPerEm value.

Review of attachment 804385 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxFont.h
@@ +533,5 @@
>      hb_blob_t* GetTableFromFontData(const void* aFontData, uint32_t aTableTag);
>  
> +    // Font's unitsPerEm from the 'head' table, if available (will be set to
> +    // kInvalidUPEM for non-sfnt font formats)
> +    uint16_t         mUnitsPerEm;

Don't indent

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: update text-svgglyphs reftests for font-specific unitsPerEm.

The scaling change here breaks all the reftests, because Liberation has a unitsPerEm of 2048. For a simple fix, I've generated a new set of test fonts based on FiraSansOT, with a unitsPerEm of 1000, so as to preserve the scaling of the glyphs.

Comment 5

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad42e99b60f3
https://hg.mozilla.org/integration/mozilla-inbound/rev/655c83236aab

Comment 6

[Jonathan Kew [:jfkthame]]

As a bonus, I believe the new test font here is responsible for fixing bugs 872486 and 872491, probably as a result of more consistent line-height metrics across the various 'sfnt' tables. So I've removed the relevant fails-if()/random-if() annotations from the reftest manifest.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ad42e99b60f3
https://hg.mozilla.org/mozilla-central/rev/655c83236aab