From 339036dd970625e03696b4533ced1e25fc4fd131 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 20:37:22 -0400 Subject: [PATCH 01/26] [kerx] Start fleshing out Format1 --- src/hb-aat-layout-kerx-table.hh | 59 ++++++++++++++++++++++++++++----- src/hb-aat-layout-morx-table.hh | 2 +- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 3cff334a9..4118d8ee1 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -99,6 +99,48 @@ struct KerxSubTableFormat0 struct KerxSubTableFormat1 { + struct EntryData + { + HBUINT16 ligActionIndex; /* Index to the first ligActionTable entry + * for processing this group, if indicated + * by the flags. */ + public: + DEFINE_SIZE_STATIC (2); + }; + + 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. */ + Reset = 0x2000, /* If set, reset the kerning data (clear the stack) */ + Reserved = 0x1FFF, /* Not used; set to 0. */ + }; + + inline driver_context_t (const KerxSubTableFormat1 *table) + {} + + inline bool is_actionable (StateTableDriver *driver, + const Entry *entry) + { + return false; // XXX return (entry->flags & Verb) && start < end; + } + inline bool transition (StateTableDriver *driver, + const Entry *entry) + { + //hb_buffer_t *buffer = driver->buffer; + //unsigned int flags = entry->flags; + + return true; + } + + public: + private: + }; + inline bool apply (hb_aat_apply_context_t *c) const { TRACE_APPLY (this); @@ -106,7 +148,10 @@ struct KerxSubTableFormat1 if (!c->plan->requested_kerning) return false; - /* TODO */ + driver_context_t dc (this); + + StateTableDriver driver (machine, c->buffer, c->font->face); + driver.drive (&dc); return_trace (true); } @@ -114,14 +159,13 @@ struct KerxSubTableFormat1 inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - return_trace (likely (c->check_struct (this) && - stateHeader.sanitize (c))); + return_trace (likely (machine.sanitize (c))); } protected: - KerxSubTableHeader header; - StateTable stateHeader; - LOffsetTo > valueTable; + KerxSubTableHeader header; + StateTable machine; + LOffsetTo, false> values; public: DEFINE_SIZE_STATIC (32); }; @@ -159,8 +203,7 @@ struct KerxSubTableFormat2 inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - return_trace (likely (c->check_struct (this) && - rowWidth.sanitize (c) && + return_trace (likely (rowWidth.sanitize (c) && leftClassTable.sanitize (c, this) && rightClassTable.sanitize (c, this) && array.sanitize (c, this))); diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 0020750ce..5de851266 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -164,7 +164,7 @@ struct RearrangementSubtable driver_context_t dc (this); - StateTableDriver driver (machine, c->buffer, c->face); + StateTableDriver driver (machine, c->buffer, c->face); driver.drive (&dc); return_trace (dc.ret); From ca54eba4846d0afda4601929556617a7ebe51714 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 20:41:16 -0400 Subject: [PATCH 02/26] [kerx] Fix bound-checking error introduced a couple commits past --- src/hb-aat-layout-kerx-table.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 4118d8ee1..0d3b330a3 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -180,7 +180,7 @@ struct KerxSubTableFormat2 unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+array), offset); if (unlikely ((const char *) v < (const char *) &array || - (const char *) v + v->static_size - (const char *) this <= header.length)) + (const char *) v + v->static_size - (const char *) this > header.length)) return 0; return *v; } @@ -284,7 +284,7 @@ struct KerxSubTableFormat6 unsigned int offset = l + r; const FWORD32 *v = &StructAtOffset (&(this+t.array), offset * sizeof (FWORD32)); if (unlikely ((const char *) v < (const char *) &t.array || - (const char *) v + v->static_size - (const char *) this <= header.length)) + (const char *) v + v->static_size - (const char *) this > header.length)) return 0; return *v; } @@ -296,7 +296,7 @@ struct KerxSubTableFormat6 unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+t.array), offset * sizeof (FWORD)); if (unlikely ((const char *) v < (const char *) &t.array || - (const char *) v + v->static_size - (const char *) this <= header.length)) + (const char *) v + v->static_size - (const char *) this > header.length)) return 0; return *v; } From c9165f5450b99e6d93e2a168b198384a221eef58 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 20:43:21 -0400 Subject: [PATCH 03/26] [kerx] More UnsizedArrayOf<> --- src/hb-aat-layout-kerx-table.hh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 0d3b330a3..edc17a398 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -205,8 +205,7 @@ struct KerxSubTableFormat2 TRACE_SANITIZE (this); return_trace (likely (rowWidth.sanitize (c) && leftClassTable.sanitize (c, this) && - rightClassTable.sanitize (c, this) && - array.sanitize (c, this))); + rightClassTable.sanitize (c, this))); } struct accelerator_t @@ -233,7 +232,8 @@ struct KerxSubTableFormat2 LOffsetTo > rightClassTable;/* Offset from beginning of this subtable to * right-hand class table. */ - LOffsetTo array; /* Offset from beginning of this subtable to + LOffsetTo, false> + array; /* Offset from beginning of this subtable to * the start of the kerning array. */ public: DEFINE_SIZE_STATIC (28); @@ -324,12 +324,10 @@ struct KerxSubTableFormat6 is_long () ? ( u.l.rowIndexTable.sanitize (c, this) && - u.l.columnIndexTable.sanitize (c, this) && - u.l.array.sanitize (c, this) + u.l.columnIndexTable.sanitize (c, this) ) : ( u.s.rowIndexTable.sanitize (c, this) && - u.s.columnIndexTable.sanitize (c, this) && - u.s.array.sanitize (c, this) + u.s.columnIndexTable.sanitize (c, this) ))); } @@ -359,13 +357,15 @@ struct KerxSubTableFormat6 { LOffsetTo > rowIndexTable; LOffsetTo > columnIndexTable; - LOffsetTo array; + LOffsetTo, false> + array; } l; struct Short { LOffsetTo > rowIndexTable; LOffsetTo > columnIndexTable; - LOffsetTo array; + LOffsetTo, false> + array; } s; } u; public: From 84967537966a76297c89460d95e7336f1bfc332d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 21:18:37 -0400 Subject: [PATCH 04/26] [kerx] Implement Format1 Untested. --- src/hb-aat-layout-kerx-table.hh | 62 +++++++++++++++++++++++++++------ src/hb-aat-layout-morx-table.hh | 2 +- 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index edc17a398..552bb0657 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -101,9 +101,9 @@ struct KerxSubTableFormat1 { struct EntryData { - HBUINT16 ligActionIndex; /* Index to the first ligActionTable entry - * for processing this group, if indicated - * by the flags. */ + HBUINT16 kernActionIndex;/* Index into the kerning value array. If + * this index is 0xFFFF, then no kerning + * is to be performed. */ public: DEFINE_SIZE_STATIC (2); }; @@ -120,25 +120,65 @@ struct KerxSubTableFormat1 Reserved = 0x1FFF, /* Not used; set to 0. */ }; - inline driver_context_t (const KerxSubTableFormat1 *table) - {} + inline driver_context_t (const KerxSubTableFormat1 *table, + hb_aat_apply_context_t *c_) : + c (c_), + kernAction (table+table->kernAction), + depth (0) {} inline bool is_actionable (StateTableDriver *driver, const Entry *entry) { - return false; // XXX return (entry->flags & Verb) && start < end; + return entry->data.kernActionIndex != 0xFFFF; } inline bool transition (StateTableDriver *driver, const Entry *entry) { - //hb_buffer_t *buffer = driver->buffer; - //unsigned int flags = entry->flags; + hb_buffer_t *buffer = driver->buffer; + unsigned int flags = entry->flags; + + if (flags & Reset) + { + depth = 0; + } + + 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->data.kernActionIndex != 0xFFFF) + { + const FWORD *actions = &kernAction[entry->data.kernActionIndex]; + if (!c->sanitizer.check_array (actions, depth)) + { + depth = 0; + return false; + } + + for (; depth; depth--) + { + unsigned int idx = stack[depth - 1]; + int v = *actions++; + /* XXX Non-forward direction... */ + if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) + buffer->pos[idx].x_advance += v; + else + buffer->pos[idx].y_advance += v; + } + } return true; } - public: private: + hb_aat_apply_context_t *c; + const UnsizedArrayOf &kernAction; + unsigned int stack[8]; + unsigned int depth; }; inline bool apply (hb_aat_apply_context_t *c) const @@ -148,7 +188,7 @@ struct KerxSubTableFormat1 if (!c->plan->requested_kerning) return false; - driver_context_t dc (this); + driver_context_t dc (this, c); StateTableDriver driver (machine, c->buffer, c->font->face); driver.drive (&dc); @@ -165,7 +205,7 @@ struct KerxSubTableFormat1 protected: KerxSubTableHeader header; StateTable machine; - LOffsetTo, false> values; + LOffsetTo, false> kernAction; public: DEFINE_SIZE_STATIC (32); }; diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 5de851266..b902fd79b 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -365,7 +365,7 @@ struct LigatureSubtable inline bool is_actionable (StateTableDriver *driver, const Entry *entry) { - return !!(entry->flags & PerformAction); + return entry->flags & PerformAction; } inline bool transition (StateTableDriver *driver, const Entry *entry) From 504cb68fc972c7f606bf9fc62015376382f78f45 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 21:29:46 -0400 Subject: [PATCH 05/26] Disable mark advance zeroing as well as mark fallback positioning if doing kerx --- src/hb-ot-shape.cc | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 3b1c93db4..e2405237f 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -114,7 +114,7 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, } plan.has_gpos_mark = !!plan.map.get_1_mask (HB_TAG ('m','a','r','k')); - if (!plan.apply_gpos) + if (!plan.apply_gpos && !plan.apply_kerx) plan.fallback_mark_positioning = true; } @@ -820,34 +820,36 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) hb_ot_layout_position_start (c->font, c->buffer); - switch (c->plan->shaper->zero_width_marks) - { - case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_EARLY: - zero_mark_widths_by_gdef (c->buffer, adjust_offsets_when_zeroing); - break; + if (!c->plan->apply_kerx) + switch (c->plan->shaper->zero_width_marks) + { + case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_EARLY: + zero_mark_widths_by_gdef (c->buffer, adjust_offsets_when_zeroing); + break; - default: - case HB_OT_SHAPE_ZERO_WIDTH_MARKS_NONE: - case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE: - break; - } + default: + case HB_OT_SHAPE_ZERO_WIDTH_MARKS_NONE: + case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE: + break; + } if (c->plan->apply_gpos) - c->plan->position (c->font, c->buffer); + ;//c->plan->position (c->font, c->buffer); else if (c->plan->apply_kerx) hb_aat_layout_position (c->plan, c->font, c->buffer); - switch (c->plan->shaper->zero_width_marks) - { - case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE: - zero_mark_widths_by_gdef (c->buffer, adjust_offsets_when_zeroing); - break; + if (!c->plan->apply_kerx) + switch (c->plan->shaper->zero_width_marks) + { + case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_LATE: + zero_mark_widths_by_gdef (c->buffer, adjust_offsets_when_zeroing); + break; - default: - case HB_OT_SHAPE_ZERO_WIDTH_MARKS_NONE: - case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_EARLY: - break; - } + default: + case HB_OT_SHAPE_ZERO_WIDTH_MARKS_NONE: + case HB_OT_SHAPE_ZERO_WIDTH_MARKS_BY_GDEF_EARLY: + break; + } /* Finishing off GPOS has to follow a certain order. */ hb_ot_layout_position_finish_advances (c->font, c->buffer); From 9f450f07b0a1593962e3b45d00f2cf93916f3466 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 21:46:58 -0400 Subject: [PATCH 06/26] [kerx] Make Format1 work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tested using Kannada MN: $ HB_OPTIONS=aat ./hb-shape Kannada\ MN.ttc -u 0C95,0CCd,C95,CCD [kn_ka.virama=0+1299|kn_ka.vattu=0+115|_blank=0@-115,0+385] $ HB_OPTIONS=aat ./hb-shape Kannada\ MN.ttc -u 0C95,0CCd,C95,CCD --features=-kern [kn_ka.virama=0+1799|kn_ka.vattu=0+230|_blank=0+0] I don't see the GPOS table in the font do the same. ¯\_(ツ)_/¯ --- src/hb-aat-layout-kerx-table.hh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 552bb0657..cc99868d1 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -123,7 +123,7 @@ struct KerxSubTableFormat1 inline driver_context_t (const KerxSubTableFormat1 *table, hb_aat_apply_context_t *c_) : c (c_), - kernAction (table+table->kernAction), + kernAction (&table->machine + table->kernAction), depth (0) {} inline bool is_actionable (StateTableDriver *driver, @@ -159,16 +159,21 @@ struct KerxSubTableFormat1 return false; } - for (; depth; depth--) + for (unsigned int i = 0; i < depth; i++) { - unsigned int idx = stack[depth - 1]; + /* 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++; /* XXX Non-forward direction... */ if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) - buffer->pos[idx].x_advance += v; + buffer->pos[idx].x_advance += c->font->em_scale_x (v); else - buffer->pos[idx].y_advance += v; + buffer->pos[idx].y_advance += c->font->em_scale_y (v); } + depth = 0; } return true; From 2b72c4b63d29eea39b646c8a1a1cfc2db732e1a6 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 21:53:14 -0400 Subject: [PATCH 07/26] [kerx] Comment --- src/hb-aat-layout-kerx-table.hh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index cc99868d1..881ab4e35 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -123,6 +123,9 @@ struct KerxSubTableFormat1 inline driver_context_t (const KerxSubTableFormat1 *table, hb_aat_apply_context_t *c_) : c (c_), + /* 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) {} From f7c45bc33ec1559c960a039b770d5c37bd82f057 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 22:15:13 -0400 Subject: [PATCH 08/26] [kerx] Allow granularly disabling kerning --- src/hb-aat-layout-kerx-table.hh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 881ab4e35..a96bad17e 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -162,6 +162,7 @@ struct KerxSubTableFormat1 return false; } + hb_mask_t kern_mask = c->plan->kern_mask; for (unsigned int i = 0; i < depth; i++) { /* Apparently, when spec says "Each pops one glyph from the kerning stack @@ -170,11 +171,14 @@ struct KerxSubTableFormat1 * list. Discovered by testing. */ unsigned int idx = stack[i]; int v = *actions++; - /* XXX Non-forward direction... */ - if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) - buffer->pos[idx].x_advance += c->font->em_scale_x (v); - else - buffer->pos[idx].y_advance += c->font->em_scale_y (v); + if (buffer->info[idx].mask & kern_mask) + { + /* XXX Non-forward direction... */ + if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction)) + buffer->pos[idx].x_advance += c->font->em_scale_x (v); + else + buffer->pos[idx].y_advance += c->font->em_scale_y (v); + } } depth = 0; } From 34caadc5c78e3d09faf11ef60bfade8f64f55de2 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 22:17:07 -0400 Subject: [PATCH 09/26] Ugh. Re-enable accidentally disabled GPOS --- 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 e2405237f..75f72edac 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -834,7 +834,7 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) } if (c->plan->apply_gpos) - ;//c->plan->position (c->font, c->buffer); + c->plan->position (c->font, c->buffer); else if (c->plan->apply_kerx) hb_aat_layout_position (c->plan, c->font, c->buffer); From 7281cb3eeb00091c6e6085895afd4a38a0516f35 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 22:56:52 -0400 Subject: [PATCH 10/26] [ankr] Start fixing --- src/hb-aat-layout-ankr-table.hh | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/hb-aat-layout-ankr-table.hh b/src/hb-aat-layout-ankr-table.hh index a197cec81..cc92ee291 100644 --- a/src/hb-aat-layout-ankr-table.hh +++ b/src/hb-aat-layout-ankr-table.hh @@ -60,17 +60,16 @@ struct ankr TRACE_SANITIZE (this); return_trace (likely (c->check_struct (this) && version == 0 && - lookupTable.sanitize (c, this) && - anchors.sanitize (c, this))); + lookupTable.sanitize (c, this))); } protected: HBUINT16 version; /* Version number (set to zero) */ HBUINT16 flags; /* Flags (currently unused; set to zero) */ - LOffsetTo > + LOffsetTo > > lookupTable; /* Offset to the table's lookup table */ - LOffsetTo > - anchors; /* Offset to the glyph data table */ + LOffsetTo + anchorData; /* Offset to the glyph data table */ public: DEFINE_SIZE_STATIC (12); From 947962a287d9aca2cb509c11f44cb5150aa6daf1 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 23:07:03 -0400 Subject: [PATCH 11/26] [ankr] Implement table access --- src/hb-aat-layout-ankr-table.hh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/hb-aat-layout-ankr-table.hh b/src/hb-aat-layout-ankr-table.hh index cc92ee291..3eac473a4 100644 --- a/src/hb-aat-layout-ankr-table.hh +++ b/src/hb-aat-layout-ankr-table.hh @@ -45,16 +45,32 @@ struct Anchor return_trace (c->check_struct (this)); } + public: FWORD xCoordinate; FWORD yCoordinate; public: DEFINE_SIZE_STATIC (4); }; +typedef LArrayOf GlyphAnchors; + struct ankr { static const hb_tag_t tableTag = HB_AAT_TAG_ankr; + inline const Anchor &get_anchor (hb_codepoint_t glyph_id, + unsigned int i, + unsigned int num_glyphs, + const char *end) const + { + unsigned int offset = (this+lookupTable).get_value_or_null (glyph_id, num_glyphs); + const GlyphAnchors &anchors = StructAtOffset (&(this+anchorData), offset); + /* TODO Use sanitizer; to avoid overflows and more. */ + if (unlikely ((const char *) &anchors + anchors.get_size () > end)) + return Null(Anchor); + return anchors[i]; + } + inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); From 28f0367aab648c486d6e8d0e13dbbb2af1b65dcc Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 00:12:49 -0400 Subject: [PATCH 12/26] [kerx] Flesh out Format4 Doesn't apply actions yet. --- src/hb-aat-layout-kerx-table.hh | 122 ++++++++++++++++++++++++++++++-- 1 file changed, 118 insertions(+), 4 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index a96bad17e..c109d003b 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -293,11 +293,121 @@ struct KerxSubTableFormat2 struct KerxSubTableFormat4 { + struct EntryData + { + HBUINT16 ankrActionIndex;/* Either 0xFFFF (for no action) or the index of + * the action to perform. */ + public: + DEFINE_SIZE_STATIC (2); + }; + + struct driver_context_t + { + static const bool in_place = true; + enum Flags + { + Mark = 0x8000, /* If set, remember this glyph as the marked glyph. */ + DontAdvance = 0x4000, /* If set, don't advance to the next glyph before + * going to the new state. */ + Reserved = 0x3FFF, /* Not used; set to 0. */ + }; + + enum SubTableFlags + { + ActionType = 0xC0000000, /* A two-bit field containing the action type. */ + Unused = 0x3F000000, /* Unused - must be zero. */ + Offset = 0x00FFFFFF, /* Masks the offset in bytes from the beginning + * of the subtable to the beginning of the control + * point table. */ + }; + + inline driver_context_t (const KerxSubTableFormat4 *table, + hb_aat_apply_context_t *c_) : + c (c_), + action_type ((table->flags & ActionType) >> 30), + ankrData ((HBUINT16 *) ((const char *) &table->machine + (table->flags & Offset))), + mark_set (false), + mark (0) {} + + inline bool is_actionable (StateTableDriver *driver, + const Entry *entry) + { + return entry->data.ankrActionIndex != 0xFFFF; + } + inline bool transition (StateTableDriver *driver, + const Entry *entry) + { + hb_buffer_t *buffer = driver->buffer; + unsigned int flags = entry->flags; + + if (mark_set && entry->data.ankrActionIndex != 0xFFFF) + { + switch (action_type) + { + case 0: /* Control Point Actions.*/ + { + /* indexed into glyph outline. */ + const HBUINT16 *data = &ankrData[entry->data.ankrActionIndex]; + if (!c->sanitizer.check_array (data, 2)) + return false; + HB_UNUSED unsigned int markControlPoint = *data++; + HB_UNUSED unsigned int currControlPoint = *data++; + /* TODO */ + } + break; + + case 1: /* Anchor Point Actions. */ + { + /* Indexed into 'ankr' table. */ + const HBUINT16 *data = &ankrData[entry->data.ankrActionIndex]; + if (!c->sanitizer.check_array (data, 2)) + return false; + HB_UNUSED unsigned int markAnchorPoint = *data++; + HB_UNUSED unsigned int currAnchorPoint = *data++; + /* TODO */ + } + break; + + case 2: /* Control Point Coordinate Actions. */ + { + const FWORD *data = (const FWORD *) &ankrData[entry->data.ankrActionIndex]; + if (!c->sanitizer.check_array (data, 4)) + return false; + HB_UNUSED int markX = *data++; + HB_UNUSED int markY = *data++; + HB_UNUSED int currX = *data++; + HB_UNUSED int currY = *data++; + /* TODO */ + } + break; + } + } + + if (flags & Mark) + { + mark_set = true; + mark = buffer->idx; + } + + return true; + } + + private: + hb_aat_apply_context_t *c; + unsigned int action_type; + const HBUINT16 *ankrData; + bool mark_set; + unsigned int mark; + }; + inline bool apply (hb_aat_apply_context_t *c) const { TRACE_APPLY (this); - /* TODO */ + driver_context_t dc (this, c); + + StateTableDriver driver (machine, c->buffer, c->font->face); + driver.drive (&dc); return_trace (true); } @@ -306,14 +416,18 @@ struct KerxSubTableFormat4 { TRACE_SANITIZE (this); - /* TODO */ - return_trace (likely (c->check_struct (this))); + /* The rest of array sanitizations are done at run-time. */ + return_trace (c->check_struct (this) && + machine.sanitize (c) && + flags.sanitize (c)); } protected: KerxSubTableHeader header; + StateTable machine; + HBUINT32 flags; public: - DEFINE_SIZE_STATIC (12); + DEFINE_SIZE_STATIC (32); }; struct KerxSubTableFormat6 From 7bb4da7d9538f3d4b1d28030d43e0c3d720d821b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 00:52:07 -0400 Subject: [PATCH 13/26] [aat] Wire up 'ankr' table to apply context --- src/hb-aat-layout-common.hh | 7 +++++-- src/hb-aat-layout.cc | 8 +++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 37ab53539..dada9c7a0 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -514,6 +514,7 @@ struct StateTableDriver }; +struct ankr; struct hb_aat_apply_context_t : hb_dispatch_context_t @@ -529,6 +530,7 @@ struct hb_aat_apply_context_t : hb_face_t *face; hb_buffer_t *buffer; hb_sanitize_context_t sanitizer; + const ankr &ankr_table; /* Unused. For debug tracing only. */ unsigned int lookup_index; @@ -537,9 +539,10 @@ struct hb_aat_apply_context_t : inline hb_aat_apply_context_t (hb_ot_shape_plan_t *plan_, hb_font_t *font_, hb_buffer_t *buffer_, - hb_blob_t *table) : + hb_blob_t *table, + const ankr &ankr_table_ = Null(ankr)) : plan (plan_), font (font_), face (font->face), buffer (buffer_), - sanitizer (), lookup_index (0), debug_depth (0) + sanitizer (), ankr_table (ankr_table_), lookup_index (0), debug_depth (0) { sanitizer.init (table); sanitizer.set_num_glyphs (face->get_num_glyphs ()); diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index 3fd4f9f89..47462ad7c 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -68,6 +68,12 @@ _get_kerx (hb_face_t *face, hb_blob_t **blob = nullptr) *blob = hb_ot_face_data (face)->kerx.get_blob (); return kerx; } +static inline const AAT::ankr& +_get_ankr (hb_face_t *face) +{ + if (unlikely (!hb_ot_shaper_face_data_ensure (face))) return Null(AAT::ankr); + return *(hb_ot_face_data (face)->ankr.get ()); +} hb_bool_t @@ -103,6 +109,6 @@ hb_aat_layout_position (hb_ot_shape_plan_t *plan, hb_blob_t *blob; const AAT::kerx& kerx = _get_kerx (font->face, &blob); - AAT::hb_aat_apply_context_t c (plan, font, buffer, blob); + AAT::hb_aat_apply_context_t c (plan, font, buffer, blob, _get_ankr (font->face)); kerx.apply (&c); } From 1622ba5943d14b2d50d45dc17fb723f4c9ddb0bb Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 01:14:18 -0400 Subject: [PATCH 14/26] [kerx] Implement Format4 'ankr'-based mark attachment Tested with Kannada MN: $ HB_OPTIONS=aat ./hb-shape Kannada\ MN.ttc -u 0CCD,0C95,0CD6 [kn_ka.vattu=0+230|kn_ai_length_mark=1@326,0+607] --- src/hb-aat-layout-common.hh | 8 ++++++-- src/hb-aat-layout-kerx-table.hh | 20 +++++++++++++++++--- src/hb-aat-layout.cc | 20 ++++++++++++++++---- src/hb-ot-layout-gpos-table.hh | 4 ---- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index dada9c7a0..78de04fa0 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -531,6 +531,7 @@ struct hb_aat_apply_context_t : hb_buffer_t *buffer; hb_sanitize_context_t sanitizer; const ankr &ankr_table; + const char *ankr_end; /* Unused. For debug tracing only. */ unsigned int lookup_index; @@ -540,9 +541,12 @@ struct hb_aat_apply_context_t : hb_font_t *font_, hb_buffer_t *buffer_, hb_blob_t *table, - const ankr &ankr_table_ = Null(ankr)) : + const ankr &ankr_table_ = Null(ankr), + const char *ankr_end_ = nullptr) : plan (plan_), font (font_), face (font->face), buffer (buffer_), - sanitizer (), ankr_table (ankr_table_), lookup_index (0), debug_depth (0) + sanitizer (), + ankr_table (ankr_table_), ankr_end (ankr_end_), + lookup_index (0), debug_depth (0) { sanitizer.init (table); sanitizer.set_num_glyphs (face->get_num_glyphs ()); diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index c109d003b..019efd57e 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -30,6 +30,7 @@ #include "hb-open-type.hh" #include "hb-aat-layout-common.hh" +#include "hb-ot-layout-gpos-table.hh" #include "hb-ot-kern-table.hh" /* @@ -362,9 +363,22 @@ struct KerxSubTableFormat4 const HBUINT16 *data = &ankrData[entry->data.ankrActionIndex]; if (!c->sanitizer.check_array (data, 2)) return false; - HB_UNUSED unsigned int markAnchorPoint = *data++; - HB_UNUSED unsigned int currAnchorPoint = *data++; - /* TODO */ + unsigned int markAnchorPoint = *data++; + unsigned int currAnchorPoint = *data++; + const Anchor markAnchor = c->ankr_table.get_anchor (c->buffer->info[mark].codepoint, + markAnchorPoint, + c->face->get_num_glyphs (), + c->ankr_end); + const Anchor currAnchor = c->ankr_table.get_anchor (c->buffer->cur ().codepoint, + currAnchorPoint, + c->face->get_num_glyphs (), + c->ankr_end); + hb_glyph_position_t &o = buffer->cur_pos(); + o.x_offset = c->font->em_scale_x (markAnchor.xCoordinate) - c->font->em_scale_x (currAnchor.xCoordinate); + o.y_offset = c->font->em_scale_y (markAnchor.yCoordinate) - c->font->em_scale_y (currAnchor.yCoordinate); + o.attach_type() = ATTACH_TYPE_MARK; + o.attach_chain() = (int) mark - (int) buffer->idx; + buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; } break; diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index 47462ad7c..73ab06d3a 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -69,10 +69,18 @@ _get_kerx (hb_face_t *face, hb_blob_t **blob = nullptr) return kerx; } static inline const AAT::ankr& -_get_ankr (hb_face_t *face) +_get_ankr (hb_face_t *face, hb_blob_t **blob = nullptr) { - if (unlikely (!hb_ot_shaper_face_data_ensure (face))) return Null(AAT::ankr); - return *(hb_ot_face_data (face)->ankr.get ()); + if (unlikely (!hb_ot_shaper_face_data_ensure (face))) + { + if (blob) + *blob = hb_blob_get_empty (); + return Null(AAT::ankr); + } + const AAT::ankr& ankr = *(hb_ot_face_data (face)->ankr.get ()); + if (blob) + *blob = hb_ot_face_data (face)->ankr.get_blob (); + return ankr; } @@ -109,6 +117,10 @@ hb_aat_layout_position (hb_ot_shape_plan_t *plan, hb_blob_t *blob; const AAT::kerx& kerx = _get_kerx (font->face, &blob); - AAT::hb_aat_apply_context_t c (plan, font, buffer, blob, _get_ankr (font->face)); + hb_blob_t *ankr_blob; + const AAT::ankr& ankr = _get_ankr (font->face, &ankr_blob); + + AAT::hb_aat_apply_context_t c (plan, font, buffer, blob, + ankr, ankr_blob->data + ankr_blob->length); kerx.apply (&c); } diff --git a/src/hb-ot-layout-gpos-table.hh b/src/hb-ot-layout-gpos-table.hh index 1a96609d0..746c24621 100644 --- a/src/hb-ot-layout-gpos-table.hh +++ b/src/hb-ot-layout-gpos-table.hh @@ -1755,10 +1755,6 @@ template struct GPOS_accelerator_t : GPOS::accelerator_t {}; -#undef attach_chain -#undef attach_type - - } /* namespace OT */ From b6bc0d4ff62e4509643db3b304306a72bbcb2c38 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 01:17:57 -0400 Subject: [PATCH 15/26] [kerx] Implement Format4 action_type=2 coordinate-based attachment Untested. --- src/hb-aat-layout-kerx-table.hh | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 019efd57e..a54e29781 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -343,6 +343,7 @@ struct KerxSubTableFormat4 if (mark_set && entry->data.ankrActionIndex != 0xFFFF) { + hb_glyph_position_t &o = buffer->cur_pos(); switch (action_type) { case 0: /* Control Point Actions.*/ @@ -373,12 +374,9 @@ struct KerxSubTableFormat4 currAnchorPoint, c->face->get_num_glyphs (), c->ankr_end); - hb_glyph_position_t &o = buffer->cur_pos(); + o.x_offset = c->font->em_scale_x (markAnchor.xCoordinate) - c->font->em_scale_x (currAnchor.xCoordinate); o.y_offset = c->font->em_scale_y (markAnchor.yCoordinate) - c->font->em_scale_y (currAnchor.yCoordinate); - o.attach_type() = ATTACH_TYPE_MARK; - o.attach_chain() = (int) mark - (int) buffer->idx; - buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; } break; @@ -387,14 +385,19 @@ struct KerxSubTableFormat4 const FWORD *data = (const FWORD *) &ankrData[entry->data.ankrActionIndex]; if (!c->sanitizer.check_array (data, 4)) return false; - HB_UNUSED int markX = *data++; - HB_UNUSED int markY = *data++; - HB_UNUSED int currX = *data++; - HB_UNUSED int currY = *data++; - /* TODO */ + int markX = *data++; + int markY = *data++; + int currX = *data++; + int currY = *data++; + + o.x_offset = c->font->em_scale_x (markX) - c->font->em_scale_x (currX); + o.y_offset = c->font->em_scale_y (markY) - c->font->em_scale_y (currY); } break; } + o.attach_type() = ATTACH_TYPE_MARK; + o.attach_chain() = (int) mark - (int) buffer->idx; + buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; } if (flags & Mark) From fbbd926dba163d9a2a6a62f380951f03363c2b14 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 01:22:29 -0400 Subject: [PATCH 16/26] [kerx] Implement Format4 action_type=1 contour-point-based attachment Untested. This concludes kerx table support! --- src/hb-aat-layout-kerx-table.hh | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index a54e29781..cd1129128 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -354,7 +354,22 @@ struct KerxSubTableFormat4 return false; HB_UNUSED unsigned int markControlPoint = *data++; HB_UNUSED unsigned int currControlPoint = *data++; - /* TODO */ + hb_position_t markX = 0; + hb_position_t markY = 0; + hb_position_t currX = 0; + hb_position_t currY = 0; + if (!c->font->get_glyph_contour_point_for_origin (c->buffer->info[mark].codepoint, + markControlPoint, + HB_DIRECTION_LTR /*XXX*/, + &markX, &markY) || + !c->font->get_glyph_contour_point_for_origin (c->buffer->cur ().codepoint, + currControlPoint, + HB_DIRECTION_LTR /*XXX*/, + &currX, &currY)) + return true; /* True, such that the machine continues. */ + + o.x_offset = markX - currX; + o.y_offset = markY - currY; } break; From 071a2cbcddcbafae9458e674c21db5001b39518d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 10:18:46 -0400 Subject: [PATCH 17/26] [trak] Clean up --- src/hb-aat-layout-trak-table.hh | 69 ++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 3b7d43881..71f169c5c 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -46,28 +46,32 @@ struct TrackTableEntry { friend struct TrackData; - inline bool sanitize (hb_sanitize_context_t *c, const void *base, - unsigned int size) const - { - TRACE_SANITIZE (this); - return_trace (likely (c->check_struct (this) && - (valuesZ.sanitize (c, base, size)))); - } - - private: inline float get_track_value () const { return track.to_float (); } - inline int get_value (const void *base, unsigned int index) const + inline int get_value (const void *base, + unsigned int index, + unsigned int nSizes) const { - return (base+valuesZ)[index]; + return hb_array_t ((base+valuesZ).arrayZ, nSizes)[index]; + } + + public: + inline bool sanitize (hb_sanitize_context_t *c, const void *base, + unsigned int nSizes) const + { + TRACE_SANITIZE (this); + return_trace (likely (c->check_struct (this) && + (valuesZ.sanitize (c, base, nSizes)))); } protected: Fixed track; /* Track value for this record. */ - NameID trackNameID; /* The 'name' table index for this track */ + NameID trackNameID; /* The 'name' table index for this track. + * (a short word or phrase like "loose" + * or "very tight") */ OffsetTo, HBUINT16, false> valuesZ; /* Offset from start of tracking table to * per-size tracking values for this track. */ @@ -78,14 +82,6 @@ struct TrackTableEntry struct TrackData { - inline bool sanitize (hb_sanitize_context_t *c, const void *base) const - { - TRACE_SANITIZE (this); - return_trace (c->check_struct (this) && - sizeTable.sanitize (c, base, nSizes) && - trackTable.sanitize (c, nTracks, base, nSizes)); - } - inline float get_tracking (const void *base, float ptem) const { /* CoreText points are CSS pixels (96 per inch), @@ -120,22 +116,31 @@ struct TrackData // TODO(ebraminio): We don't attempt to extrapolate to larger or // smaller values for now but we should do, per spec if (size_index == sizes) - return trackTableEntry->get_value (base, sizes - 1); + return trackTableEntry->get_value (base, sizes - 1, nSizes); if (size_index == 0 || size_table[size_index] == fixed_size) - return trackTableEntry->get_value (base, size_index); + return trackTableEntry->get_value (base, size_index, nSizes); float s0 = size_table[size_index - 1].to_float (); float s1 = size_table[size_index].to_float (); float t = (csspx - s0) / (s1 - s0); - return (float) t * trackTableEntry->get_value (base, size_index) + - ((float) 1.0 - t) * trackTableEntry->get_value (base, size_index - 1); + return (float) t * trackTableEntry->get_value (base, size_index, nSizes) + + ((float) 1.0 - t) * trackTableEntry->get_value (base, size_index - 1, nSizes); + } + + inline bool sanitize (hb_sanitize_context_t *c, const void *base) const + { + TRACE_SANITIZE (this); + return_trace (c->check_struct (this) && + sizeTable.sanitize (c, base, nSizes) && + trackTable.sanitize (c, nTracks, base, nSizes)); } protected: HBUINT16 nTracks; /* Number of separate tracks included in this table. */ HBUINT16 nSizes; /* Number of point sizes included in this table. */ LOffsetTo, false> - sizeTable; /* Offset to array[nSizes] of size values. */ + sizeTable; /* Offset from start of the tracking table to + * Array[nSizes] of size values.. */ UnsizedArrayOf trackTable; /* Array[nTracks] of TrackTableEntry records. */ @@ -194,15 +199,17 @@ struct trak } protected: - FixedVersion<> version; /* Version of the tracking table--currently - * 0x00010000u for version 1.0. */ - HBUINT16 format; /* Format of the tracking table */ - OffsetTo horizData; /* TrackData for horizontal text */ - OffsetTo vertData; /* TrackData for vertical text */ + FixedVersion<> version; /* Version of the tracking table + * (0x00010000u for version 1.0). */ + HBUINT16 format; /* Format of the tracking table (set to 0). */ + OffsetTo horizData; /* Offset from start of tracking table to TrackData + * for horizontal text (or 0 if none). */ + OffsetTo vertData; /* Offset from start of tracking table to TrackData + * for vertical text (or 0 if none). */ HBUINT16 reserved; /* Reserved. Set to 0. */ public: - DEFINE_SIZE_MIN (12); + DEFINE_SIZE_STATIC (12); }; } /* namespace AAT */ From d06c4a867f0d383d8c27f2957e646d9e3fe6853b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 10:22:01 -0400 Subject: [PATCH 18/26] [trak] Only adjust around first glyph Assumes graphemes only have one base glyph. --- src/hb-aat-layout-trak-table.hh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 71f169c5c..dbad449be 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -174,24 +174,24 @@ struct trak { const TrackData &trackData = this+horizData; float tracking = trackData.get_tracking (this, ptem); - hb_position_t advance_to_add = c->font->em_scalef_x (tracking / 2); + hb_position_t offset_to_add = c->font->em_scalef_x (tracking / 2); + hb_position_t advance_to_add = c->font->em_scalef_x (tracking); foreach_grapheme (buffer, start, end) { - buffer->pos[start].x_offset += advance_to_add; buffer->pos[start].x_advance += advance_to_add; - buffer->pos[end].x_advance += advance_to_add; + buffer->pos[start].x_offset += offset_to_add; } } else { const TrackData &trackData = this+vertData; float tracking = trackData.get_tracking (this, ptem); - hb_position_t advance_to_add = c->font->em_scalef_y (tracking / 2); + hb_position_t offset_to_add = c->font->em_scalef_y (tracking / 2); + hb_position_t advance_to_add = c->font->em_scalef_y (tracking); foreach_grapheme (buffer, start, end) { - buffer->pos[start].y_offset += advance_to_add; buffer->pos[start].y_advance += advance_to_add; - buffer->pos[end].y_advance += advance_to_add; + buffer->pos[start].y_offset += offset_to_add; } } From a5be380cae9b49ed85c8620f1921209ef61a72ad Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 10:29:02 -0400 Subject: [PATCH 19/26] [trak] More --- src/hb-aat-layout-trak-table.hh | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index dbad449be..9054922b8 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -98,33 +98,41 @@ struct TrackData unsigned int sizes = nSizes; const TrackTableEntry *trackTableEntry = nullptr; - for (unsigned int i = 0; i < sizes; ++i) - // For now we only seek for track entries with zero tracking value - if (trackTable[i].get_track_value () == 0.f) - trackTableEntry = &trackTable[0]; + for (unsigned int i = 0; i < sizes; i++) + { + /* Note: Seems like the track entries are sorted by values. But the + * spec doesn't explicitly say that. It just mentions it in the example. */ + + /* For now we only seek for track entries with zero tracking value */ + + if (trackTable[i].get_track_value () == 0.f) + { + trackTableEntry = &trackTable[0]; + break; + } + } - // We couldn't match any, exit if (!trackTableEntry) return 0.; /* TODO bfind() */ unsigned int size_index; UnsizedArrayOf size_table = base+sizeTable; - for (size_index = 0; size_index < sizes; ++size_index) + for (size_index = 0; size_index < sizes; size_index++) if (size_table[size_index] >= fixed_size) break; // TODO(ebraminio): We don't attempt to extrapolate to larger or // smaller values for now but we should do, per spec if (size_index == sizes) - return trackTableEntry->get_value (base, sizes - 1, nSizes); + return trackTableEntry->get_value (base, sizes - 1, sizes); if (size_index == 0 || size_table[size_index] == fixed_size) - return trackTableEntry->get_value (base, size_index, nSizes); + return trackTableEntry->get_value (base, size_index, sizes); float s0 = size_table[size_index - 1].to_float (); float s1 = size_table[size_index].to_float (); float t = (csspx - s0) / (s1 - s0); - return (float) t * trackTableEntry->get_value (base, size_index, nSizes) + - ((float) 1.0 - t) * trackTableEntry->get_value (base, size_index - 1, nSizes); + return (float) t * trackTableEntry->get_value (base, size_index, sizes) + + ((float) 1.0 - t) * trackTableEntry->get_value (base, size_index - 1, sizes); } inline bool sanitize (hb_sanitize_context_t *c, const void *base) const From 451f3de521ff1b7f4d3b8ebb2cc0b95d88c9314a Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 10:30:32 -0400 Subject: [PATCH 20/26] [trak] Fix counting --- src/hb-aat-layout-trak-table.hh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 9054922b8..63dd890c5 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -95,10 +95,9 @@ struct TrackData /* XXX Clean this up. Make it work with nSizes==1 and 0. */ - unsigned int sizes = nSizes; - const TrackTableEntry *trackTableEntry = nullptr; - for (unsigned int i = 0; i < sizes; i++) + unsigned int count = nTracks; + for (unsigned int i = 0; i < count; i++) { /* Note: Seems like the track entries are sorted by values. But the * spec doesn't explicitly say that. It just mentions it in the example. */ @@ -111,9 +110,10 @@ struct TrackData break; } } - if (!trackTableEntry) return 0.; + unsigned int sizes = nSizes; + /* TODO bfind() */ unsigned int size_index; UnsizedArrayOf size_table = base+sizeTable; From 3d7dea6dfdc9e75dcca100a79525aa3736dbe29c Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 10:32:08 -0400 Subject: [PATCH 21/26] [trak] Handle nSizes=0 and 1 --- src/hb-aat-layout-trak-table.hh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 63dd890c5..368c55c12 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -93,7 +93,9 @@ struct TrackData Fixed fixed_size; fixed_size.set_float (csspx); - /* XXX Clean this up. Make it work with nSizes==1 and 0. */ + /* + * Choose track. + */ const TrackTableEntry *trackTableEntry = nullptr; unsigned int count = nTracks; @@ -112,8 +114,15 @@ struct TrackData } if (!trackTableEntry) return 0.; + /* + * Choose size. + */ + unsigned int sizes = nSizes; + if (!sizes) return 0.; + if (sizes == 1) return trackTableEntry->get_value (base, 0, sizes); + /* TODO bfind() */ unsigned int size_index; UnsizedArrayOf size_table = base+sizeTable; From d6a12dba6da6262fd9e5d8397b46ac8516136cae Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 11:10:06 -0400 Subject: [PATCH 22/26] [trak] Fix, and hook up Works beautifully! Test coming. --- src/hb-aat-layout-common.hh | 4 ++-- src/hb-aat-layout-trak-table.hh | 6 ++++-- src/hb-aat-layout.cc | 25 ++++++++++++++++++++++++- src/hb-aat-layout.hh | 8 ++++++++ src/hb-ot-shape.cc | 2 ++ 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 78de04fa0..78a27a74c 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -540,7 +540,7 @@ struct hb_aat_apply_context_t : inline hb_aat_apply_context_t (hb_ot_shape_plan_t *plan_, hb_font_t *font_, hb_buffer_t *buffer_, - hb_blob_t *table, + hb_blob_t *blob = const_cast (&Null(hb_blob_t)), const ankr &ankr_table_ = Null(ankr), const char *ankr_end_ = nullptr) : plan (plan_), font (font_), face (font->face), buffer (buffer_), @@ -548,7 +548,7 @@ struct hb_aat_apply_context_t : ankr_table (ankr_table_), ankr_end (ankr_end_), lookup_index (0), debug_depth (0) { - sanitizer.init (table); + sanitizer.init (blob); sanitizer.set_num_glyphs (face->get_num_glyphs ()); sanitizer.start_processing (); sanitizer.set_max_ops (HB_SANITIZE_MAX_OPS_MAX); diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index 368c55c12..b2d694132 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -125,9 +125,9 @@ struct TrackData /* TODO bfind() */ unsigned int size_index; - UnsizedArrayOf size_table = base+sizeTable; + hb_array_t size_table ((base+sizeTable).arrayZ, sizes); for (size_index = 0; size_index < sizes; size_index++) - if (size_table[size_index] >= fixed_size) + if ((int) size_table[size_index] >= (int) fixed_size) break; // TODO(ebraminio): We don't attempt to extrapolate to larger or @@ -169,6 +169,8 @@ struct trak { static const hb_tag_t tableTag = HB_AAT_TAG_trak; + inline bool has_data (void) const { return version.to_int () != 0; } + inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index 73ab06d3a..2b86ba8c7 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -37,7 +37,7 @@ #include "hb-aat-ltag-table.hh" // Just so we compile it; unused otherwise. /* - * morx/kerx/trak/ankr + * morx/kerx/trak */ static inline const AAT::morx& @@ -82,6 +82,12 @@ _get_ankr (hb_face_t *face, hb_blob_t **blob = nullptr) *blob = hb_ot_face_data (face)->ankr.get_blob (); return ankr; } +static inline const AAT::trak& +_get_trak (hb_face_t *face) +{ + if (unlikely (!hb_ot_shaper_face_data_ensure (face))) return Null(AAT::trak); + return *(hb_ot_face_data (face)->trak.get ()); +} hb_bool_t @@ -124,3 +130,20 @@ hb_aat_layout_position (hb_ot_shape_plan_t *plan, ankr, ankr_blob->data + ankr_blob->length); kerx.apply (&c); } + +hb_bool_t +hb_aat_layout_has_tracking (hb_face_t *face) +{ + return _get_trak (face).has_data (); +} + +void +hb_aat_layout_track (hb_ot_shape_plan_t *plan, + hb_font_t *font, + hb_buffer_t *buffer) +{ + const AAT::trak& trak = _get_trak (font->face); + + AAT::hb_aat_apply_context_t c (plan, font, buffer); + trak.apply (&c); +} diff --git a/src/hb-aat-layout.hh b/src/hb-aat-layout.hh index aafc3278f..897c26aa0 100644 --- a/src/hb-aat-layout.hh +++ b/src/hb-aat-layout.hh @@ -47,4 +47,12 @@ hb_aat_layout_position (hb_ot_shape_plan_t *plan, hb_font_t *font, hb_buffer_t *buffer); +HB_INTERNAL hb_bool_t +hb_aat_layout_has_tracking (hb_face_t *face); + +HB_INTERNAL void +hb_aat_layout_track (hb_ot_shape_plan_t *plan, + hb_font_t *font, + hb_buffer_t *buffer); + #endif /* HB_AAT_LAYOUT_HH */ diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 75f72edac..3ca54ac8c 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -838,6 +838,8 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) else if (c->plan->apply_kerx) hb_aat_layout_position (c->plan, c->font, c->buffer); + hb_aat_layout_track (c->plan, c->font, c->buffer); + if (!c->plan->apply_kerx) switch (c->plan->shaper->zero_width_marks) { From 04f72e8990ea61ffc6b62105c75e0a3e1b1ebab4 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 11:25:07 -0400 Subject: [PATCH 23/26] [trak] Implement extrapolation This concludes trak, as well as AAT shaping support! --- src/hb-aat-layout-trak-table.hh | 39 +++++++++++++++++---------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index b2d694132..e02721750 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -82,6 +82,22 @@ struct TrackTableEntry struct TrackData { + inline float interpolate_at (unsigned int idx, + float target_size, + const TrackTableEntry &trackTableEntry, + const void *base) const + { + unsigned int sizes = nSizes; + hb_array_t size_table ((base+sizeTable).arrayZ, sizes); + + float s0 = size_table[idx].to_float (); + float s1 = size_table[idx + 1].to_float (); + float t = unlikely (s0 == s1) ? 0.f : (target_size - s0) / (s1 - s0); + return (float) t * trackTableEntry.get_value (base, idx + 1, sizes) + + ((float) 1.0 - t) * trackTableEntry.get_value (base, idx, sizes); + return 0; + } + inline float get_tracking (const void *base, float ptem) const { /* CoreText points are CSS pixels (96 per inch), @@ -90,13 +106,10 @@ struct TrackData * https://developer.apple.com/library/content/documentation/GraphicsAnimation/Conceptual/HighResolutionOSX/Explained/Explained.html */ float csspx = ptem * 96.f / 72.f; - Fixed fixed_size; - fixed_size.set_float (csspx); /* * Choose track. */ - const TrackTableEntry *trackTableEntry = nullptr; unsigned int count = nTracks; for (unsigned int i = 0; i < count; i++) @@ -117,31 +130,19 @@ struct TrackData /* * Choose size. */ - unsigned int sizes = nSizes; - if (!sizes) return 0.; if (sizes == 1) return trackTableEntry->get_value (base, 0, sizes); /* TODO bfind() */ - unsigned int size_index; hb_array_t size_table ((base+sizeTable).arrayZ, sizes); + unsigned int size_index; for (size_index = 0; size_index < sizes; size_index++) - if ((int) size_table[size_index] >= (int) fixed_size) + if (size_table[size_index].to_float () >= csspx) break; - // TODO(ebraminio): We don't attempt to extrapolate to larger or - // smaller values for now but we should do, per spec - if (size_index == sizes) - return trackTableEntry->get_value (base, sizes - 1, sizes); - if (size_index == 0 || size_table[size_index] == fixed_size) - return trackTableEntry->get_value (base, size_index, sizes); - - float s0 = size_table[size_index - 1].to_float (); - float s1 = size_table[size_index].to_float (); - float t = (csspx - s0) / (s1 - s0); - return (float) t * trackTableEntry->get_value (base, size_index, sizes) + - ((float) 1.0 - t) * trackTableEntry->get_value (base, size_index - 1, sizes); + return interpolate_at (size_index ? size_index - 1 : 0, csspx, + *trackTableEntry, base); } inline bool sanitize (hb_sanitize_context_t *c, const void *base) const From 100e95f48e3d137c654d206e858d6419ea62a12c Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 11:30:45 -0400 Subject: [PATCH 24/26] [trak] Add tests --- test/shaping/data/in-house/Makefile.sources | 1 + test/shaping/data/in-house/fonts/TestTRAK.ttf | Bin 0 -> 2456 bytes test/shaping/data/in-house/tests/aat-trak.tests | 8 ++++++++ 3 files changed, 9 insertions(+) create mode 100644 test/shaping/data/in-house/fonts/TestTRAK.ttf create mode 100644 test/shaping/data/in-house/tests/aat-trak.tests diff --git a/test/shaping/data/in-house/Makefile.sources b/test/shaping/data/in-house/Makefile.sources index 1f7eff202..f2402740f 100644 --- a/test/shaping/data/in-house/Makefile.sources +++ b/test/shaping/data/in-house/Makefile.sources @@ -1,4 +1,5 @@ TESTS = \ + tests/aat-trak.tests \ tests/arabic-fallback-shaping.tests \ tests/arabic-feature-order.tests \ tests/arabic-like-joining.tests \ diff --git a/test/shaping/data/in-house/fonts/TestTRAK.ttf b/test/shaping/data/in-house/fonts/TestTRAK.ttf new file mode 100644 index 0000000000000000000000000000000000000000..07ae3afd81ecfbf1c36c3e5a173c36b6a754513d GIT binary patch literal 2456 zcmai0O-vg{6n?X7Y(OqxY_9RE8-M$PNN6E4 zwWm{wdE0Bp+*sQ{SJMz!eh_2OH-Ya;FBX^Ua!dk$7kpbLn@rUFY?J$ z(>!sLttM)cdWuqwCStI4Yc!9{n<#$3ZbMQlyepgme_Ws0>l9%7xq@!UDPucz{PYR< zm1#_rfngmO)}g`oLhMfgyTcm6=P>uHBjhNze_W9#R77`R6O^}DNHZof}M_FKn*+1$}?e3$C78F2jtdnm#1e|>h16!dN#&)F2-Zo z_9?4%Sj}0jr*V}l(Yg4^+?`EdIOqzBiWpYIl1p;Bj6jZ|nuS~M-Tb_<&eULOI%x>~ zgja9#`okfAbEK6;t4>ybbu_IwUSCK%V2Wb12Lq@bs-zWBF$QlrGU;=-DdSTsY4?EC zDtEQD40wzP4u$sZMM-)Tzkl-v3w7EY6RzfN+=iW+c&kyU;SfLVIO_=O3iE@Ga~?F5 z;HbZ&SDNkdxVl^A1JN_dSlkf#AL+E!np20Z)~WVvEPj#Sz@3;K)iHOST0^YL8)S%L zIDq&D>_n)lqrsuU3GR0-o#$T~Qyu6Cx=u4{0;kggHPA$vHsPGa%Cs5uwKDwzbJJ>> zZlDwF>oRR8vtXt%%2JN5Ql2i*0;Q=4%TLe)_`(d(5U5kMgta8}<|#${AsL4xNqsZ} zz5t6EnxYXpjxhxvmnlOD_}jfIKWDwSYCVm(1;jgw*h?C+X;9}uUC|<>u%>HdG=wQ= z6@XzL^2?w(-aP0c_QiFlfqVkv1R0pa# zVL!2LQz%>UD(d^sV()qQNLAPvkruYS^p^KELZdhd`U{7t^cCv=f2|%uk-Er3UM4V+ znOF@o*NpTfGR2xvZ5-pHbuM`!pUkA@$Fhq#+uXTKB6(?SN3s=%!5i-r==^PE_!jZT z@Otuhl&@{IE*X~-4AWj)0e6G13V3@=WU0`$Y6iNec8wNrW_4ZuAB^ZM`Z|C8OCSCR D*gN_- literal 0 HcmV?d00001 diff --git a/test/shaping/data/in-house/tests/aat-trak.tests b/test/shaping/data/in-house/tests/aat-trak.tests new file mode 100644 index 000000000..9e6505585 --- /dev/null +++ b/test/shaping/data/in-house/tests/aat-trak.tests @@ -0,0 +1,8 @@ +../fonts/TestTRAK.ttf::U+0041,U+0042,U+0043:[A.alt=0+1000|B=1+1000|C.alt=2+1000] +../fonts/TestTRAK.ttf:--font-ptem=.5:U+0041,U+0042,U+0043:[A.alt=0@100,0+1200|B=1@100,0+1200|C.alt=2@100,0+1200] +../fonts/TestTRAK.ttf:--font-ptem=1:U+0041,U+0042,U+0043:[A.alt=0@100,0+1200|B=1@100,0+1200|C.alt=2@100,0+1200] +../fonts/TestTRAK.ttf:--font-ptem=2:U+0041,U+0042,U+0043:[A.alt=0@93,0+1187|B=1@93,0+1187|C.alt=2@93,0+1187] +../fonts/TestTRAK.ttf:--font-ptem=9:U+0041,U+0042,U+0043:[A.alt=0+1000|B=1+1000|C.alt=2+1000] +../fonts/TestTRAK.ttf:--font-ptem=24:U+0041,U+0042,U+0043:[A.alt=0@-12,0+976|B=1@-12,0+976|C.alt=2@-12,0+976] +../fonts/TestTRAK.ttf:--font-ptem=72:U+0041,U+0042,U+0043:[A.alt=0@-50,0+900|B=1@-50,0+900|C.alt=2@-50,0+900] +../fonts/TestTRAK.ttf:--font-ptem=144:U+0041,U+0042,U+0043:[A.alt=0@-100,0+800|B=1@-100,0+800|C.alt=2@-100,0+800] From b59a428af08d6451a47f40ed01e594815ebf6303 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 13:24:17 -0400 Subject: [PATCH 25/26] Minor --- src/hb-ot-shape.cc | 6 +++++- src/hb-ot-shape.hh | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 3ca54ac8c..2b147e345 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -116,6 +116,9 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, 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; + + /* Currently we always apply trak. */ + plan.apply_trak = hb_aat_layout_has_tracking (face); } @@ -838,7 +841,8 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) else if (c->plan->apply_kerx) hb_aat_layout_position (c->plan, c->font, c->buffer); - hb_aat_layout_track (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) switch (c->plan->shaper->zero_width_marks) diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh index 4943c5155..c9c0d3e09 100644 --- a/src/hb-ot-shape.hh +++ b/src/hb-ot-shape.hh @@ -50,10 +50,11 @@ struct hb_ot_shape_plan_t bool fallback_kerning : 1; bool fallback_mark_positioning : 1; - bool apply_morx : 1; + bool apply_gpos : 1; bool apply_kerx : 1; bool apply_kern : 1; - bool apply_gpos : 1; + bool apply_morx : 1; + bool apply_trak : 1; inline void collect_lookups (hb_tag_t table_tag, hb_set_t *lookups) const From 0b9d60e1a1c4b7867ac907bbd7c004191a14e697 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 11 Oct 2018 13:26:58 -0400 Subject: [PATCH 26/26] [aat] Apply kerx if GPOS kern was not applied Ned tells me this is what Apple does. --- src/hb-ot-shape.cc | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 2b147e345..a5538871b 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -100,17 +100,15 @@ 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) + if (plan.requested_kerning && !plan.apply_kerx && !has_gpos_kern) { - if (plan.apply_kerx) - ;/* kerx supercedes kern. */ - else if (!has_gpos_kern) - { - if (hb_ot_layout_has_kerning (face)) - plan.apply_kern = true; - else - plan.fallback_kerning = true; - } + /* 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)) + plan.apply_kern = true; + else + plan.fallback_kerning = true; } plan.has_gpos_mark = !!plan.map.get_1_mask (HB_TAG ('m','a','r','k'));