From 220a5991baa213b7bd173ea02090dc6fc8aef655 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 13:51:39 -0500 Subject: [PATCH 01/66] [kern/kerx] Fix trace numbering --- src/hb-aat-layout-kerx-table.hh | 1 + src/hb-ot-kern-table.hh | 1 + 2 files changed, 2 insertions(+) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index ca2fdb493..0c9a0ffc2 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -720,6 +720,7 @@ struct kerx skip: table = &StructAfter (*table); + c->set_lookup_index (c->lookup_index + 1); } } diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 66e827d17..213b9cfcb 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -596,6 +596,7 @@ struct KernTable skip: st = &StructAfter (*st); + c->set_lookup_index (c->lookup_index + 1); } } From 164eedd9181345d84d5f8059475ad4b97784fd46 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 13:18:27 -0500 Subject: [PATCH 02/66] [kern] Minor --- src/hb-ot-kern-table.hh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 213b9cfcb..82b8b67cd 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -478,8 +478,11 @@ struct KernSubTable inline unsigned int get_size (void) const { return u.header.length; } inline unsigned int get_type (void) const { return u.header.format; } - inline bool is_simple (void) const - { return !(u.header.coverage & (u.header.CrossStream | u.header.Variation)); } + inline bool is_crossStream (void) const + { return u.header.coverage & u.header.CrossStream; } + + inline bool is_variation (void) const + { return u.header.coverage & u.header.Variation; } inline bool is_horizontal (void) const { return (u.header.coverage & u.header.Direction) == u.header.DirectionHorizontal; } @@ -548,7 +551,7 @@ struct KernTable unsigned int count = thiz()->nTables; for (unsigned int i = 0; i < count; i++) { - if (!st->is_simple () || !st->is_horizontal ()) + if (!st->is_variation () || !st->is_crossStream () || !st->is_horizontal ()) continue; if (st->is_override ()) v = 0; @@ -569,14 +572,15 @@ struct KernTable unsigned int last_override = 0; for (unsigned int i = 0; i < count; i++) { - if (st->is_simple () && st->is_override ()) + if (!st->is_variation () && !st->is_crossStream () && + st->is_override ()) last_override = i; st = &StructAfter (*st); } st = CastP (&thiz()->dataZ); for (unsigned int i = 0; i < count; i++) { - if (!st->is_simple ()) + if (st->is_variation () || st->is_crossStream ()) goto skip; if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != st->is_horizontal ()) From 10e6f708f30986bab9f7b506935f2555d6b79ff4 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 13:32:13 -0500 Subject: [PATCH 03/66] [kern] Minor --- src/hb-ot-kern-table.hh | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 82b8b67cd..fa2b91cc7 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -478,18 +478,6 @@ struct KernSubTable inline unsigned int get_size (void) const { return u.header.length; } inline unsigned int get_type (void) const { return u.header.format; } - inline bool is_crossStream (void) const - { return u.header.coverage & u.header.CrossStream; } - - inline bool is_variation (void) const - { return u.header.coverage & u.header.Variation; } - - inline bool is_horizontal (void) const - { return (u.header.coverage & u.header.Direction) == u.header.DirectionHorizontal; } - - inline bool is_override (void) const - { return bool (u.header.coverage & u.header.Override); } - inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const { switch (get_type ()) { @@ -523,7 +511,7 @@ struct KernSubTable return_trace (dispatch (c)); } - protected: + public: union { KernSubTableHeader header; KernSubTableFormat0 format0; @@ -551,9 +539,11 @@ struct KernTable unsigned int count = thiz()->nTables; for (unsigned int i = 0; i < count; i++) { - if (!st->is_variation () || !st->is_crossStream () || !st->is_horizontal ()) + if ((st->u.header.coverage & + (st->u.header.Variation | st->u.header.CrossStream | st->u.header.Direction)) != + st->u.header.DirectionHorizontal) continue; - if (st->is_override ()) + if (st->u.header.coverage & st->u.header.Override) v = 0; v += st->get_kerning (left, right); st = &StructAfter (*st); @@ -572,18 +562,20 @@ struct KernTable unsigned int last_override = 0; for (unsigned int i = 0; i < count; i++) { - if (!st->is_variation () && !st->is_crossStream () && - st->is_override ()) + if (!(st->u.header.coverage & (st->u.header.Variation | st->u.header.CrossStream)) && + (st->u.header.coverage & st->u.header.Override)) last_override = i; st = &StructAfter (*st); } st = CastP (&thiz()->dataZ); for (unsigned int i = 0; i < count; i++) { - if (st->is_variation () || st->is_crossStream ()) + if (st->u.header.coverage & + (st->u.header.Variation | st->u.header.CrossStream)) goto skip; - if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != st->is_horizontal ()) + if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != + ((st->u.header.coverage & st->u.header.Direction) == st->u.header.DirectionHorizontal)) goto skip; if (i < last_override) From c0383c6bb725bed2a48485988a427348384f3f87 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 15:07:19 -0500 Subject: [PATCH 04/66] Minor --- src/hb-aat-layout-common.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index ee2136ed9..ddfd04b87 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -365,7 +365,7 @@ namespace AAT { enum { DELETED_GLYPH = 0xFFFF }; /* - * Extended State Table + * (Extended) State Table */ template From b11830c09e0d78bbdaf86ef02191d00b3d8256c4 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 15:23:18 -0500 Subject: [PATCH 05/66] [kern] Improve Format 2 Still disabled. --- src/hb-aat-layout-common.hh | 20 +++++++++----------- src/hb-ot-kern-table.hh | 35 ++++++----------------------------- 2 files changed, 15 insertions(+), 40 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index ddfd04b87..539941d85 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -528,24 +528,22 @@ struct StateTable struct ClassTable { - inline unsigned int get_class (hb_codepoint_t glyph_id) const + inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int outOfRange=0) const { - return firstGlyph <= glyph_id && glyph_id - firstGlyph < glyphCount ? classArrayZ[glyph_id - firstGlyph] : 1; + unsigned int i = glyph_id - firstGlyph; + return i >= classArray.len ? outOfRange : classArray.arrayZ[i]; } inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - return_trace (c->check_struct (this) && classArrayZ.sanitize (c, glyphCount)); + return_trace (c->check_struct (this) && classArray.sanitize (c)); } protected: - GlyphID firstGlyph; /* First glyph index included in the trimmed array. */ - HBUINT16 glyphCount; /* Total number of glyphs (equivalent to the last - * glyph minus the value of firstGlyph plus 1). */ - UnsizedArrayOf - classArrayZ; /* The class codes (indexed by glyph index minus - * firstGlyph). */ + GlyphID firstGlyph; /* First glyph index included in the trimmed array. */ + ArrayOf classArray; /* The class codes (indexed by glyph index minus + * firstGlyph). */ public: - DEFINE_SIZE_ARRAY (4, classArrayZ); + DEFINE_SIZE_ARRAY (4, classArray); }; struct MortTypes @@ -557,7 +555,7 @@ struct MortTypes { inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int num_glyphs HB_UNUSED) const { - return ClassTable::get_class (glyph_id); + return ClassTable::get_class (glyph_id, 1); } }; template diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index fa2b91cc7..c54c10086 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -319,44 +319,21 @@ struct KernSubTableFormat1 DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 10); }; -struct KernClassTable -{ - inline unsigned int get_class (hb_codepoint_t g) const { return classes[g - firstGlyph]; } - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - return_trace (c->check_struct (this) && - classes.sanitize (c)); - } - - protected: - HBUINT16 firstGlyph; /* First glyph in class range. */ - ArrayOf classes; /* Glyph classes. */ - public: - DEFINE_SIZE_ARRAY (4, classes); -}; - template struct KernSubTableFormat2 { inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, AAT::hb_aat_apply_context_t *c) const { - /* This subtable is disabled. It's not cleaer to me *exactly* where the offests are - * based from. I *think* they should be based from beginning of kern subtable wrapper, - * *NOT* "this". Since we know of no fonts that use this subtable, we are disabling - * it. Someday fix it and re-enable. */ + /* Disabled until we find a font to test this. Note that OT vs AAT specify + * different ClassTable. OT's has 16bit entries, while AAT has 8bit entries. + * I've not seen any in the wild. */ return 0; unsigned int l = (this+leftClassTable).get_class (left); unsigned int r = (this+rightClassTable).get_class (right); unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+array), offset); -#if 0 - if (unlikely ((const char *) v < (const char *) &array || - (const char *) v > (const char *) end - 2)) -#endif - return 0; + if (unlikely (!v->sanitize (&c->sanitizer))) return 0; return *v; } @@ -400,9 +377,9 @@ struct KernSubTableFormat2 protected: KernSubTableHeader header; HBUINT16 rowWidth; /* The width, in bytes, of a row in the table. */ - OffsetTo leftClassTable; /* Offset from beginning of this subtable to + OffsetTo leftClassTable; /* Offset from beginning of this subtable to * left-hand class table. */ - OffsetTo rightClassTable;/* Offset from beginning of this subtable to + OffsetTo rightClassTable;/* Offset from beginning of this subtable to * right-hand class table. */ OffsetTo array; /* Offset from beginning of this subtable to * the start of the kerning array. */ From 01bf43ac01576a6415336cc56c74bb1a872566d1 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 14:48:42 -0500 Subject: [PATCH 06/66] [kern] Implement CrossStream kerning --- src/hb-aat-layout-kerx-table.hh | 75 +++++++++++++++++++++------------ src/hb-ot-kern-table.hh | 63 ++++++++++++++++++++++----- 2 files changed, 101 insertions(+), 37 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 0c9a0ffc2..550abf31b 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -62,6 +62,20 @@ kerxTupleKern (int value, struct KerxSubTableHeader { + enum Coverage + { + Vertical = 0x80000000, /* Set if table has vertical kerning values. */ + CrossStream = 0x40000000, /* Set if table has cross-stream kerning values. */ + Variation = 0x20000000, /* Set if table has variation kerning values. */ + Backwards = 0x10000000, /* If clear, process the glyphs forwards, that + * is, from first to last in the glyph stream. + * If we, process them from last to first. + * This flag only applies to state-table based + * 'kerx' subtables (types 1 and 4). */ + Reserved = 0x0FFFFF00, /* Reserved, set to zero. */ + SubtableType = 0x000000FF, /* Subtable type. */ + }; + inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); @@ -95,6 +109,9 @@ struct KerxSubTableFormat0 if (!c->plan->requested_kerning) return false; + if (header.coverage & header.CrossStream) + return false; + accelerator_t accel (*this, c); hb_kern_machine_t machine (accel); machine.kern (c->font, c->buffer, c->plan->kern_mask); @@ -161,7 +178,8 @@ struct KerxSubTableFormat1 * similar to offsets in morx table, NOT from beginning of this table, like * other subtables in kerx. Discovered via testing. */ kernAction (&table->machine + table->kernAction), - depth (0) {} + depth (0), + crossStream (table->header.coverage & table->header.CrossStream) {} inline bool is_actionable (StateTableDriver *driver HB_UNUSED, const Entry *entry) @@ -209,15 +227,25 @@ struct KerxSubTableFormat1 { if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) { - buffer->pos[idx].x_advance += c->font->em_scale_x (v); - if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) - buffer->pos[idx].x_offset += c->font->em_scale_x (v); + if (crossStream) + buffer->pos[idx].y_offset += c->font->em_scale_y (v); + else + { + buffer->pos[idx].x_advance += c->font->em_scale_x (v); + if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) + buffer->pos[idx].x_offset += c->font->em_scale_x (v); + } } else { - buffer->pos[idx].y_advance += c->font->em_scale_y (v); - if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) - buffer->pos[idx].y_offset += c->font->em_scale_y (v); + if (crossStream) + buffer->pos[idx].x_offset += c->font->em_scale_x (v); + else + { + buffer->pos[idx].y_advance += c->font->em_scale_y (v); + if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) + buffer->pos[idx].y_offset += c->font->em_scale_y (v); + } } } } @@ -232,6 +260,7 @@ struct KerxSubTableFormat1 const UnsizedArrayOf &kernAction; unsigned int stack[8]; unsigned int depth; + bool crossStream; }; inline bool apply (hb_aat_apply_context_t *c) const @@ -241,6 +270,9 @@ struct KerxSubTableFormat1 if (!c->plan->requested_kerning) return false; + if (header.coverage & header.CrossStream) + return false; + if (header.tupleCount) return_trace (false); /* TODO kerxTupleKern */ @@ -289,6 +321,9 @@ struct KerxSubTableFormat2 if (!c->plan->requested_kerning) return false; + if (header.coverage & header.CrossStream) + return false; + accelerator_t accel (*this, c); hb_kern_machine_t machine (accel); machine.kern (c->font, c->buffer, c->plan->kern_mask); @@ -547,6 +582,9 @@ struct KerxSubTableFormat6 if (!c->plan->requested_kerning) return false; + if (header.coverage & header.CrossStream) + return false; + accelerator_t accel (*this, c); hb_kern_machine_t machine (accel); machine.kern (c->font, c->buffer, c->plan->kern_mask); @@ -615,21 +653,7 @@ struct KerxTable friend struct kerx; inline unsigned int get_size (void) const { return u.header.length; } - inline unsigned int get_type (void) const { return u.header.coverage & SubtableType; } - - enum Coverage - { - Vertical = 0x80000000, /* Set if table has vertical kerning values. */ - CrossStream = 0x40000000, /* Set if table has cross-stream kerning values. */ - Variation = 0x20000000, /* Set if table has variation kerning values. */ - Backwards = 0x10000000, /* If clear, process the glyphs forwards, that - * is, from first to last in the glyph stream. - * If we, process them from last to first. - * This flag only applies to state-table based - * 'kerx' subtables (types 1 and 4). */ - Reserved = 0x0FFFFF00, /* Reserved, set to zero. */ - SubtableType = 0x000000FF, /* Subtable type. */ - }; + inline unsigned int get_type (void) const { return u.header.coverage & u.header.SubtableType; } template inline typename context_t::return_t dispatch (context_t *c) const @@ -689,14 +713,11 @@ struct kerx { bool reverse; - if (table->u.header.coverage & (KerxTable::CrossStream)) - goto skip; /* We do NOT handle cross-stream. */ - if (HB_DIRECTION_IS_VERTICAL (c->buffer->props.direction) != - bool (table->u.header.coverage & KerxTable::Vertical)) + bool (table->u.header.coverage & table->u.header.Vertical)) goto skip; - reverse = bool (table->u.header.coverage & KerxTable::Backwards) != + reverse = bool (table->u.header.coverage & table->u.header.Backwards) != HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); if (!c->buffer->message (c->font, "start kerx subtable %d", c->lookup_index)) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index c54c10086..30f7091be 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -174,6 +174,9 @@ struct KernSubTableFormat0 if (!c->plan->requested_kerning) return false; + if (header.coverage & header.CrossStream) + return false; + hb_kern_machine_t machine (*this); machine.kern (c->font, c->buffer, c->plan->kern_mask); @@ -218,7 +221,9 @@ struct KernSubTableFormat1 * similar to offsets in morx table, NOT from beginning of this table, like * other subtables in kerx. Discovered via testing. */ kernAction (&table->machine + table->kernAction), - depth (0) {} + depth (0), + crossStream (table->header.coverage & table->header.CrossStream), + crossOffset (0) {} inline bool is_actionable (AAT::StateTableDriver *driver HB_UNUSED, const AAT::Entry *entry) @@ -241,7 +246,9 @@ struct KernSubTableFormat1 if (entry->flags & Offset) { - unsigned int kernIndex = AAT::MortTypes::offsetToIndex (entry->flags & Offset, &table->machine, kernAction.arrayZ); + unsigned int kernIndex = AAT::MortTypes::offsetToIndex (entry->flags & Offset, + &table->machine, + kernAction.arrayZ); const FWORD *actions = &kernAction[kernIndex]; if (!c->sanitizer.check_array (actions, depth)) { @@ -258,21 +265,50 @@ struct KernSubTableFormat1 * list. Discovered by testing. */ unsigned int idx = stack[i]; int v = *actions++; + + /* The following two flags are undocumented in the spec, but described + * in the example. */ + bool last = v & 1; + v = v & ~1; + if (v == 0x8000) + { + crossOffset = 0; + v = 0; + } + if (idx < buffer->len && buffer->info[idx].mask & kern_mask) { if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) { - buffer->pos[idx].x_advance += c->font->em_scale_x (v); - if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) - buffer->pos[idx].x_offset += c->font->em_scale_x (v); + if (crossStream) + { + crossOffset += v; + buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); + } + else + { + buffer->pos[idx].x_advance += c->font->em_scale_x (v); + if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) + buffer->pos[idx].x_offset += c->font->em_scale_x (v); + } } else { - buffer->pos[idx].y_advance += c->font->em_scale_y (v); - if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) - buffer->pos[idx].y_offset += c->font->em_scale_y (v); + if (crossStream) + { + crossOffset += v; + buffer->pos[idx].x_offset += c->font->em_scale_x (crossOffset); + } + else + { + buffer->pos[idx].y_advance += c->font->em_scale_y (v); + if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) + buffer->pos[idx].y_offset += c->font->em_scale_y (v); + } } } + if (last) + break; } depth = 0; } @@ -286,6 +322,8 @@ struct KernSubTableFormat1 const UnsizedArrayOf &kernAction; unsigned int stack[8]; unsigned int depth; + bool crossStream; + int crossOffset; }; inline bool apply (AAT::hb_aat_apply_context_t *c) const @@ -344,6 +382,9 @@ struct KernSubTableFormat2 if (!c->plan->requested_kerning) return false; + if (header.coverage & header.CrossStream) + return false; + accelerator_t accel (*this, c); hb_kern_machine_t machine (accel); machine.kern (c->font, c->buffer, c->plan->kern_mask); @@ -412,6 +453,9 @@ struct KernSubTableFormat3 if (!c->plan->requested_kerning) return false; + if (header.coverage & header.CrossStream) + return false; + hb_kern_machine_t machine (*this); machine.kern (c->font, c->buffer, c->plan->kern_mask); @@ -547,8 +591,7 @@ struct KernTable st = CastP (&thiz()->dataZ); for (unsigned int i = 0; i < count; i++) { - if (st->u.header.coverage & - (st->u.header.Variation | st->u.header.CrossStream)) + if (st->u.header.coverage & st->u.header.Variation) goto skip; if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != From e8c47724638c29d78001905610c662de99c59cad Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 17:16:04 -0500 Subject: [PATCH 07/66] [kern] XXX Negate CrossKerning sign Not sure why, but seems to better match GeezaPro Arabic w CoreText. Quite possibly I'm doing something very wrong... --- src/hb-ot-kern-table.hh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 30f7091be..c7282a3a0 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -282,7 +282,8 @@ struct KernSubTableFormat1 { if (crossStream) { - crossOffset += v; + /* XXX Why negative, not positive?!?! */ + crossOffset -= v; buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); } else From 9c04b6058306cd4b2123a33a7cbeb47505434217 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 18:35:58 -0500 Subject: [PATCH 08/66] [kern] In Format1, adjust how kerning is done In a series of kerns in one action, kern all but last glyph forward, and the last one backward. Seems to better match what CoreText is doing. Test cases, with GeezaPro Arabic: $ ./hb-shape GeezaPro_10_10.ttc -u U+0631,U+0628 [u0628.beh=1+1415|u0631.reh=0@-202,0+700] $ ./hb-shape GeezaPro_10_10.ttc -u U+0628,U+064F [u064f.damma=0@0,-250+-250|u0628.beh=0@250,0+1665] In a later change, I'll make kern machine avoid producing negative kerns. --- src/hb-ot-kern-table.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index c7282a3a0..980b7e150 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -289,7 +289,7 @@ struct KernSubTableFormat1 else { buffer->pos[idx].x_advance += c->font->em_scale_x (v); - if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) + if (last) buffer->pos[idx].x_offset += c->font->em_scale_x (v); } } @@ -303,7 +303,7 @@ struct KernSubTableFormat1 else { buffer->pos[idx].y_advance += c->font->em_scale_y (v); - if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) + if (last) buffer->pos[idx].y_offset += c->font->em_scale_y (v); } } From 9810f0b80e5b6580a7a15debcec073dfc9ca759f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 19:24:04 -0500 Subject: [PATCH 09/66] [kern] Minor --- src/hb-ot-kern-table.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 980b7e150..0e60c582a 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -269,7 +269,7 @@ struct KernSubTableFormat1 /* The following two flags are undocumented in the spec, but described * in the example. */ bool last = v & 1; - v = v & ~1; + v &= ~1; if (v == 0x8000) { crossOffset = 0; From 4d003b8503f9c984abe2ac0de8c526a276ea8e54 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 21:04:02 -0500 Subject: [PATCH 10/66] [kern] Add TODO --- src/hb-ot-kern-table.hh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 0e60c582a..bce0aa3e7 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -225,6 +225,15 @@ struct KernSubTableFormat1 crossStream (table->header.coverage & table->header.CrossStream), crossOffset (0) {} + /* TODO + * + * "Because the stateTableOffset in the state table header is (strictly + * speaking) redundant, some 'kern' tables use it to record an initial + * state where that should not be StartOfText. To determine if this is + * done, calculate what the stateTableOffset should be. If it's different + * from the actual stateTableOffset, use it as the initial state." + */ + inline bool is_actionable (AAT::StateTableDriver *driver HB_UNUSED, const AAT::Entry *entry) { From 564e8ac0465d8ced3a98ecb55d09ffaa45eefc2f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 21:04:40 -0500 Subject: [PATCH 11/66] [kern] Adjust some more Getting closer. So many open questions still... --- src/hb-ot-kern-table.hh | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index bce0aa3e7..2dccc40ee 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -266,13 +266,9 @@ struct KernSubTableFormat1 } hb_mask_t kern_mask = c->plan->kern_mask; - for (unsigned int i = 0; i < depth; i++) + while (depth--) { - /* Apparently, when spec says "Each pops one glyph from the kerning stack - * and applies the kerning value to it.", it doesn't mean it in that order. - * The deepest item in the stack corresponds to the first item in the action - * list. Discovered by testing. */ - unsigned int idx = stack[i]; + unsigned int idx = stack[depth]; int v = *actions++; /* The following two flags are undocumented in the spec, but described @@ -291,29 +287,41 @@ struct KernSubTableFormat1 { if (crossStream) { + if (!buffer->pos[idx].y_offset) + buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); /* XXX Why negative, not positive?!?! */ - crossOffset -= v; - buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); + v = -v; + crossOffset += v; } else { - buffer->pos[idx].x_advance += c->font->em_scale_x (v); - if (last) + if (!buffer->pos[idx].x_offset) + { + buffer->pos[idx].x_advance += c->font->em_scale_x (v); buffer->pos[idx].x_offset += c->font->em_scale_x (v); + } } } else { if (crossStream) { - crossOffset += v; - buffer->pos[idx].x_offset += c->font->em_scale_x (crossOffset); + /* CoreText doesn't do crossStream kerning in vertical. */ +#if 0 + if (!buffer->pos[idx].x_offset) + buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); +#endif + /* XXX Why negative, not positive?!?! */ + v = -v; + crossOffset += v; } else { - buffer->pos[idx].y_advance += c->font->em_scale_y (v); - if (last) + if (!buffer->pos[idx].y_offset) + { + buffer->pos[idx].y_advance += c->font->em_scale_y (v); buffer->pos[idx].y_offset += c->font->em_scale_y (v); + } } } } From 80a33b9ac351d81793f35a92e0255ffbf5ceb8b9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 21:41:28 -0500 Subject: [PATCH 12/66] [kern] More tweaks Solves a mystery or two. I'm fairly confident this is what CoreText does now. --- src/hb-ot-kern-table.hh | 42 ++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 2dccc40ee..28bca8439 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -266,15 +266,27 @@ struct KernSubTableFormat1 } hb_mask_t kern_mask = c->plan->kern_mask; - while (depth--) - { - unsigned int idx = stack[depth]; - int v = *actions++; - /* The following two flags are undocumented in the spec, but described - * in the example. */ - bool last = v & 1; + /* "Each pops one glyph from the kerning stack and applies the kerning value to it. + * The end of the list is marked by an odd value... */ + unsigned int i; + for (i = 0; i < depth; i++) + if (actions[i] & 1) + { + i++; + break; + } + for (; i; i--) + { + unsigned int idx = stack[depth - i]; + int v = actions[i - 1]; + + /* "The end of the list is marked by an odd value..." + * Ignore it. */ v &= ~1; + + /* The following flag is undocumented in the spec, but described + * in the example. */ if (v == 0x8000) { crossOffset = 0; @@ -287,11 +299,9 @@ struct KernSubTableFormat1 { if (crossStream) { + crossOffset += v; if (!buffer->pos[idx].y_offset) buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); - /* XXX Why negative, not positive?!?! */ - v = -v; - crossOffset += v; } else { @@ -307,13 +317,9 @@ struct KernSubTableFormat1 if (crossStream) { /* CoreText doesn't do crossStream kerning in vertical. */ -#if 0 - if (!buffer->pos[idx].x_offset) - buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); -#endif - /* XXX Why negative, not positive?!?! */ - v = -v; - crossOffset += v; + //crossOffset += v; + //if (!buffer->pos[idx].x_offset) + // buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); } else { @@ -325,8 +331,6 @@ struct KernSubTableFormat1 } } } - if (last) - break; } depth = 0; } From 0123976a0c1e2f629252969a7ff632dc2b1dbbc9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 6 Nov 2018 21:45:40 -0500 Subject: [PATCH 13/66] [kerx] Adjust CrossStream kern to match 'kern' table --- src/hb-aat-layout-kerx-table.hh | 62 +++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 550abf31b..93d3ffb7f 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -179,7 +179,8 @@ struct KerxSubTableFormat1 * other subtables in kerx. Discovered via testing. */ kernAction (&table->machine + table->kernAction), depth (0), - crossStream (table->header.coverage & table->header.CrossStream) {} + crossStream (table->header.coverage & table->header.CrossStream), + crossOffset (0) {} inline bool is_actionable (StateTableDriver *driver HB_UNUSED, const Entry *entry) @@ -215,36 +216,68 @@ struct KerxSubTableFormat1 } hb_mask_t kern_mask = c->plan->kern_mask; - for (unsigned int i = 0; i < depth; i++) + + /* From Apple 'kern' spec: + * "Each pops one glyph from the kerning stack and applies the kerning value to it. + * The end of the list is marked by an odd value... */ + unsigned int i; + for (i = 0; i < depth; i++) + if (actions[i] & 1) + { + i++; + break; + } + for (; i; i--) { - /* Apparently, when spec says "Each pops one glyph from the kerning stack - * and applies the kerning value to it.", it doesn't mean it in that order. - * The deepest item in the stack corresponds to the first item in the action - * list. Discovered by testing. */ - unsigned int idx = stack[i]; - int v = *actions++; + unsigned int idx = stack[depth - i]; + int v = actions[i - 1]; + + /* "The end of the list is marked by an odd value..." + * Ignore it. */ + v &= ~1; + + /* The following flag is undocumented in the spec, but described + * in the 'kern' table example. */ + if (v == 0x8000) + { + crossOffset = 0; + v = 0; + } if (idx < buffer->len && buffer->info[idx].mask & kern_mask) { if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) { if (crossStream) - buffer->pos[idx].y_offset += c->font->em_scale_y (v); + { + crossOffset += v; + if (!buffer->pos[idx].y_offset) + buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); + } else { - buffer->pos[idx].x_advance += c->font->em_scale_x (v); - if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) + if (!buffer->pos[idx].x_offset) + { + buffer->pos[idx].x_advance += c->font->em_scale_x (v); buffer->pos[idx].x_offset += c->font->em_scale_x (v); + } } } else { if (crossStream) - buffer->pos[idx].x_offset += c->font->em_scale_x (v); + { + /* CoreText doesn't do crossStream kerning in vertical. */ + //crossOffset += v; + //if (!buffer->pos[idx].x_offset) + // buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); + } else { - buffer->pos[idx].y_advance += c->font->em_scale_y (v); - if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction)) + if (!buffer->pos[idx].y_offset) + { + buffer->pos[idx].y_advance += c->font->em_scale_y (v); buffer->pos[idx].y_offset += c->font->em_scale_y (v); + } } } } @@ -261,6 +294,7 @@ struct KerxSubTableFormat1 unsigned int stack[8]; unsigned int depth; bool crossStream; + int crossOffset; }; inline bool apply (hb_aat_apply_context_t *c) const From 8d0f797139e853d13cb2383d541c2e691d9dbae3 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 00:04:40 -0500 Subject: [PATCH 14/66] [kern/kerx] Fix "reset" magic value --- src/hb-aat-layout-kerx-table.hh | 2 +- src/hb-ot-kern-table.hh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 93d3ffb7f..efbe7ed3a 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -238,7 +238,7 @@ struct KerxSubTableFormat1 /* The following flag is undocumented in the spec, but described * in the 'kern' table example. */ - if (v == 0x8000) + if (v == -0x8000) { crossOffset = 0; v = 0; diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 28bca8439..ec7ce929f 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -287,7 +287,7 @@ struct KernSubTableFormat1 /* The following flag is undocumented in the spec, but described * in the example. */ - if (v == 0x8000) + if (v == -0x8000) { crossOffset = 0; v = 0; From 59e04e42312293c30714a666c4479e209aec3c0e Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 00:25:48 -0500 Subject: [PATCH 15/66] [kern/kerx] Fix cursive joining MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tested with Waseem TTC: $ hb-shape Waseem.ttc جحخج [F1Jeem_R2=3@0,180+479|M1Khah_L2_R2=2@0,682+403|M1Hah_L2_R2=1@0,1184+403|I1Jeem_L2=0@0,1184+744] --- src/hb-aat-layout-kerx-table.hh | 2 ++ src/hb-ot-kern-table.hh | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index efbe7ed3a..6f63aa408 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -284,6 +284,8 @@ struct KerxSubTableFormat1 } depth = 0; } + else + buffer->pos[buffer->idx].y_offset += c->font->em_scale_y (crossOffset); return true; } diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index ec7ce929f..b8006005f 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -334,6 +334,8 @@ struct KernSubTableFormat1 } depth = 0; } + else + buffer->pos[buffer->idx].y_offset += c->font->em_scale_y (crossOffset); return true; } From befac337ca2c705e2cea60a9a92e40e0dbbc40aa Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 09:53:02 -0500 Subject: [PATCH 16/66] [kern] Remove Override business Not used in any fonts. Not well-specified when mixing kerning with Cross-Stream positioning. --- src/hb-ot-kern-table.hh | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index b8006005f..33e682714 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -588,8 +588,6 @@ struct KernTable (st->u.header.Variation | st->u.header.CrossStream | st->u.header.Direction)) != st->u.header.DirectionHorizontal) continue; - if (st->u.header.coverage & st->u.header.Override) - v = 0; v += st->get_kerning (left, right); st = &StructAfter (*st); } @@ -603,15 +601,6 @@ struct KernTable c->set_lookup_index (0); const SubTable *st = CastP (&thiz()->dataZ); unsigned int count = thiz()->nTables; - /* If there's an override subtable, skip subtables before that. */ - unsigned int last_override = 0; - for (unsigned int i = 0; i < count; i++) - { - if (!(st->u.header.coverage & (st->u.header.Variation | st->u.header.CrossStream)) && - (st->u.header.coverage & st->u.header.Override)) - last_override = i; - st = &StructAfter (*st); - } st = CastP (&thiz()->dataZ); for (unsigned int i = 0; i < count; i++) { @@ -622,9 +611,6 @@ struct KernTable ((st->u.header.coverage & st->u.header.Direction) == st->u.header.DirectionHorizontal)) goto skip; - if (i < last_override) - goto skip; - if (!c->buffer->message (c->font, "start kern subtable %d", c->lookup_index)) goto skip; @@ -719,8 +705,6 @@ struct KernAAT : KernTable CrossStream = 0x40u, Variation = 0x20u, - Override = 0x00u, /* Not supported. */ - DirectionHorizontal= 0x00u }; From a6acff252c72457ecfa856fd6c57081b3a4290dd Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 10:19:46 -0500 Subject: [PATCH 17/66] [kerx] Towards sharing subtables with kern --- src/hb-aat-layout-kerx-table.hh | 41 ++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 6f63aa408..995e01387 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -90,6 +90,7 @@ struct KerxSubTableHeader DEFINE_SIZE_STATIC (12); }; +template struct KerxSubTableFormat0 { inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, @@ -141,13 +142,14 @@ struct KerxSubTableFormat0 } protected: - KerxSubTableHeader header; + KernSubTableHeader header; BinSearchArrayOf pairs; /* Sorted kern records. */ public: - DEFINE_SIZE_ARRAY (28, pairs); + DEFINE_SIZE_ARRAY (KernSubTableHeader::static_size + 16, pairs); }; +template struct KerxSubTableFormat1 { struct EntryData @@ -329,13 +331,14 @@ struct KerxSubTableFormat1 } protected: - KerxSubTableHeader header; + KernSubTableHeader header; StateTable machine; LOffsetTo, false> kernAction; public: - DEFINE_SIZE_STATIC (32); + DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 20); }; +template struct KerxSubTableFormat2 { inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, @@ -390,7 +393,7 @@ struct KerxSubTableFormat2 } protected: - KerxSubTableHeader header; + KernSubTableHeader header; HBUINT32 rowWidth; /* The width, in bytes, of a row in the table. */ LOffsetTo, false> leftClassTable; /* Offset from beginning of this subtable to @@ -402,9 +405,10 @@ struct KerxSubTableFormat2 array; /* Offset from beginning of this subtable to * the start of the kerning array. */ public: - DEFINE_SIZE_STATIC (28); + DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 16); }; +template struct KerxSubTableFormat4 { struct EntryData @@ -566,14 +570,15 @@ struct KerxSubTableFormat4 } protected: - KerxSubTableHeader header; + KernSubTableHeader header; StateTable machine; HBUINT32 flags; public: - DEFINE_SIZE_STATIC (32); + DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 20); }; +template struct KerxSubTableFormat6 { enum Flags @@ -589,7 +594,7 @@ struct KerxSubTableFormat6 unsigned int num_glyphs = c->sanitizer.get_num_glyphs (); if (is_long ()) { - const U::Long &t = u.l; + const typename U::Long &t = u.l; unsigned int l = (this+t.rowIndexTable).get_value_or_null (left, num_glyphs); unsigned int r = (this+t.columnIndexTable).get_value_or_null (right, num_glyphs); unsigned int offset = l + r; @@ -601,7 +606,7 @@ struct KerxSubTableFormat6 } else { - const U::Short &t = u.s; + const typename U::Short &t = u.s; unsigned int l = (this+t.rowIndexTable).get_value_or_null (left, num_glyphs); unsigned int r = (this+t.columnIndexTable).get_value_or_null (right, num_glyphs); unsigned int offset = l + r; @@ -660,7 +665,7 @@ struct KerxSubTableFormat6 }; protected: - KerxSubTableHeader header; + KernSubTableHeader header; HBUINT32 flags; HBUINT16 rowCount; HBUINT16 columnCount; @@ -681,7 +686,7 @@ struct KerxSubTableFormat6 } u; LOffsetTo, false> vector; public: - DEFINE_SIZE_STATIC (36); + DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 24); }; struct KerxTable @@ -718,12 +723,12 @@ struct KerxTable protected: union { - KerxSubTableHeader header; - KerxSubTableFormat0 format0; - KerxSubTableFormat1 format1; - KerxSubTableFormat2 format2; - KerxSubTableFormat4 format4; - KerxSubTableFormat6 format6; + KerxSubTableHeader header; + KerxSubTableFormat0 format0; + KerxSubTableFormat1 format1; + KerxSubTableFormat2 format2; + KerxSubTableFormat4 format4; + KerxSubTableFormat6 format6; } u; public: DEFINE_SIZE_MIN (12); From d0f8f4c200670bc0bfbffbf301139a3613865a7f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 10:25:25 -0500 Subject: [PATCH 18/66] [kern] Move kern machine to hb-kern.hh --- src/Makefile.sources | 1 + src/hb-aat-layout-kerx-table.hh | 5 +- src/hb-kern.hh | 153 ++++++++++++++++++++++++++++++++ src/hb-ot-kern-table.hh | 115 +----------------------- src/hb-ot-shape-fallback.cc | 2 +- 5 files changed, 157 insertions(+), 119 deletions(-) create mode 100644 src/hb-kern.hh diff --git a/src/Makefile.sources b/src/Makefile.sources index 561fbed6d..2cada4bbc 100644 --- a/src/Makefile.sources +++ b/src/Makefile.sources @@ -16,6 +16,7 @@ HB_BASE_sources = \ hb-font.hh \ hb-font.cc \ hb-iter.hh \ + hb-kern.hh \ hb-map.hh \ hb-map.cc \ hb-machinery.hh \ diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 995e01387..11d7f5c76 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -28,10 +28,7 @@ #ifndef HB_AAT_LAYOUT_KERX_TABLE_HH #define HB_AAT_LAYOUT_KERX_TABLE_HH -#include "hb-open-type.hh" -#include "hb-aat-layout-common.hh" -#include "hb-ot-layout-gpos-table.hh" -#include "hb-ot-kern-table.hh" +#include "hb-kern.hh" /* * kerx -- Extended Kerning diff --git a/src/hb-kern.hh b/src/hb-kern.hh new file mode 100644 index 000000000..de7269600 --- /dev/null +++ b/src/hb-kern.hh @@ -0,0 +1,153 @@ +/* + * Copyright © 2017 Google, Inc. + * + * This is part of HarfBuzz, a text shaping library. + * + * Permission is hereby granted, without written agreement and without + * license or royalty fees, to use, copy, modify, and distribute this + * software and its documentation for any purpose, provided that the + * above copyright notice and the following two paragraphs appear in + * all copies of this software. + * + * IN NO EVENT SHALL THE COPYRIGHT HOLDER BE LIABLE TO ANY PARTY FOR + * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES + * ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS DOCUMENTATION, EVEN + * IF THE COPYRIGHT HOLDER HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH + * DAMAGE. + * + * THE COPYRIGHT HOLDER SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, + * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS FOR A PARTICULAR PURPOSE. THE SOFTWARE PROVIDED HEREUNDER IS + * ON AN "AS IS" BASIS, AND THE COPYRIGHT HOLDER HAS NO OBLIGATION TO + * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. + * + * Google Author(s): Behdad Esfahbod + */ + +#ifndef HB_KERN_HH +#define HB_KERN_HH + +#include "hb-open-type.hh" +#include "hb-aat-layout-common.hh" +#include "hb-ot-layout-gpos-table.hh" + + +namespace OT { + + +template +struct hb_kern_machine_t +{ + hb_kern_machine_t (const Driver &driver_) : driver (driver_) {} + + HB_NO_SANITIZE_SIGNED_INTEGER_OVERFLOW + inline void kern (hb_font_t *font, + hb_buffer_t *buffer, + hb_mask_t kern_mask, + bool scale = true) const + { + OT::hb_ot_apply_context_t c (1, font, buffer); + c.set_lookup_mask (kern_mask); + c.set_lookup_props (OT::LookupFlag::IgnoreMarks); + OT::hb_ot_apply_context_t::skipping_iterator_t &skippy_iter = c.iter_input; + skippy_iter.init (&c); + + bool horizontal = HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction); + unsigned int count = buffer->len; + hb_glyph_info_t *info = buffer->info; + hb_glyph_position_t *pos = buffer->pos; + for (unsigned int idx = 0; idx < count;) + { + if (!(info[idx].mask & kern_mask)) + { + idx++; + continue; + } + + skippy_iter.reset (idx, 1); + if (!skippy_iter.next ()) + { + idx++; + continue; + } + + unsigned int i = idx; + unsigned int j = skippy_iter.idx; + + hb_position_t kern = driver.get_kerning (info[i].codepoint, + info[j].codepoint); + + + if (likely (!kern)) + goto skip; + + + if (horizontal) + { + if (scale) + kern = font->em_scale_x (kern); + hb_position_t kern1 = kern >> 1; + hb_position_t kern2 = kern - kern1; + pos[i].x_advance += kern1; + pos[j].x_advance += kern2; + pos[j].x_offset += kern2; + } + else + { + if (scale) + kern = font->em_scale_y (kern); + hb_position_t kern1 = kern >> 1; + hb_position_t kern2 = kern - kern1; + pos[i].y_advance += kern1; + pos[j].y_advance += kern2; + pos[j].y_offset += kern2; + } + + buffer->unsafe_to_break (i, j + 1); + + skip: + idx = skippy_iter.idx; + } + } + + const Driver &driver; +}; + + +struct hb_glyph_pair_t +{ + hb_codepoint_t left; + hb_codepoint_t right; +}; + +struct KernPair +{ + inline int get_kerning (void) const + { return value; } + + inline int cmp (const hb_glyph_pair_t &o) const + { + int ret = left.cmp (o.left); + if (ret) return ret; + return right.cmp (o.right); + } + + inline bool sanitize (hb_sanitize_context_t *c) const + { + TRACE_SANITIZE (this); + return_trace (c->check_struct (this)); + } + + protected: + GlyphID left; + GlyphID right; + FWORD value; + public: + DEFINE_SIZE_STATIC (6); +}; + + +} /* namespace OT */ + + +#endif /* HB_KERN_HH */ diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 33e682714..40d36a336 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -27,89 +27,8 @@ #ifndef HB_OT_KERN_TABLE_HH #define HB_OT_KERN_TABLE_HH -#include "hb-open-type.hh" +#include "hb-kern.hh" #include "hb-ot-shape.hh" -#include "hb-ot-layout-gsubgpos.hh" -#include "hb-aat-layout-common.hh" - - -template -struct hb_kern_machine_t -{ - hb_kern_machine_t (const Driver &driver_) : driver (driver_) {} - - HB_NO_SANITIZE_SIGNED_INTEGER_OVERFLOW - inline void kern (hb_font_t *font, - hb_buffer_t *buffer, - hb_mask_t kern_mask, - bool scale = true) const - { - OT::hb_ot_apply_context_t c (1, font, buffer); - c.set_lookup_mask (kern_mask); - c.set_lookup_props (OT::LookupFlag::IgnoreMarks); - OT::hb_ot_apply_context_t::skipping_iterator_t &skippy_iter = c.iter_input; - skippy_iter.init (&c); - - bool horizontal = HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction); - unsigned int count = buffer->len; - hb_glyph_info_t *info = buffer->info; - hb_glyph_position_t *pos = buffer->pos; - for (unsigned int idx = 0; idx < count;) - { - if (!(info[idx].mask & kern_mask)) - { - idx++; - continue; - } - - skippy_iter.reset (idx, 1); - if (!skippy_iter.next ()) - { - idx++; - continue; - } - - unsigned int i = idx; - unsigned int j = skippy_iter.idx; - - hb_position_t kern = driver.get_kerning (info[i].codepoint, - info[j].codepoint); - - - if (likely (!kern)) - goto skip; - - - if (horizontal) - { - if (scale) - kern = font->em_scale_x (kern); - hb_position_t kern1 = kern >> 1; - hb_position_t kern2 = kern - kern1; - pos[i].x_advance += kern1; - pos[j].x_advance += kern2; - pos[j].x_offset += kern2; - } - else - { - if (scale) - kern = font->em_scale_y (kern); - hb_position_t kern1 = kern >> 1; - hb_position_t kern2 = kern - kern1; - pos[i].y_advance += kern1; - pos[j].y_advance += kern2; - pos[j].y_offset += kern2; - } - - buffer->unsafe_to_break (i, j + 1); - - skip: - idx = skippy_iter.idx; - } - } - - const Driver &driver; -}; /* @@ -123,38 +42,6 @@ struct hb_kern_machine_t namespace OT { -struct hb_glyph_pair_t -{ - hb_codepoint_t left; - hb_codepoint_t right; -}; - -struct KernPair -{ - inline int get_kerning (void) const - { return value; } - - inline int cmp (const hb_glyph_pair_t &o) const - { - int ret = left.cmp (o.left); - if (ret) return ret; - return right.cmp (o.right); - } - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - return_trace (c->check_struct (this)); - } - - protected: - GlyphID left; - GlyphID right; - FWORD value; - public: - DEFINE_SIZE_STATIC (6); -}; - template struct KernSubTableFormat0 { diff --git a/src/hb-ot-shape-fallback.cc b/src/hb-ot-shape-fallback.cc index 766efe20d..3742d6704 100644 --- a/src/hb-ot-shape-fallback.cc +++ b/src/hb-ot-shape-fallback.cc @@ -465,7 +465,7 @@ _hb_ot_shape_fallback_kern (const hb_ot_shape_plan_t *plan, !font->has_glyph_v_kerning_func ()) return; hb_ot_shape_fallback_kern_driver_t driver (font, buffer); - hb_kern_machine_t machine (driver); + OT::hb_kern_machine_t machine (driver); machine.kern (font, buffer, plan->kern_mask, false); } From 540ccc38b0f95804d08047f8b2d059bfd1e09337 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 10:33:46 -0500 Subject: [PATCH 19/66] [kern/kerx] More towards sharing --- src/hb-aat-layout-kerx-table.hh | 7 ++++--- src/hb-ot-kern-table.hh | 10 +++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 11d7f5c76..a2f0c642d 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -59,6 +59,8 @@ kerxTupleKern (int value, struct KerxSubTableHeader { + typedef MorxTypes Types; + enum Coverage { Vertical = 0x80000000, /* Set if table has vertical kerning values. */ @@ -134,13 +136,12 @@ struct KerxSubTableFormat0 inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - return_trace (likely (c->check_struct (this) && - pairs.sanitize (c))); + return_trace (likely (pairs.sanitize (c))); } protected: KernSubTableHeader header; - BinSearchArrayOf + BinSearchArrayOf pairs; /* Sorted kern records. */ public: DEFINE_SIZE_ARRAY (KernSubTableHeader::static_size + 16, pairs); diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 40d36a336..9f7546623 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -49,8 +49,7 @@ struct KernSubTableFormat0 { hb_glyph_pair_t pair = {left, right}; int i = pairs.bsearch (pair); - if (i == -1) - return 0; + if (i == -1) return 0; return pairs[i].get_kerning (); } @@ -78,7 +77,8 @@ struct KernSubTableFormat0 protected: KernSubTableHeader header; - BinSearchArrayOf pairs; /* Array of kerning pairs. */ + BinSearchArrayOf + pairs; /* Array of kerning pairs. */ public: DEFINE_SIZE_ARRAY (KernSubTableHeader::static_size + 8, pairs); }; @@ -543,6 +543,8 @@ struct KernOT : KernTable struct SubTableHeader { + typedef AAT::MortTypes Types; + enum Coverage { Direction = 0x01u, @@ -586,6 +588,8 @@ struct KernAAT : KernTable struct SubTableHeader { + typedef AAT::MortTypes Types; + enum Coverage { Direction = 0x80u, From c97dde5d55929df394fbe57c1ba1a725592c6732 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 10:39:39 -0500 Subject: [PATCH 20/66] [kern/kerx] Towards merge more --- src/hb-aat-layout-kerx-table.hh | 75 ++++++++++++++++++++++++--------- src/hb-kern.hh | 33 --------------- src/hb-ot-kern-table.hh | 7 ++- 3 files changed, 58 insertions(+), 57 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index a2f0c642d..dcab790d2 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -29,6 +29,7 @@ #define HB_AAT_LAYOUT_KERX_TABLE_HH #include "hb-kern.hh" +#include "hb-aat-layout-ankr-table.hh" /* * kerx -- Extended Kerning @@ -57,36 +58,36 @@ kerxTupleKern (int value, } -struct KerxSubTableHeader +struct hb_glyph_pair_t { - typedef MorxTypes Types; + hb_codepoint_t left; + hb_codepoint_t right; +}; - enum Coverage +struct KernPair +{ + inline int get_kerning (void) const + { return value; } + + inline int cmp (const hb_glyph_pair_t &o) const { - Vertical = 0x80000000, /* Set if table has vertical kerning values. */ - CrossStream = 0x40000000, /* Set if table has cross-stream kerning values. */ - Variation = 0x20000000, /* Set if table has variation kerning values. */ - Backwards = 0x10000000, /* If clear, process the glyphs forwards, that - * is, from first to last in the glyph stream. - * If we, process them from last to first. - * This flag only applies to state-table based - * 'kerx' subtables (types 1 and 4). */ - Reserved = 0x0FFFFF00, /* Reserved, set to zero. */ - SubtableType = 0x000000FF, /* Subtable type. */ - }; + int ret = left.cmp (o.left); + if (ret) return ret; + return right.cmp (o.right); + } inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - return_trace (likely (c->check_struct (this))); + return_trace (c->check_struct (this)); } + protected: + GlyphID left; + GlyphID right; + FWORD value; public: - HBUINT32 length; - HBUINT32 coverage; - HBUINT32 tupleCount; - public: - DEFINE_SIZE_STATIC (12); + DEFINE_SIZE_STATIC (6); }; template @@ -687,6 +688,39 @@ struct KerxSubTableFormat6 DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 24); }; + +struct KerxSubTableHeader +{ + typedef MorxTypes Types; + + enum Coverage + { + Vertical = 0x80000000, /* Set if table has vertical kerning values. */ + CrossStream = 0x40000000, /* Set if table has cross-stream kerning values. */ + Variation = 0x20000000, /* Set if table has variation kerning values. */ + Backwards = 0x10000000, /* If clear, process the glyphs forwards, that + * is, from first to last in the glyph stream. + * If we, process them from last to first. + * This flag only applies to state-table based + * 'kerx' subtables (types 1 and 4). */ + Reserved = 0x0FFFFF00, /* Reserved, set to zero. */ + SubtableType = 0x000000FF, /* Subtable type. */ + }; + + inline bool sanitize (hb_sanitize_context_t *c) const + { + TRACE_SANITIZE (this); + return_trace (likely (c->check_struct (this))); + } + + public: + HBUINT32 length; + HBUINT32 coverage; + HBUINT32 tupleCount; + public: + DEFINE_SIZE_STATIC (12); +}; + struct KerxTable { friend struct kerx; @@ -816,6 +850,7 @@ struct kerx DEFINE_SIZE_MIN (8); }; + } /* namespace AAT */ diff --git a/src/hb-kern.hh b/src/hb-kern.hh index de7269600..60e625c4b 100644 --- a/src/hb-kern.hh +++ b/src/hb-kern.hh @@ -114,39 +114,6 @@ struct hb_kern_machine_t }; -struct hb_glyph_pair_t -{ - hb_codepoint_t left; - hb_codepoint_t right; -}; - -struct KernPair -{ - inline int get_kerning (void) const - { return value; } - - inline int cmp (const hb_glyph_pair_t &o) const - { - int ret = left.cmp (o.left); - if (ret) return ret; - return right.cmp (o.right); - } - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - return_trace (c->check_struct (this)); - } - - protected: - GlyphID left; - GlyphID right; - FWORD value; - public: - DEFINE_SIZE_STATIC (6); -}; - - } /* namespace OT */ diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 9f7546623..c7c15e880 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -27,8 +27,7 @@ #ifndef HB_OT_KERN_TABLE_HH #define HB_OT_KERN_TABLE_HH -#include "hb-kern.hh" -#include "hb-ot-shape.hh" +#include "hb-aat-layout-kerx-table.hh" /* @@ -47,7 +46,7 @@ struct KernSubTableFormat0 { inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const { - hb_glyph_pair_t pair = {left, right}; + AAT::hb_glyph_pair_t pair = {left, right}; int i = pairs.bsearch (pair); if (i == -1) return 0; return pairs[i].get_kerning (); @@ -77,7 +76,7 @@ struct KernSubTableFormat0 protected: KernSubTableHeader header; - BinSearchArrayOf + BinSearchArrayOf pairs; /* Array of kerning pairs. */ public: DEFINE_SIZE_ARRAY (KernSubTableHeader::static_size + 8, pairs); From 5b17853547ca6848ee652ef6990a81bb345ac06f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 10:45:25 -0500 Subject: [PATCH 21/66] [kern/kerx] Share Format0 --- src/hb-aat-layout-kerx-table.hh | 22 +++++++++++---- src/hb-ot-kern-table.hh | 50 +++++---------------------------- 2 files changed, 23 insertions(+), 49 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index dcab790d2..9cdba8797 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -93,6 +93,14 @@ struct KernPair template struct KerxSubTableFormat0 { + inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const + { + hb_glyph_pair_t pair = {left, right}; + int i = pairs.bsearch (pair); + if (i == -1) return 0; + return pairs[i].get_kerning (); + } + inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, hb_aat_apply_context_t *c) const { @@ -100,7 +108,7 @@ struct KerxSubTableFormat0 int i = pairs.bsearch (pair); if (i == -1) return 0; int v = pairs[i].get_kerning (); - return kerxTupleKern (v, header.tupleCount, this, c); + return kerxTupleKern (v, header.tuple_count (), this, c); } inline bool apply (hb_aat_apply_context_t *c) const @@ -310,7 +318,7 @@ struct KerxSubTableFormat1 if (header.coverage & header.CrossStream) return false; - if (header.tupleCount) + if (header.tuple_count ()) return_trace (false); /* TODO kerxTupleKern */ driver_context_t dc (this, c); @@ -349,7 +357,7 @@ struct KerxSubTableFormat2 unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+array), offset); if (unlikely (!v->sanitize (&c->sanitizer))) return 0; - return kerxTupleKern (*v, header.tupleCount, this, c); + return kerxTupleKern (*v, header.tuple_count (), this, c); } inline bool apply (hb_aat_apply_context_t *c) const @@ -601,7 +609,7 @@ struct KerxSubTableFormat6 if (unlikely (hb_unsigned_mul_overflows (offset, sizeof (FWORD32)))) return 0; const FWORD32 *v = &StructAtOffset (&(this+t.array), offset * sizeof (FWORD32)); if (unlikely (!v->sanitize (&c->sanitizer))) return 0; - return kerxTupleKern (*v, header.tupleCount, &(this+vector), c); + return kerxTupleKern (*v, header.tuple_count (), &(this+vector), c); } else { @@ -611,7 +619,7 @@ struct KerxSubTableFormat6 unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+t.array), offset * sizeof (FWORD)); if (unlikely (!v->sanitize (&c->sanitizer))) return 0; - return kerxTupleKern (*v, header.tupleCount, &(this+vector), c); + return kerxTupleKern (*v, header.tuple_count (), &(this+vector), c); } } @@ -646,7 +654,7 @@ struct KerxSubTableFormat6 u.s.columnIndexTable.sanitize (c, this) && c->check_range (this, u.s.array) )) && - (header.tupleCount == 0 || + (header.tuple_count () == 0 || c->check_range (this, vector)))); } @@ -693,6 +701,8 @@ struct KerxSubTableHeader { typedef MorxTypes Types; + unsigned int tuple_count (void) const { return tupleCount; } + enum Coverage { Vertical = 0x80000000, /* Set if table has vertical kerning values. */ diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index c7c15e880..e065b6ff0 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -41,47 +41,6 @@ namespace OT { -template -struct KernSubTableFormat0 -{ - inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const - { - AAT::hb_glyph_pair_t pair = {left, right}; - int i = pairs.bsearch (pair); - if (i == -1) return 0; - return pairs[i].get_kerning (); - } - - inline bool apply (AAT::hb_aat_apply_context_t *c) const - { - TRACE_APPLY (this); - - if (!c->plan->requested_kerning) - return false; - - if (header.coverage & header.CrossStream) - return false; - - hb_kern_machine_t machine (*this); - machine.kern (c->font, c->buffer, c->plan->kern_mask); - - return_trace (true); - } - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - return_trace (pairs.sanitize (c)); - } - - protected: - KernSubTableHeader header; - BinSearchArrayOf - pairs; /* Array of kerning pairs. */ - public: - DEFINE_SIZE_ARRAY (KernSubTableHeader::static_size + 8, pairs); -}; - template struct KernSubTableFormat1 { @@ -445,7 +404,7 @@ struct KernSubTable public: union { KernSubTableHeader header; - KernSubTableFormat0 format0; + AAT::KerxSubTableFormat0 format0; KernSubTableFormat1 format1; KernSubTableFormat2 format2; KernSubTableFormat3 format3; @@ -544,6 +503,8 @@ struct KernOT : KernTable { typedef AAT::MortTypes Types; + unsigned int tuple_count (void) const { return 0; } + enum Coverage { Direction = 0x01u, @@ -589,6 +550,8 @@ struct KernAAT : KernTable { typedef AAT::MortTypes Types; + unsigned int tuple_count (void) const { return 0; } + enum Coverage { Direction = 0x80u, @@ -609,7 +572,8 @@ struct KernAAT : KernTable HBUINT8 coverage; /* Coverage bits. */ HBUINT8 format; /* Subtable format. */ HBUINT16 tupleIndex; /* The tuple index (used for variations fonts). - * This value specifies which tuple this subtable covers. */ + * This value specifies which tuple this subtable covers. + * Note: We don't implement. */ public: DEFINE_SIZE_STATIC (8); }; From e890753ebbf0d20c1c86796837918d530610df3b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 10:58:50 -0500 Subject: [PATCH 22/66] [morx] Minor --- src/hb-aat-layout-morx-table.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 2bc60182e..c06af4210 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -375,14 +375,14 @@ struct LigatureEntry Reserved = 0x1FFF, /* These bits are reserved and should be set to 0. */ }; - typedef struct + struct EntryData { HBUINT16 ligActionIndex; /* Index to the first ligActionTable entry * for processing this group, if indicated * by the flags. */ public: DEFINE_SIZE_STATIC (2); - } EntryData; + }; template static inline bool performAction (Flags flags) @@ -428,11 +428,11 @@ struct LigatureSubtable struct driver_context_t { + static const bool in_place = false; enum { DontAdvance = LigatureEntryT::DontAdvance, }; - static const bool in_place = false; enum LigActionFlags { LigActionLast = 0x80000000, /* This is the last action in the list. This also From ce3451dc2aad2241c148953842e696e9f53b5deb Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:02:04 -0500 Subject: [PATCH 23/66] [kerx] Towards sharing Format1 --- src/hb-aat-layout-kerx-table.hh | 61 +++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 9cdba8797..e72378233 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -156,9 +156,22 @@ struct KerxSubTableFormat0 DEFINE_SIZE_ARRAY (KernSubTableHeader::static_size + 16, pairs); }; -template -struct KerxSubTableFormat1 + +template +struct Format1Entry; + +template <> +struct Format1Entry { + enum Flags + { + Push = 0x8000, /* If set, push this glyph on the kerning stack. */ + DontAdvance = 0x4000, /* If set, don't advance to the next glyph + * before going to the new state. */ + Reset = 0x2000, /* If set, reset the kerning data (clear the stack) */ + Reserved = 0x1FFF, /* Not used; set to 0. */ + }; + struct EntryData { HBUINT16 kernActionIndex;/* Index into the kerning value array. If @@ -167,17 +180,39 @@ struct KerxSubTableFormat1 public: DEFINE_SIZE_STATIC (2); }; +}; +template <> +struct Format1Entry +{ + enum Flags + { + Push = 0x8000, /* If set, push this glyph on the kerning stack. */ + DontAdvance = 0x4000, /* If set, don't advance to the next glyph + * before going to the new state. */ + Offset = 0x3FFF, /* Byte offset from beginning of subtable to the + * value table for the glyphs on the kerning stack. */ + + Reset = 0x0000, /* Not supported? */ + }; + + typedef void EntryData; +}; + +template +struct KerxSubTableFormat1 +{ + typedef typename KernSubTableHeader::Types Types; + typedef typename Types::HBUINT HBUINT; + + typedef Format1Entry Format1EntryT; + typedef typename Format1EntryT::EntryData EntryData; struct driver_context_t { static const bool in_place = true; - enum Flags + enum { - Push = 0x8000, /* If set, push this glyph on the kerning stack. */ - DontAdvance = 0x4000, /* If set, don't advance to the next glyph - * before going to the new state. */ - Reset = 0x2000, /* If set, reset the kerning data (clear the stack) */ - Reserved = 0x1FFF, /* Not used; set to 0. */ + DontAdvance = Format1EntryT::DontAdvance, }; inline driver_context_t (const KerxSubTableFormat1 *table, @@ -202,12 +237,10 @@ struct KerxSubTableFormat1 hb_buffer_t *buffer = driver->buffer; unsigned int flags = entry->flags; - if (flags & Reset) - { + if (flags & Format1EntryT::Reset) depth = 0; - } - if (flags & Push) + if (flags & Format1EntryT::Push) { if (likely (depth < ARRAY_LENGTH (stack))) stack[depth++] = buffer->idx; @@ -339,8 +372,8 @@ struct KerxSubTableFormat1 protected: KernSubTableHeader header; - StateTable machine; - LOffsetTo, false> kernAction; + StateTable machine; + OffsetTo, HBUINT, false>kernAction; public: DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 20); }; From b693fd0dc6c7979dcacdff060ecf12a2e107071d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:05:28 -0500 Subject: [PATCH 24/66] [morx] Simplify --- src/hb-aat-layout-morx-table.hh | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index c06af4210..784b36cf4 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -384,12 +384,10 @@ struct LigatureEntry DEFINE_SIZE_STATIC (2); }; - template - static inline bool performAction (Flags flags) - { return flags & PerformAction; } + static inline bool performAction (const Entry *entry) + { return entry->flags & PerformAction; } - template - static inline unsigned int ligActionIndex (Entry &entry, Flags flags) + static inline unsigned int ligActionIndex (const Entry *entry) { return entry->data.ligActionIndex; } }; template <> @@ -408,13 +406,11 @@ struct LigatureEntry typedef void EntryData; - template - static inline bool performAction (Flags flags) - { return flags & Offset; } + static inline bool performAction (const Entry *entry) + { return entry->flags & Offset; } - template - static inline unsigned int ligActionIndex (Entry &entry, Flags flags) - { return flags & 0x3FFF; } + static inline unsigned int ligActionIndex (const Entry *entry) + { return entry->flags & 0x3FFF; } }; @@ -458,16 +454,15 @@ struct LigatureSubtable inline bool is_actionable (StateTableDriver *driver HB_UNUSED, const Entry *entry) { - return LigatureEntryT::performAction (entry->flags); + return LigatureEntryT::performAction (entry); } inline bool transition (StateTableDriver *driver, const Entry *entry) { hb_buffer_t *buffer = driver->buffer; - unsigned int flags = entry->flags; DEBUG_MSG (APPLY, nullptr, "Ligature transition at %d", buffer->idx); - if (flags & LigatureEntryT::SetComponent) + if (entry->flags & LigatureEntryT::SetComponent) { if (unlikely (match_length >= ARRAY_LENGTH (match_positions))) return false; @@ -480,11 +475,11 @@ struct LigatureSubtable DEBUG_MSG (APPLY, nullptr, "Set component at %d", buffer->out_len); } - if (LigatureEntryT::performAction (flags)) + if (LigatureEntryT::performAction (entry)) { DEBUG_MSG (APPLY, nullptr, "Perform action with %d", match_length); unsigned int end = buffer->out_len; - unsigned int action_idx = LigatureEntryT::ligActionIndex (entry, flags); + unsigned int action_idx = LigatureEntryT::ligActionIndex (entry); unsigned int action; unsigned int ligature_idx = 0; From d5c88af4a23bffc09840c43e6b1403b64a9f74d5 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:20:14 -0500 Subject: [PATCH 25/66] [kerx] More towards sharing Format1 --- src/hb-aat-layout-kerx-table.hh | 35 +++++++++++++++++++++++++++++---- src/hb-aat-layout-morx-table.hh | 10 ++++++---- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index e72378233..2a12a0b56 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -180,6 +180,12 @@ struct Format1Entry public: DEFINE_SIZE_STATIC (2); }; + + static inline bool performAction (const Entry *entry) + { return entry->data.kernActionIndex != 0xFFFF; } + + static inline unsigned int kernActionIndex (const Entry *entry) + { return entry->data.kernActionIndex; } }; template <> struct Format1Entry @@ -196,6 +202,12 @@ struct Format1Entry }; typedef void EntryData; + + static inline bool performAction (const Entry *entry) + { return entry->flags & Offset; } + + static inline unsigned int kernActionIndex (const Entry *entry) + { return entry->flags & Offset; } }; template @@ -215,9 +227,10 @@ struct KerxSubTableFormat1 DontAdvance = Format1EntryT::DontAdvance, }; - inline driver_context_t (const KerxSubTableFormat1 *table, + inline driver_context_t (const KerxSubTableFormat1 *table_, hb_aat_apply_context_t *c_) : c (c_), + table (table_), /* Apparently the offset kernAction is from the beginning of the state-machine, * similar to offsets in morx table, NOT from beginning of this table, like * other subtables in kerx. Discovered via testing. */ @@ -226,10 +239,21 @@ struct KerxSubTableFormat1 crossStream (table->header.coverage & table->header.CrossStream), crossOffset (0) {} + + /* TODO + * 'kern' table has this pecularity, we don't currently implement. + * + * "Because the stateTableOffset in the state table header is (strictly + * speaking) redundant, some 'kern' tables use it to record an initial + * state where that should not be StartOfText. To determine if this is + * done, calculate what the stateTableOffset should be. If it's different + * from the actual stateTableOffset, use it as the initial state." + */ + inline bool is_actionable (StateTableDriver *driver HB_UNUSED, const Entry *entry) { - return entry->data.kernActionIndex != 0xFFFF; + return Format1EntryT::performAction (entry); } inline bool transition (StateTableDriver *driver, const Entry *entry) @@ -248,9 +272,11 @@ struct KerxSubTableFormat1 depth = 0; /* Probably not what CoreText does, but better? */ } - if (entry->data.kernActionIndex != 0xFFFF) + if (Format1EntryT::performAction (entry)) { - const FWORD *actions = &kernAction[entry->data.kernActionIndex]; + unsigned int kern_idx = Format1EntryT::kernActionIndex (entry); + kern_idx = Types::offsetToIndex (kern_idx, &table->machine, kernAction.arrayZ); + const FWORD *actions = &kernAction[kern_idx]; if (!c->sanitizer.check_array (actions, depth)) { depth = 0; @@ -334,6 +360,7 @@ struct KerxSubTableFormat1 private: hb_aat_apply_context_t *c; + const KerxSubTableFormat1 *table; const UnsizedArrayOf &kernAction; unsigned int stack[8]; unsigned int depth; diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 784b36cf4..51f14682c 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -410,7 +410,7 @@ struct LigatureEntry { return entry->flags & Offset; } static inline unsigned int ligActionIndex (const Entry *entry) - { return entry->flags & 0x3FFF; } + { return entry->flags & Offset; } }; @@ -479,9 +479,6 @@ struct LigatureSubtable { DEBUG_MSG (APPLY, nullptr, "Perform action with %d", match_length); unsigned int end = buffer->out_len; - unsigned int action_idx = LigatureEntryT::ligActionIndex (entry); - unsigned int action; - unsigned int ligature_idx = 0; if (unlikely (!match_length)) return true; @@ -490,8 +487,13 @@ struct LigatureSubtable return false; // TODO Work on previous instead? unsigned int cursor = match_length; + + unsigned int action_idx = LigatureEntryT::ligActionIndex (entry); action_idx = Types::offsetToIndex (action_idx, table, ligAction.arrayZ); const HBUINT32 *actionData = &ligAction[action_idx]; + + unsigned int ligature_idx = 0; + unsigned int action; do { if (unlikely (!cursor)) From f5f4ca7871ec2be2b5666a7b9e6e5e28133b8393 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:21:09 -0500 Subject: [PATCH 26/66] [kern/kerx] Enable crossStream kerning in vertical CoreText doesn't, but no reason we shouldn't do. --- src/hb-aat-layout-kerx-table.hh | 8 ++++---- src/hb-ot-kern-table.hh | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 2a12a0b56..34183270d 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -334,10 +334,10 @@ struct KerxSubTableFormat1 { if (crossStream) { - /* CoreText doesn't do crossStream kerning in vertical. */ - //crossOffset += v; - //if (!buffer->pos[idx].x_offset) - // buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); + /* CoreText doesn't do crossStream kerning in vertical. We do. */ + crossOffset += v; + if (!buffer->pos[idx].x_offset) + buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); } else { diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index e065b6ff0..e205b4b36 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -161,10 +161,10 @@ struct KernSubTableFormat1 { if (crossStream) { - /* CoreText doesn't do crossStream kerning in vertical. */ - //crossOffset += v; - //if (!buffer->pos[idx].x_offset) - // buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); + /* CoreText doesn't do crossStream kerning in vertical. We do. */ + crossOffset += v; + if (!buffer->pos[idx].x_offset) + buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); } else { From 2a720911964a00ad607ff712be09ea3ea0925c9b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:25:55 -0500 Subject: [PATCH 27/66] [kerx] Minor --- src/hb-aat-layout-kerx-table.hh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 34183270d..66b8b8e6a 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -311,6 +311,7 @@ struct KerxSubTableFormat1 crossOffset = 0; v = 0; } + if (idx < buffer->len && buffer->info[idx].mask & kern_mask) { if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) @@ -402,7 +403,7 @@ struct KerxSubTableFormat1 StateTable machine; OffsetTo, HBUINT, false>kernAction; public: - DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 20); + DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 5 * sizeof (HBUINT)); }; template From a244190afa90ac253724a2ff23a3bdf0c507d0e6 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:43:25 -0500 Subject: [PATCH 28/66] [kerx] Minor --- src/hb-aat-layout-kerx-table.hh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 66b8b8e6a..c2b989c19 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -524,7 +524,6 @@ struct KerxSubTableFormat4 const Entry *entry) { hb_buffer_t *buffer = driver->buffer; - unsigned int flags = entry->flags; if (mark_set && entry->data.ankrActionIndex != 0xFFFF && buffer->idx < buffer->len) { @@ -600,7 +599,7 @@ struct KerxSubTableFormat4 buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; } - if (flags & Mark) + if (entry->flags & Mark) { mark_set = true; mark = buffer->idx; From c808e444da12840ac3ab1d78569504b9b7e876f9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:28:36 -0500 Subject: [PATCH 29/66] [kern/kerx] Share Format1 subtable --- src/hb-aat-layout-kerx-table.hh | 9 +- src/hb-ot-kern-table.hh | 187 +------------------------------- 2 files changed, 4 insertions(+), 192 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index c2b989c19..2b26d496a 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -250,12 +250,12 @@ struct KerxSubTableFormat1 * from the actual stateTableOffset, use it as the initial state." */ - inline bool is_actionable (StateTableDriver *driver HB_UNUSED, + inline bool is_actionable (StateTableDriver *driver HB_UNUSED, const Entry *entry) { return Format1EntryT::performAction (entry); } - inline bool transition (StateTableDriver *driver, + inline bool transition (StateTableDriver *driver, const Entry *entry) { hb_buffer_t *buffer = driver->buffer; @@ -376,15 +376,12 @@ struct KerxSubTableFormat1 if (!c->plan->requested_kerning) return false; - if (header.coverage & header.CrossStream) - return false; - if (header.tuple_count ()) return_trace (false); /* TODO kerxTupleKern */ driver_context_t dc (this, c); - StateTableDriver driver (machine, c->buffer, c->font->face); + StateTableDriver driver (machine, c->buffer, c->font->face); driver.drive (&dc); return_trace (true); diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index e205b4b36..6f1b46a93 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -41,191 +41,6 @@ namespace OT { -template -struct KernSubTableFormat1 -{ - typedef void EntryData; - - struct driver_context_t - { - static const bool in_place = true; - enum Flags - { - Push = 0x8000, /* If set, push this glyph on the kerning stack. */ - DontAdvance = 0x4000, /* If set, don't advance to the next glyph - * before going to the new state. */ - Offset = 0x3FFF, /* Byte offset from beginning of subtable to the - * value table for the glyphs on the kerning stack. */ - }; - - inline driver_context_t (const KernSubTableFormat1 *table_, - AAT::hb_aat_apply_context_t *c_) : - c (c_), - table (table_), - /* Apparently the offset kernAction is from the beginning of the state-machine, - * similar to offsets in morx table, NOT from beginning of this table, like - * other subtables in kerx. Discovered via testing. */ - kernAction (&table->machine + table->kernAction), - depth (0), - crossStream (table->header.coverage & table->header.CrossStream), - crossOffset (0) {} - - /* TODO - * - * "Because the stateTableOffset in the state table header is (strictly - * speaking) redundant, some 'kern' tables use it to record an initial - * state where that should not be StartOfText. To determine if this is - * done, calculate what the stateTableOffset should be. If it's different - * from the actual stateTableOffset, use it as the initial state." - */ - - inline bool is_actionable (AAT::StateTableDriver *driver HB_UNUSED, - const AAT::Entry *entry) - { - return entry->flags & Offset; - } - inline bool transition (AAT::StateTableDriver *driver, - const AAT::Entry *entry) - { - hb_buffer_t *buffer = driver->buffer; - unsigned int flags = entry->flags; - - if (flags & Push) - { - if (likely (depth < ARRAY_LENGTH (stack))) - stack[depth++] = buffer->idx; - else - depth = 0; /* Probably not what CoreText does, but better? */ - } - - if (entry->flags & Offset) - { - unsigned int kernIndex = AAT::MortTypes::offsetToIndex (entry->flags & Offset, - &table->machine, - kernAction.arrayZ); - const FWORD *actions = &kernAction[kernIndex]; - if (!c->sanitizer.check_array (actions, depth)) - { - depth = 0; - return false; - } - - hb_mask_t kern_mask = c->plan->kern_mask; - - /* "Each pops one glyph from the kerning stack and applies the kerning value to it. - * The end of the list is marked by an odd value... */ - unsigned int i; - for (i = 0; i < depth; i++) - if (actions[i] & 1) - { - i++; - break; - } - for (; i; i--) - { - unsigned int idx = stack[depth - i]; - int v = actions[i - 1]; - - /* "The end of the list is marked by an odd value..." - * Ignore it. */ - v &= ~1; - - /* The following flag is undocumented in the spec, but described - * in the example. */ - if (v == -0x8000) - { - crossOffset = 0; - v = 0; - } - - if (idx < buffer->len && buffer->info[idx].mask & kern_mask) - { - if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) - { - if (crossStream) - { - crossOffset += v; - if (!buffer->pos[idx].y_offset) - buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); - } - else - { - if (!buffer->pos[idx].x_offset) - { - buffer->pos[idx].x_advance += c->font->em_scale_x (v); - buffer->pos[idx].x_offset += c->font->em_scale_x (v); - } - } - } - else - { - if (crossStream) - { - /* CoreText doesn't do crossStream kerning in vertical. We do. */ - crossOffset += v; - if (!buffer->pos[idx].x_offset) - buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); - } - else - { - if (!buffer->pos[idx].y_offset) - { - buffer->pos[idx].y_advance += c->font->em_scale_y (v); - buffer->pos[idx].y_offset += c->font->em_scale_y (v); - } - } - } - } - } - depth = 0; - } - else - buffer->pos[buffer->idx].y_offset += c->font->em_scale_y (crossOffset); - - return true; - } - - private: - AAT::hb_aat_apply_context_t *c; - const KernSubTableFormat1 *table; - const UnsizedArrayOf &kernAction; - unsigned int stack[8]; - unsigned int depth; - bool crossStream; - int crossOffset; - }; - - inline bool apply (AAT::hb_aat_apply_context_t *c) const - { - TRACE_APPLY (this); - - if (!c->plan->requested_kerning) - return false; - - driver_context_t dc (this, c); - - AAT::StateTableDriver driver (machine, c->buffer, c->font->face); - driver.drive (&dc); - - return_trace (true); - } - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - /* The rest of array sanitizations are done at run-time. */ - return_trace (likely (c->check_struct (this) && - machine.sanitize (c))); - } - - protected: - KernSubTableHeader header; - AAT::StateTable machine; - OffsetTo, HBUINT16, false> kernAction; - public: - DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 10); -}; - template struct KernSubTableFormat2 { @@ -405,7 +220,7 @@ struct KernSubTable union { KernSubTableHeader header; AAT::KerxSubTableFormat0 format0; - KernSubTableFormat1 format1; + AAT::KerxSubTableFormat1 format1; KernSubTableFormat2 format2; KernSubTableFormat3 format3; } u; From 241ba7da518adee334fff105ae19dfb051868a57 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:51:40 -0500 Subject: [PATCH 30/66] [morx/kerx] Rename types --- src/hb-aat-layout-common.hh | 4 ++-- src/hb-aat-layout-kerx-table.hh | 17 +++++++++-------- src/hb-aat-layout-morx-table.hh | 4 ++-- src/hb-ot-kern-table.hh | 4 ++-- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 539941d85..cede80c87 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -546,7 +546,7 @@ struct ClassTable DEFINE_SIZE_ARRAY (4, classArray); }; -struct MortTypes +struct ObsoleteTypes { static const bool extended = false; typedef HBUINT16 HBUINT; @@ -573,7 +573,7 @@ struct MortTypes return offsetToIndex (2 * offset, base, array); } }; -struct MorxTypes +struct ExtendedTypes { static const bool extended = true; typedef HBUINT32 HBUINT; diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 2b26d496a..6469e5cf1 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -476,6 +476,8 @@ struct KerxSubTableFormat2 template struct KerxSubTableFormat4 { + typedef ExtendedTypes Types; + struct EntryData { HBUINT16 ankrActionIndex;/* Either 0xFFFF (for no action) or the index of @@ -512,12 +514,12 @@ struct KerxSubTableFormat4 mark_set (false), mark (0) {} - inline bool is_actionable (StateTableDriver *driver HB_UNUSED, + inline bool is_actionable (StateTableDriver *driver HB_UNUSED, const Entry *entry) { return entry->data.ankrActionIndex != 0xFFFF; } - inline bool transition (StateTableDriver *driver, + inline bool transition (StateTableDriver *driver, const Entry *entry) { hb_buffer_t *buffer = driver->buffer; @@ -619,7 +621,7 @@ struct KerxSubTableFormat4 driver_context_t dc (this, c); - StateTableDriver driver (machine, c->buffer, c->font->face); + StateTableDriver driver (machine, c->buffer, c->font->face); driver.drive (&dc); return_trace (true); @@ -634,10 +636,9 @@ struct KerxSubTableFormat4 } protected: - KernSubTableHeader header; - StateTable - machine; - HBUINT32 flags; + KernSubTableHeader header; + StateTable machine; + HBUINT32 flags; public: DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 20); }; @@ -756,7 +757,7 @@ struct KerxSubTableFormat6 struct KerxSubTableHeader { - typedef MorxTypes Types; + typedef ExtendedTypes Types; unsigned int tuple_count (void) const { return tupleCount; } diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 51f14682c..a364f7ac7 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -1166,11 +1166,11 @@ struct mortmorx DEFINE_SIZE_MIN (8); }; -struct morx : mortmorx +struct morx : mortmorx { static const hb_tag_t tableTag = HB_AAT_TAG_morx; }; -struct mort : mortmorx +struct mort : mortmorx { static const hb_tag_t tableTag = HB_AAT_TAG_mort; }; diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 6f1b46a93..3f771d2d5 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -316,7 +316,7 @@ struct KernOT : KernTable struct SubTableHeader { - typedef AAT::MortTypes Types; + typedef AAT::ObsoleteTypes Types; unsigned int tuple_count (void) const { return 0; } @@ -363,7 +363,7 @@ struct KernAAT : KernTable struct SubTableHeader { - typedef AAT::MortTypes Types; + typedef AAT::ObsoleteTypes Types; unsigned int tuple_count (void) const { return 0; } From e72e041c3cda164b2ffb02d770b35d0d70954818 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 11:56:36 -0500 Subject: [PATCH 31/66] [kerx] Rename --- src/hb-aat-layout-kerx-table.hh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 6469e5cf1..9e1105dec 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -789,7 +789,7 @@ struct KerxSubTableHeader DEFINE_SIZE_STATIC (12); }; -struct KerxTable +struct KerxSubTable { friend struct kerx; @@ -848,17 +848,17 @@ struct kerx inline void apply (hb_aat_apply_context_t *c) const { c->set_lookup_index (0); - const KerxTable *table = &firstTable; + const KerxSubTable *st = &firstTable; unsigned int count = tableCount; for (unsigned int i = 0; i < count; i++) { bool reverse; if (HB_DIRECTION_IS_VERTICAL (c->buffer->props.direction) != - bool (table->u.header.coverage & table->u.header.Vertical)) + bool (st->u.header.coverage & st->u.header.Vertical)) goto skip; - reverse = bool (table->u.header.coverage & table->u.header.Backwards) != + reverse = bool (st->u.header.coverage & st->u.header.Backwards) != HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); if (!c->buffer->message (c->font, "start kerx subtable %d", c->lookup_index)) @@ -867,13 +867,13 @@ struct kerx if (reverse) c->buffer->reverse (); - c->sanitizer.set_object (*table); + c->sanitizer.set_object (*st); /* XXX Reverse-kern is not working yet... * hb_kern_machine_t would need to know that it's reverse-kerning. * Or better yet, make it work in reverse as well, so we don't have * to reverse and reverse back? */ - table->dispatch (c); + st->dispatch (c); if (reverse) c->buffer->reverse (); @@ -881,7 +881,7 @@ struct kerx (void) c->buffer->message (c->font, "end kerx subtable %d", c->lookup_index); skip: - table = &StructAfter (*table); + st = &StructAfter (*st); c->set_lookup_index (c->lookup_index + 1); } } @@ -893,13 +893,13 @@ struct kerx !tableCount.sanitize (c)) return_trace (false); - const KerxTable *table = &firstTable; + const KerxSubTable *st = &firstTable; unsigned int count = tableCount; for (unsigned int i = 0; i < count; i++) { - if (!table->sanitize (c)) + if (!st->sanitize (c)) return_trace (false); - table = &StructAfter (*table); + st = &StructAfter (*st); } return_trace (true); @@ -911,7 +911,7 @@ struct kerx HBUINT16 unused; /* Set to 0. */ HBUINT32 tableCount; /* The number of subtables included in the extended kerning * table. */ - KerxTable firstTable; /* Subtables. */ + KerxSubTable firstTable; /* Subtables. */ /*subtableGlyphCoverageArray*/ /* Only if version >= 3. We don't use. */ public: From d5c0ca210fef315fd039e5b1825a865f36606a3f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:08:44 -0500 Subject: [PATCH 32/66] [aat] Minor --- src/hb-aat-layout-common.hh | 16 ++++++++++------ src/hb-ot-kern-table.hh | 4 ++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index cede80c87..34c61e93c 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -436,7 +436,7 @@ struct StateTable inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int num_glyphs) const { if (unlikely (glyph_id == DELETED_GLYPH)) return CLASS_DELETED_GLYPH; - return (this+classTable).get_class (glyph_id, num_glyphs); + return (this+classTable).get_class (glyph_id, num_glyphs, 1); } inline const Entry *get_entries () const @@ -528,7 +528,7 @@ struct StateTable struct ClassTable { - inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int outOfRange=0) const + inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int outOfRange) const { unsigned int i = glyph_id - firstGlyph; return i >= classArray.len ? outOfRange : classArray.arrayZ[i]; @@ -553,9 +553,11 @@ struct ObsoleteTypes typedef HBUINT8 HBUSHORT; struct ClassType : ClassTable { - inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int num_glyphs HB_UNUSED) const + inline unsigned int get_class (hb_codepoint_t glyph_id, + unsigned int num_glyphs HB_UNUSED, + unsigned int outOfRange) const { - return ClassTable::get_class (glyph_id, 1); + return ClassTable::get_class (glyph_id, outOfRange); } }; template @@ -580,10 +582,12 @@ struct ExtendedTypes typedef HBUINT16 HBUSHORT; struct ClassType : Lookup { - inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int num_glyphs) const + inline unsigned int get_class (hb_codepoint_t glyph_id, + unsigned int num_glyphs, + unsigned int outOfRange) const { const HBUINT16 *v = get_value (glyph_id, num_glyphs); - return v ? *v : 1; + return v ? *v : outOfRange; } }; template diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 3f771d2d5..870d80013 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -51,8 +51,8 @@ struct KernSubTableFormat2 * different ClassTable. OT's has 16bit entries, while AAT has 8bit entries. * I've not seen any in the wild. */ return 0; - unsigned int l = (this+leftClassTable).get_class (left); - unsigned int r = (this+rightClassTable).get_class (right); + unsigned int l = (this+leftClassTable).get_class (left, 0); + unsigned int r = (this+rightClassTable).get_class (right, 0); unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+array), offset); if (unlikely (!v->sanitize (&c->sanitizer))) return 0; From 8faec4e33486616fdc0d690ad80d4a38a73c8182 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:16:38 -0500 Subject: [PATCH 33/66] [kerx] Towards merging Format2 --- src/hb-aat-layout-kerx-table.hh | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 9e1105dec..418412557 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -406,12 +406,15 @@ struct KerxSubTableFormat1 template struct KerxSubTableFormat2 { + typedef typename KernSubTableHeader::Types Types; + typedef typename Types::HBUINT HBUINT; + inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, hb_aat_apply_context_t *c) const { unsigned int num_glyphs = c->sanitizer.get_num_glyphs (); - unsigned int l = (this+leftClassTable).get_value_or_null (left, num_glyphs); - unsigned int r = (this+rightClassTable).get_value_or_null (right, num_glyphs); + unsigned int l = (this+leftClassTable).get_class (left, num_glyphs, 0); + unsigned int r = (this+rightClassTable).get_class (right, num_glyphs, 0); unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+array), offset); if (unlikely (!v->sanitize (&c->sanitizer))) return 0; @@ -459,18 +462,18 @@ struct KerxSubTableFormat2 protected: KernSubTableHeader header; - HBUINT32 rowWidth; /* The width, in bytes, of a row in the table. */ - LOffsetTo, false> + HBUINT rowWidth; /* The width, in bytes, of a row in the table. */ + OffsetTo leftClassTable; /* Offset from beginning of this subtable to * left-hand class table. */ - LOffsetTo, false> + OffsetTo rightClassTable;/* Offset from beginning of this subtable to * right-hand class table. */ - LOffsetTo, false> + OffsetTo, HBUINT, false> array; /* Offset from beginning of this subtable to * the start of the kerning array. */ public: - DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 16); + DEFINE_SIZE_STATIC (KernSubTableHeader::static_size + 4 * sizeof (HBUINT)); }; template From 1a5ef8490034f4bd8965a3c71d34a5930ebe11b7 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:19:52 -0500 Subject: [PATCH 34/66] [kern/kerx] Share Format2 This, enables Format2 for kern table, which was disabled before. --- src/hb-aat-layout-kerx-table.hh | 6 +++ src/hb-ot-kern-table.hh | 73 +-------------------------------- 2 files changed, 7 insertions(+), 72 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 418412557..d55d5f7a5 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -460,6 +460,12 @@ struct KerxSubTableFormat2 c->check_range (this, array))); } + /* Note: + * OT kern table specifies ClassTable as having 16-bit entries, whereas + * AAT kern table specifies them as having 8bit entries. + * I've not seen any fonts with this format in kern table. + * We follow AAT. */ + protected: KernSubTableHeader header; HBUINT rowWidth; /* The width, in bytes, of a row in the table. */ diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 870d80013..27829d84f 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -41,77 +41,6 @@ namespace OT { -template -struct KernSubTableFormat2 -{ - inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, - AAT::hb_aat_apply_context_t *c) const - { - /* Disabled until we find a font to test this. Note that OT vs AAT specify - * different ClassTable. OT's has 16bit entries, while AAT has 8bit entries. - * I've not seen any in the wild. */ - return 0; - unsigned int l = (this+leftClassTable).get_class (left, 0); - unsigned int r = (this+rightClassTable).get_class (right, 0); - unsigned int offset = l + r; - const FWORD *v = &StructAtOffset (&(this+array), offset); - if (unlikely (!v->sanitize (&c->sanitizer))) return 0; - return *v; - } - - inline bool apply (AAT::hb_aat_apply_context_t *c) const - { - TRACE_APPLY (this); - - if (!c->plan->requested_kerning) - return false; - - if (header.coverage & header.CrossStream) - return false; - - accelerator_t accel (*this, c); - hb_kern_machine_t machine (accel); - machine.kern (c->font, c->buffer, c->plan->kern_mask); - - return_trace (true); - } - - struct accelerator_t - { - const KernSubTableFormat2 &table; - AAT::hb_aat_apply_context_t *c; - - inline accelerator_t (const KernSubTableFormat2 &table_, - AAT::hb_aat_apply_context_t *c_) : - table (table_), c (c_) {} - - inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const - { return table.get_kerning (left, right, c); } - }; - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - return_trace (true); /* Disabled. See above. */ - return_trace (c->check_struct (this) && - leftClassTable.sanitize (c, this) && - rightClassTable.sanitize (c, this) && - array.sanitize (c, this)); - } - - protected: - KernSubTableHeader header; - HBUINT16 rowWidth; /* The width, in bytes, of a row in the table. */ - OffsetTo leftClassTable; /* Offset from beginning of this subtable to - * left-hand class table. */ - OffsetTo rightClassTable;/* Offset from beginning of this subtable to - * right-hand class table. */ - OffsetTo array; /* Offset from beginning of this subtable to - * the start of the kerning array. */ - public: - DEFINE_SIZE_MIN (KernSubTableHeader::static_size + 8); -}; - template struct KernSubTableFormat3 { @@ -221,7 +150,7 @@ struct KernSubTable KernSubTableHeader header; AAT::KerxSubTableFormat0 format0; AAT::KerxSubTableFormat1 format1; - KernSubTableFormat2 format2; + AAT::KerxSubTableFormat2 format2; KernSubTableFormat3 format3; } u; public: From 330508497d301c0ba5d5fb5d0900b62c191aabb5 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:27:44 -0500 Subject: [PATCH 35/66] [kern/kerx] Minor --- src/hb-aat-layout-kerx-table.hh | 2 +- src/hb-ot-kern-table.hh | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index d55d5f7a5..a395e76ab 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -768,7 +768,7 @@ struct KerxSubTableHeader { typedef ExtendedTypes Types; - unsigned int tuple_count (void) const { return tupleCount; } + inline unsigned int tuple_count (void) const { return tupleCount; } enum Coverage { diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 27829d84f..f8b1aa7a5 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -247,7 +247,8 @@ struct KernOT : KernTable { typedef AAT::ObsoleteTypes Types; - unsigned int tuple_count (void) const { return 0; } + inline unsigned int tuple_count (void) const { return 0; } + enum Coverage { @@ -294,7 +295,7 @@ struct KernAAT : KernTable { typedef AAT::ObsoleteTypes Types; - unsigned int tuple_count (void) const { return 0; } + inline unsigned int tuple_count (void) const { return 0; } enum Coverage { From f5e0a63a22f91720a997f5070b84e982e57de661 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:32:39 -0500 Subject: [PATCH 36/66] [kern/kerx] Towards sharing KernTable --- src/hb-aat-layout-kerx-table.hh | 4 ++-- src/hb-ot-kern-table.hh | 19 +++++++------------ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index a395e76ab..9a221bc5a 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -769,6 +769,7 @@ struct KerxSubTableHeader typedef ExtendedTypes Types; inline unsigned int tuple_count (void) const { return tupleCount; } + inline bool is_horizontal (void) const { return !(coverage & Vertical); } enum Coverage { @@ -863,8 +864,7 @@ struct kerx { bool reverse; - if (HB_DIRECTION_IS_VERTICAL (c->buffer->props.direction) != - bool (st->u.header.coverage & st->u.header.Vertical)) + if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != st->u.header.is_horizontal ()) goto skip; reverse = bool (st->u.header.coverage & st->u.header.Backwards) != diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index f8b1aa7a5..89da9e36f 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -173,9 +173,8 @@ struct KernTable unsigned int count = thiz()->nTables; for (unsigned int i = 0; i < count; i++) { - if ((st->u.header.coverage & - (st->u.header.Variation | st->u.header.CrossStream | st->u.header.Direction)) != - st->u.header.DirectionHorizontal) + if ((st->u.header.coverage & (st->u.header.Variation | st->u.header.CrossStream)) || + !st->u.header.is_horizontal ()) continue; v += st->get_kerning (left, right); st = &StructAfter (*st); @@ -196,8 +195,7 @@ struct KernTable if (st->u.header.coverage & st->u.header.Variation) goto skip; - if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != - ((st->u.header.coverage & st->u.header.Direction) == st->u.header.DirectionHorizontal)) + if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != st->u.header.is_horizontal ()) goto skip; if (!c->buffer->message (c->font, "start kern subtable %d", c->lookup_index)) @@ -248,18 +246,16 @@ struct KernOT : KernTable typedef AAT::ObsoleteTypes Types; inline unsigned int tuple_count (void) const { return 0; } - + inline bool is_horizontal (void) const { return (coverage & Horizontal); } enum Coverage { - Direction = 0x01u, + Horizontal = 0x01u, Minimum = 0x02u, CrossStream = 0x04u, Override = 0x08u, Variation = 0x00u, /* Not supported. */ - - DirectionHorizontal= 0x01u }; inline bool sanitize (hb_sanitize_context_t *c) const @@ -296,14 +292,13 @@ struct KernAAT : KernTable typedef AAT::ObsoleteTypes Types; inline unsigned int tuple_count (void) const { return 0; } + inline bool is_horizontal (void) const { return !(coverage & Vertical); } enum Coverage { - Direction = 0x80u, + Vertical = 0x80u, CrossStream = 0x40u, Variation = 0x20u, - - DirectionHorizontal= 0x00u }; inline bool sanitize (hb_sanitize_context_t *c) const From f8c3df7d4a685bb86a1c15a5ef95485e8ef30305 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:48:06 -0500 Subject: [PATCH 37/66] [kern/kerx] Minor --- src/hb-aat-layout-kerx-table.hh | 5 +++-- src/hb-ot-kern-table.hh | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 9a221bc5a..636380260 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -852,6 +852,7 @@ public: struct kerx { static const hb_tag_t tableTag = HB_AAT_TAG_kerx; + static const uint16_t minVersion = 2; inline bool has_data (void) const { return version != 0; } @@ -898,8 +899,7 @@ struct kerx inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - if (!version.sanitize (c) || version < 2 || - !tableCount.sanitize (c)) + if (!version.sanitize (c) || version < minVersion || !tableCount.sanitize (c)) return_trace (false); const KerxSubTable *st = &firstTable; @@ -921,6 +921,7 @@ struct kerx HBUINT32 tableCount; /* The number of subtables included in the extended kerning * table. */ KerxSubTable firstTable; /* Subtables. */ + UnsizedArrayOf dataZ; /*subtableGlyphCoverageArray*/ /* Only if version >= 3. We don't use. */ public: diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 89da9e36f..538451195 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -170,7 +170,7 @@ struct KernTable int v = 0; const SubTable *st = CastP (&thiz()->dataZ); - unsigned int count = thiz()->nTables; + unsigned int count = thiz()->tableCount; for (unsigned int i = 0; i < count; i++) { if ((st->u.header.coverage & (st->u.header.Variation | st->u.header.CrossStream)) || @@ -188,7 +188,7 @@ struct KernTable c->set_lookup_index (0); const SubTable *st = CastP (&thiz()->dataZ); - unsigned int count = thiz()->nTables; + unsigned int count = thiz()->tableCount; st = CastP (&thiz()->dataZ); for (unsigned int i = 0; i < count; i++) { @@ -216,14 +216,15 @@ struct KernTable inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - if (unlikely (!c->check_struct (thiz()) || - thiz()->version != T::VERSION)) + if (unlikely (!thiz()->version.sanitize (c) || + thiz()->version < T::minVersion || + !thiz()->tableCount.sanitize (c))) return_trace (false); typedef KernSubTable SubTable; const SubTable *st = CastP (&thiz()->dataZ); - unsigned int count = thiz()->nTables; + unsigned int count = thiz()->tableCount; for (unsigned int i = 0; i < count; i++) { if (unlikely (!st->sanitize (c))) @@ -239,7 +240,7 @@ struct KernOT : KernTable { friend struct KernTable; - static const uint16_t VERSION = 0x0000u; + static const uint16_t minVersion = 0; struct SubTableHeader { @@ -275,7 +276,7 @@ struct KernOT : KernTable protected: HBUINT16 version; /* Version--0x0000u */ - HBUINT16 nTables; /* Number of subtables in the kerning table. */ + HBUINT16 tableCount; /* Number of subtables in the kerning table. */ UnsizedArrayOf dataZ; public: DEFINE_SIZE_ARRAY (4, dataZ); @@ -285,7 +286,7 @@ struct KernAAT : KernTable { friend struct KernTable; - static const uint32_t VERSION = 0x00010000u; + static const uint32_t minVersion = 0x00010000u; struct SubTableHeader { @@ -320,7 +321,7 @@ struct KernAAT : KernTable protected: HBUINT32 version; /* Version--0x00010000u */ - HBUINT32 nTables; /* Number of subtables in the kerning table. */ + HBUINT32 tableCount; /* Number of subtables in the kerning table. */ UnsizedArrayOf dataZ; public: DEFINE_SIZE_ARRAY (8, dataZ); From 8e9f6cd0fddd572e048487aae3141d3dbb1b99cb Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:49:20 -0500 Subject: [PATCH 38/66] [kerx] More minor --- src/hb-aat-layout-kerx-table.hh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 636380260..2bbd11aac 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -859,7 +859,7 @@ struct kerx inline void apply (hb_aat_apply_context_t *c) const { c->set_lookup_index (0); - const KerxSubTable *st = &firstTable; + const KerxSubTable *st = &firstSubTable; unsigned int count = tableCount; for (unsigned int i = 0; i < count; i++) { @@ -902,7 +902,7 @@ struct kerx if (!version.sanitize (c) || version < minVersion || !tableCount.sanitize (c)) return_trace (false); - const KerxSubTable *st = &firstTable; + const KerxSubTable *st = &firstSubTable; unsigned int count = tableCount; for (unsigned int i = 0; i < count; i++) { @@ -920,8 +920,7 @@ struct kerx HBUINT16 unused; /* Set to 0. */ HBUINT32 tableCount; /* The number of subtables included in the extended kerning * table. */ - KerxSubTable firstTable; /* Subtables. */ - UnsizedArrayOf dataZ; + KerxSubTable firstSubTable; /* Subtables. */ /*subtableGlyphCoverageArray*/ /* Only if version >= 3. We don't use. */ public: From 1ff300464a1075b8cd5311970afbbcf4bb3b6f3d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:51:49 -0500 Subject: [PATCH 39/66] [kern] Massage more --- src/hb-ot-kern-table.hh | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 538451195..461a2448a 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -169,7 +169,7 @@ struct KernTable typedef KernSubTable SubTable; int v = 0; - const SubTable *st = CastP (&thiz()->dataZ); + const SubTable *st = &thiz()->firstSubTable; unsigned int count = thiz()->tableCount; for (unsigned int i = 0; i < count; i++) { @@ -187,9 +187,8 @@ struct KernTable typedef KernSubTable SubTable; c->set_lookup_index (0); - const SubTable *st = CastP (&thiz()->dataZ); + const SubTable *st = &thiz()->firstSubTable; unsigned int count = thiz()->tableCount; - st = CastP (&thiz()->dataZ); for (unsigned int i = 0; i < count; i++) { if (st->u.header.coverage & st->u.header.Variation) @@ -223,7 +222,7 @@ struct KernTable typedef KernSubTable SubTable; - const SubTable *st = CastP (&thiz()->dataZ); + const SubTable *st = &thiz()->firstSubTable; unsigned int count = thiz()->tableCount; for (unsigned int i = 0; i < count; i++) { @@ -277,9 +276,9 @@ struct KernOT : KernTable protected: HBUINT16 version; /* Version--0x0000u */ HBUINT16 tableCount; /* Number of subtables in the kerning table. */ - UnsizedArrayOf dataZ; + KernSubTable firstSubTable; /* Subtables. */ public: - DEFINE_SIZE_ARRAY (4, dataZ); + DEFINE_SIZE_MIN (4); }; struct KernAAT : KernTable @@ -322,9 +321,9 @@ struct KernAAT : KernTable protected: HBUINT32 version; /* Version--0x00010000u */ HBUINT32 tableCount; /* Number of subtables in the kerning table. */ - UnsizedArrayOf dataZ; + KernSubTable firstSubTable; /* Subtables. */ public: - DEFINE_SIZE_ARRAY (8, dataZ); + DEFINE_SIZE_MIN (8); }; struct kern From 30af5b4a4c2071599dc87bc092a7329befcc45cc Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 12:57:10 -0500 Subject: [PATCH 40/66] [kern] Move code --- src/hb-ot-kern-table.hh | 132 +++++++++++++++++++++------------------- 1 file changed, 69 insertions(+), 63 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 461a2448a..5046a0da5 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -235,44 +235,47 @@ struct KernTable } }; + +struct KernOTSubTableHeader +{ + typedef AAT::ObsoleteTypes Types; + + inline unsigned int tuple_count (void) const { return 0; } + inline bool is_horizontal (void) const { return (coverage & Horizontal); } + + enum Coverage + { + Horizontal = 0x01u, + Minimum = 0x02u, + CrossStream = 0x04u, + Override = 0x08u, + + Variation = 0x00u, /* Not supported. */ + }; + + inline bool sanitize (hb_sanitize_context_t *c) const + { + TRACE_SANITIZE (this); + return_trace (c->check_struct (this)); + } + + public: + HBUINT16 versionZ; /* Unused. */ + HBUINT16 length; /* Length of the subtable (including this header). */ + HBUINT8 format; /* Subtable format. */ + HBUINT8 coverage; /* Coverage bits. */ + public: + DEFINE_SIZE_STATIC (6); +}; + struct KernOT : KernTable { friend struct KernTable; + typedef KernOTSubTableHeader SubTableHeader; + static const uint16_t minVersion = 0; - struct SubTableHeader - { - typedef AAT::ObsoleteTypes Types; - - inline unsigned int tuple_count (void) const { return 0; } - inline bool is_horizontal (void) const { return (coverage & Horizontal); } - - enum Coverage - { - Horizontal = 0x01u, - Minimum = 0x02u, - CrossStream = 0x04u, - Override = 0x08u, - - Variation = 0x00u, /* Not supported. */ - }; - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - return_trace (c->check_struct (this)); - } - - public: - HBUINT16 versionZ; /* Unused. */ - HBUINT16 length; /* Length of the subtable (including this header). */ - HBUINT8 format; /* Subtable format. */ - HBUINT8 coverage; /* Coverage bits. */ - public: - DEFINE_SIZE_STATIC (6); - }; - protected: HBUINT16 version; /* Version--0x0000u */ HBUINT16 tableCount; /* Number of subtables in the kerning table. */ @@ -281,43 +284,46 @@ struct KernOT : KernTable DEFINE_SIZE_MIN (4); }; + +struct KernAATSubTableHeader +{ + typedef AAT::ObsoleteTypes Types; + + inline unsigned int tuple_count (void) const { return 0; } + inline bool is_horizontal (void) const { return !(coverage & Vertical); } + + enum Coverage + { + Vertical = 0x80u, + CrossStream = 0x40u, + Variation = 0x20u, + }; + + inline bool sanitize (hb_sanitize_context_t *c) const + { + TRACE_SANITIZE (this); + return_trace (c->check_struct (this)); + } + + public: + HBUINT32 length; /* Length of the subtable (including this header). */ + HBUINT8 coverage; /* Coverage bits. */ + HBUINT8 format; /* Subtable format. */ + HBUINT16 tupleIndex; /* The tuple index (used for variations fonts). + * This value specifies which tuple this subtable covers. + * Note: We don't implement. */ + public: + DEFINE_SIZE_STATIC (8); +}; + struct KernAAT : KernTable { friend struct KernTable; + typedef KernAATSubTableHeader SubTableHeader; + static const uint32_t minVersion = 0x00010000u; - struct SubTableHeader - { - typedef AAT::ObsoleteTypes Types; - - inline unsigned int tuple_count (void) const { return 0; } - inline bool is_horizontal (void) const { return !(coverage & Vertical); } - - enum Coverage - { - Vertical = 0x80u, - CrossStream = 0x40u, - Variation = 0x20u, - }; - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - return_trace (c->check_struct (this)); - } - - public: - HBUINT32 length; /* Length of the subtable (including this header). */ - HBUINT8 coverage; /* Coverage bits. */ - HBUINT8 format; /* Subtable format. */ - HBUINT16 tupleIndex; /* The tuple index (used for variations fonts). - * This value specifies which tuple this subtable covers. - * Note: We don't implement. */ - public: - DEFINE_SIZE_STATIC (8); - }; - protected: HBUINT32 version; /* Version--0x00010000u */ HBUINT32 tableCount; /* Number of subtables in the kerning table. */ From ab57bcae0fd4505c80bb4ccdef6838bb2805ce79 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 13:04:21 -0500 Subject: [PATCH 41/66] [kern] Minor --- src/hb-ot-kern-table.hh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 5046a0da5..693c4f29f 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -166,7 +166,7 @@ struct KernTable inline int get_h_kerning (hb_codepoint_t left, hb_codepoint_t right) const { - typedef KernSubTable SubTable; + typedef typename T::SubTable SubTable; int v = 0; const SubTable *st = &thiz()->firstSubTable; @@ -184,7 +184,7 @@ struct KernTable inline void apply (AAT::hb_aat_apply_context_t *c) const { - typedef KernSubTable SubTable; + typedef typename T::SubTable SubTable; c->set_lookup_index (0); const SubTable *st = &thiz()->firstSubTable; @@ -220,7 +220,7 @@ struct KernTable !thiz()->tableCount.sanitize (c))) return_trace (false); - typedef KernSubTable SubTable; + typedef typename T::SubTable SubTable; const SubTable *st = &thiz()->firstSubTable; unsigned int count = thiz()->tableCount; @@ -273,6 +273,7 @@ struct KernOT : KernTable friend struct KernTable; typedef KernOTSubTableHeader SubTableHeader; + typedef KernSubTable SubTable; static const uint16_t minVersion = 0; @@ -321,6 +322,7 @@ struct KernAAT : KernTable friend struct KernTable; typedef KernAATSubTableHeader SubTableHeader; + typedef KernSubTable SubTable; static const uint32_t minVersion = 0x00010000u; From 89ec095979bde94bd203ed2c394f6e40629e9e78 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 13:10:05 -0500 Subject: [PATCH 42/66] [kern] Disable Format1 and Format3 for OT-style tables --- src/hb-ot-kern-table.hh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 693c4f29f..0102bb2c9 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -128,9 +128,9 @@ struct KernSubTable TRACE_DISPATCH (this, subtable_type); switch (subtable_type) { case 0: return_trace (c->dispatch (u.format0)); - case 1: return_trace (c->dispatch (u.format1)); + case 1: return_trace (u.header.apple ? c->dispatch (u.format1) : c->default_return_value ()); case 2: return_trace (c->dispatch (u.format2)); - case 3: return_trace (c->dispatch (u.format3)); + case 3: return_trace (u.header.apple ? c->dispatch (u.format3) : c->default_return_value ()); default: return_trace (c->default_return_value ()); } } @@ -238,6 +238,7 @@ struct KernTable struct KernOTSubTableHeader { + static const bool apple = false; typedef AAT::ObsoleteTypes Types; inline unsigned int tuple_count (void) const { return 0; } @@ -288,6 +289,7 @@ struct KernOT : KernTable struct KernAATSubTableHeader { + static const bool apple = true; typedef AAT::ObsoleteTypes Types; inline unsigned int tuple_count (void) const { return 0; } From db6e658e8c0c4953c2f026f6a67a5d2fb4bdc204 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 13:33:23 -0500 Subject: [PATCH 43/66] [kern/kerx] More towards sharing KernTable --- src/hb-aat-layout-kerx-table.hh | 48 +++++++++++++------------ src/hb-ot-kern-table.hh | 63 ++++++++++++++++++++++----------- 2 files changed, 69 insertions(+), 42 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 2bbd11aac..3a76f0fa4 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -773,16 +773,16 @@ struct KerxSubTableHeader enum Coverage { - Vertical = 0x80000000, /* Set if table has vertical kerning values. */ - CrossStream = 0x40000000, /* Set if table has cross-stream kerning values. */ - Variation = 0x20000000, /* Set if table has variation kerning values. */ - Backwards = 0x10000000, /* If clear, process the glyphs forwards, that - * is, from first to last in the glyph stream. - * If we, process them from last to first. - * This flag only applies to state-table based - * 'kerx' subtables (types 1 and 4). */ - Reserved = 0x0FFFFF00, /* Reserved, set to zero. */ - SubtableType = 0x000000FF, /* Subtable type. */ + Vertical = 0x80000000u, /* Set if table has vertical kerning values. */ + CrossStream = 0x40000000u, /* Set if table has cross-stream kerning values. */ + Variation = 0x20000000u, /* Set if table has variation kerning values. */ + Backwards = 0x10000000u, /* If clear, process the glyphs forwards, that + * is, from first to last in the glyph stream. + * If we, process them from last to first. + * This flag only applies to state-table based + * 'kerx' subtables (types 1 and 4). */ + Reserved = 0x0FFFFF00u, /* Reserved, set to zero. */ + SubtableType= 0x000000FFu, /* Subtable type. */ }; inline bool sanitize (hb_sanitize_context_t *c) const @@ -854,12 +854,16 @@ struct kerx static const hb_tag_t tableTag = HB_AAT_TAG_kerx; static const uint16_t minVersion = 2; + typedef KerxSubTableHeader SubTableHeader; + typedef SubTableHeader::Types Types; + typedef KerxSubTable SubTable; + inline bool has_data (void) const { return version != 0; } inline void apply (hb_aat_apply_context_t *c) const { c->set_lookup_index (0); - const KerxSubTable *st = &firstSubTable; + const SubTable *st = &firstSubTable; unsigned int count = tableCount; for (unsigned int i = 0; i < count; i++) { @@ -871,7 +875,7 @@ struct kerx reverse = bool (st->u.header.coverage & st->u.header.Backwards) != HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); - if (!c->buffer->message (c->font, "start kerx subtable %d", c->lookup_index)) + if (!c->buffer->message (c->font, "start %c%c%c%c subtable %d", HB_UNTAG (tableTag), c->lookup_index)) goto skip; if (reverse) @@ -879,19 +883,17 @@ struct kerx c->sanitizer.set_object (*st); - /* XXX Reverse-kern is not working yet... - * hb_kern_machine_t would need to know that it's reverse-kerning. - * Or better yet, make it work in reverse as well, so we don't have - * to reverse and reverse back? */ + /* XXX Reverse-kern is probably not working yet... + * hb_kern_machine_t would need to know that it's reverse-kerning. */ st->dispatch (c); if (reverse) c->buffer->reverse (); - (void) c->buffer->message (c->font, "end kerx subtable %d", c->lookup_index); + (void) c->buffer->message (c->font, "end %c%c%c%c subtable %d", HB_UNTAG (tableTag), c->lookup_index); skip: - st = &StructAfter (*st); + st = &StructAfter (*st); c->set_lookup_index (c->lookup_index + 1); } } @@ -899,16 +901,18 @@ struct kerx inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - if (!version.sanitize (c) || version < minVersion || !tableCount.sanitize (c)) + if (unlikely (!version.sanitize (c) || + version < minVersion || + !tableCount.sanitize (c))) return_trace (false); - const KerxSubTable *st = &firstSubTable; + const SubTable *st = &firstSubTable; unsigned int count = tableCount; for (unsigned int i = 0; i < count; i++) { if (!st->sanitize (c)) return_trace (false); - st = &StructAfter (*st); + st = &StructAfter (*st); } return_trace (true); @@ -920,7 +924,7 @@ struct kerx HBUINT16 unused; /* Set to 0. */ HBUINT32 tableCount; /* The number of subtables included in the extended kerning * table. */ - KerxSubTable firstSubTable; /* Subtables. */ + SubTable firstSubTable; /* Subtables. */ /*subtableGlyphCoverageArray*/ /* Only if version >= 3. We don't use. */ public: diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 0102bb2c9..0976d16da 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -191,20 +191,34 @@ struct KernTable unsigned int count = thiz()->tableCount; for (unsigned int i = 0; i < count; i++) { - if (st->u.header.coverage & st->u.header.Variation) + bool reverse; + + if (!T::Types::extended && (st->u.header.coverage & st->u.header.Variation)) goto skip; if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != st->u.header.is_horizontal ()) goto skip; - if (!c->buffer->message (c->font, "start kern subtable %d", c->lookup_index)) + reverse = T::Types::extended /* TODO remove after kern application is moved earlier. */ && + bool (st->u.header.coverage & st->u.header.Backwards) != + HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); + + if (!c->buffer->message (c->font, "start %c%c%c%c subtable %d", HB_UNTAG (thiz()->tableTag), c->lookup_index)) goto skip; + if (reverse) + c->buffer->reverse (); + c->sanitizer.set_object (*st); + /* XXX Reverse-kern is probably not working yet... + * hb_kern_machine_t would need to know that it's reverse-kerning. */ st->dispatch (c); - (void) c->buffer->message (c->font, "end kern subtable %d", c->lookup_index); + if (reverse) + c->buffer->reverse (); + + (void) c->buffer->message (c->font, "end %c%c%c%c subtable %d", HB_UNTAG (thiz()->tableTag), c->lookup_index); skip: st = &StructAfter (*st); @@ -247,11 +261,13 @@ struct KernOTSubTableHeader enum Coverage { Horizontal = 0x01u, - Minimum = 0x02u, + Minimum = 0x02u, CrossStream = 0x04u, - Override = 0x08u, + Override = 0x08u, - Variation = 0x00u, /* Not supported. */ + /* Not supported: */ + Backwards = 0x00u, + Variation = 0x00u, }; inline bool sanitize (hb_sanitize_context_t *c) const @@ -273,15 +289,17 @@ struct KernOT : KernTable { friend struct KernTable; - typedef KernOTSubTableHeader SubTableHeader; - typedef KernSubTable SubTable; - + static const hb_tag_t tableTag = HB_OT_TAG_kern; static const uint16_t minVersion = 0; + typedef KernOTSubTableHeader SubTableHeader; + typedef SubTableHeader::Types Types; + typedef KernSubTable SubTable; + protected: - HBUINT16 version; /* Version--0x0000u */ - HBUINT16 tableCount; /* Number of subtables in the kerning table. */ - KernSubTable firstSubTable; /* Subtables. */ + HBUINT16 version; /* Version--0x0000u */ + HBUINT16 tableCount; /* Number of subtables in the kerning table. */ + SubTable firstSubTable; /* Subtables. */ public: DEFINE_SIZE_MIN (4); }; @@ -297,9 +315,12 @@ struct KernAATSubTableHeader enum Coverage { - Vertical = 0x80u, + Vertical = 0x80u, CrossStream = 0x40u, - Variation = 0x20u, + Variation = 0x20u, + + /* Not supported: */ + Backwards = 0x00u, }; inline bool sanitize (hb_sanitize_context_t *c) const @@ -323,15 +344,17 @@ struct KernAAT : KernTable { friend struct KernTable; - typedef KernAATSubTableHeader SubTableHeader; - typedef KernSubTable SubTable; - + static const hb_tag_t tableTag = HB_OT_TAG_kern; static const uint32_t minVersion = 0x00010000u; + typedef KernAATSubTableHeader SubTableHeader; + typedef SubTableHeader::Types Types; + typedef KernSubTable SubTable; + protected: - HBUINT32 version; /* Version--0x00010000u */ - HBUINT32 tableCount; /* Number of subtables in the kerning table. */ - KernSubTable firstSubTable; /* Subtables. */ + HBUINT32 version; /* Version--0x00010000u */ + HBUINT32 tableCount; /* Number of subtables in the kerning table. */ + SubTable firstSubTable; /* Subtables. */ public: DEFINE_SIZE_MIN (8); }; From c038f5be6b70b8edffc701dd3e4e3cd08d14e2f0 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 13:35:06 -0500 Subject: [PATCH 44/66] [fallback] Minor --- src/hb-ot-shape-fallback.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-ot-shape-fallback.cc b/src/hb-ot-shape-fallback.cc index 3742d6704..cfd06e7fd 100644 --- a/src/hb-ot-shape-fallback.cc +++ b/src/hb-ot-shape-fallback.cc @@ -25,7 +25,7 @@ */ #include "hb-ot-shape-fallback.hh" -#include "hb-ot-kern-table.hh" +#include "hb-kern.hh" static unsigned int recategorize_combining_class (hb_codepoint_t u, From 14772da06f9c67d0d40712369e26064e3dee2a91 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 13:40:22 -0500 Subject: [PATCH 45/66] [kern/kerx] Share KernTable, renamed to KerxTable --- src/hb-aat-layout-kerx-table.hh | 81 ++++++++++++++++++------- src/hb-ot-kern-table.hh | 103 ++------------------------------ 2 files changed, 63 insertions(+), 121 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 3a76f0fa4..8febd32db 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -831,7 +831,7 @@ struct KerxSubTable return_trace (dispatch (c)); } -protected: + public: union { KerxSubTableHeader header; KerxSubTableFormat0 format0; @@ -840,7 +840,7 @@ protected: KerxSubTableFormat4 format4; KerxSubTableFormat6 format6; } u; -public: + public: DEFINE_SIZE_MIN (12); }; @@ -849,33 +849,52 @@ public: * The 'kerx' Table */ -struct kerx +template +struct KerxTable { - static const hb_tag_t tableTag = HB_AAT_TAG_kerx; - static const uint16_t minVersion = 2; + /* https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern */ + inline const T* thiz (void) const { return static_cast (this); } - typedef KerxSubTableHeader SubTableHeader; - typedef SubTableHeader::Types Types; - typedef KerxSubTable SubTable; - - inline bool has_data (void) const { return version != 0; } - - inline void apply (hb_aat_apply_context_t *c) const + inline int get_h_kerning (hb_codepoint_t left, hb_codepoint_t right) const { + typedef typename T::SubTable SubTable; + + int v = 0; + const SubTable *st = &thiz()->firstSubTable; + unsigned int count = thiz()->tableCount; + for (unsigned int i = 0; i < count; i++) + { + if ((st->u.header.coverage & (st->u.header.Variation | st->u.header.CrossStream)) || + !st->u.header.is_horizontal ()) + continue; + v += st->get_kerning (left, right); + st = &StructAfter (*st); + } + return v; + } + + inline void apply (AAT::hb_aat_apply_context_t *c) const + { + typedef typename T::SubTable SubTable; + c->set_lookup_index (0); - const SubTable *st = &firstSubTable; - unsigned int count = tableCount; + const SubTable *st = &thiz()->firstSubTable; + unsigned int count = thiz()->tableCount; for (unsigned int i = 0; i < count; i++) { bool reverse; + if (!T::Types::extended && (st->u.header.coverage & st->u.header.Variation)) + goto skip; + if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != st->u.header.is_horizontal ()) goto skip; - reverse = bool (st->u.header.coverage & st->u.header.Backwards) != + reverse = T::Types::extended /* TODO remove after kern application is moved earlier. */ && + bool (st->u.header.coverage & st->u.header.Backwards) != HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); - if (!c->buffer->message (c->font, "start %c%c%c%c subtable %d", HB_UNTAG (tableTag), c->lookup_index)) + if (!c->buffer->message (c->font, "start %c%c%c%c subtable %d", HB_UNTAG (thiz()->tableTag), c->lookup_index)) goto skip; if (reverse) @@ -890,7 +909,7 @@ struct kerx if (reverse) c->buffer->reverse (); - (void) c->buffer->message (c->font, "end %c%c%c%c subtable %d", HB_UNTAG (tableTag), c->lookup_index); + (void) c->buffer->message (c->font, "end %c%c%c%c subtable %d", HB_UNTAG (thiz()->tableTag), c->lookup_index); skip: st = &StructAfter (*st); @@ -901,22 +920,38 @@ struct kerx inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - if (unlikely (!version.sanitize (c) || - version < minVersion || - !tableCount.sanitize (c))) + if (unlikely (!thiz()->version.sanitize (c) || + thiz()->version < T::minVersion || + !thiz()->tableCount.sanitize (c))) return_trace (false); - const SubTable *st = &firstSubTable; - unsigned int count = tableCount; + typedef typename T::SubTable SubTable; + + const SubTable *st = &thiz()->firstSubTable; + unsigned int count = thiz()->tableCount; for (unsigned int i = 0; i < count; i++) { - if (!st->sanitize (c)) + if (unlikely (!st->sanitize (c))) return_trace (false); st = &StructAfter (*st); } return_trace (true); } +}; + +struct kerx : KerxTable +{ + friend struct KerxTable; + + static const hb_tag_t tableTag = HB_AAT_TAG_kerx; + static const uint16_t minVersion = 2; + + typedef KerxSubTableHeader SubTableHeader; + typedef SubTableHeader::Types Types; + typedef KerxSubTable SubTable; + + inline bool has_data (void) const { return version; } protected: HBUINT16 version; /* The version number of the extended kerning table diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 0976d16da..b90f7fba7 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -158,98 +158,6 @@ struct KernSubTable }; -template -struct KernTable -{ - /* https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern */ - inline const T* thiz (void) const { return static_cast (this); } - - inline int get_h_kerning (hb_codepoint_t left, hb_codepoint_t right) const - { - typedef typename T::SubTable SubTable; - - int v = 0; - const SubTable *st = &thiz()->firstSubTable; - unsigned int count = thiz()->tableCount; - for (unsigned int i = 0; i < count; i++) - { - if ((st->u.header.coverage & (st->u.header.Variation | st->u.header.CrossStream)) || - !st->u.header.is_horizontal ()) - continue; - v += st->get_kerning (left, right); - st = &StructAfter (*st); - } - return v; - } - - inline void apply (AAT::hb_aat_apply_context_t *c) const - { - typedef typename T::SubTable SubTable; - - c->set_lookup_index (0); - const SubTable *st = &thiz()->firstSubTable; - unsigned int count = thiz()->tableCount; - for (unsigned int i = 0; i < count; i++) - { - bool reverse; - - if (!T::Types::extended && (st->u.header.coverage & st->u.header.Variation)) - goto skip; - - if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != st->u.header.is_horizontal ()) - goto skip; - - reverse = T::Types::extended /* TODO remove after kern application is moved earlier. */ && - bool (st->u.header.coverage & st->u.header.Backwards) != - HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); - - if (!c->buffer->message (c->font, "start %c%c%c%c subtable %d", HB_UNTAG (thiz()->tableTag), c->lookup_index)) - goto skip; - - if (reverse) - c->buffer->reverse (); - - c->sanitizer.set_object (*st); - - /* XXX Reverse-kern is probably not working yet... - * hb_kern_machine_t would need to know that it's reverse-kerning. */ - st->dispatch (c); - - if (reverse) - c->buffer->reverse (); - - (void) c->buffer->message (c->font, "end %c%c%c%c subtable %d", HB_UNTAG (thiz()->tableTag), c->lookup_index); - - skip: - st = &StructAfter (*st); - c->set_lookup_index (c->lookup_index + 1); - } - } - - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - if (unlikely (!thiz()->version.sanitize (c) || - thiz()->version < T::minVersion || - !thiz()->tableCount.sanitize (c))) - return_trace (false); - - typedef typename T::SubTable SubTable; - - const SubTable *st = &thiz()->firstSubTable; - unsigned int count = thiz()->tableCount; - for (unsigned int i = 0; i < count; i++) - { - if (unlikely (!st->sanitize (c))) - return_trace (false); - st = &StructAfter (*st); - } - - return_trace (true); - } -}; - - struct KernOTSubTableHeader { static const bool apple = false; @@ -285,9 +193,9 @@ struct KernOTSubTableHeader DEFINE_SIZE_STATIC (6); }; -struct KernOT : KernTable +struct KernOT : AAT::KerxTable { - friend struct KernTable; + friend struct AAT::KerxTable; static const hb_tag_t tableTag = HB_OT_TAG_kern; static const uint16_t minVersion = 0; @@ -340,9 +248,9 @@ struct KernAATSubTableHeader DEFINE_SIZE_STATIC (8); }; -struct KernAAT : KernTable +struct KernAAT : AAT::KerxTable { - friend struct KernTable; + friend struct AAT::KerxTable; static const hb_tag_t tableTag = HB_OT_TAG_kern; static const uint32_t minVersion = 0x00010000u; @@ -363,8 +271,7 @@ struct kern { static const hb_tag_t tableTag = HB_OT_TAG_kern; - inline bool has_data (void) const - { return u.version32 != 0; } + inline bool has_data (void) const { return u.version32; } inline int get_h_kerning (hb_codepoint_t left, hb_codepoint_t right) const { From 39b4ef6f18605e85c68cbcec534e137fc831dbca Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 13:48:45 -0500 Subject: [PATCH 46/66] [kerx] Better sanitize tupleKerning --- src/hb-aat-layout-kerx-table.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 8febd32db..c1b968468 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -53,7 +53,7 @@ kerxTupleKern (int value, unsigned int offset = value; const FWORD *pv = &StructAtOffset (base, offset); - if (unlikely (!pv->sanitize (&c->sanitizer))) return 0; + if (unlikely (!c->sanitizer.check_array (pv, tupleCount))) return 0; return *pv; } From f4bad0086e40c70d66d6514f038ddda1411657c8 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 13:51:17 -0500 Subject: [PATCH 47/66] [kerx] Implement tupleKerning for Format1 Untested. --- src/hb-aat-layout-kerx-table.hh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index c1b968468..6bbeda764 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -295,10 +295,12 @@ struct KerxSubTableFormat1 i++; break; } + unsigned int tuple_count = table->header.tuple_count (); + tuple_count = tuple_count ? tuple_count : 1; for (; i; i--) { unsigned int idx = stack[depth - i]; - int v = actions[i - 1]; + int v = actions[(i - 1) * tuple_count]; /* "The end of the list is marked by an odd value..." * Ignore it. */ @@ -376,9 +378,6 @@ struct KerxSubTableFormat1 if (!c->plan->requested_kerning) return false; - if (header.tuple_count ()) - return_trace (false); /* TODO kerxTupleKern */ - driver_context_t dc (this, c); StateTableDriver driver (machine, c->buffer, c->font->face); From 0c3b061ac244fa8a8657366e1b95523503fdf7be Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 13:58:41 -0500 Subject: [PATCH 48/66] [kern] Apply erlier, where GPOS/kerx are applied --- src/hb-aat-layout-kerx-table.hh | 3 +-- src/hb-ot-shape-fallback.cc | 9 +++++++++ src/hb-ot-shape.cc | 12 +++++------- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 6bbeda764..8b87e176f 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -889,8 +889,7 @@ struct KerxTable if (HB_DIRECTION_IS_HORIZONTAL (c->buffer->props.direction) != st->u.header.is_horizontal ()) goto skip; - reverse = T::Types::extended /* TODO remove after kern application is moved earlier. */ && - bool (st->u.header.coverage & st->u.header.Backwards) != + reverse = bool (st->u.header.coverage & st->u.header.Backwards) != HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); if (!c->buffer->message (c->font, "start %c%c%c%c subtable %d", HB_UNTAG (thiz()->tableTag), c->lookup_index)) diff --git a/src/hb-ot-shape-fallback.cc b/src/hb-ot-shape-fallback.cc index cfd06e7fd..bbc410ad6 100644 --- a/src/hb-ot-shape-fallback.cc +++ b/src/hb-ot-shape-fallback.cc @@ -464,9 +464,18 @@ _hb_ot_shape_fallback_kern (const hb_ot_shape_plan_t *plan, !font->has_glyph_h_kerning_func () : !font->has_glyph_v_kerning_func ()) return; + + bool reverse = HB_DIRECTION_IS_BACKWARD (buffer->props.direction); + + if (reverse) + buffer->reverse (); + hb_ot_shape_fallback_kern_driver_t driver (font, buffer); OT::hb_kern_machine_t machine (driver); machine.kern (font, buffer, plan->kern_mask, false); + + if (reverse) + buffer->reverse (); } diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 98c29a483..d2f771320 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -866,10 +866,15 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) break; } + /* XXX Clean up relationship between these. */ if (c->plan->apply_gpos) c->plan->position (c->font, c->buffer); else if (c->plan->apply_kerx) hb_aat_layout_position (c->plan, c->font, c->buffer); + else if (c->plan->apply_kern) + hb_ot_layout_kern (c->plan, c->font, c->buffer); + else if (c->plan->fallback_kerning) + _hb_ot_shape_fallback_kern (c->plan, c->font, c->buffer); if (c->plan->apply_trak) hb_aat_layout_track (c->plan, c->font, c->buffer); @@ -915,13 +920,6 @@ hb_ot_position (const hb_ot_shape_context_t *c) if (HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction)) hb_buffer_reverse (c->buffer); - /* Visual fallback goes here. */ - - if (c->plan->apply_kern) - hb_ot_layout_kern (c->plan, c->font, c->buffer); - else if (c->plan->fallback_kerning) - _hb_ot_shape_fallback_kern (c->plan, c->font, c->buffer); - _hb_buffer_deallocate_gsubgpos_vars (c->buffer); } From 649cc3ef2773950b0b5884d9d1caf414aac888bf Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 14:04:04 -0500 Subject: [PATCH 49/66] [kerx] Don't disable crossKerning if kern feature is off --- src/hb-aat-layout-kerx-table.hh | 9 +++++---- src/hb-ot-shape.cc | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 8b87e176f..aabbd4c98 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -314,7 +314,7 @@ struct KerxSubTableFormat1 v = 0; } - if (idx < buffer->len && buffer->info[idx].mask & kern_mask) + if (idx < buffer->len) { if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) { @@ -324,7 +324,7 @@ struct KerxSubTableFormat1 if (!buffer->pos[idx].y_offset) buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); } - else + else if (buffer->info[idx].mask & kern_mask) { if (!buffer->pos[idx].x_offset) { @@ -342,7 +342,7 @@ struct KerxSubTableFormat1 if (!buffer->pos[idx].x_offset) buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); } - else + else if (buffer->info[idx].mask & kern_mask) { if (!buffer->pos[idx].y_offset) { @@ -375,7 +375,8 @@ struct KerxSubTableFormat1 { TRACE_APPLY (this); - if (!c->plan->requested_kerning) + if (!c->plan->requested_kerning && + !(header.coverage & header.CrossStream)) return false; driver_context_t dc (this, c); diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index d2f771320..bc4c24980 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -124,7 +124,7 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, else if (hb_aat_layout_has_positioning (face)) plan.apply_kerx = true; - if (plan.requested_kerning && !plan.apply_kerx && !has_gpos_kern) + if (!plan.apply_kerx && !has_gpos_kern) { /* Apparently Apple applies kerx if GPOS kern was not applied. */ if (hb_aat_layout_has_positioning (face)) From e10a856eb24ae45e301c3ffa778caa4c0a995bb9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 14:11:48 -0500 Subject: [PATCH 50/66] [kerx] Format --- src/hb-aat-layout-kerx-table.hh | 58 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index aabbd4c98..09c720874 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -300,10 +300,11 @@ struct KerxSubTableFormat1 for (; i; i--) { unsigned int idx = stack[depth - i]; + if (idx >= buffer->len) continue; + int v = actions[(i - 1) * tuple_count]; - /* "The end of the list is marked by an odd value..." - * Ignore it. */ + /* "The end of the list is marked by an odd value..." Ignore it. */ v &= ~1; /* The following flag is undocumented in the spec, but described @@ -314,41 +315,38 @@ struct KerxSubTableFormat1 v = 0; } - if (idx < buffer->len) + if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) { - if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) + if (crossStream) { - if (crossStream) + crossOffset += v; + if (!buffer->pos[idx].y_offset) + buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); + } + else if (buffer->info[idx].mask & kern_mask) + { + if (!buffer->pos[idx].x_offset) { - crossOffset += v; - if (!buffer->pos[idx].y_offset) - buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); - } - else if (buffer->info[idx].mask & kern_mask) - { - if (!buffer->pos[idx].x_offset) - { - buffer->pos[idx].x_advance += c->font->em_scale_x (v); - buffer->pos[idx].x_offset += c->font->em_scale_x (v); - } + buffer->pos[idx].x_advance += c->font->em_scale_x (v); + buffer->pos[idx].x_offset += c->font->em_scale_x (v); } } - else + } + else + { + if (crossStream) { - if (crossStream) + /* CoreText doesn't do crossStream kerning in vertical. We do. */ + crossOffset += v; + if (!buffer->pos[idx].x_offset) + buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); + } + else if (buffer->info[idx].mask & kern_mask) + { + if (!buffer->pos[idx].y_offset) { - /* CoreText doesn't do crossStream kerning in vertical. We do. */ - crossOffset += v; - if (!buffer->pos[idx].x_offset) - buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); - } - else if (buffer->info[idx].mask & kern_mask) - { - if (!buffer->pos[idx].y_offset) - { - buffer->pos[idx].y_advance += c->font->em_scale_y (v); - buffer->pos[idx].y_offset += c->font->em_scale_y (v); - } + buffer->pos[idx].y_advance += c->font->em_scale_y (v); + buffer->pos[idx].y_offset += c->font->em_scale_y (v); } } } From b2f687c2569a3cc0b1cd0335c5ca0f8d193f8a39 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 14:38:29 -0500 Subject: [PATCH 51/66] [kerx] Use GPOS attachment facilities for CrossStream kerning --- src/hb-aat-layout-kerx-table.hh | 51 ++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 09c720874..4ef5d9e26 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -236,9 +236,7 @@ struct KerxSubTableFormat1 * other subtables in kerx. Discovered via testing. */ kernAction (&table->machine + table->kernAction), depth (0), - crossStream (table->header.coverage & table->header.CrossStream), - crossOffset (0) {} - + crossStream (table->header.coverage & table->header.CrossStream) {} /* TODO * 'kern' table has this pecularity, we don't currently implement. @@ -307,21 +305,25 @@ struct KerxSubTableFormat1 /* "The end of the list is marked by an odd value..." Ignore it. */ v &= ~1; + hb_glyph_position_t &o = buffer->pos[idx]; + /* The following flag is undocumented in the spec, but described * in the 'kern' table example. */ if (v == -0x8000) { - crossOffset = 0; - v = 0; + o.attach_type() = ATTACH_TYPE_NONE; + o.attach_chain() = 0; + o.x_offset = o.y_offset = 0; } - - if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) + else if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) { if (crossStream) { - crossOffset += v; - if (!buffer->pos[idx].y_offset) - buffer->pos[idx].y_offset += c->font->em_scale_y (crossOffset); + if (buffer->pos[idx].attach_type() && !buffer->pos[idx].y_offset) + { + o.y_offset = c->font->em_scale_y (v); + buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; + } } else if (buffer->info[idx].mask & kern_mask) { @@ -337,9 +339,11 @@ struct KerxSubTableFormat1 if (crossStream) { /* CoreText doesn't do crossStream kerning in vertical. We do. */ - crossOffset += v; - if (!buffer->pos[idx].x_offset) - buffer->pos[idx].x_offset = c->font->em_scale_x (crossOffset); + if (buffer->pos[idx].attach_type() && !buffer->pos[idx].x_offset) + { + o.x_offset = c->font->em_scale_x (v); + buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; + } } else if (buffer->info[idx].mask & kern_mask) { @@ -353,8 +357,6 @@ struct KerxSubTableFormat1 } depth = 0; } - else - buffer->pos[buffer->idx].y_offset += c->font->em_scale_y (crossOffset); return true; } @@ -366,7 +368,6 @@ struct KerxSubTableFormat1 unsigned int stack[8]; unsigned int depth; bool crossStream; - int crossOffset; }; inline bool apply (hb_aat_apply_context_t *c) const @@ -875,6 +876,7 @@ struct KerxTable { typedef typename T::SubTable SubTable; + bool seenCrossStream = false; c->set_lookup_index (0); const SubTable *st = &thiz()->firstSubTable; unsigned int count = thiz()->tableCount; @@ -894,6 +896,23 @@ struct KerxTable if (!c->buffer->message (c->font, "start %c%c%c%c subtable %d", HB_UNTAG (thiz()->tableTag), c->lookup_index)) goto skip; + if (!seenCrossStream && + (st->u.header.coverage & st->u.header.CrossStream)) + { + /* Attach all glyphs into a chain. */ + seenCrossStream = true; + hb_glyph_position_t *pos = c->buffer->pos; + unsigned int count = c->buffer->len; + for (unsigned int i = 0; i < count; i++) + { + pos[i].attach_type() = ATTACH_TYPE_CURSIVE; + pos[i].attach_chain() = HB_DIRECTION_IS_FORWARD (c->buffer->props.direction) ? -1 : +1; + /* We intentionally don't set HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT, + * since there needs to be a non-zero attachment for post-positioning to + * be needed. */ + } + } + if (reverse) c->buffer->reverse (); From 0eb4157011e78c332d781e28b54b020aa08957c0 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 14:42:15 -0500 Subject: [PATCH 52/66] [kerx] Disable backwards-kerning for non-state-machine tables That's what the spec says for Backwards flag, only applicable to formats 1 and 4. --- src/hb-aat-layout-kerx-table.hh | 8 +++----- src/hb-ot-kern-table.hh | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 4ef5d9e26..222285d46 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -118,7 +118,7 @@ struct KerxSubTableFormat0 if (!c->plan->requested_kerning) return false; - if (header.coverage & header.CrossStream) + if (header.coverage & (header.CrossStream | header.Backwards)) return false; accelerator_t accel (*this, c); @@ -427,7 +427,7 @@ struct KerxSubTableFormat2 if (!c->plan->requested_kerning) return false; - if (header.coverage & header.CrossStream) + if (header.coverage & (header.CrossStream | header.Backwards)) return false; accelerator_t accel (*this, c); @@ -696,7 +696,7 @@ struct KerxSubTableFormat6 if (!c->plan->requested_kerning) return false; - if (header.coverage & header.CrossStream) + if (header.coverage & (header.CrossStream | header.Backwards)) return false; accelerator_t accel (*this, c); @@ -918,8 +918,6 @@ struct KerxTable c->sanitizer.set_object (*st); - /* XXX Reverse-kern is probably not working yet... - * hb_kern_machine_t would need to know that it's reverse-kerning. */ st->dispatch (c); if (reverse) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index b90f7fba7..b2c29e303 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -66,7 +66,7 @@ struct KernSubTableFormat3 if (!c->plan->requested_kerning) return false; - if (header.coverage & header.CrossStream) + if (header.coverage & (header.CrossStream | header.Backwards)) return false; hb_kern_machine_t machine (*this); From 7a9629f2f11a11d1c064662a08a0172ac2001668 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 14:52:36 -0500 Subject: [PATCH 53/66] [kerx] Implement CrossStream kerning for non-state-machine subtables Untested. --- src/hb-aat-layout-kerx-table.hh | 12 ++++----- src/hb-kern.hh | 43 ++++++++++++++++++++++++--------- src/hb-ot-kern-table.hh | 4 +-- 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 222285d46..dc3ecc582 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -118,11 +118,11 @@ struct KerxSubTableFormat0 if (!c->plan->requested_kerning) return false; - if (header.coverage & (header.CrossStream | header.Backwards)) + if (header.coverage & header.Backwards) return false; accelerator_t accel (*this, c); - hb_kern_machine_t machine (accel); + hb_kern_machine_t machine (accel, header.coverage & header.CrossStream); machine.kern (c->font, c->buffer, c->plan->kern_mask); return_trace (true); @@ -427,11 +427,11 @@ struct KerxSubTableFormat2 if (!c->plan->requested_kerning) return false; - if (header.coverage & (header.CrossStream | header.Backwards)) + if (header.coverage & header.Backwards) return false; accelerator_t accel (*this, c); - hb_kern_machine_t machine (accel); + hb_kern_machine_t machine (accel, header.coverage & header.CrossStream); machine.kern (c->font, c->buffer, c->plan->kern_mask); return_trace (true); @@ -696,11 +696,11 @@ struct KerxSubTableFormat6 if (!c->plan->requested_kerning) return false; - if (header.coverage & (header.CrossStream | header.Backwards)) + if (header.coverage & header.Backwards) return false; accelerator_t accel (*this, c); - hb_kern_machine_t machine (accel); + hb_kern_machine_t machine (accel, header.coverage & header.CrossStream); machine.kern (c->font, c->buffer, c->plan->kern_mask); return_trace (true); diff --git a/src/hb-kern.hh b/src/hb-kern.hh index 60e625c4b..aa01b470f 100644 --- a/src/hb-kern.hh +++ b/src/hb-kern.hh @@ -38,7 +38,10 @@ namespace OT { template struct hb_kern_machine_t { - hb_kern_machine_t (const Driver &driver_) : driver (driver_) {} + hb_kern_machine_t (const Driver &driver_, + bool crossStream_ = false) : + driver (driver_), + crossStream (crossStream_) {} HB_NO_SANITIZE_SIGNED_INTEGER_OVERFLOW inline void kern (hb_font_t *font, @@ -81,26 +84,41 @@ struct hb_kern_machine_t if (likely (!kern)) goto skip; - if (horizontal) { if (scale) kern = font->em_scale_x (kern); - hb_position_t kern1 = kern >> 1; - hb_position_t kern2 = kern - kern1; - pos[i].x_advance += kern1; - pos[j].x_advance += kern2; - pos[j].x_offset += kern2; + if (crossStream) + { + pos[j].y_offset = kern; + buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; + } + else + { + hb_position_t kern1 = kern >> 1; + hb_position_t kern2 = kern - kern1; + pos[i].x_advance += kern1; + pos[j].x_advance += kern2; + pos[j].x_offset += kern2; + } } else { if (scale) kern = font->em_scale_y (kern); - hb_position_t kern1 = kern >> 1; - hb_position_t kern2 = kern - kern1; - pos[i].y_advance += kern1; - pos[j].y_advance += kern2; - pos[j].y_offset += kern2; + if (crossStream) + { + pos[j].x_offset = kern; + buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; + } + else + { + hb_position_t kern1 = kern >> 1; + hb_position_t kern2 = kern - kern1; + pos[i].y_advance += kern1; + pos[j].y_advance += kern2; + pos[j].y_offset += kern2; + } } buffer->unsafe_to_break (i, j + 1); @@ -111,6 +129,7 @@ struct hb_kern_machine_t } const Driver &driver; + bool crossStream; }; diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index b2c29e303..95306ece3 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -66,10 +66,10 @@ struct KernSubTableFormat3 if (!c->plan->requested_kerning) return false; - if (header.coverage & (header.CrossStream | header.Backwards)) + if (header.coverage & header.Backwards) return false; - hb_kern_machine_t machine (*this); + hb_kern_machine_t machine (*this, header.coverage & header.CrossStream); machine.kern (c->font, c->buffer, c->plan->kern_mask); return_trace (true); From 501a364d9bb6c5828f9d660bae8b6e93b158b275 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 15:02:16 -0500 Subject: [PATCH 54/66] [GPOS] Add TODO item --- src/hb-ot-layout-gpos-table.hh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh index 743a97931..0dcbb5a0d 100644 --- a/src/hb-ot-layout-gpos-table.hh +++ b/src/hb-ot-layout-gpos-table.hh @@ -837,6 +837,8 @@ struct PairPosFormat2 unsigned int klass2 = (this+classDef2).get_class (buffer->info[skippy_iter.idx].codepoint); if (unlikely (klass1 >= class1Count || klass2 >= class2Count)) return_trace (false); + /* TODO Only unsafe_to_break if kerning values not zero... + * https://github.com/harfbuzz/harfbuzz/issues/1365 */ buffer->unsafe_to_break (buffer->idx, skippy_iter.idx + 1); const Value *v = &values[record_len * (klass1 * class2Count + klass2)]; valueFormat1.apply_value (c, this, v, buffer->cur_pos()); From 6ee6cd93d8c61389cf242e42a531cc6e7214b21a Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 15:40:55 -0500 Subject: [PATCH 55/66] [GPOS] Only mark unsafe-to-break if kerning happened Fixes https://github.com/harfbuzz/harfbuzz/issues/1365 --- src/hb-ot-layout-gpos-table.hh | 56 +++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh index 0dcbb5a0d..9e1b026d7 100644 --- a/src/hb-ot-layout-gpos-table.hh +++ b/src/hb-ot-layout-gpos-table.hh @@ -103,56 +103,58 @@ struct ValueFormat : HBUINT16 inline unsigned int get_size (void) const { return get_len () * Value::static_size; } - void apply_value (hb_ot_apply_context_t *c, + bool apply_value (hb_ot_apply_context_t *c, const void *base, const Value *values, hb_glyph_position_t &glyph_pos) const { + bool ret = false; unsigned int format = *this; - if (!format) return; + if (!format) return ret; hb_font_t *font = c->font; hb_bool_t horizontal = HB_DIRECTION_IS_HORIZONTAL (c->direction); - if (format & xPlacement) glyph_pos.x_offset += font->em_scale_x (get_short (values++)); - if (format & yPlacement) glyph_pos.y_offset += font->em_scale_y (get_short (values++)); + if (format & xPlacement) glyph_pos.x_offset += font->em_scale_x (get_short (values++, &ret)); + if (format & yPlacement) glyph_pos.y_offset += font->em_scale_y (get_short (values++, &ret)); if (format & xAdvance) { - if (likely (horizontal)) glyph_pos.x_advance += font->em_scale_x (get_short (values)); + if (likely (horizontal)) glyph_pos.x_advance += font->em_scale_x (get_short (values, &ret)); values++; } /* y_advance values grow downward but font-space grows upward, hence negation */ if (format & yAdvance) { - if (unlikely (!horizontal)) glyph_pos.y_advance -= font->em_scale_y (get_short (values)); + if (unlikely (!horizontal)) glyph_pos.y_advance -= font->em_scale_y (get_short (values, &ret)); values++; } - if (!has_device ()) return; + if (!has_device ()) return ret; bool use_x_device = font->x_ppem || font->num_coords; bool use_y_device = font->y_ppem || font->num_coords; - if (!use_x_device && !use_y_device) return; + if (!use_x_device && !use_y_device) return ret; const VariationStore &store = c->var_store; /* pixel -> fractional pixel */ if (format & xPlaDevice) { - if (use_x_device) glyph_pos.x_offset += (base + get_device (values)).get_x_delta (font, store); + if (use_x_device) glyph_pos.x_offset += (base + get_device (values, &ret)).get_x_delta (font, store); values++; } if (format & yPlaDevice) { - if (use_y_device) glyph_pos.y_offset += (base + get_device (values)).get_y_delta (font, store); + if (use_y_device) glyph_pos.y_offset += (base + get_device (values, &ret)).get_y_delta (font, store); values++; } if (format & xAdvDevice) { - if (horizontal && use_x_device) glyph_pos.x_advance += (base + get_device (values)).get_x_delta (font, store); + if (horizontal && use_x_device) glyph_pos.x_advance += (base + get_device (values, &ret)).get_x_delta (font, store); values++; } if (format & yAdvDevice) { /* y_advance values grow downward but font-space grows upward, hence negation */ - if (!horizontal && use_y_device) glyph_pos.y_advance -= (base + get_device (values)).get_y_delta (font, store); + if (!horizontal && use_y_device) glyph_pos.y_advance -= (base + get_device (values, &ret)).get_y_delta (font, store); values++; } + return ret; } private: @@ -175,11 +177,17 @@ struct ValueFormat : HBUINT16 static inline OffsetTo& get_device (Value* value) { return *CastP > (value); } - static inline const OffsetTo& get_device (const Value* value) - { return *CastP > (value); } + static inline const OffsetTo& get_device (const Value* value, bool *worked=nullptr) + { + if (worked) *worked |= *value; + return *CastP > (value); + } - static inline const HBINT16& get_short (const Value* value) - { return *CastP (value); } + static inline const HBINT16& get_short (const Value* value, bool *worked=nullptr) + { + if (worked) *worked |= *value; + return *CastP (value); + } public: @@ -672,9 +680,10 @@ struct PairSet min = mid + 1; else { - buffer->unsafe_to_break (buffer->idx, pos + 1); - valueFormats[0].apply_value (c, this, &record->values[0], buffer->cur_pos()); - valueFormats[1].apply_value (c, this, &record->values[len1], buffer->pos[pos]); + /* Note the intentional use of "|" instead of short-circuit "||". */ + if (valueFormats[0].apply_value (c, this, &record->values[0], buffer->cur_pos()) | + valueFormats[1].apply_value (c, this, &record->values[len1], buffer->pos[pos])) + buffer->unsafe_to_break (buffer->idx, pos + 1); if (len2) pos++; buffer->idx = pos; @@ -837,12 +846,11 @@ struct PairPosFormat2 unsigned int klass2 = (this+classDef2).get_class (buffer->info[skippy_iter.idx].codepoint); if (unlikely (klass1 >= class1Count || klass2 >= class2Count)) return_trace (false); - /* TODO Only unsafe_to_break if kerning values not zero... - * https://github.com/harfbuzz/harfbuzz/issues/1365 */ - buffer->unsafe_to_break (buffer->idx, skippy_iter.idx + 1); const Value *v = &values[record_len * (klass1 * class2Count + klass2)]; - valueFormat1.apply_value (c, this, v, buffer->cur_pos()); - valueFormat2.apply_value (c, this, v + len1, buffer->pos[skippy_iter.idx]); + /* Note the intentional use of "|" instead of short-circuit "||". */ + if (valueFormat1.apply_value (c, this, v, buffer->cur_pos()) | + valueFormat2.apply_value (c, this, v + len1, buffer->pos[skippy_iter.idx])) + buffer->unsafe_to_break (buffer->idx, skippy_iter.idx + 1); buffer->idx = skippy_iter.idx; if (len2) From ea579f9ccc87718d4c2ca8945a997e6679428a12 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 15:44:40 -0500 Subject: [PATCH 56/66] [kerx] Fix peculiar indexing that was needed previously Not needed now that we use GPOS attachment for cursive kerx. --- src/hb-aat-layout-kerx-table.hh | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index dc3ecc582..84fa6dcd7 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -286,23 +286,18 @@ struct KerxSubTableFormat1 /* From Apple 'kern' spec: * "Each pops one glyph from the kerning stack and applies the kerning value to it. * The end of the list is marked by an odd value... */ - unsigned int i; - for (i = 0; i < depth; i++) - if (actions[i] & 1) - { - i++; - break; - } unsigned int tuple_count = table->header.tuple_count (); tuple_count = tuple_count ? tuple_count : 1; - for (; i; i--) + bool last = false; + while (!last && depth--) { - unsigned int idx = stack[depth - i]; + unsigned int idx = stack[depth]; + int v = *actions; + actions += tuple_count; if (idx >= buffer->len) continue; - int v = actions[(i - 1) * tuple_count]; - - /* "The end of the list is marked by an odd value..." Ignore it. */ + /* "The end of the list is marked by an odd value..." */ + last = v & 1; v &= ~1; hb_glyph_position_t &o = buffer->pos[idx]; @@ -355,7 +350,6 @@ struct KerxSubTableFormat1 } } } - depth = 0; } return true; From bc06e2805ae55f5c152dfb70ee91c75830ad8f54 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 16:02:40 -0500 Subject: [PATCH 57/66] [kerx/kern] Add has_cross_stream() --- src/hb-aat-layout-kerx-table.hh | 15 +++++++++++++++ src/hb-ot-kern-table.hh | 9 +++++++++ 2 files changed, 24 insertions(+) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 84fa6dcd7..422120949 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -848,6 +848,21 @@ struct KerxTable /* https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern */ inline const T* thiz (void) const { return static_cast (this); } + inline bool has_cross_stream (void) const + { + typedef typename T::SubTable SubTable; + + const SubTable *st = &thiz()->firstSubTable; + unsigned int count = thiz()->tableCount; + for (unsigned int i = 0; i < count; i++) + { + if (st->u.header.coverage & st->u.header.CrossStream) + return true; + st = &StructAfter (*st); + } + return false; + } + inline int get_h_kerning (hb_codepoint_t left, hb_codepoint_t right) const { typedef typename T::SubTable SubTable; diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 95306ece3..690532e29 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -273,6 +273,15 @@ struct kern inline bool has_data (void) const { return u.version32; } + inline bool has_cross_stream (void) const + { + switch (u.major) { + case 0: return u.ot.has_cross_stream (); + case 1: return u.aat.has_cross_stream (); + default:return false; + } + } + inline int get_h_kerning (hb_codepoint_t left, hb_codepoint_t right) const { switch (u.major) { From 9af983af24788afad4b37bd2297b86cdca7c5c29 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 16:03:09 -0500 Subject: [PATCH 58/66] [kern] Switch to dispatch --- src/hb-aat-layout-kerx-table.hh | 7 +++++-- src/hb-ot-kern-table.hh | 28 +++++++++++++++------------- src/hb-ot-layout.cc | 6 ++---- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 422120949..272ebfd95 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -881,10 +881,11 @@ struct KerxTable return v; } - inline void apply (AAT::hb_aat_apply_context_t *c) const + inline bool apply (AAT::hb_aat_apply_context_t *c) const { typedef typename T::SubTable SubTable; + bool ret = false; bool seenCrossStream = false; c->set_lookup_index (0); const SubTable *st = &thiz()->firstSubTable; @@ -927,7 +928,7 @@ struct KerxTable c->sanitizer.set_object (*st); - st->dispatch (c); + ret |= st->dispatch (c); if (reverse) c->buffer->reverse (); @@ -938,6 +939,8 @@ struct KerxTable st = &StructAfter (*st); c->set_lookup_index (c->lookup_index + 1); } + + return ret; } inline bool sanitize (hb_sanitize_context_t *c) const diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index 690532e29..14e065684 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -272,10 +272,11 @@ struct kern static const hb_tag_t tableTag = HB_OT_TAG_kern; inline bool has_data (void) const { return u.version32; } + inline unsigned int get_type (void) const { return u.major; } inline bool has_cross_stream (void) const { - switch (u.major) { + switch (get_type ()) { case 0: return u.ot.has_cross_stream (); case 1: return u.aat.has_cross_stream (); default:return false; @@ -284,20 +285,25 @@ struct kern inline int get_h_kerning (hb_codepoint_t left, hb_codepoint_t right) const { - switch (u.major) { + switch (get_type ()) { case 0: return u.ot.get_h_kerning (left, right); case 1: return u.aat.get_h_kerning (left, right); default:return 0; } } - inline void apply (AAT::hb_aat_apply_context_t *c) const + inline bool apply (AAT::hb_aat_apply_context_t *c) const + { return dispatch (c); } + + template + inline typename context_t::return_t dispatch (context_t *c) const { - /* TODO Switch to dispatch(). */ - switch (u.major) { - case 0: u.ot.apply (c); return; - case 1: u.aat.apply (c); return; - default: return; + unsigned int subtable_type = get_type (); + TRACE_DISPATCH (this, subtable_type); + switch (subtable_type) { + case 0: return_trace (c->dispatch (u.ot)); + case 1: return_trace (c->dispatch (u.aat)); + default: return_trace (c->default_return_value ()); } } @@ -305,11 +311,7 @@ struct kern { TRACE_SANITIZE (this); if (!u.version32.sanitize (c)) return_trace (false); - switch (u.major) { - case 0: return_trace (u.ot.sanitize (c)); - case 1: return_trace (u.aat.sanitize (c)); - default:return_trace (true); - } + return_trace (dispatch (c)); } protected: diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index 6da1d3fdc..9b93bb085 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -1275,10 +1275,8 @@ apply_backward (OT::hb_ot_apply_context_t *c, if (accel.may_have (buffer->cur().codepoint) && (buffer->cur().mask & c->lookup_mask) && c->check_glyph_property (&buffer->cur(), c->lookup_props)) - { - if (accel.apply (c)) - ret = true; - } + ret |= accel.apply (c); + /* The reverse lookup doesn't "advance" cursor (for good reason). */ buffer->idx--; From 41cff7afc916048810a7ea4aa33ecdee7401df74 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 16:05:36 -0500 Subject: [PATCH 59/66] Minor --- src/hb-ot-shape.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index bc4c24980..dfe2a88c9 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -903,6 +903,9 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) c->font->subtract_glyph_h_origin (info[i].codepoint, &pos[i].x_offset, &pos[i].y_offset); + + if (c->plan->fallback_mark_positioning && c->plan->shaper->fallback_position) + _hb_ot_shape_fallback_mark_position (c->plan, c->font, c->buffer); } static inline void @@ -914,9 +917,6 @@ hb_ot_position (const hb_ot_shape_context_t *c) hb_ot_position_complex (c); - if (c->plan->fallback_mark_positioning && c->plan->shaper->fallback_position) - _hb_ot_shape_fallback_mark_position (c->plan, c->font, c->buffer); - if (HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction)) hb_buffer_reverse (c->buffer); From 5cf6f94dfd30a468ab8464435e846811c39d9226 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 16:07:22 -0500 Subject: [PATCH 60/66] Don't apply both kerx and kern Ouch! --- src/hb-ot-shape.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index dfe2a88c9..455c8d689 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -129,7 +129,7 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, /* Apparently Apple applies kerx if GPOS kern was not applied. */ if (hb_aat_layout_has_positioning (face)) plan.apply_kerx = true; - if (hb_ot_layout_has_kerning (face)) + else if (hb_ot_layout_has_kerning (face)) plan.apply_kern = true; else plan.fallback_kerning = true; From ca23567f41a2d6389f6fd2483a994cf5aa6aeaf8 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 16:19:51 -0500 Subject: [PATCH 61/66] Disable fallback mark positioning if kern table has cross-stream kerning Happens even if the cross-stream kerning is for cursive attachment only. Oh well.. --- src/hb-ot-layout.cc | 6 ++++++ src/hb-ot-layout.hh | 3 +++ src/hb-ot-shape.cc | 14 ++++++-------- src/hb-ot-shape.hh | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index 9b93bb085..cdc7755ad 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -63,6 +63,12 @@ hb_ot_layout_has_kerning (hb_face_t *face) return face->table.kern->has_data (); } +hb_bool_t +hb_ot_layout_has_cross_kerning (hb_face_t *face) +{ + return face->table.kern->has_cross_stream (); +} + void hb_ot_layout_kern (hb_ot_shape_plan_t *plan, hb_font_t *font, diff --git a/src/hb-ot-layout.hh b/src/hb-ot-layout.hh index 437ae4776..ee6e0d254 100644 --- a/src/hb-ot-layout.hh +++ b/src/hb-ot-layout.hh @@ -48,6 +48,9 @@ struct hb_ot_shape_plan_t; HB_INTERNAL hb_bool_t hb_ot_layout_has_kerning (hb_face_t *face); +HB_INTERNAL hb_bool_t +hb_ot_layout_has_cross_kerning (hb_face_t *face); + HB_INTERNAL void hb_ot_layout_kern (hb_ot_shape_plan_t *plan, hb_font_t *font, diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 455c8d689..9b6d47089 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -131,13 +131,12 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, plan.apply_kerx = true; else if (hb_ot_layout_has_kerning (face)) plan.apply_kern = true; - else - plan.fallback_kerning = true; } + bool has_kern_mark = plan.apply_kern && hb_ot_layout_has_cross_kerning (face); + plan.zero_marks = !plan.apply_kerx && !has_kern_mark; plan.has_gpos_mark = !!plan.map.get_1_mask (HB_TAG ('m','a','r','k')); - if (!plan.apply_gpos && !plan.apply_kerx) - plan.fallback_mark_positioning = true; + plan.fallback_mark_positioning = !plan.apply_gpos && !plan.apply_kerx && !has_kern_mark; /* Currently we always apply trak. */ plan.apply_trak = plan.requested_tracking && hb_aat_layout_has_tracking (face); @@ -853,7 +852,7 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) hb_ot_layout_position_start (c->font, c->buffer); - if (!c->plan->apply_kerx) + if (c->plan->zero_marks) switch (c->plan->shaper->zero_width_marks) { case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_EARLY: @@ -866,20 +865,19 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) break; } - /* XXX Clean up relationship between these. */ if (c->plan->apply_gpos) c->plan->position (c->font, c->buffer); else if (c->plan->apply_kerx) hb_aat_layout_position (c->plan, c->font, c->buffer); else if (c->plan->apply_kern) hb_ot_layout_kern (c->plan, c->font, c->buffer); - else if (c->plan->fallback_kerning) + else _hb_ot_shape_fallback_kern (c->plan, c->font, c->buffer); if (c->plan->apply_trak) hb_aat_layout_track (c->plan, c->font, c->buffer); - if (!c->plan->apply_kerx) + if (c->plan->zero_marks) switch (c->plan->shaper->zero_width_marks) { case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE: diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh index 1cb9e24d7..655e28d6d 100644 --- a/src/hb-ot-shape.hh +++ b/src/hb-ot-shape.hh @@ -51,8 +51,8 @@ struct hb_ot_shape_plan_t bool requested_tracking : 1; bool has_frac : 1; bool has_gpos_mark : 1; + bool zero_marks : 1; bool fallback_glyph_classes : 1; - bool fallback_kerning : 1; bool fallback_mark_positioning : 1; bool apply_gpos : 1; From 1909072235e59eb80f9169300279b65779b932a4 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 16:42:16 -0500 Subject: [PATCH 62/66] [aat] Add debug info to state machine --- src/hb-aat-layout-common.hh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 34c61e93c..6efc99aa4 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -452,6 +452,7 @@ struct StateTable const Entry *entries = (this+entryTable).arrayZ; unsigned int entry = states[state * nClasses + klass]; + DEBUG_MSG (APPLY, nullptr, "e%d", entry); return &entries[entry]; } @@ -629,6 +630,7 @@ struct StateTableDriver unsigned int klass = buffer->idx < buffer->len ? machine.get_class (buffer->info[buffer->idx].codepoint, num_glyphs) : (unsigned) StateTable::CLASS_END_OF_TEXT; + DEBUG_MSG (APPLY, nullptr, "c%d at %d", klass, buffer->idx); const Entry *entry = machine.get_entryZ (state, klass); if (unlikely (!entry)) break; @@ -661,6 +663,7 @@ struct StateTableDriver last_was_dont_advance = (entry->flags & context_t::DontAdvance) && buffer->max_ops-- > 0; state = machine.new_state (entry->newState); + DEBUG_MSG (APPLY, nullptr, "s%d", state); if (buffer->idx == buffer->len) break; From 385f78b3123f268e4c7ff423621e5ce9e8a5c54b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 17:19:21 -0500 Subject: [PATCH 63/66] [aat] Remove deleted-glyhs after applying kerx/kern Finally: Fixes https://github.com/harfbuzz/harfbuzz/issues/1356 Test case: $ ./hb-shape GeezaPro.ttc -u U+0628,U+064A,U+064E,U+0651,U+0629 [u0629.final.tehMarbuta=4+713|u064e_u0651.shaddaFatha=1@0,-200+0|u064a.medial.yeh=1+656|u0628.initial.beh=0+656] The mark positioning (kern table CrossStream kerning) only works if deleted glyph (as result of ligation) is still in stream and pushed through the state machine. --- src/hb-aat-layout-morx-table.hh | 16 ----- src/hb-aat-layout.cc | 30 +++++++++- src/hb-aat-layout.hh | 12 +++- src/hb-ot-layout-gpos-table.hh | 6 +- src/hb-ot-layout.cc | 60 +++++++++++++++++-- src/hb-ot-layout.hh | 23 +++---- src/hb-ot-shape.cc | 102 +++++++++++--------------------- 7 files changed, 142 insertions(+), 107 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index a364f7ac7..28e3c10d0 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -1106,21 +1106,6 @@ struct mortmorx } } - inline static void remove_deleted_glyphs (hb_buffer_t *buffer) - { - if (unlikely (!buffer->successful)) return; - - buffer->clear_output (); - for (buffer->idx = 0; buffer->idx < buffer->len && buffer->successful;) - { - if (unlikely (buffer->cur().codepoint == DELETED_GLYPH)) - buffer->skip_glyph (); - else - buffer->next_glyph (); - } - buffer->swap_buffers (); - } - inline void apply (hb_aat_apply_context_t *c) const { if (unlikely (!c->buffer->successful)) return; @@ -1133,7 +1118,6 @@ struct mortmorx if (unlikely (!c->buffer->successful)) return; chain = &StructAfter > (*chain); } - remove_deleted_glyphs (c->buffer); } inline bool sanitize (hb_sanitize_context_t *c) const diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index 5a4e227ca..b3b56c08d 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -193,7 +193,7 @@ hb_aat_layout_compile_map (const hb_aat_map_builder_t *mapper, } -hb_bool_t +bool hb_aat_layout_has_substitution (hb_face_t *face) { return face->table.morx->has_data () || @@ -224,8 +224,32 @@ hb_aat_layout_substitute (hb_ot_shape_plan_t *plan, } } +void +hb_aat_layout_zero_width_deleted_glyphs (hb_buffer_t *buffer) +{ + unsigned int count = buffer->len; + hb_glyph_info_t *info = buffer->info; + hb_glyph_position_t *pos = buffer->pos; + for (unsigned int i = 0; i < count; i++) + if (unlikely (info[i].codepoint == AAT::DELETED_GLYPH)) + pos[i].x_advance = pos[i].y_advance = 0; +} -hb_bool_t +static bool +is_deleted_glyph (const hb_glyph_info_t *info) +{ + return info->codepoint == AAT::DELETED_GLYPH; +} + +void +hb_aat_layout_remove_deleted_glyphs (hb_buffer_t *buffer) +{ + hb_ot_layout_delete_glyphs_inplace (buffer, is_deleted_glyph); +} + + + +bool hb_aat_layout_has_positioning (hb_face_t *face) { return face->table.kerx->has_data (); @@ -248,7 +272,7 @@ hb_aat_layout_position (hb_ot_shape_plan_t *plan, } -hb_bool_t +bool hb_aat_layout_has_tracking (hb_face_t *face) { return face->table.trak->has_data (); diff --git a/src/hb-aat-layout.hh b/src/hb-aat-layout.hh index 8a558e6aa..97935a02f 100644 --- a/src/hb-aat-layout.hh +++ b/src/hb-aat-layout.hh @@ -56,7 +56,7 @@ HB_INTERNAL void hb_aat_layout_compile_map (const hb_aat_map_builder_t *mapper, hb_aat_map_t *map); -HB_INTERNAL hb_bool_t +HB_INTERNAL bool hb_aat_layout_has_substitution (hb_face_t *face); HB_INTERNAL void @@ -64,7 +64,13 @@ hb_aat_layout_substitute (hb_ot_shape_plan_t *plan, hb_font_t *font, hb_buffer_t *buffer); -HB_INTERNAL hb_bool_t +HB_INTERNAL void +hb_aat_layout_zero_width_deleted_glyphs (hb_buffer_t *buffer); + +HB_INTERNAL void +hb_aat_layout_remove_deleted_glyphs (hb_buffer_t *buffer); + +HB_INTERNAL bool hb_aat_layout_has_positioning (hb_face_t *face); HB_INTERNAL void @@ -72,7 +78,7 @@ hb_aat_layout_position (hb_ot_shape_plan_t *plan, hb_font_t *font, hb_buffer_t *buffer); -HB_INTERNAL hb_bool_t +HB_INTERNAL bool hb_aat_layout_has_tracking (hb_face_t *face); HB_INTERNAL void diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh index 9e1b026d7..fc74af47a 100644 --- a/src/hb-ot-layout-gpos-table.hh +++ b/src/hb-ot-layout-gpos-table.hh @@ -113,7 +113,7 @@ struct ValueFormat : HBUINT16 if (!format) return ret; hb_font_t *font = c->font; - hb_bool_t horizontal = HB_DIRECTION_IS_HORIZONTAL (c->direction); + bool horizontal = HB_DIRECTION_IS_HORIZONTAL (c->direction); if (format & xPlacement) glyph_pos.x_offset += font->em_scale_x (get_short (values++, &ret)); if (format & yPlacement) glyph_pos.y_offset += font->em_scale_y (get_short (values++, &ret)); @@ -271,10 +271,10 @@ struct AnchorFormat2 unsigned int x_ppem = font->x_ppem; unsigned int y_ppem = font->y_ppem; hb_position_t cx = 0, cy = 0; - hb_bool_t ret; + bool ret; ret = (x_ppem || y_ppem) && - font->get_glyph_contour_point_for_origin (glyph_id, anchorPoint, HB_DIRECTION_LTR, &cx, &cy); + font->get_glyph_contour_point_for_origin (glyph_id, anchorPoint, HB_DIRECTION_LTR, &cx, &cy); *x = ret && x_ppem ? cx : font->em_fscale_x (xCoordinate); *y = ret && y_ppem ? cy : font->em_fscale_y (yCoordinate); } diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index cdc7755ad..eb5140fc2 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -57,13 +57,13 @@ * kern */ -hb_bool_t +bool hb_ot_layout_has_kerning (hb_face_t *face) { return face->table.kern->has_data (); } -hb_bool_t +bool hb_ot_layout_has_cross_kerning (hb_face_t *face) { return face->table.kern->has_cross_stream (); @@ -423,7 +423,7 @@ hb_ot_layout_table_get_feature_tags (hb_face_t *face, return g.get_feature_tags (start_offset, feature_count, feature_tags); } -hb_bool_t +bool hb_ot_layout_table_find_feature (hb_face_t *face, hb_tag_t table_tag, hb_tag_t feature_tag, @@ -933,12 +933,12 @@ hb_ot_layout_lookup_would_substitute (hb_face_t *face, zero_context); } -hb_bool_t +bool hb_ot_layout_lookup_would_substitute_fast (hb_face_t *face, unsigned int lookup_index, const hb_codepoint_t *glyphs, unsigned int glyphs_length, - hb_bool_t zero_context) + bool zero_context) { if (unlikely (lookup_index >= face->table.GSUB->lookup_count)) return false; OT::hb_would_apply_context_t c (face, glyphs, glyphs_length, (bool) zero_context); @@ -955,6 +955,56 @@ hb_ot_layout_substitute_start (hb_font_t *font, _hb_ot_layout_set_glyph_props (font, buffer); } +void +hb_ot_layout_delete_glyphs_inplace (hb_buffer_t *buffer, + bool (*filter) (const hb_glyph_info_t *info)) +{ + /* Merge clusters and delete filtered glyphs. + * NOTE! We can't use out-buffer as we have positioning data. */ + unsigned int j = 0; + unsigned int count = buffer->len; + hb_glyph_info_t *info = buffer->info; + hb_glyph_position_t *pos = buffer->pos; + for (unsigned int i = 0; i < count; i++) + { + if (filter (&info[i])) + { + /* Merge clusters. + * Same logic as buffer->delete_glyph(), but for in-place removal. */ + + unsigned int cluster = info[i].cluster; + if (i + 1 < count && cluster == info[i + 1].cluster) + continue; /* Cluster survives; do nothing. */ + + if (j) + { + /* Merge cluster backward. */ + if (cluster < info[j - 1].cluster) + { + unsigned int mask = info[i].mask; + unsigned int old_cluster = info[j - 1].cluster; + for (unsigned k = j; k && info[k - 1].cluster == old_cluster; k--) + buffer->set_cluster (info[k - 1], cluster, mask); + } + continue; + } + + if (i + 1 < count) + buffer->merge_clusters (i, i + 2); /* Merge cluster forward. */ + + continue; + } + + if (j != i) + { + info[j] = info[i]; + pos[j] = pos[i]; + } + j++; + } + buffer->len = j; +} + /** * hb_ot_layout_lookup_substitute_closure: * diff --git a/src/hb-ot-layout.hh b/src/hb-ot-layout.hh index ee6e0d254..b21825316 100644 --- a/src/hb-ot-layout.hh +++ b/src/hb-ot-layout.hh @@ -45,10 +45,10 @@ struct hb_ot_shape_plan_t; * kern */ -HB_INTERNAL hb_bool_t +HB_INTERNAL bool hb_ot_layout_has_kerning (hb_face_t *face); -HB_INTERNAL hb_bool_t +HB_INTERNAL bool hb_ot_layout_has_cross_kerning (hb_face_t *face); HB_INTERNAL void @@ -59,7 +59,7 @@ hb_ot_layout_kern (hb_ot_shape_plan_t *plan, /* Private API corresponding to hb-ot-layout.h: */ -HB_INTERNAL hb_bool_t +HB_INTERNAL bool hb_ot_layout_table_find_feature (hb_face_t *face, hb_tag_t table_tag, hb_tag_t feature_tag, @@ -93,12 +93,12 @@ HB_MARK_AS_FLAG_T (hb_ot_layout_glyph_props_flags_t); * GSUB/GPOS */ -HB_INTERNAL hb_bool_t +HB_INTERNAL bool hb_ot_layout_lookup_would_substitute_fast (hb_face_t *face, unsigned int lookup_index, const hb_codepoint_t *glyphs, unsigned int glyphs_length, - hb_bool_t zero_context); + bool zero_context); /* Should be called before all the substitute_lookup's are done. */ @@ -106,6 +106,9 @@ HB_INTERNAL void hb_ot_layout_substitute_start (hb_font_t *font, hb_buffer_t *buffer); +HB_INTERNAL void +hb_ot_layout_delete_glyphs_inplace (hb_buffer_t *buffer, + bool (*filter) (const hb_glyph_info_t *info)); namespace OT { struct hb_ot_apply_context_t; @@ -306,13 +309,13 @@ _hb_glyph_info_get_unicode_space_fallback_type (const hb_glyph_info_t *info) static inline bool _hb_glyph_info_ligated (const hb_glyph_info_t *info); -static inline hb_bool_t +static inline bool _hb_glyph_info_is_default_ignorable (const hb_glyph_info_t *info) { return (info->unicode_props() & UPROPS_MASK_IGNORABLE) && !_hb_glyph_info_ligated (info); } -static inline hb_bool_t +static inline bool _hb_glyph_info_is_default_ignorable_and_not_hidden (const hb_glyph_info_t *info) { return ((info->unicode_props() & (UPROPS_MASK_IGNORABLE|UPROPS_MASK_HIDDEN)) @@ -366,17 +369,17 @@ _hb_glyph_info_is_unicode_format (const hb_glyph_info_t *info) return _hb_glyph_info_get_general_category (info) == HB_UNICODE_GENERAL_CATEGORY_FORMAT; } -static inline hb_bool_t +static inline bool _hb_glyph_info_is_zwnj (const hb_glyph_info_t *info) { return _hb_glyph_info_is_unicode_format (info) && (info->unicode_props() & UPROPS_MASK_Cf_ZWNJ); } -static inline hb_bool_t +static inline bool _hb_glyph_info_is_zwj (const hb_glyph_info_t *info) { return _hb_glyph_info_is_unicode_format (info) && (info->unicode_props() & UPROPS_MASK_Cf_ZWJ); } -static inline hb_bool_t +static inline bool _hb_glyph_info_is_joiner (const hb_glyph_info_t *info) { return _hb_glyph_info_is_unicode_format (info) && (info->unicode_props() & (UPROPS_MASK_Cf_ZWNJ|UPROPS_MASK_Cf_ZWJ)); diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 9b6d47089..99d6b9d40 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -476,7 +476,9 @@ hb_ensure_native_direction (hb_buffer_t *buffer) } -/* Substitute */ +/* + * Substitute + */ static inline void hb_ot_mirror_chars (const hb_ot_shape_context_t *c) @@ -582,10 +584,8 @@ hb_ot_shape_setup_masks (const hb_ot_shape_context_t *c) } static void -hb_ot_zero_width_default_ignorables (const hb_ot_shape_context_t *c) +hb_ot_zero_width_default_ignorables (const hb_buffer_t *buffer) { - hb_buffer_t *buffer = c->buffer; - if (!(buffer->scratch_flags & HB_BUFFER_SCRATCH_FLAG_HAS_DEFAULT_IGNORABLES) || (buffer->flags & HB_BUFFER_FLAG_PRESERVE_DEFAULT_IGNORABLES) || (buffer->flags & HB_BUFFER_FLAG_REMOVE_DEFAULT_IGNORABLES)) @@ -601,21 +601,19 @@ hb_ot_zero_width_default_ignorables (const hb_ot_shape_context_t *c) } static void -hb_ot_hide_default_ignorables (const hb_ot_shape_context_t *c) +hb_ot_hide_default_ignorables (hb_buffer_t *buffer, + hb_font_t *font) { - hb_buffer_t *buffer = c->buffer; - if (!(buffer->scratch_flags & HB_BUFFER_SCRATCH_FLAG_HAS_DEFAULT_IGNORABLES) || (buffer->flags & HB_BUFFER_FLAG_PRESERVE_DEFAULT_IGNORABLES)) return; unsigned int count = buffer->len; hb_glyph_info_t *info = buffer->info; - hb_glyph_position_t *pos = buffer->pos; - hb_codepoint_t invisible = c->buffer->invisible; + hb_codepoint_t invisible = buffer->invisible; if (!(buffer->flags & HB_BUFFER_FLAG_REMOVE_DEFAULT_IGNORABLES) && - (invisible || c->font->get_nominal_glyph (' ', &invisible))) + (invisible || font->get_nominal_glyph (' ', &invisible))) { /* Replace default-ignorables with a zero-advance invisible glyph. */ for (unsigned int i = 0; i < count; i++) @@ -625,49 +623,7 @@ hb_ot_hide_default_ignorables (const hb_ot_shape_context_t *c) } } else - { - /* Merge clusters and delete default-ignorables. - * NOTE! We can't use out-buffer as we have positioning data. */ - unsigned int j = 0; - for (unsigned int i = 0; i < count; i++) - { - if (_hb_glyph_info_is_default_ignorable (&info[i])) - { - /* Merge clusters. - * Same logic as buffer->delete_glyph(), but for in-place removal. */ - - unsigned int cluster = info[i].cluster; - if (i + 1 < count && cluster == info[i + 1].cluster) - continue; /* Cluster survives; do nothing. */ - - if (j) - { - /* Merge cluster backward. */ - if (cluster < info[j - 1].cluster) - { - unsigned int mask = info[i].mask; - unsigned int old_cluster = info[j - 1].cluster; - for (unsigned k = j; k && info[k - 1].cluster == old_cluster; k--) - buffer->set_cluster (info[k - 1], cluster, mask); - } - continue; - } - - if (i + 1 < count) - buffer->merge_clusters (i, i + 2); /* Merge cluster forward. */ - - continue; - } - - if (j != i) - { - info[j] = info[i]; - pos[j] = pos[i]; - } - j++; - } - buffer->len = j; - } + hb_ot_layout_delete_glyphs_inplace (buffer, _hb_glyph_info_is_default_ignorable); } @@ -684,10 +640,10 @@ hb_ot_map_glyphs_fast (hb_buffer_t *buffer) } static inline void -hb_synthesize_glyph_classes (const hb_ot_shape_context_t *c) +hb_synthesize_glyph_classes (hb_buffer_t *buffer) { - unsigned int count = c->buffer->len; - hb_glyph_info_t *info = c->buffer->info; + unsigned int count = buffer->len; + hb_glyph_info_t *info = buffer->info; for (unsigned int i = 0; i < count; i++) { hb_ot_layout_glyph_props_flags_t klass; @@ -739,7 +695,7 @@ hb_ot_substitute_complex (const hb_ot_shape_context_t *c) hb_ot_layout_substitute_start (c->font, buffer); if (c->plan->fallback_glyph_classes) - hb_synthesize_glyph_classes (c); + hb_synthesize_glyph_classes (c->buffer); if (unlikely (c->plan->apply_morx)) hb_aat_layout_substitute (c->plan, c->font, c->buffer); @@ -748,7 +704,7 @@ hb_ot_substitute_complex (const hb_ot_shape_context_t *c) } static inline void -hb_ot_substitute (const hb_ot_shape_context_t *c) +hb_ot_substitute_pre (const hb_ot_shape_context_t *c) { hb_ot_substitute_default (c); @@ -757,7 +713,21 @@ hb_ot_substitute (const hb_ot_shape_context_t *c) hb_ot_substitute_complex (c); } -/* Position */ +static inline void +hb_ot_substitute_post (const hb_ot_shape_context_t *c) +{ + hb_ot_hide_default_ignorables (c->buffer, c->font); + if (c->plan->apply_morx) + hb_aat_layout_remove_deleted_glyphs (c->buffer); + + if (c->plan->shaper->postprocess_glyphs) + c->plan->shaper->postprocess_glyphs (c->plan, c->buffer, c->font); +} + + +/* + * Position + */ static inline void adjust_mark_offsets (hb_glyph_position_t *pos) @@ -890,9 +860,11 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) break; } - /* Finishing off GPOS has to follow a certain order. */ + /* Finish off. Has to follow a certain order. */ hb_ot_layout_position_finish_advances (c->font, c->buffer); - hb_ot_zero_width_default_ignorables (c); + hb_ot_zero_width_default_ignorables (c->buffer); + if (c->plan->apply_morx) + hb_aat_layout_zero_width_deleted_glyphs (c->buffer); hb_ot_layout_position_finish_offsets (c->font, c->buffer); /* The nil glyph_h_origin() func returns 0, so no need to apply it. */ @@ -983,13 +955,9 @@ hb_ot_shape_internal (hb_ot_shape_context_t *c) if (c->plan->shaper->preprocess_text) c->plan->shaper->preprocess_text (c->plan, c->buffer, c->font); - hb_ot_substitute (c); + hb_ot_substitute_pre (c); hb_ot_position (c); - - hb_ot_hide_default_ignorables (c); - - if (c->plan->shaper->postprocess_glyphs) - c->plan->shaper->postprocess_glyphs (c->plan, c->buffer, c->font); + hb_ot_substitute_post (c); hb_propagate_flags (c->buffer); From 29c5302376ff2bc8f04b0fc0efba3ce40ef564a7 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 17:29:37 -0500 Subject: [PATCH 64/66] [morx] Minor --- src/hb-aat-layout-morx-table.hh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 28e3c10d0..430732702 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -461,7 +461,7 @@ struct LigatureSubtable { hb_buffer_t *buffer = driver->buffer; - DEBUG_MSG (APPLY, nullptr, "Ligature transition at %d", buffer->idx); + DEBUG_MSG (APPLY, nullptr, "Ligature transition at %u", buffer->idx); if (entry->flags & LigatureEntryT::SetComponent) { if (unlikely (match_length >= ARRAY_LENGTH (match_positions))) @@ -472,12 +472,12 @@ struct LigatureSubtable match_length--; match_positions[match_length++] = buffer->out_len; - DEBUG_MSG (APPLY, nullptr, "Set component at %d", buffer->out_len); + DEBUG_MSG (APPLY, nullptr, "Set component at %u", buffer->out_len); } if (LigatureEntryT::performAction (entry)) { - DEBUG_MSG (APPLY, nullptr, "Perform action with %d", match_length); + DEBUG_MSG (APPLY, nullptr, "Perform action with %u", match_length); unsigned int end = buffer->out_len; if (unlikely (!match_length)) @@ -504,7 +504,7 @@ struct LigatureSubtable break; } - DEBUG_MSG (APPLY, nullptr, "Moving to stack position %d", cursor - 1); + DEBUG_MSG (APPLY, nullptr, "Moving to stack position %u", cursor - 1); buffer->move_to (match_positions[--cursor]); if (unlikely (!actionData->sanitize (&c->sanitizer))) return false; @@ -520,7 +520,7 @@ struct LigatureSubtable if (unlikely (!componentData.sanitize (&c->sanitizer))) return false; ligature_idx += componentData; - DEBUG_MSG (APPLY, nullptr, "Action store %d last %d", + DEBUG_MSG (APPLY, nullptr, "Action store %u last %u", bool (action & LigActionStore), bool (action & LigActionLast)); if (action & (LigActionStore | LigActionLast)) @@ -530,7 +530,7 @@ struct LigatureSubtable if (unlikely (!ligatureData.sanitize (&c->sanitizer))) return false; hb_codepoint_t lig = ligatureData; - DEBUG_MSG (APPLY, nullptr, "Produced ligature %d", lig); + DEBUG_MSG (APPLY, nullptr, "Produced ligature %u", lig); buffer->replace_glyph (lig); unsigned int lig_end = match_positions[match_length - 1] + 1; From 006386be3a069199ebaf22bcc55fa7233c62e0d5 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 18:04:53 -0500 Subject: [PATCH 65/66] [kern] Implement negative state numbers Let the fuzzing bots rip this code apart... --- src/hb-aat-layout-common.hh | 90 ++++++++++++++++++++++++--------- src/hb-aat-layout-kerx-table.hh | 10 ---- 2 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 6efc99aa4..6a16d70c4 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -430,8 +430,8 @@ struct StateTable CLASS_END_OF_LINE = 3, }; - inline unsigned int new_state (unsigned int newState) const - { return Types::extended ? newState : (newState - stateArrayTable) / nClasses; } + inline int new_state (unsigned int newState) const + { return Types::extended ? newState : ((int) newState - (int) stateArrayTable) / nClasses; } inline unsigned int get_class (hb_codepoint_t glyph_id, unsigned int num_glyphs) const { @@ -444,7 +444,7 @@ struct StateTable return (this+entryTable).arrayZ; } - inline const Entry *get_entryZ (unsigned int state, unsigned int klass) const + inline const Entry *get_entryZ (int state, unsigned int klass) const { if (unlikely (klass >= nClasses)) return nullptr; @@ -452,7 +452,7 @@ struct StateTable const Entry *entries = (this+entryTable).arrayZ; unsigned int entry = states[state * nClasses + klass]; - DEBUG_MSG (APPLY, nullptr, "e%d", entry); + DEBUG_MSG (APPLY, nullptr, "e%u", entry); return &entries[entry]; } @@ -468,28 +468,69 @@ struct StateTable const Entry *entries = (this+entryTable).arrayZ; unsigned int num_classes = nClasses; + if (unlikely (hb_unsigned_mul_overflows (num_classes, states[0].static_size))) + return_trace (false); + unsigned int row_stride = num_classes * states[0].static_size; - unsigned int num_states = 1; + /* Apple 'kern' table has this peculiarity: + * + * "Because the stateTableOffset in the state table header is (strictly + * speaking) redundant, some 'kern' tables use it to record an initial + * state where that should not be StartOfText. To determine if this is + * done, calculate what the stateTableOffset should be. If it's different + * from the actual stateTableOffset, use it as the initial state." + * + * We implement this by calling the initial state zero, but allow *negative* + * states if the start state indeed was not the first state. Since the code + * is shared, this will also apply to 'mort' table. The 'kerx' / 'morx' + * tables are not affected since those address states by index, not offset. + */ + + int min_state = 0; + int max_state = 0; unsigned int num_entries = 0; - unsigned int state = 0; + int state_pos = 0; + int state_neg = 0; unsigned int entry = 0; - while (state < num_states) + while (min_state < state_neg || state_pos <= max_state) { - if (unlikely (hb_unsigned_mul_overflows (num_classes, states[0].static_size))) - return_trace (false); + if (min_state < state_neg) + { + /* Negative states. */ + if (unlikely (hb_unsigned_mul_overflows (min_state, num_classes))) + return_trace (false); + if (unlikely (!c->check_array (&states[min_state * num_classes], -min_state, row_stride))) + return_trace (false); + if ((c->max_ops -= state_neg - min_state) < 0) + return_trace (false); + { /* Sweep new states. */ + const HBUSHORT *stop = &states[min_state * num_classes]; + if (unlikely (stop > states)) + return_trace (false); + for (const HBUSHORT *p = states; stop < p; p--) + num_entries = MAX (num_entries, *(p - 1) + 1); + state_neg = min_state; + } + } - if (unlikely (!c->check_array (states, - num_states, - num_classes * states[0].static_size))) - return_trace (false); - if ((c->max_ops -= num_states - state) < 0) - return_trace (false); - { /* Sweep new states. */ - const HBUSHORT *stop = &states[num_states * num_classes]; - for (const HBUSHORT *p = &states[state * num_classes]; p < stop; p++) - num_entries = MAX (num_entries, *p + 1); - state = num_states; + if (state_pos <= max_state) + { + /* Positive states. */ + if (unlikely (!c->check_array (states, max_state + 1, row_stride))) + return_trace (false); + if ((c->max_ops -= max_state - state_pos + 1) < 0) + return_trace (false); + { /* Sweep new states. */ + if (unlikely (hb_unsigned_mul_overflows ((max_state + 1), num_classes))) + return_trace (false); + const HBUSHORT *stop = &states[(max_state + 1) * num_classes]; + if (unlikely (stop < states)) + return_trace (false); + for (const HBUSHORT *p = &states[state_pos * num_classes]; p < stop; p++) + num_entries = MAX (num_entries, *p + 1); + state_pos = max_state + 1; + } } if (unlikely (!c->check_array (entries, num_entries))) @@ -500,8 +541,9 @@ struct StateTable const Entry *stop = &entries[num_entries]; for (const Entry *p = &entries[entry]; p < stop; p++) { - unsigned int newState = new_state (p->newState); - num_states = MAX (num_states, newState + 1); + int newState = new_state (p->newState); + min_state = MIN (min_state, newState); + max_state = MAX (max_state, newState); } entry = num_entries; } @@ -623,14 +665,14 @@ struct StateTableDriver if (!c->in_place) buffer->clear_output (); - unsigned int state = StateTable::STATE_START_OF_TEXT; + int state = StateTable::STATE_START_OF_TEXT; bool last_was_dont_advance = false; for (buffer->idx = 0; buffer->successful;) { unsigned int klass = buffer->idx < buffer->len ? machine.get_class (buffer->info[buffer->idx].codepoint, num_glyphs) : (unsigned) StateTable::CLASS_END_OF_TEXT; - DEBUG_MSG (APPLY, nullptr, "c%d at %d", klass, buffer->idx); + DEBUG_MSG (APPLY, nullptr, "c%u at %u", klass, buffer->idx); const Entry *entry = machine.get_entryZ (state, klass); if (unlikely (!entry)) break; diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 272ebfd95..cc8281491 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -238,16 +238,6 @@ struct KerxSubTableFormat1 depth (0), crossStream (table->header.coverage & table->header.CrossStream) {} - /* TODO - * 'kern' table has this pecularity, we don't currently implement. - * - * "Because the stateTableOffset in the state table header is (strictly - * speaking) redundant, some 'kern' tables use it to record an initial - * state where that should not be StartOfText. To determine if this is - * done, calculate what the stateTableOffset should be. If it's different - * from the actual stateTableOffset, use it as the initial state." - */ - inline bool is_actionable (StateTableDriver *driver HB_UNUSED, const Entry *entry) { From b18a56a290bf5330e81019b33f15e6951dd86a8b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 7 Nov 2018 18:13:22 -0500 Subject: [PATCH 66/66] [kerx] Comment --- src/hb-aat-layout-kerx-table.hh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index cc8281491..3cd80acfd 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -292,6 +292,10 @@ struct KerxSubTableFormat1 hb_glyph_position_t &o = buffer->pos[idx]; + /* Testing shows that CoreText only applies kern (cross-stream or not) + * if none has been applied by previous subtables. That is, it does + * NOT seem to accumulate as otherwise implied by specs. */ + /* The following flag is undocumented in the spec, but described * in the 'kern' table example. */ if (v == -0x8000)