[aat] Remove deleted-glyhs after applying kerx/kern

Finally:  Fixes https://github.com/harfbuzz/harfbuzz/issues/1356

Test case:
$ ./hb-shape GeezaPro.ttc -u U+0628,U+064A,U+064E,U+0651,U+0629
[u0629.final.tehMarbuta=4+713|u064e_u0651.shaddaFatha=1@0,-200+0|u064a.medial.yeh=1+656|u0628.initial.beh=0+656]

The mark positioning (kern table CrossStream kerning) only works if deleted
glyph (as result of ligation) is still in stream and pushed through the
state machine.

src/hb-aat-layout-morx-table.hh

@@ -1106,21 +1106,6 @@ struct mortmorx
     }
   }
 
-  inline static void remove_deleted_glyphs (hb_buffer_t *buffer)
-  {
-    if (unlikely (!buffer->successful)) return;
-
-    buffer->clear_output ();
-    for (buffer->idx = 0; buffer->idx < buffer->len && buffer->successful;)
-    {
-      if (unlikely (buffer->cur().codepoint == DELETED_GLYPH))
-        buffer->skip_glyph ();
-      else
-        buffer->next_glyph ();
-    }
-    buffer->swap_buffers ();
-  }
-
   inline void apply (hb_aat_apply_context_t *c) const
   {
     if (unlikely (!c->buffer->successful)) return;
@@ -1133,7 +1118,6 @@ struct mortmorx
       if (unlikely (!c->buffer->successful)) return;
       chain = &StructAfter<Chain<Types> > (*chain);
     }
-    remove_deleted_glyphs (c->buffer);
   }
 
   inline bool sanitize (hb_sanitize_context_t *c) const

src/hb-aat-layout.cc

@@ -193,7 +193,7 @@ hb_aat_layout_compile_map (const hb_aat_map_builder_t *mapper,
 }
 
 
-hb_bool_t
+bool
 hb_aat_layout_has_substitution (hb_face_t *face)
 {
   return face->table.morx->has_data () ||
@@ -224,8 +224,32 @@ hb_aat_layout_substitute (hb_ot_shape_plan_t *plan,
   }
 }
 
+void
+hb_aat_layout_zero_width_deleted_glyphs (hb_buffer_t *buffer)
+{
+  unsigned int count = buffer->len;
+  hb_glyph_info_t *info = buffer->info;
+  hb_glyph_position_t *pos = buffer->pos;
+  for (unsigned int i = 0; i < count; i++)
+    if (unlikely (info[i].codepoint == AAT::DELETED_GLYPH))
+      pos[i].x_advance = pos[i].y_advance = 0;
+}
 
-hb_bool_t
+static bool
+is_deleted_glyph (const hb_glyph_info_t *info)
+{
+  return info->codepoint == AAT::DELETED_GLYPH;
+}
+
+void
+hb_aat_layout_remove_deleted_glyphs (hb_buffer_t *buffer)
+{
+  hb_ot_layout_delete_glyphs_inplace (buffer, is_deleted_glyph);
+}
+
+
+
+bool
 hb_aat_layout_has_positioning (hb_face_t *face)
 {
   return face->table.kerx->has_data ();
@@ -248,7 +272,7 @@ hb_aat_layout_position (hb_ot_shape_plan_t *plan,
 }
 
 
-hb_bool_t
+bool
 hb_aat_layout_has_tracking (hb_face_t *face)
 {
   return face->table.trak->has_data ();

src/hb-aat-layout.hh

@@ -56,7 +56,7 @@ HB_INTERNAL void
 hb_aat_layout_compile_map (const hb_aat_map_builder_t *mapper,
 			   hb_aat_map_t *map);
 
-HB_INTERNAL hb_bool_t
+HB_INTERNAL bool
 hb_aat_layout_has_substitution (hb_face_t *face);
 
 HB_INTERNAL void
@@ -64,7 +64,13 @@ hb_aat_layout_substitute (hb_ot_shape_plan_t *plan,
 			  hb_font_t *font,
 			  hb_buffer_t *buffer);
 
-HB_INTERNAL hb_bool_t
+HB_INTERNAL void
+hb_aat_layout_zero_width_deleted_glyphs (hb_buffer_t *buffer);
+
+HB_INTERNAL void
+hb_aat_layout_remove_deleted_glyphs (hb_buffer_t *buffer);
+
+HB_INTERNAL bool
 hb_aat_layout_has_positioning (hb_face_t *face);
 
 HB_INTERNAL void
@@ -72,7 +78,7 @@ hb_aat_layout_position (hb_ot_shape_plan_t *plan,
 			hb_font_t *font,
 			hb_buffer_t *buffer);
 
-HB_INTERNAL hb_bool_t
+HB_INTERNAL bool
 hb_aat_layout_has_tracking (hb_face_t *face);
 
 HB_INTERNAL void

src/hb-ot-layout-gpos-table.hh

@@ -113,7 +113,7 @@ struct ValueFormat : HBUINT16
     if (!format) return ret;
 
     hb_font_t *font = c->font;
-    hb_bool_t horizontal = HB_DIRECTION_IS_HORIZONTAL (c->direction);
+    bool horizontal = HB_DIRECTION_IS_HORIZONTAL (c->direction);
 
     if (format & xPlacement) glyph_pos.x_offset  += font->em_scale_x (get_short (values++, &ret));
     if (format & yPlacement) glyph_pos.y_offset  += font->em_scale_y (get_short (values++, &ret));
@@ -271,10 +271,10 @@ struct AnchorFormat2
     unsigned int x_ppem = font->x_ppem;
     unsigned int y_ppem = font->y_ppem;
     hb_position_t cx = 0, cy = 0;
-    hb_bool_t ret;
+    bool ret;
 
     ret = (x_ppem || y_ppem) &&
-	   font->get_glyph_contour_point_for_origin (glyph_id, anchorPoint, HB_DIRECTION_LTR, &cx, &cy);
+	  font->get_glyph_contour_point_for_origin (glyph_id, anchorPoint, HB_DIRECTION_LTR, &cx, &cy);
     *x = ret && x_ppem ? cx : font->em_fscale_x (xCoordinate);
     *y = ret && y_ppem ? cy : font->em_fscale_y (yCoordinate);
   }

src/hb-ot-layout.cc

@@ -57,13 +57,13 @@
  * kern
  */
 
-hb_bool_t
+bool
 hb_ot_layout_has_kerning (hb_face_t *face)
 {
   return face->table.kern->has_data ();
 }
 
-hb_bool_t
+bool
 hb_ot_layout_has_cross_kerning (hb_face_t *face)
 {
   return face->table.kern->has_cross_stream ();
@@ -423,7 +423,7 @@ hb_ot_layout_table_get_feature_tags (hb_face_t    *face,
   return g.get_feature_tags (start_offset, feature_count, feature_tags);
 }
 
-hb_bool_t
+bool
 hb_ot_layout_table_find_feature (hb_face_t    *face,
 				 hb_tag_t      table_tag,
 				 hb_tag_t      feature_tag,
@@ -933,12 +933,12 @@ hb_ot_layout_lookup_would_substitute (hb_face_t            *face,
 						    zero_context);
 }
 
-hb_bool_t
+bool
 hb_ot_layout_lookup_would_substitute_fast (hb_face_t            *face,
 					   unsigned int          lookup_index,
 					   const hb_codepoint_t *glyphs,
 					   unsigned int          glyphs_length,
-					   hb_bool_t             zero_context)
+					   bool                  zero_context)
 {
   if (unlikely (lookup_index >= face->table.GSUB->lookup_count)) return false;
   OT::hb_would_apply_context_t c (face, glyphs, glyphs_length, (bool) zero_context);
@@ -955,6 +955,56 @@ hb_ot_layout_substitute_start (hb_font_t    *font,
 _hb_ot_layout_set_glyph_props (font, buffer);
 }
 
+void
+hb_ot_layout_delete_glyphs_inplace (hb_buffer_t *buffer,
+				    bool (*filter) (const hb_glyph_info_t *info))
+{
+  /* Merge clusters and delete filtered glyphs.
+   * NOTE! We can't use out-buffer as we have positioning data. */
+  unsigned int j = 0;
+  unsigned int count = buffer->len;
+  hb_glyph_info_t *info = buffer->info;
+  hb_glyph_position_t *pos = buffer->pos;
+  for (unsigned int i = 0; i < count; i++)
+  {
+    if (filter (&info[i]))
+    {
+      /* Merge clusters.
+       * Same logic as buffer->delete_glyph(), but for in-place removal. */
+
+      unsigned int cluster = info[i].cluster;
+      if (i + 1 < count && cluster == info[i + 1].cluster)
+	continue; /* Cluster survives; do nothing. */
+
+      if (j)
+      {
+	/* Merge cluster backward. */
+	if (cluster < info[j - 1].cluster)
+	{
+	  unsigned int mask = info[i].mask;
+	  unsigned int old_cluster = info[j - 1].cluster;
+	  for (unsigned k = j; k && info[k - 1].cluster == old_cluster; k--)
+	    buffer->set_cluster (info[k - 1], cluster, mask);
+	}
+	continue;
+      }
+
+      if (i + 1 < count)
+	buffer->merge_clusters (i, i + 2); /* Merge cluster forward. */
+
+      continue;
+    }
+
+    if (j != i)
+    {
+      info[j] = info[i];
+      pos[j] = pos[i];
+    }
+    j++;
+  }
+  buffer->len = j;
+}
+
 /**
  * hb_ot_layout_lookup_substitute_closure:
  *

src/hb-ot-layout.hh

@@ -45,10 +45,10 @@ struct hb_ot_shape_plan_t;
  * kern
  */
 
-HB_INTERNAL hb_bool_t
+HB_INTERNAL bool
 hb_ot_layout_has_kerning (hb_face_t *face);
 
-HB_INTERNAL hb_bool_t
+HB_INTERNAL bool
 hb_ot_layout_has_cross_kerning (hb_face_t *face);
 
 HB_INTERNAL void
@@ -59,7 +59,7 @@ hb_ot_layout_kern (hb_ot_shape_plan_t *plan,
 
 /* Private API corresponding to hb-ot-layout.h: */
 
-HB_INTERNAL hb_bool_t
+HB_INTERNAL bool
 hb_ot_layout_table_find_feature (hb_face_t    *face,
 				 hb_tag_t      table_tag,
 				 hb_tag_t      feature_tag,
@@ -93,12 +93,12 @@ HB_MARK_AS_FLAG_T (hb_ot_layout_glyph_props_flags_t);
  * GSUB/GPOS
  */
 
-HB_INTERNAL hb_bool_t
+HB_INTERNAL bool
 hb_ot_layout_lookup_would_substitute_fast (hb_face_t            *face,
 					   unsigned int          lookup_index,
 					   const hb_codepoint_t *glyphs,
 					   unsigned int          glyphs_length,
-					   hb_bool_t             zero_context);
+					   bool                  zero_context);
 
 
 /* Should be called before all the substitute_lookup's are done. */
@@ -106,6 +106,9 @@ HB_INTERNAL void
 hb_ot_layout_substitute_start (hb_font_t    *font,
 			       hb_buffer_t  *buffer);
 
+HB_INTERNAL void
+hb_ot_layout_delete_glyphs_inplace (hb_buffer_t *buffer,
+				    bool (*filter) (const hb_glyph_info_t *info));
 
 namespace OT {
   struct hb_ot_apply_context_t;
@@ -306,13 +309,13 @@ _hb_glyph_info_get_unicode_space_fallback_type (const hb_glyph_info_t *info)
 
 static inline bool _hb_glyph_info_ligated (const hb_glyph_info_t *info);
 
-static inline hb_bool_t
+static inline bool
 _hb_glyph_info_is_default_ignorable (const hb_glyph_info_t *info)
 {
   return (info->unicode_props() & UPROPS_MASK_IGNORABLE) &&
 	 !_hb_glyph_info_ligated (info);
 }
-static inline hb_bool_t
+static inline bool
 _hb_glyph_info_is_default_ignorable_and_not_hidden (const hb_glyph_info_t *info)
 {
   return ((info->unicode_props() & (UPROPS_MASK_IGNORABLE|UPROPS_MASK_HIDDEN))
@@ -366,17 +369,17 @@ _hb_glyph_info_is_unicode_format (const hb_glyph_info_t *info)
   return _hb_glyph_info_get_general_category (info) ==
 	 HB_UNICODE_GENERAL_CATEGORY_FORMAT;
 }
-static inline hb_bool_t
+static inline bool
 _hb_glyph_info_is_zwnj (const hb_glyph_info_t *info)
 {
   return _hb_glyph_info_is_unicode_format (info) && (info->unicode_props() & UPROPS_MASK_Cf_ZWNJ);
 }
-static inline hb_bool_t
+static inline bool
 _hb_glyph_info_is_zwj (const hb_glyph_info_t *info)
 {
   return _hb_glyph_info_is_unicode_format (info) && (info->unicode_props() & UPROPS_MASK_Cf_ZWJ);
 }
-static inline hb_bool_t
+static inline bool
 _hb_glyph_info_is_joiner (const hb_glyph_info_t *info)
 {
   return _hb_glyph_info_is_unicode_format (info) && (info->unicode_props() & (UPROPS_MASK_Cf_ZWNJ|UPROPS_MASK_Cf_ZWJ));

src/hb-ot-shape.cc

@@ -476,7 +476,9 @@ hb_ensure_native_direction (hb_buffer_t *buffer)
 }
 
 
-/* Substitute */
+/*
+ * Substitute
+ */
 
 static inline void
 hb_ot_mirror_chars (const hb_ot_shape_context_t *c)
@@ -582,10 +584,8 @@ hb_ot_shape_setup_masks (const hb_ot_shape_context_t *c)
 }
 
 static void
-hb_ot_zero_width_default_ignorables (const hb_ot_shape_context_t *c)
+hb_ot_zero_width_default_ignorables (const hb_buffer_t *buffer)
 {
-  hb_buffer_t *buffer = c->buffer;
-
   if (!(buffer->scratch_flags & HB_BUFFER_SCRATCH_FLAG_HAS_DEFAULT_IGNORABLES) ||
       (buffer->flags & HB_BUFFER_FLAG_PRESERVE_DEFAULT_IGNORABLES) ||
       (buffer->flags & HB_BUFFER_FLAG_REMOVE_DEFAULT_IGNORABLES))
@@ -601,21 +601,19 @@ hb_ot_zero_width_default_ignorables (const hb_ot_shape_context_t *c)
 }
 
 static void
-hb_ot_hide_default_ignorables (const hb_ot_shape_context_t *c)
+hb_ot_hide_default_ignorables (hb_buffer_t *buffer,
+			       hb_font_t   *font)
 {
-  hb_buffer_t *buffer = c->buffer;
-
   if (!(buffer->scratch_flags & HB_BUFFER_SCRATCH_FLAG_HAS_DEFAULT_IGNORABLES) ||
       (buffer->flags & HB_BUFFER_FLAG_PRESERVE_DEFAULT_IGNORABLES))
     return;
 
   unsigned int count = buffer->len;
   hb_glyph_info_t *info = buffer->info;
-  hb_glyph_position_t *pos = buffer->pos;
 
-  hb_codepoint_t invisible = c->buffer->invisible;
+  hb_codepoint_t invisible = buffer->invisible;
   if (!(buffer->flags & HB_BUFFER_FLAG_REMOVE_DEFAULT_IGNORABLES) &&
-      (invisible || c->font->get_nominal_glyph (' ', &invisible)))
+      (invisible || font->get_nominal_glyph (' ', &invisible)))
   {
     /* Replace default-ignorables with a zero-advance invisible glyph. */
     for (unsigned int i = 0; i < count; i++)
@@ -625,49 +623,7 @@ hb_ot_hide_default_ignorables (const hb_ot_shape_context_t *c)
     }
   }
   else
-  {
+    hb_ot_layout_delete_glyphs_inplace (buffer, _hb_glyph_info_is_default_ignorable);
-    /* Merge clusters and delete default-ignorables.
-     * NOTE! We can't use out-buffer as we have positioning data. */
-    unsigned int j = 0;
-    for (unsigned int i = 0; i < count; i++)
-    {
-      if (_hb_glyph_info_is_default_ignorable (&info[i]))
-      {
-	/* Merge clusters.
-	 * Same logic as buffer->delete_glyph(), but for in-place removal. */
-
-	unsigned int cluster = info[i].cluster;
-	if (i + 1 < count && cluster == info[i + 1].cluster)
-	  continue; /* Cluster survives; do nothing. */
-
-	if (j)
-	{
-	  /* Merge cluster backward. */
-	  if (cluster < info[j - 1].cluster)
-	  {
-	    unsigned int mask = info[i].mask;
-	    unsigned int old_cluster = info[j - 1].cluster;
-	    for (unsigned k = j; k && info[k - 1].cluster == old_cluster; k--)
-	      buffer->set_cluster (info[k - 1], cluster, mask);
-	  }
-	  continue;
-	}
-
-	if (i + 1 < count)
-	  buffer->merge_clusters (i, i + 2); /* Merge cluster forward. */
-
-	continue;
-      }
-
-      if (j != i)
-      {
-	info[j] = info[i];
-	pos[j] = pos[i];
-      }
-      j++;
-    }
-    buffer->len = j;
-  }
 }
 
 
@@ -684,10 +640,10 @@ hb_ot_map_glyphs_fast (hb_buffer_t  *buffer)
 }
 
 static inline void
-hb_synthesize_glyph_classes (const hb_ot_shape_context_t *c)
+hb_synthesize_glyph_classes (hb_buffer_t *buffer)
 {
-  unsigned int count = c->buffer->len;
+  unsigned int count = buffer->len;
-  hb_glyph_info_t *info = c->buffer->info;
+  hb_glyph_info_t *info = buffer->info;
   for (unsigned int i = 0; i < count; i++)
   {
     hb_ot_layout_glyph_props_flags_t klass;
@@ -739,7 +695,7 @@ hb_ot_substitute_complex (const hb_ot_shape_context_t *c)
   hb_ot_layout_substitute_start (c->font, buffer);
 
   if (c->plan->fallback_glyph_classes)
-    hb_synthesize_glyph_classes (c);
+    hb_synthesize_glyph_classes (c->buffer);
 
   if (unlikely (c->plan->apply_morx))
     hb_aat_layout_substitute (c->plan, c->font, c->buffer);
@@ -748,7 +704,7 @@ hb_ot_substitute_complex (const hb_ot_shape_context_t *c)
 }
 
 static inline void
-hb_ot_substitute (const hb_ot_shape_context_t *c)
+hb_ot_substitute_pre (const hb_ot_shape_context_t *c)
 {
   hb_ot_substitute_default (c);
 
@@ -757,7 +713,21 @@ hb_ot_substitute (const hb_ot_shape_context_t *c)
   hb_ot_substitute_complex (c);
 }
 
-/* Position */
+static inline void
+hb_ot_substitute_post (const hb_ot_shape_context_t *c)
+{
+  hb_ot_hide_default_ignorables (c->buffer, c->font);
+  if (c->plan->apply_morx)
+    hb_aat_layout_remove_deleted_glyphs (c->buffer);
+
+  if (c->plan->shaper->postprocess_glyphs)
+    c->plan->shaper->postprocess_glyphs (c->plan, c->buffer, c->font);
+}
+
+
+/*
+ * Position
+ */
 
 static inline void
 adjust_mark_offsets (hb_glyph_position_t *pos)
@@ -890,9 +860,11 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c)
 	break;
     }
 
-  /* Finishing off GPOS has to follow a certain order. */
+  /* Finish off.  Has to follow a certain order. */
   hb_ot_layout_position_finish_advances (c->font, c->buffer);
-  hb_ot_zero_width_default_ignorables (c);
+  hb_ot_zero_width_default_ignorables (c->buffer);
+  if (c->plan->apply_morx)
+    hb_aat_layout_zero_width_deleted_glyphs (c->buffer);
   hb_ot_layout_position_finish_offsets (c->font, c->buffer);
 
   /* The nil glyph_h_origin() func returns 0, so no need to apply it. */
@@ -983,13 +955,9 @@ hb_ot_shape_internal (hb_ot_shape_context_t *c)
   if (c->plan->shaper->preprocess_text)
     c->plan->shaper->preprocess_text (c->plan, c->buffer, c->font);
 
-  hb_ot_substitute (c);
+  hb_ot_substitute_pre (c);
   hb_ot_position (c);
-
+  hb_ot_substitute_post (c);
-  hb_ot_hide_default_ignorables (c);
-
-  if (c->plan->shaper->postprocess_glyphs)
-    c->plan->shaper->postprocess_glyphs (c->plan, c->buffer, c->font);
 
   hb_propagate_flags (c->buffer);