Closed Bug 1090869 Opened 5 years ago Closed 5 years ago

Only input and context glyphs, not output, are significant when checking for features that involve <space>

Categories

(Core :: Graphics: Text, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 --- fixed
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

Details

Attachments

(1 file)

We don't need to include the lookups' output glyphs in the sets we collect during HasLookupRuleWithGlyph() and HasLookupRuleWithGlyphByScript(); only the input glyph (sequence) and context are important here.

By not collecting output, we avoid hitting a pathologically inefficient case in harfbuzz which causes a multi-second delay when loading the example at http://behdad.org/urdu/.

Behdad is about to fix the HB issue upstream, but in any case we can reduce the work we do by passing nullptr for the output parameter to collect_glyphs.
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Attachment #8513388 - Flags: review?(jdaggett) → review+
Comment on attachment 8513388 [details] [diff] [review]
Don't collect output glyphs when checking for features involving <space>.

Approval Request Comment
[Feature/regressing bug #]:
This code dates back to 761442, later revised in bug 921858. So it's fixing a longstanding issue, but with Google apparently working on a font that's badly affected by this, it'd be nice to ship the fix sooner rather than later.

[User impact if declined]:
Multi-second hang on first use of opentype fonts with particular kinds of complex lookups. Load http://behdad.org/urdu/ for an example.

[Describe test coverage new/current, TBPL]:
Landed on inbound. Existing font reftests cover this code.

[Risks and why]: 
Minimal. Trivial patch that just avoids doing redundant work.

[String/UUID change made/needed]:
None.
Attachment #8513388 - Flags: approval-mozilla-beta?
Attachment #8513388 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/5dcc6258e024
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Attachment #8513388 - Flags: approval-mozilla-beta?
Attachment #8513388 - Flags: approval-mozilla-beta+
Attachment #8513388 - Flags: approval-mozilla-aurora?
Attachment #8513388 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.