From 69f9fbc4200442a35484d3c790ae8f4979be5d60 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 17 Mar 2016 11:59:43 -0700 Subject: [PATCH] Synthesize GDEF glyph class for any glyph that does not have one in GDEF Previously we only synthesized GDEF glyph classes if the glyphClassDef array in GDEF was null. This worked well enough, and is indeed what OpenType requires: "If the font does not include a GlyphClassDef table, the client must define and maintain this information when using the GSUB and GPOS tables." That sentence does not quite make sense since one needs Unicode properties as well, but is close enough. However, looks like Arial Unicode as shipped on WinXP, does have GDEF glyph class array, but defines no classes for Hebrew. This results in Hebrew marks not getting their widths zeroed. So, with this change, we synthesize glyph class for any glyph that is not specified in the GDEF glyph class table. Since, from our point of view, a glyph not being listed in that table is a font bug, any unwanted consequence of this change is a font bug :). Note that we still don't get the same rendering as Uniscribe, since Uniscribe seems to do fallback positioning as well, even though the font does have a GPOS table (which does NOT cover Hebrew!). We are not going to try to match that though. Test string for Arial Unicode: U+05E9,U+05B8,U+05C1,U+05DC Before: [gid1166=3+991|gid1142=0+737|gid5798=0+1434] After: [gid1166=3+991|gid1142=0+0|gid5798=0+1434] Uniscribe: [gid1166=3+991|gid1142=0@348,0+0|gid5798=0+1434] Note that our new output matches what we were generating until July 2014, because the Hebrew shaper used to zero mark advances based on Unicode, NOT GDEF. That's 9e834e29e0b657f0555df1ab9cea79ff7abcf08d. Reported by Greg Douglas. --- src/hb-ot-layout-gsub-table.hh | 22 ++++++++++++++++++++-- src/hb-ot-shape.cc | 29 ----------------------------- 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh index 38c2c6405..7de56cf2a 100644 --- a/src/hb-ot-layout-gsub-table.hh +++ b/src/hb-ot-layout-gsub-table.hh @@ -1288,10 +1288,28 @@ GSUB::substitute_start (hb_font_t *font, hb_buffer_t *buffer) const GDEF &gdef = *hb_ot_layout_from_face (font->face)->gdef; unsigned int count = buffer->len; + hb_glyph_info_t *info = buffer->info; for (unsigned int i = 0; i < count; i++) { - _hb_glyph_info_set_glyph_props (&buffer->info[i], gdef.get_glyph_props (buffer->info[i].codepoint)); - _hb_glyph_info_clear_lig_props (&buffer->info[i]); + unsigned int props = gdef.get_glyph_props (info[i].codepoint); + if (!props) + { + /* Never mark default-ignorables as marks. + * They won't get in the way of lookups anyway, + * but having them as mark will cause them to be skipped + * over if the lookup-flag says so, but at least for the + * Mongolian variation selectors, looks like Uniscribe + * marks them as non-mark. Some Mongolian fonts without + * GDEF rely on this. Another notable character that + * this applies to is COMBINING GRAPHEME JOINER. */ + props = (_hb_glyph_info_get_general_category (&info[i]) != + HB_UNICODE_GENERAL_CATEGORY_NON_SPACING_MARK || + _hb_glyph_info_is_default_ignorable (&info[i])) ? + HB_OT_LAYOUT_GLYPH_PROPS_BASE_GLYPH : + HB_OT_LAYOUT_GLYPH_PROPS_MARK; + } + _hb_glyph_info_set_glyph_props (&info[i], props); + _hb_glyph_info_clear_lig_props (&info[i]); buffer->info[i].syllable() = 0; } } diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 2ba9b78a3..7811cb7f8 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -523,32 +523,6 @@ hb_ot_map_glyphs_fast (hb_buffer_t *buffer) buffer->content_type = HB_BUFFER_CONTENT_TYPE_GLYPHS; } -static inline void -hb_synthesize_glyph_classes (hb_ot_shape_context_t *c) -{ - unsigned int count = c->buffer->len; - hb_glyph_info_t *info = c->buffer->info; - for (unsigned int i = 0; i < count; i++) - { - hb_ot_layout_glyph_props_flags_t klass; - - /* Never mark default-ignorables as marks. - * They won't get in the way of lookups anyway, - * but having them as mark will cause them to be skipped - * over if the lookup-flag says so, but at least for the - * Mongolian variation selectors, looks like Uniscribe - * marks them as non-mark. Some Mongolian fonts without - * GDEF rely on this. Another notable character that - * this applies to is COMBINING GRAPHEME JOINER. */ - klass = (_hb_glyph_info_get_general_category (&info[i]) != - HB_UNICODE_GENERAL_CATEGORY_NON_SPACING_MARK || - _hb_glyph_info_is_default_ignorable (&info[i])) ? - HB_OT_LAYOUT_GLYPH_PROPS_BASE_GLYPH : - HB_OT_LAYOUT_GLYPH_PROPS_MARK; - _hb_glyph_info_set_glyph_props (&info[i], klass); - } -} - static inline void hb_ot_substitute_default (hb_ot_shape_context_t *c) { @@ -580,9 +554,6 @@ hb_ot_substitute_complex (hb_ot_shape_context_t *c) hb_ot_layout_substitute_start (c->font, buffer); - if (!hb_ot_layout_has_glyph_classes (c->face)) - hb_synthesize_glyph_classes (c); - c->plan->substitute (c->font, buffer); return;