Closed Bug 916048 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: Graphics: Text, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

Attachments

(2 files)

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.
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.
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.
Attachment #804385 - Flags: review?(roc)
Assignee: nobody → jfkthame
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
Attachment #804385 - Flags: review?(roc) → review+
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.
Attachment #804637 - Flags: review?(roc)
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.
https://hg.mozilla.org/mozilla-central/rev/ad42e99b60f3
https://hg.mozilla.org/mozilla-central/rev/655c83236aab
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.