glyph-range indexes in the SVG table are not searched correctly

RESOLVED FIXED in mozilla24

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

unspecified
mozilla24
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

5 years ago
Created attachment 759753 [details] [diff] [review]
patch, fix the comparison function to treat the glyph range as a closed interval

The documentation for the 'SVG ' table, at both [1] and [2], says that the glyph document index entries have fields [startGlyphId, endGlyphId] to define the range, implying that it is defined as a closed interval that includes the specified start and end glyph IDs.

However, the gfxSVGGlyphs::CompareIndexEntries function at http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxSVGGlyphs.cpp#107 assumes it is dealing with a half-open interval [startGlyphId, endGlyphId) where the specified end glyph ID is NOT considered to be part of the range.

As a result, it will fail to find and use an SVG glyph whose ID is the endGlyphId of the relevant index entry.

I ran into this when creating test fonts for bug 875629, and worked around it by simply having my font-creation tool write half-open intervals in the SVG glyph index. However, we need to make the code actually match the spec. (Alternatively, we could change the spec, but I don't see any reason to prefer that option.)


[1] https://wiki.mozilla.org/SVGOpenTypeFonts?title=SVGOpenTypeFonts#The_SVG_table
[2] http://dev.w3.org/SVG/modules/fonts/SVG-OpenType.html#index
Attachment #759753 - Flags: review?(roc)
(Assignee)

Comment 1

5 years ago
Created attachment 759754 [details] [diff] [review]
update test fonts from bug 875629 for corrected handling of the SVG glyph index

This just updates the test fonts from bug 875629 to replace the half-open interval in their glyph indexes with a closed interval.
Attachment #759754 - Flags: review?(roc)
(Assignee)

Updated

5 years ago
Attachment #759754 - Attachment description: update test fonts from for corrected handling of the SVG glyph index → update test fonts from bug 875629 for corrected handling of the SVG glyph index
(Assignee)

Comment 2

5 years ago
Created attachment 759761 [details] [diff] [review]
pt 3 - update the sanitizer check for the glyph index ranges.

Hmm, the sanitizer code in ots/src/svg.cc to check the index ranges is also wrong - it allows the startGlyphId of a range to be equal to the endGlyphId of the preceding range, which would be correct if they were half-open intervals, but is wrong for closed intervals. Additional fix attached.
Attachment #759761 - Flags: review?(roc)
(Assignee)

Comment 3

5 years ago
Created attachment 760333 [details] [diff] [review]
pt 4 - update the svg.woff test font with corrected glyph index entries

There's one last piece here: we need to rebuild the original svg.woff test font using a corrected tool to generate the proper glyph index entries; otherwise the updated sanitizer code will drop the 'SVG ' table from it due to overlapping ranges, making all the tests fail.
Attachment #760333 - Flags: review?(roc)
You need to log in before you can comment on or make changes to this bug.