From 54e6efadd6a30587736858d3fb497ed4e5e5f252 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sun, 4 Feb 2018 14:58:02 -0500 Subject: [PATCH] [aat] Fix unsafe-to-break At any position, if state is not zero, mark unsafe-to-break before, unless we can reason it safe. At any position, if there's an action entry for end-of-text, mark unsafe to break. Also changes buffer diff impl to allow for flag differences as long as the buffer glyph flags are superset of reference glyph flags. With this, all MORX tests pass. --- src/hb-aat-layout-common-private.hh | 31 ++++++++++++++++++++----- src/hb-aat-layout-morx-table.hh | 35 +++++++++++++++++++---------- src/hb-buffer.cc | 2 +- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/hb-aat-layout-common-private.hh b/src/hb-aat-layout-common-private.hh index 1ea8318fb..74f9fdb0d 100644 --- a/src/hb-aat-layout-common-private.hh +++ b/src/hb-aat-layout-common-private.hh @@ -614,8 +614,7 @@ struct StateTableDriver hb_face_t *face_) : machine (machine_), buffer (buffer_), - num_glyphs (face_->get_num_glyphs ()), - last_zero (0) {} + num_glyphs (face_->get_num_glyphs ()) {} template inline void drive (context_t *c) @@ -629,9 +628,6 @@ struct StateTableDriver bool last_was_dont_advance = false; for (buffer->idx = 0;;) { - if (!state) - last_zero = buffer->idx; - unsigned int klass = buffer->idx < buffer->len ? machine.get_class (info[buffer->idx].codepoint, num_glyphs) : 0 /* End of text */; @@ -639,6 +635,30 @@ struct StateTableDriver if (unlikely (!entry)) break; + /* Unsafe-to-break before this if not in state 0, as things might + * go differently if we start from state 0 here. */ + if (state && buffer->idx) + { + /* Special-case easy cases: if starting here at state 0 is not + * actionable, and leads to the same next state, then it's safe. + * Let's hope... Maybe disable the conditional later, if proves + * insufficient. */ + const Entry *start_entry = machine.get_entryZ (0, klass); + if (start_entry->newState != entry->newState || + (start_entry->flags & context_t::DontAdvance) != (entry->flags & context_t::DontAdvance) || + c->is_actionable (this, entry) || + c->is_actionable (this, start_entry)) + buffer->unsafe_to_break (buffer->idx - 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_entryZ (state, 0); + if (c->is_actionable (this, end_entry)) + buffer->unsafe_to_break (buffer->idx, buffer->idx + 2); + } + if (unlikely (!c->transition (this, entry))) break; @@ -665,7 +685,6 @@ struct StateTableDriver const StateTable &machine; hb_buffer_t *buffer; unsigned int num_glyphs; - unsigned int last_zero; }; diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index 1f8ac4064..4e75dd3a7 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -58,9 +58,13 @@ struct RearrangementSubtable inline driver_context_t (const RearrangementSubtable *table) : ret (false), - start (0), end (0), - last_zero_before_start (0) {} + start (0), end (0) {} + inline bool is_actionable (StateTableDriver *driver, + const Entry *entry) + { + return (entry->flags & Verb) && start < end; + } inline bool transition (StateTableDriver *driver, const Entry *entry) { @@ -68,10 +72,7 @@ struct RearrangementSubtable unsigned int flags = entry->flags; if (flags & MarkFirst) - { start = buffer->idx; - last_zero_before_start = driver->last_zero; - } if (flags & MarkLast) end = MIN (buffer->idx + 1, buffer->len); @@ -110,7 +111,7 @@ struct RearrangementSubtable if (end - start >= l + r) { - buffer->unsafe_to_break (last_zero_before_start, MIN (buffer->idx + 1, buffer->len)); + buffer->merge_clusters (start, MIN (buffer->idx + 1, buffer->len)); buffer->merge_clusters (start, end); hb_glyph_info_t *info = buffer->info; @@ -147,7 +148,6 @@ struct RearrangementSubtable private: unsigned int start; unsigned int end; - unsigned int last_zero_before_start; }; inline bool apply (hb_aat_apply_context_t *c) const @@ -200,9 +200,18 @@ struct ContextualSubtable ret (false), mark_set (false), mark (0), - last_zero_before_mark (0), subs (table+table->substitutionTables) {} + inline bool is_actionable (StateTableDriver *driver, + 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; + } inline bool transition (StateTableDriver *driver, const Entry *entry) { @@ -220,7 +229,7 @@ struct ContextualSubtable const GlyphID *replacement = lookup.get_value (info[mark].codepoint, driver->num_glyphs); if (replacement) { - buffer->unsafe_to_break (last_zero_before_mark, MIN (buffer->idx + 1, buffer->len)); + buffer->unsafe_to_break (mark, MIN (buffer->idx + 1, buffer->len)); info[mark].codepoint = *replacement; ret = true; } @@ -233,7 +242,6 @@ struct ContextualSubtable const GlyphID *replacement = lookup.get_value (info[idx].codepoint, driver->num_glyphs); if (replacement) { - buffer->unsafe_to_break (driver->last_zero, idx + 1); info[idx].codepoint = *replacement; ret = true; } @@ -243,7 +251,6 @@ struct ContextualSubtable { mark_set = true; mark = buffer->idx; - last_zero_before_mark = driver->last_zero; } return true; @@ -254,7 +261,6 @@ struct ContextualSubtable private: bool mark_set; unsigned int mark; - unsigned int last_zero_before_mark; const UnsizedOffsetListOf, HBUINT32> &subs; }; @@ -344,6 +350,11 @@ struct LigatureSubtable ligature (table+table->ligature), match_length (0) {} + inline bool is_actionable (StateTableDriver *driver, + const Entry *entry) + { + return !!(entry->flags & PerformAction); + } inline bool transition (StateTableDriver *driver, const Entry *entry) { diff --git a/src/hb-buffer.cc b/src/hb-buffer.cc index 7ead43b01..7fb000dd3 100644 --- a/src/hb-buffer.cc +++ b/src/hb-buffer.cc @@ -1933,7 +1933,7 @@ hb_buffer_diff (hb_buffer_t *buffer, result |= HB_BUFFER_DIFF_FLAG_CODEPOINT_MISMATCH; if (buf_info->cluster != ref_info->cluster) result |= HB_BUFFER_DIFF_FLAG_CLUSTER_MISMATCH; - if ((buf_info->mask & HB_GLYPH_FLAG_DEFINED) != (ref_info->mask & HB_GLYPH_FLAG_DEFINED)) + if ((buf_info->mask & HB_GLYPH_FLAG_DEFINED) & (ref_info->mask & HB_GLYPH_FLAG_DEFINED) != (ref_info->mask & HB_GLYPH_FLAG_DEFINED)) result |= HB_BUFFER_DIFF_FLAG_GLYPH_FLAGS_MISMATCH; if (contains && ref_info->codepoint == dottedcircle_glyph) result |= HB_BUFFER_DIFF_FLAG_DOTTED_CIRCLE_PRESENT;