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>
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
Target Milestone: --- → mozilla36
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.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.