From 359dead960c825edeb4587915a511d323f1c1f2a Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 6 May 2016 16:19:19 +0100 Subject: [PATCH] Allow MultipleSubst to delete glyph Fixes https://github.com/behdad/harfbuzz/issues/253 Hopefully we got the logic right. --- src/hb-ot-layout-gsub-table.hh | 17 +++++++---------- src/hb-ot-layout-gsubgpos-private.hh | 11 +++++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh index 7de56cf2a..22031f43c 100644 --- 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 56c501533..997d22550 100644 --- 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. */