Merge pull request #5119 from harfbuzz/aat-deleted-glyph

[AAT] Clean up deleted-glyph handling

src/hb-aat-layout-common.hh

@@ -29,6 +29,8 @@
 
 #include "hb-aat-layout.hh"
 #include "hb-aat-map.hh"
+#include "hb-ot-layout-common.hh"
+#include "hb-ot-layout-gdef-table.hh"
 #include "hb-open-type.hh"
 #include "hb-cache.hh"
 #include "hb-bit-set.hh"
@@ -48,6 +50,8 @@ struct ankr;
 using hb_aat_class_cache_t = hb_cache_t<15, 8, 7>;
 static_assert (sizeof (hb_aat_class_cache_t) == 256, "");
 
+enum { DELETED_GLYPH = 0xFFFF };
+
 struct hb_aat_apply_context_t :
        hb_dispatch_context_t<hb_aat_apply_context_t, bool, HB_DEBUG_APPLY>
 {
@@ -64,7 +68,8 @@ struct hb_aat_apply_context_t :
   hb_buffer_t *buffer;
   hb_sanitize_context_t sanitizer;
   const ankr *ankr_table;
-  const OT::GDEF *gdef_table;
+  const OT::GDEF &gdef;
+  bool has_glyph_classes;
   const hb_sorted_vector_t<hb_aat_map_t::range_flags_t> *range_flags = nullptr;
   bool using_buffer_glyph_set = false;
   hb_bit_set_t buffer_glyph_set;
@@ -106,6 +111,63 @@ struct hb_aat_apply_context_t :
 	return true;
     return false;
   }
+
+  template <typename T>
+  HB_NODISCARD bool output_glyphs (unsigned int count,
+				   const T *glyphs)
+  {
+    for (unsigned int i = 0; i < count; i++)
+    {
+      if (glyphs[i] == DELETED_GLYPH)
+      {
+	buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_DEFAULT_IGNORABLES;
+	_hb_glyph_info_set_default_ignorable (&buffer->cur());
+      }
+      else
+      {
+	buffer_glyph_set.add (glyphs[i]);
+#ifndef HB_NO_OT_LAYOUT
+	if (has_glyph_classes)
+	  _hb_glyph_info_set_glyph_props (&buffer->cur(),
+					  gdef.get_glyph_props (glyphs[i]));
+#endif
+      }
+      if (unlikely (!buffer->output_glyph (glyphs[i]))) return false;
+    }
+    return true;
+  }
+
+  HB_NODISCARD bool replace_glyph (hb_codepoint_t glyph)
+  {
+    if (glyph == DELETED_GLYPH)
+      return delete_glyph ();
+
+    buffer_glyph_set.add (glyph);
+#ifndef HB_NO_OT_LAYOUT
+    if (has_glyph_classes)
+      _hb_glyph_info_set_glyph_props (&buffer->cur(),
+				      gdef.get_glyph_props (glyph));
+#endif
+    return buffer->replace_glyph (glyph);
+  }
+
+  HB_NODISCARD bool delete_glyph ()
+  {
+    buffer->scratch_flags |= HB_BUFFER_SCRATCH_FLAG_HAS_DEFAULT_IGNORABLES;
+    _hb_glyph_info_set_default_ignorable (&buffer->cur());
+    return buffer->replace_glyph (DELETED_GLYPH);
+  }
+
+  void replace_glyph_inplace (unsigned i, hb_codepoint_t glyph)
+  {
+    buffer->info[i].codepoint = glyph;
+    buffer_glyph_set.add (glyph);
+#ifndef HB_NO_OT_LAYOUT
+    if (has_glyph_classes)
+      _hb_glyph_info_set_glyph_props (&buffer->info[i],
+				      gdef.get_glyph_props (glyph));
+#endif
+  }
 };
 
 
@@ -113,8 +175,6 @@ struct hb_aat_apply_context_t :
  * Lookup Table
  */
 
-enum { DELETED_GLYPH = 0xFFFF };
-
 template <typename T> struct Lookup;
 
 template <typename T>

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

@@ -30,8 +30,6 @@
 #include "hb-open-type.hh"
 #include "hb-aat-layout-common.hh"
 #include "hb-ot-layout.hh"
-#include "hb-ot-layout-common.hh"
-#include "hb-ot-layout-gdef-table.hh"
 #include "hb-aat-map.hh"
 
 /*
@@ -242,9 +240,7 @@ struct ContextualSubtable
 	ret (false),
 	c (c_),
 	table (table_),
-	gdef (*c->gdef_table),
 	mark_set (false),
-	has_glyph_classes (gdef.has_glyph_classes ()),
 	mark (0),
 	subs (table+table->substitutionTables) {}
 
@@ -281,12 +277,7 @@ struct ContextualSubtable
       if (replacement)
       {
 	buffer->unsafe_to_break (mark, hb_min (buffer->idx + 1, buffer->len));
-	hb_codepoint_t glyph = *replacement;
-	buffer->info[mark].codepoint = glyph;
-	c->buffer_glyph_set.add (glyph);
-	if (has_glyph_classes)
-	  _hb_glyph_info_set_glyph_props (&buffer->info[mark],
-					  gdef.get_glyph_props (*replacement));
+	c->replace_glyph_inplace (mark, *replacement);
 	ret = true;
       }
 
@@ -312,12 +303,7 @@ struct ContextualSubtable
       }
       if (replacement)
       {
-	hb_codepoint_t glyph = *replacement;
-	buffer->info[idx].codepoint = glyph;
-	c->buffer_glyph_set.add (glyph);
-	if (has_glyph_classes)
-	  _hb_glyph_info_set_glyph_props (&buffer->info[idx],
-					  gdef.get_glyph_props (*replacement));
+	c->replace_glyph_inplace (idx, *replacement);
 	ret = true;
       }
 
@@ -333,9 +319,7 @@ struct ContextualSubtable
     hb_aat_apply_context_t *c;
     const ContextualSubtable *table;
     private:
-    const OT::GDEF &gdef;
     bool mark_set;
-    bool has_glyph_classes;
     unsigned int mark;
     const UnsizedListOfOffset16To<Lookup<HBGlyphID16>, HBUINT, void, false> &subs;
   };
@@ -581,7 +565,7 @@ struct LigatureSubtable
 	    hb_codepoint_t lig = ligatureData;
 
 	    DEBUG_MSG (APPLY, nullptr, "Produced ligature %u", lig);
-	    if (unlikely (!buffer->replace_glyph (lig))) return;
+	    if (unlikely (!c->replace_glyph (lig))) return;
 
 	    unsigned int lig_end = match_positions[(match_length - 1u) % ARRAY_LENGTH (match_positions)] + 1u;
 	    /* Now go and delete all subsequent components. */
@@ -589,8 +573,7 @@ struct LigatureSubtable
 	    {
 	      DEBUG_MSG (APPLY, nullptr, "Skipping ligature component");
 	      if (unlikely (!buffer->move_to (match_positions[--match_length % ARRAY_LENGTH (match_positions)]))) return;
-	      _hb_glyph_info_set_default_ignorable (&buffer->cur());
-	      if (unlikely (!buffer->replace_glyph (DELETED_GLYPH))) return;
+	      if (!c->delete_glyph ()) return;
 	    }
 
 	    if (unlikely (!buffer->move_to (lig_end))) return;
@@ -671,9 +654,6 @@ struct NoncontextualSubtable
       return_trace (false);
     }
 
-    const OT::GDEF &gdef (*c->gdef_table);
-    bool has_glyph_classes = gdef.has_glyph_classes ();
-
     bool ret = false;
     unsigned int num_glyphs = c->face->get_num_glyphs ();
 
@@ -703,12 +683,7 @@ struct NoncontextualSubtable
       const HBGlyphID16 *replacement = substitute.get_value (info[i].codepoint, num_glyphs);
       if (replacement)
       {
-	hb_codepoint_t glyph = *replacement;
-	info[i].codepoint = glyph;
-	c->buffer_glyph_set.add (glyph);
-	if (has_glyph_classes)
-	  _hb_glyph_info_set_glyph_props (&info[i],
-					  gdef.get_glyph_props (*replacement));
+	c->replace_glyph_inplace (i, *replacement);
 	ret = true;
       }
     }
@@ -850,9 +825,7 @@ struct InsertionSubtable
 	if (buffer->idx < buffer->len && !before)
 	  if (unlikely (!buffer->copy_glyph ())) return;
 	/* TODO We ignore KashidaLike setting. */
-	if (unlikely (!buffer->replace_glyphs (0, count, glyphs))) return;
-	for (unsigned int i = 0; i < count; i++)
-	  c->buffer_glyph_set.add (glyphs[i]);
+	if (unlikely (!c->output_glyphs (count, glyphs))) return;
 	ret = true;
 	if (buffer->idx < buffer->len && !before)
 	  buffer->skip_glyph ();
@@ -881,7 +854,8 @@ struct InsertionSubtable
 	if (buffer->idx < buffer->len && !before)
 	  if (unlikely (!buffer->copy_glyph ())) return;
 	/* TODO We ignore KashidaLike setting. */
-	if (unlikely (!buffer->replace_glyphs (0, count, glyphs))) return;
+	if (unlikely (!c->output_glyphs (count, glyphs))) return;
+	ret = true;
 	if (buffer->idx < buffer->len && !before)
 	  buffer->skip_glyph ();
 

src/hb-aat-layout.cc

@@ -58,13 +58,14 @@ AAT::hb_aat_apply_context_t::hb_aat_apply_context_t (const hb_ot_shape_plan_t *p
 						       buffer (buffer_),
 						       sanitizer (),
 						       ankr_table (&Null (AAT::ankr)),
-						       gdef_table (
+						       gdef (
 #ifndef HB_NO_OT_LAYOUT
-							 face->table.GDEF->table
+							 *face->table.GDEF->table
 #else
-							 &Null (GDEF)
+							 Null (GDEF)
 #endif
 						       ),
+						       has_glyph_classes (gdef.has_glyph_classes ()),
 						       lookup_index (0)
 {
   sanitizer.init (blob);
@@ -320,21 +321,11 @@ hb_aat_layout_substitute (const 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 = pos[i].x_offset = pos[i].y_offset = 0;
-}
-
 static bool
 is_deleted_glyph (const hb_glyph_info_t *info)
 {
-  return info->codepoint == AAT::DELETED_GLYPH;
+  return info->codepoint == AAT::DELETED_GLYPH &&
+	 _hb_glyph_info_is_default_ignorable (info);
 }
 
 void

src/hb-aat-layout.hh

@@ -60,9 +60,6 @@ hb_aat_layout_substitute (const hb_ot_shape_plan_t *plan,
 			  const hb_feature_t *features,
 			  unsigned num_features);
 
-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);
 

src/hb-ot-shape.cc

@@ -1109,10 +1109,6 @@ hb_ot_position_plan (const hb_ot_shape_context_t *c)
   /* 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->buffer);
-#ifndef HB_NO_AAT_SHAPE
-  if (c->plan->apply_morx)
-    hb_aat_layout_zero_width_deleted_glyphs (c->buffer);
-#endif
   hb_ot_layout_position_finish_offsets (c->font, c->buffer);
 
   /* The nil glyph_h_origin() func returns 0, so no need to apply it. */