src/hb-aat-layout-common-private.hh |   31 +++++++++++++++++----
 src/hb-aat-layout-morx-table.hh     |   52 ++++++++++++++++++++++++------------
 src/hb-buffer.cc                    |    2 -
 3 files changed, 62 insertions(+), 23 deletions(-)

New commits:
commit 54e6efadd6a30587736858d3fb497ed4e5e5f252
Author: Behdad Esfahbod <beh...@behdad.org>
Date:   Sun Feb 4 14:58:02 2018 -0500

    [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.

diff --git a/src/hb-aat-layout-common-private.hh 
b/src/hb-aat-layout-common-private.hh
index 1ea8318f..74f9fdb0 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 <typename context_t>
   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<EntryData> *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<EntryData> *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<EntryData> &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 1f8ac406..4e75dd3a 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<void> *driver,
+                              const Entry<void> *entry)
+    {
+      return (entry->flags & Verb) && start < end;
+    }
     inline bool transition (StateTableDriver<void> *driver,
                            const Entry<void> *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<EntryData> *driver,
+                              const Entry<EntryData> *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<EntryData> *driver,
                            const Entry<EntryData> *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<Lookup<GlyphID>, HBUINT32> &subs;
   };
 
@@ -344,6 +350,11 @@ struct LigatureSubtable
        ligature (table+table->ligature),
        match_length (0) {}
 
+    inline bool is_actionable (StateTableDriver<EntryData> *driver,
+                              const Entry<EntryData> *entry)
+    {
+      return !!(entry->flags & PerformAction);
+    }
     inline bool transition (StateTableDriver<EntryData> *driver,
                            const Entry<EntryData> *entry)
     {
diff --git a/src/hb-buffer.cc b/src/hb-buffer.cc
index 7ead43b0..7fb000dd 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;
commit 89b1906d990658c763f35113c8978a5e21bffc22
Author: Behdad Esfahbod <beh...@behdad.org>
Date:   Sun Feb 4 14:45:02 2018 -0500

    [aat] More adjustment to ContextualSubtable
    
    See comment.
    
    With this, MORX-20 passes if I turn --verify off.  Our unsafe-to-break
    logic is currently broken in presence of end-of-text actions.  That's,
    ugh, extra work to fix.  Let me try...

diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh
index ba9729c5..1f8ac406 100644
--- a/src/hb-aat-layout-morx-table.hh
+++ b/src/hb-aat-layout-morx-table.hh
@@ -198,6 +198,7 @@ struct ContextualSubtable
 
     inline driver_context_t (const ContextualSubtable *table) :
        ret (false),
+       mark_set (false),
        mark (0),
        last_zero_before_mark (0),
        subs (table+table->substitutionTables) {}
@@ -207,6 +208,11 @@ struct ContextualSubtable
     {
       hb_buffer_t *buffer = driver->buffer;
 
+      /* 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;
+
       if (entry->data.markIndex != 0xFFFF)
       {
        const Lookup<GlyphID> &lookup = subs[entry->data.markIndex];
@@ -235,6 +241,7 @@ struct ContextualSubtable
 
       if (entry->flags & SetMark)
       {
+       mark_set = true;
        mark = buffer->idx;
        last_zero_before_mark = driver->last_zero;
       }
@@ -245,6 +252,7 @@ struct ContextualSubtable
     public:
     bool ret;
     private:
+    bool mark_set;
     unsigned int mark;
     unsigned int last_zero_before_mark;
     const UnsizedOffsetListOf<Lookup<GlyphID>, HBUINT32> &subs;
commit 8be596f0b76543e19644c0b77c1bcf4d9e783c2b
Author: Behdad Esfahbod <beh...@behdad.org>
Date:   Sun Feb 4 14:40:17 2018 -0500

    [aat] In ContextualSubstitute, apply end-of-text action to last glyph

diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh
index 68c32c43..ba9729c5 100644
--- a/src/hb-aat-layout-morx-table.hh
+++ b/src/hb-aat-layout-morx-table.hh
@@ -219,15 +219,16 @@ struct ContextualSubtable
          ret = true;
        }
       }
-      if (entry->data.currentIndex != 0xFFFF && buffer->idx < buffer->len)
+      if (entry->data.currentIndex != 0xFFFF)
       {
+        unsigned int idx = MIN (buffer->idx, buffer->len - 1);
        const Lookup<GlyphID> &lookup = subs[entry->data.currentIndex];
        hb_glyph_info_t *info = buffer->info;
-       const GlyphID *replacement = lookup.get_value 
(info[buffer->idx].codepoint, driver->num_glyphs);
+       const GlyphID *replacement = lookup.get_value (info[idx].codepoint, 
driver->num_glyphs);
        if (replacement)
        {
-         buffer->unsafe_to_break (driver->last_zero, MIN (buffer->idx + 1, 
buffer->len));
-         info[buffer->idx].codepoint = *replacement;
+         buffer->unsafe_to_break (driver->last_zero, idx + 1);
+         info[idx].codepoint = *replacement;
          ret = true;
        }
       }
commit c0b1c7eb2eed67147adec3d2c9e02d01f279c8f4
Author: Behdad Esfahbod <beh...@behdad.org>
Date:   Sun Feb 4 14:19:41 2018 -0500

    [aat] Remove unneeded check

diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh
index ffe4d03a..68c32c43 100644
--- a/src/hb-aat-layout-morx-table.hh
+++ b/src/hb-aat-layout-morx-table.hh
@@ -207,7 +207,7 @@ struct ContextualSubtable
     {
       hb_buffer_t *buffer = driver->buffer;
 
-      if (entry->data.markIndex != 0xFFFF && mark < buffer->len)
+      if (entry->data.markIndex != 0xFFFF)
       {
        const Lookup<GlyphID> &lookup = subs[entry->data.markIndex];
        hb_glyph_info_t *info = buffer->info;
_______________________________________________
HarfBuzz mailing list
HarfBuzz@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/harfbuzz

Reply via email to