Closed Bug 1263956 Opened 8 years ago Closed 8 years ago

adopt new harfbuzz API for character-to-glyph mapping functions

Categories

(Core :: Graphics: Text, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: jfkthame, Assigned: jfkthame)

References

Details

Attachments

(1 file, 1 obsolete file)

From the harfbuzz 1.2.3 change log:

-----
Added get_nominal_glyph() and get_variation_glyph() instead of get_glyph()

New API:
- hb_font_get_nominal_glyph_func_t
- hb_font_get_variation_glyph_func_t
- hb_font_funcs_set_nominal_glyph_func()
- hb_font_funcs_set_variation_glyph_func()
- hb_font_get_nominal_glyph()
- hb_font_get_variation_glyph()

Deprecated API:
- hb_font_get_glyph_func_t
- hb_font_funcs_set_glyph_func()

Clients that implement their own font-funcs are encouraged to replace
their get_glyph() implementation with a get_nominal_glyph() and
get_variation_glyph() pair.  The variation version can assume that
variation_selector argument is not zero.  Old (deprecated) functions
will continue working indefinitely using internal gymnastics; it is
just more efficient to use the new functions.
-----

We should replace our current get_glyph() implementation with separate get_nominal_glyph() and get_variation_glyph() functions, as recommended.
This is basically just splitting our existing all-in-one gfxHarfBuzzShaper::GetGlyph() into two parts, one for nominal glyphs and one for variations.
Attachment #8740451 - Flags: review?(jmuizelaar)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8740451 [details] [diff] [review]
Adopt the new harfbuzz API for char-to-glyph mapping functions

Review of attachment 8740451 [details] [diff] [review]:
-----------------------------------------------------------------

While looking at this I was wondering if we have any reason to keep the GetGlyph() implementation in gfxFT2FontBase. Is this just doing the same work?
Attachment #8740451 - Flags: review?(jmuizelaar) → review+
Hmm, good question.... I think (but would want to check more carefully before doing anything) the answer may be that we needed a Freetype-based implementation for the sake of non-sfnt fonts (X11 bitmaps, etc). The new gfxFcPlatformFontList code doesn't support these, so perhaps we can drop that code -- but not yet, as the old fontconfig code is still present and can be preffed back on.

Once we're ready to move completely and permanently to the gfxFcPlatformFontList backend, we may be able to simplify other parts of the Freetype-related code because we'll know that we are only dealing with sfnt-format fonts.
Ugh, tryserver says I did something wrong... (comment 2). Will check & fix.
OK, it's a trivial fix: I'd over-simplified GetVariationGlyph slightly, so that it returned early without handling UVS fallback for compatibility characters properly. Carrying over r=jrmuizel; new try run, to double-check: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f8e618fd569.
Attachment #8740451 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d821f105ae9aa7e168dffc08fe33b15f7215b18
Bug 1263956 - Adopt the new harfbuzz API for char-to-glyph mapping functions. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/9d821f105ae9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.