From c4e36f97b6df1eb5fba588b09ae1630bb5c49589 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 17:06:16 +0100 Subject: [PATCH 1/9] [AAT] Minor --- src/hb-aat-layout-common.hh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index df41741f5..84f043311 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -529,6 +529,7 @@ struct StateTable { TRACE_SANITIZE (this); if (unlikely (!(c->check_struct (this) && + nClasses >= 4 /* Ensure pre-defined classes fit. */ && classTable.sanitize (c, this)))) return_trace (false); const HBUSHORT *states = (this+stateArrayTable).arrayZ; @@ -773,7 +774,7 @@ struct StateTableDriver /* Unsafe-to-break if end-of-text would kick in here. */ if (buffer->idx + 2 <= buffer->len) { - const Entry *end_entry = machine.get_entryZ (state, 0); + const Entry *end_entry = machine.get_entryZ (state, StateTable::CLASS_END_OF_TEXT); if (c->is_actionable (this, end_entry)) buffer->unsafe_to_break (buffer->idx, buffer->idx + 2); } From c4623db4a3f963394db940449007fa3312460993 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 17:10:12 +0100 Subject: [PATCH 2/9] [AAT] Minor --- src/hb-aat-layout-common.hh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 84f043311..e3b2a2be1 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -572,7 +572,7 @@ struct StateTable -min_state, row_stride))) return_trace (false); - if ((c->max_ops -= state_neg - min_state) < 0) + if ((c->max_ops -= state_neg - min_state) <= 0) return_trace (false); { /* Sweep new states. */ const HBUSHORT *stop = &states[min_state * num_classes]; @@ -591,7 +591,7 @@ struct StateTable max_state + 1, row_stride))) return_trace (false); - if ((c->max_ops -= max_state - state_pos + 1) < 0) + 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))) @@ -607,7 +607,7 @@ struct StateTable if (unlikely (!c->check_array (entries, num_entries))) return_trace (false); - if ((c->max_ops -= num_entries - entry) < 0) + if ((c->max_ops -= num_entries - entry) <= 0) return_trace (false); { /* Sweep new entries. */ const Entry *stop = &entries[num_entries]; @@ -746,7 +746,6 @@ struct StateTableDriver buffer->clear_output (); 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 ? @@ -782,15 +781,13 @@ struct StateTableDriver if (unlikely (!c->transition (this, entry))) break; - 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; - if (!last_was_dont_advance) + if (!(entry->flags & context_t::DontAdvance) || buffer->max_ops-- <= 0) buffer->next_glyph (); } From 299eca0c3b28c99add006420bc667431d874fb2e Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 17:17:00 +0100 Subject: [PATCH 3/9] [AAT] Handle out-of-bounds classes --- src/hb-aat-layout-common.hh | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index e3b2a2be1..c6205e752 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -511,9 +511,10 @@ struct StateTable const Entry *get_entries () const { return (this+entryTable).arrayZ; } - const Entry *get_entryZ (int state, unsigned int klass) const + const Entry &get_entry (int state, unsigned int klass) const { - if (unlikely (klass >= nClasses)) return nullptr; + if (unlikely (klass >= nClasses)) + klass = StateTable >::CLASS_OUT_OF_BOUNDS; const HBUSHORT *states = (this+stateArrayTable).arrayZ; const Entry *entries = (this+entryTable).arrayZ; @@ -521,7 +522,7 @@ struct StateTable unsigned int entry = states[state * nClasses + klass]; DEBUG_MSG (APPLY, nullptr, "e%u", entry); - return &entries[entry]; + return entries[entry]; } bool sanitize (hb_sanitize_context_t *c, @@ -752,9 +753,7 @@ struct StateTableDriver machine.get_class (buffer->info[buffer->idx].codepoint, num_glyphs) : (unsigned) StateTable::CLASS_END_OF_TEXT; DEBUG_MSG (APPLY, nullptr, "c%u at %u", klass, buffer->idx); - const Entry *entry = machine.get_entryZ (state, klass); - if (unlikely (!entry)) - break; + const Entry *entry = &machine.get_entry (state, klass); /* Unsafe-to-break before this if not in state 0, as things might * go differently if we start from state 0 here. @@ -773,7 +772,7 @@ struct StateTableDriver /* Unsafe-to-break if end-of-text would kick in here. */ if (buffer->idx + 2 <= buffer->len) { - const Entry *end_entry = machine.get_entryZ (state, StateTable::CLASS_END_OF_TEXT); + const Entry *end_entry = &machine.get_entry (state, StateTable::CLASS_END_OF_TEXT); if (c->is_actionable (this, end_entry)) buffer->unsafe_to_break (buffer->idx, buffer->idx + 2); } From 1ec90514f69efc329691186466f62373efa863b1 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 17:21:41 +0100 Subject: [PATCH 4/9] [AAT] Minor --- src/hb-aat-layout-common.hh | 12 +++--- src/hb-aat-layout-kerx-table.hh | 38 +++++++++--------- src/hb-aat-layout-morx-table.hh | 68 ++++++++++++++++----------------- 3 files changed, 59 insertions(+), 59 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index c6205e752..76ba98997 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -753,7 +753,7 @@ struct StateTableDriver machine.get_class (buffer->info[buffer->idx].codepoint, num_glyphs) : (unsigned) StateTable::CLASS_END_OF_TEXT; DEBUG_MSG (APPLY, nullptr, "c%u at %u", klass, buffer->idx); - const Entry *entry = &machine.get_entry (state, klass); + const Entry &entry = machine.get_entry (state, klass); /* Unsafe-to-break before this if not in state 0, as things might * go differently if we start from state 0 here. @@ -764,15 +764,15 @@ struct StateTableDriver /* If there's no action and we're just epsilon-transitioning to state 0, * safe to break. */ if (c->is_actionable (this, entry) || - !(entry->newState == StateTable::STATE_START_OF_TEXT && - entry->flags == context_t::DontAdvance)) + !(entry.newState == StateTable::STATE_START_OF_TEXT && + entry.flags == context_t::DontAdvance)) buffer->unsafe_to_break_from_outbuffer (buffer->backtrack_len () - 1, buffer->idx + 1); } /* Unsafe-to-break if end-of-text would kick in here. */ if (buffer->idx + 2 <= buffer->len) { - const Entry *end_entry = &machine.get_entry (state, StateTable::CLASS_END_OF_TEXT); + const Entry &end_entry = machine.get_entry (state, StateTable::CLASS_END_OF_TEXT); if (c->is_actionable (this, end_entry)) buffer->unsafe_to_break (buffer->idx, buffer->idx + 2); } @@ -780,13 +780,13 @@ struct StateTableDriver if (unlikely (!c->transition (this, entry))) break; - state = machine.new_state (entry->newState); + state = machine.new_state (entry.newState); DEBUG_MSG (APPLY, nullptr, "s%d", state); if (buffer->idx == buffer->len) break; - if (!(entry->flags & context_t::DontAdvance) || buffer->max_ops-- <= 0) + if (!(entry.flags & context_t::DontAdvance) || buffer->max_ops-- <= 0) buffer->next_glyph (); } diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index ac562e408..0630fba6c 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -170,11 +170,11 @@ struct Format1Entry DEFINE_SIZE_STATIC (2); }; - static bool performAction (const Entry *entry) - { return entry->data.kernActionIndex != 0xFFFF; } + static bool performAction (const Entry &entry) + { return entry.data.kernActionIndex != 0xFFFF; } - static unsigned int kernActionIndex (const Entry *entry) - { return entry->data.kernActionIndex; } + static unsigned int kernActionIndex (const Entry &entry) + { return entry.data.kernActionIndex; } }; template <> struct Format1Entry @@ -192,11 +192,11 @@ struct Format1Entry typedef void EntryData; - static bool performAction (const Entry *entry) - { return entry->flags & Offset; } + static bool performAction (const Entry &entry) + { return entry.flags & Offset; } - static unsigned int kernActionIndex (const Entry *entry) - { return entry->flags & Offset; } + static unsigned int kernActionIndex (const Entry &entry) + { return entry.flags & Offset; } }; template @@ -228,15 +228,15 @@ struct KerxSubTableFormat1 crossStream (table->header.coverage & table->header.CrossStream) {} bool is_actionable (StateTableDriver *driver HB_UNUSED, - const Entry *entry) + const Entry &entry) { return Format1EntryT::performAction (entry); } bool transition (StateTableDriver *driver, - const Entry *entry) + const Entry &entry) { hb_buffer_t *buffer = driver->buffer; - unsigned int flags = entry->flags; + unsigned int flags = entry.flags; if (flags & Format1EntryT::Reset) depth = 0; @@ -498,16 +498,16 @@ struct KerxSubTableFormat4 mark (0) {} bool is_actionable (StateTableDriver *driver HB_UNUSED, - const Entry *entry) + const Entry &entry) { - return entry->data.ankrActionIndex != 0xFFFF; + return entry.data.ankrActionIndex != 0xFFFF; } bool transition (StateTableDriver *driver, - const Entry *entry) + const Entry &entry) { hb_buffer_t *buffer = driver->buffer; - if (mark_set && entry->data.ankrActionIndex != 0xFFFF && buffer->idx < buffer->len) + if (mark_set && entry.data.ankrActionIndex != 0xFFFF && buffer->idx < buffer->len) { hb_glyph_position_t &o = buffer->cur_pos(); switch (action_type) @@ -515,7 +515,7 @@ struct KerxSubTableFormat4 case 0: /* Control Point Actions.*/ { /* indexed into glyph outline. */ - const HBUINT16 *data = &ankrData[entry->data.ankrActionIndex]; + const HBUINT16 *data = &ankrData[entry.data.ankrActionIndex]; if (!c->sanitizer.check_array (data, 2)) return false; HB_UNUSED unsigned int markControlPoint = *data++; @@ -542,7 +542,7 @@ struct KerxSubTableFormat4 case 1: /* Anchor Point Actions. */ { /* Indexed into 'ankr' table. */ - const HBUINT16 *data = &ankrData[entry->data.ankrActionIndex]; + const HBUINT16 *data = &ankrData[entry.data.ankrActionIndex]; if (!c->sanitizer.check_array (data, 2)) return false; unsigned int markAnchorPoint = *data++; @@ -561,7 +561,7 @@ struct KerxSubTableFormat4 case 2: /* Control Point Coordinate Actions. */ { - const FWORD *data = (const FWORD *) &ankrData[entry->data.ankrActionIndex]; + const FWORD *data = (const FWORD *) &ankrData[entry.data.ankrActionIndex]; if (!c->sanitizer.check_array (data, 4)) return false; int markX = *data++; @@ -579,7 +579,7 @@ struct KerxSubTableFormat4 buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_GPOS_ATTACHMENT; } - if (entry->flags & Mark) + if (entry.flags & Mark) { mark_set = true; mark = buffer->idx; diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 2ebbc3511..564a618e0 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -74,15 +74,15 @@ struct RearrangementSubtable start (0), end (0) {} bool is_actionable (StateTableDriver *driver HB_UNUSED, - const Entry *entry) + const Entry &entry) { - return (entry->flags & Verb) && start < end; + return (entry.flags & Verb) && start < end; } bool transition (StateTableDriver *driver, - const Entry *entry) + const Entry &entry) { hb_buffer_t *buffer = driver->buffer; - unsigned int flags = entry->flags; + unsigned int flags = entry.flags; if (flags & MarkFirst) start = buffer->idx; @@ -223,17 +223,17 @@ struct ContextualSubtable subs (table+table->substitutionTables) {} bool is_actionable (StateTableDriver *driver, - const Entry *entry) + const Entry &entry) { hb_buffer_t *buffer = driver->buffer; if (buffer->idx == buffer->len && !mark_set) return false; - return entry->data.markIndex != 0xFFFF || entry->data.currentIndex != 0xFFFF; + return entry.data.markIndex != 0xFFFF || entry.data.currentIndex != 0xFFFF; } bool transition (StateTableDriver *driver, - const Entry *entry) + const Entry &entry) { hb_buffer_t *buffer = driver->buffer; @@ -247,15 +247,15 @@ struct ContextualSubtable replacement = nullptr; if (Types::extended) { - if (entry->data.markIndex != 0xFFFF) + if (entry.data.markIndex != 0xFFFF) { - const Lookup &lookup = subs[entry->data.markIndex]; + const Lookup &lookup = subs[entry.data.markIndex]; replacement = lookup.get_value (buffer->info[mark].codepoint, driver->num_glyphs); } } else { - unsigned int offset = entry->data.markIndex + buffer->info[mark].codepoint; + unsigned int offset = entry.data.markIndex + buffer->info[mark].codepoint; const UnsizedArrayOf &subs_old = (const UnsizedArrayOf &) subs; replacement = &subs_old[Types::wordOffsetToIndex (offset, table, subs_old.arrayZ)]; if (!replacement->sanitize (&c->sanitizer) || !*replacement) @@ -272,15 +272,15 @@ struct ContextualSubtable unsigned int idx = MIN (buffer->idx, buffer->len - 1); if (Types::extended) { - if (entry->data.currentIndex != 0xFFFF) + if (entry.data.currentIndex != 0xFFFF) { - const Lookup &lookup = subs[entry->data.currentIndex]; + const Lookup &lookup = subs[entry.data.currentIndex]; replacement = lookup.get_value (buffer->info[idx].codepoint, driver->num_glyphs); } } else { - unsigned int offset = entry->data.currentIndex + buffer->info[idx].codepoint; + unsigned int offset = entry.data.currentIndex + buffer->info[idx].codepoint; const UnsizedArrayOf &subs_old = (const UnsizedArrayOf &) subs; replacement = &subs_old[Types::wordOffsetToIndex (offset, table, subs_old.arrayZ)]; if (!replacement->sanitize (&c->sanitizer) || !*replacement) @@ -292,7 +292,7 @@ struct ContextualSubtable ret = true; } - if (entry->flags & SetMark) + if (entry.flags & SetMark) { mark_set = true; mark = buffer->idx; @@ -385,11 +385,11 @@ struct LigatureEntry DEFINE_SIZE_STATIC (2); }; - static bool performAction (const Entry *entry) - { return entry->flags & PerformAction; } + static bool performAction (const Entry &entry) + { return entry.flags & PerformAction; } - static unsigned int ligActionIndex (const Entry *entry) - { return entry->data.ligActionIndex; } + static unsigned int ligActionIndex (const Entry &entry) + { return entry.data.ligActionIndex; } }; template <> struct LigatureEntry @@ -407,11 +407,11 @@ struct LigatureEntry typedef void EntryData; - static bool performAction (const Entry *entry) - { return entry->flags & Offset; } + static bool performAction (const Entry &entry) + { return entry.flags & Offset; } - static unsigned int ligActionIndex (const Entry *entry) - { return entry->flags & Offset; } + static unsigned int ligActionIndex (const Entry &entry) + { return entry.flags & Offset; } }; @@ -453,17 +453,17 @@ struct LigatureSubtable match_length (0) {} bool is_actionable (StateTableDriver *driver HB_UNUSED, - const Entry *entry) + const Entry &entry) { return LigatureEntryT::performAction (entry); } bool transition (StateTableDriver *driver, - const Entry *entry) + const Entry &entry) { hb_buffer_t *buffer = driver->buffer; DEBUG_MSG (APPLY, nullptr, "Ligature transition at %u", buffer->idx); - if (entry->flags & LigatureEntryT::SetComponent) + if (entry.flags & LigatureEntryT::SetComponent) { if (unlikely (match_length >= ARRAY_LENGTH (match_positions))) return false; @@ -718,23 +718,23 @@ struct InsertionSubtable insertionAction (table+table->insertionAction) {} bool is_actionable (StateTableDriver *driver HB_UNUSED, - const Entry *entry) + const Entry &entry) { - return (entry->flags & (CurrentInsertCount | MarkedInsertCount)) && - (entry->data.currentInsertIndex != 0xFFFF ||entry->data.markedInsertIndex != 0xFFFF); + return (entry.flags & (CurrentInsertCount | MarkedInsertCount)) && + (entry.data.currentInsertIndex != 0xFFFF ||entry.data.markedInsertIndex != 0xFFFF); } bool transition (StateTableDriver *driver, - const Entry *entry) + const Entry &entry) { hb_buffer_t *buffer = driver->buffer; - unsigned int flags = entry->flags; + unsigned int flags = entry.flags; unsigned mark_loc = buffer->out_len; - if (entry->data.markedInsertIndex != 0xFFFF) + if (entry.data.markedInsertIndex != 0xFFFF) { unsigned int count = (flags & MarkedInsertCount); - unsigned int start = entry->data.markedInsertIndex; + unsigned int start = entry.data.markedInsertIndex; const GlyphID *glyphs = &insertionAction[start]; if (unlikely (!c->sanitizer.check_array (glyphs, count))) return false; @@ -759,10 +759,10 @@ struct InsertionSubtable if (flags & SetMark) mark = mark_loc; - if (entry->data.currentInsertIndex != 0xFFFF) + if (entry.data.currentInsertIndex != 0xFFFF) { unsigned int count = (flags & CurrentInsertCount) >> 5; - unsigned int start = entry->data.currentInsertIndex; + unsigned int start = entry.data.currentInsertIndex; const GlyphID *glyphs = &insertionAction[start]; if (unlikely (!c->sanitizer.check_array (glyphs, count))) return false; From e234bb6a428cd6c8ddf57eb078cd51b9d1f25ba8 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 17:23:11 +0100 Subject: [PATCH 5/9] [AAT] Ignore machine errors and continue --- 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 76ba98997..516a72210 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -778,7 +778,7 @@ struct StateTableDriver } if (unlikely (!c->transition (this, entry))) - break; + ;//break; Ignore error. state = machine.new_state (entry.newState); DEBUG_MSG (APPLY, nullptr, "s%d", state); From b976940243bf1f174bd6abb85955789ef2631d24 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 18:01:07 +0100 Subject: [PATCH 6/9] [AAT] Handle transition errors during machine operation Before we used to give up. Now, just ignore error and continue processing. Fixes https://github.com/harfbuzz/harfbuzz/issues/1531 --- src/hb-aat-layout-common.hh | 3 +-- src/hb-aat-layout-kerx-table.hh | 21 +++++++------------- src/hb-aat-layout-morx-table.hh | 34 +++++++++++++-------------------- 3 files changed, 21 insertions(+), 37 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 516a72210..27ade28fe 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -777,8 +777,7 @@ struct StateTableDriver buffer->unsafe_to_break (buffer->idx, buffer->idx + 2); } - if (unlikely (!c->transition (this, entry))) - ;//break; Ignore error. + c->transition (this, entry); state = machine.new_state (entry.newState); DEBUG_MSG (APPLY, nullptr, "s%d", state); diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 0630fba6c..a64c80738 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -232,7 +232,7 @@ struct KerxSubTableFormat1 { return Format1EntryT::performAction (entry); } - bool transition (StateTableDriver *driver, + void transition (StateTableDriver *driver, const Entry &entry) { hb_buffer_t *buffer = driver->buffer; @@ -259,7 +259,7 @@ struct KerxSubTableFormat1 if (!c->sanitizer.check_array (actions, depth, tuple_count)) { depth = 0; - return false; + return; } hb_mask_t kern_mask = c->plan->kern_mask; @@ -334,8 +334,6 @@ struct KerxSubTableFormat1 } } } - - return true; } private: @@ -502,7 +500,7 @@ struct KerxSubTableFormat4 { return entry.data.ankrActionIndex != 0xFFFF; } - bool transition (StateTableDriver *driver, + void transition (StateTableDriver *driver, const Entry &entry) { hb_buffer_t *buffer = driver->buffer; @@ -516,8 +514,7 @@ struct KerxSubTableFormat4 { /* indexed into glyph outline. */ const HBUINT16 *data = &ankrData[entry.data.ankrActionIndex]; - if (!c->sanitizer.check_array (data, 2)) - return false; + if (!c->sanitizer.check_array (data, 2)) return; HB_UNUSED unsigned int markControlPoint = *data++; HB_UNUSED unsigned int currControlPoint = *data++; hb_position_t markX = 0; @@ -532,7 +529,7 @@ struct KerxSubTableFormat4 currControlPoint, HB_DIRECTION_LTR /*XXX*/, &currX, &currY)) - return true; /* True, such that the machine continues. */ + return; o.x_offset = markX - currX; o.y_offset = markY - currY; @@ -543,8 +540,7 @@ struct KerxSubTableFormat4 { /* Indexed into 'ankr' table. */ const HBUINT16 *data = &ankrData[entry.data.ankrActionIndex]; - if (!c->sanitizer.check_array (data, 2)) - return false; + if (!c->sanitizer.check_array (data, 2)) return; unsigned int markAnchorPoint = *data++; unsigned int currAnchorPoint = *data++; const Anchor &markAnchor = c->ankr_table->get_anchor (c->buffer->info[mark].codepoint, @@ -562,8 +558,7 @@ struct KerxSubTableFormat4 case 2: /* Control Point Coordinate Actions. */ { const FWORD *data = (const FWORD *) &ankrData[entry.data.ankrActionIndex]; - if (!c->sanitizer.check_array (data, 4)) - return false; + if (!c->sanitizer.check_array (data, 4)) return; int markX = *data++; int markY = *data++; int currX = *data++; @@ -584,8 +579,6 @@ struct KerxSubTableFormat4 mark_set = true; mark = buffer->idx; } - - return true; } private: diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 564a618e0..8fc6c5daf 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -78,7 +78,7 @@ struct RearrangementSubtable { return (entry.flags & Verb) && start < end; } - bool transition (StateTableDriver *driver, + void transition (StateTableDriver *driver, const Entry &entry) { hb_buffer_t *buffer = driver->buffer; @@ -152,8 +152,6 @@ struct RearrangementSubtable } } } - - return true; } public: @@ -232,7 +230,7 @@ struct ContextualSubtable return entry.data.markIndex != 0xFFFF || entry.data.currentIndex != 0xFFFF; } - bool transition (StateTableDriver *driver, + void transition (StateTableDriver *driver, const Entry &entry) { hb_buffer_t *buffer = driver->buffer; @@ -240,7 +238,7 @@ struct ContextualSubtable /* Looks like CoreText applies neither mark nor current substitution for * end-of-text if mark was not explicitly set. */ if (buffer->idx == buffer->len && !mark_set) - return true; + return; const GlyphID *replacement; @@ -297,8 +295,6 @@ struct ContextualSubtable mark_set = true; mark = buffer->idx; } - - return true; } public: @@ -457,7 +453,7 @@ struct LigatureSubtable { return LigatureEntryT::performAction (entry); } - bool transition (StateTableDriver *driver, + void transition (StateTableDriver *driver, const Entry &entry) { hb_buffer_t *buffer = driver->buffer; @@ -466,7 +462,7 @@ struct LigatureSubtable if (entry.flags & LigatureEntryT::SetComponent) { if (unlikely (match_length >= ARRAY_LENGTH (match_positions))) - return false; + match_length = 0; /* TODO Use a ring buffer instead. */ /* Never mark same index twice, in case DontAdvance was used... */ if (match_length && match_positions[match_length - 1] == buffer->out_len) @@ -482,10 +478,10 @@ struct LigatureSubtable unsigned int end = buffer->out_len; if (unlikely (!match_length)) - return true; + return; if (buffer->idx >= buffer->len) - return false; // TODO Work on previous instead? + return; // TODO Work on previous instead? unsigned int cursor = match_length; @@ -508,7 +504,7 @@ struct LigatureSubtable 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; + if (unlikely (!actionData->sanitize (&c->sanitizer))) break; action = *actionData; uint32_t uoffset = action & LigActionOffset; @@ -518,7 +514,7 @@ struct LigatureSubtable unsigned int component_idx = buffer->cur().codepoint + offset; component_idx = Types::wordOffsetToIndex (component_idx, table, component.arrayZ); const HBUINT16 &componentData = component[component_idx]; - if (unlikely (!componentData.sanitize (&c->sanitizer))) return false; + if (unlikely (!componentData.sanitize (&c->sanitizer))) break; ligature_idx += componentData; DEBUG_MSG (APPLY, nullptr, "Action store %u last %u", @@ -528,7 +524,7 @@ struct LigatureSubtable { ligature_idx = Types::offsetToIndex (ligature_idx, table, ligature.arrayZ); const GlyphID &ligatureData = ligature[ligature_idx]; - if (unlikely (!ligatureData.sanitize (&c->sanitizer))) return false; + if (unlikely (!ligatureData.sanitize (&c->sanitizer))) break; hb_codepoint_t lig = ligatureData; DEBUG_MSG (APPLY, nullptr, "Produced ligature %u", lig); @@ -552,8 +548,6 @@ struct LigatureSubtable while (!(action & LigActionLast)); buffer->move_to (end); } - - return true; } public: @@ -723,7 +717,7 @@ struct InsertionSubtable return (entry.flags & (CurrentInsertCount | MarkedInsertCount)) && (entry.data.currentInsertIndex != 0xFFFF ||entry.data.markedInsertIndex != 0xFFFF); } - bool transition (StateTableDriver *driver, + void transition (StateTableDriver *driver, const Entry &entry) { hb_buffer_t *buffer = driver->buffer; @@ -736,7 +730,7 @@ struct InsertionSubtable unsigned int count = (flags & MarkedInsertCount); unsigned int start = entry.data.markedInsertIndex; const GlyphID *glyphs = &insertionAction[start]; - if (unlikely (!c->sanitizer.check_array (glyphs, count))) return false; + if (unlikely (!c->sanitizer.check_array (glyphs, count))) count = 0; bool before = flags & MarkedInsertBefore; @@ -764,7 +758,7 @@ struct InsertionSubtable unsigned int count = (flags & CurrentInsertCount) >> 5; unsigned int start = entry.data.currentInsertIndex; const GlyphID *glyphs = &insertionAction[start]; - if (unlikely (!c->sanitizer.check_array (glyphs, count))) return false; + if (unlikely (!c->sanitizer.check_array (glyphs, count))) count = 0; bool before = flags & CurrentInsertBefore; @@ -795,8 +789,6 @@ struct InsertionSubtable */ buffer->move_to ((flags & DontAdvance) ? end : end + count); } - - return true; } public: From 7886b1578fceee368ae5afe206ff98f50e1c42e3 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 18:06:17 +0100 Subject: [PATCH 7/9] Whitespace --- src/hb-aat-layout-morx-table.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 8fc6c5daf..6e7bb0399 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -481,7 +481,7 @@ struct LigatureSubtable return; if (buffer->idx >= buffer->len) - return; // TODO Work on previous instead? + return; /* TODO Work on previous instead? */ unsigned int cursor = match_length; From a371a28cda23805cbea22867e0a3ed53ecb811ed Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 18:12:25 +0100 Subject: [PATCH 8/9] [AAT] Use a ring buffer for ligature stack I think Apple does very similarly, but probably with a stack size of 16. We do it with a stack size that is currently set to 64. Fixes https://github.com/harfbuzz/harfbuzz/issues/1531 --- src/hb-aat-layout-morx-table.hh | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 6e7bb0399..c2a75a8a9 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -461,14 +461,11 @@ struct LigatureSubtable DEBUG_MSG (APPLY, nullptr, "Ligature transition at %u", buffer->idx); if (entry.flags & LigatureEntryT::SetComponent) { - if (unlikely (match_length >= ARRAY_LENGTH (match_positions))) - match_length = 0; /* TODO Use a ring buffer instead. */ - /* Never mark same index twice, in case DontAdvance was used... */ - if (match_length && match_positions[match_length - 1] == buffer->out_len) + if (match_length && match_positions[(match_length - 1u) % ARRAY_LENGTH (match_positions)] == buffer->out_len) match_length--; - match_positions[match_length++] = buffer->out_len; + match_positions[match_length++ % ARRAY_LENGTH (match_positions)] = buffer->out_len; DEBUG_MSG (APPLY, nullptr, "Set component at %u", buffer->out_len); } @@ -502,7 +499,7 @@ struct LigatureSubtable } DEBUG_MSG (APPLY, nullptr, "Moving to stack position %u", cursor - 1); - buffer->move_to (match_positions[--cursor]); + buffer->move_to (match_positions[--cursor % ARRAY_LENGTH (match_positions)]); if (unlikely (!actionData->sanitize (&c->sanitizer))) break; action = *actionData; @@ -530,17 +527,17 @@ struct LigatureSubtable DEBUG_MSG (APPLY, nullptr, "Produced ligature %u", lig); buffer->replace_glyph (lig); - unsigned int lig_end = match_positions[match_length - 1] + 1; + unsigned int lig_end = match_positions[(match_length - 1) % ARRAY_LENGTH (match_positions)] + 1; /* Now go and delete all subsequent components. */ while (match_length - 1 > cursor) { DEBUG_MSG (APPLY, nullptr, "Skipping ligature component"); - buffer->move_to (match_positions[--match_length]); + buffer->move_to (match_positions[--match_length % ARRAY_LENGTH (match_positions)]); buffer->replace_glyph (DELETED_GLYPH); } buffer->move_to (lig_end); - buffer->merge_out_clusters (match_positions[cursor], buffer->out_len); + buffer->merge_out_clusters (match_positions[cursor % ARRAY_LENGTH (match_positions)], buffer->out_len); } actionData++; From e970de48bcbdccd29350f331288c0a98f7846c16 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 24 Jan 2019 18:16:17 +0100 Subject: [PATCH 9/9] [AAT] Minor sign --- src/hb-aat-layout-morx-table.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index c2a75a8a9..4a1d959ef 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -527,9 +527,9 @@ struct LigatureSubtable DEBUG_MSG (APPLY, nullptr, "Produced ligature %u", lig); buffer->replace_glyph (lig); - unsigned int lig_end = match_positions[(match_length - 1) % ARRAY_LENGTH (match_positions)] + 1; + unsigned int lig_end = match_positions[(match_length - 1u) % ARRAY_LENGTH (match_positions)] + 1u; /* Now go and delete all subsequent components. */ - while (match_length - 1 > cursor) + while (match_length - 1u > cursor) { DEBUG_MSG (APPLY, nullptr, "Skipping ligature component"); buffer->move_to (match_positions[--match_length % ARRAY_LENGTH (match_positions)]);