Allow MultipleSubst to delete glyph

Fixes https://github.com/behdad/harfbuzz/issues/253

Hopefully we got the logic right.

diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh
index 7de56cf..22031f4 100644 (file)
--- a/src/hb-ot-layout-gsub-table.hh
+++ b/src/hb-ot-layout-gsub-table.hh
@@ -265,16 +265,6 @@ struct Sequence
     TRACE_APPLY (this);
     unsigned int count = substitute.len;
 
-    /* TODO:
-     * Testing shows that Uniscribe actually allows zero-len susbstitute,
-     * which essentially deletes a glyph.  We don't allow for now.  It
-     * can be confusing to the client since the cluster from the deleted
-     * glyph won't be merged with any output cluster...  Also, currently
-     * buffer->move_to() makes assumptions about this too.  Perhaps fix
-     * in the future after figuring out what to do with the clusters.
-     */
-    if (unlikely (!count)) return_trace (false);
-
     /* Special-case to make it in-place and not consider this
      * as a "multiplied" substitution. */
     if (unlikely (count == 1))
@@ -282,6 +272,13 @@ struct Sequence
       c->replace_glyph (substitute.array[0]);
       return_trace (true);
     }
+    /* Spec disallows this, but Uniscribe allows it.
+     * https://github.com/behdad/harfbuzz/issues/253 */
+    else if (unlikely (count == 0))
+    {
+      c->buffer->delete_glyph ();
+      return_trace (true);
+    }
 
     unsigned int klass = _hb_glyph_info_is_ligature (&c->buffer->cur()) ?
                         HB_OT_LAYOUT_GLYPH_PROPS_BASE_GLYPH : 0;

diff --git a/src/hb-ot-layout-gsubgpos-private.hh b/src/hb-ot-layout-gsubgpos-private.hh
index 56c5015..997d225 100644 (file)
--- a/src/hb-ot-layout-gsubgpos-private.hh
+++ b/src/hb-ot-layout-gsubgpos-private.hh
@@ -996,10 +996,13 @@ static inline bool apply_lookup (hb_apply_context_t *c,
 
     /* Recursed lookup changed buffer len.  Adjust. */
 
-    /* end can't go back past the current match position.
-     * Note: this is only true because we do NOT allow MultipleSubst
-     * with zero sequence len. */
-    end = MAX (MIN((int) match_positions[idx] + 1, (int) new_len), int (end) + delta);
+    end = int (end) + delta;
+    if (end <= match_positions[idx])
+    {
+      /* There can't be any further changes. */
+      assert (end == match_positions[idx]);
+      break;
+    }
 
     unsigned int next = idx + 1; /* next now is the position after the recursed lookup. */