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

RESOLVED FIXED in mozilla26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

unspecified
mozilla26
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Assignee

Description

6 years ago
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.
Assignee

Comment 1

6 years ago
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.
Assignee

Comment 2

6 years ago
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

Updated

6 years ago
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+
Assignee

Comment 4

6 years ago
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)
Assignee

Comment 6

6 years ago
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
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.