[HarfBuzz] improving hb_ot_hide_default_ignorables

[Jonathan Kew]

Hi Behdad,

With the change in variation selector handling, stray (unsupported) VS 
codes are no longer consumed by the get_glyph callback; they remain in 
the buffer and are individually mapped to glyphs. That's all very well; 
but in most cases they'll just map to .notdef.

Eventually, hb_ot_hide_default_ignorables() will run across these 
.notdef glyphs and replace them with zero-width space glyphs. All's good...

...except in the (unlikely) case where the font does not actually 
support the <space> character. This will be almost vanishingly rare, of 
course - but unfortunately it happens with the "MarkA" font that's used 
for a variety of rendering tests.

This prompted me to look into patching hb_ot_hide_default_ignorables() 
so that it will work even if there's no <space> in the font. ISTM we can 
fix this by simply deleting the ignorable altogether in this case. It'll 
be a bit less performant than the current replace-with-zero-width-space 
implementation, because it requires shifting all the following glyphs up 
in the buffer, so I've retained the existing implementation for the 
(usual) case where <space> is available.

Suggested patch is attached.

JK
-------------- next part --------------
From: Jonathan Kew <jkew at mozilla.com>

diff --git a/gfx/harfbuzz/src/hb-ot-shape.cc b/gfx/harfbuzz/src/hb-ot-shape.cc
--- a/gfx/harfbuzz/src/hb-ot-shape.cc
+++ b/gfx/harfbuzz/src/hb-ot-shape.cc
@@ -531,28 +531,40 @@ static void
 hb_ot_hide_default_ignorables (hb_ot_shape_context_t *c)
 {
   if (c->buffer->flags & HB_BUFFER_FLAG_PRESERVE_DEFAULT_IGNORABLES)
     return;
 
   hb_codepoint_t space = 0;
 
   unsigned int count = c->buffer->len;
+  hb_glyph_info_t *info = c->buffer->info;
+  hb_glyph_position_t *pos = c->buffer->pos;
   for (unsigned int i = 0; i < count; i++)
-    if (unlikely (!is_a_ligature (c->buffer->info[i]) &&
-		  _hb_glyph_info_is_default_ignorable (&c->buffer->info[i])))
+    if (unlikely (!is_a_ligature (info[i]) &&
+		  _hb_glyph_info_is_default_ignorable (&info[i])))
     {
       if (!space) {
         /* We assume that the space glyph is not gid0. */
-        if (unlikely (!c->font->get_glyph (' ', 0, &space)) || !space)
-	return; /* No point! */
+	if (unlikely (!c->font->get_glyph (' ', 0, &space)) || !space) {
+	  /*
+	   * Failed to find a space glyph(!), so delete the ignorable entirely
+	   * by shortening the buffer and shifting everything up.
+	   */
+	  c->buffer->len = --count;
+	  for (unsigned int j = i; j < count; j++) {
+	    info[j] = info[j + 1];
+	    pos[j] = pos[j + 1];
+	  }
+	  continue;
+	}
       }
-      c->buffer->info[i].codepoint = space;
-      c->buffer->pos[i].x_advance = 0;
-      c->buffer->pos[i].y_advance = 0;
+      info[i].codepoint = space;
+      pos[i].x_advance = 0;
+      pos[i].y_advance = 0;
     }
 }
 
 
 /* Pull it all together! */
 
 static void
 hb_ot_shape_internal (hb_ot_shape_context_t *c)