[HarfBuzz] handling of variation selectors

[Jonathan Kew]

I think there's a problem that arises due to the change in 
79d1007a501fd63c0ba4d51038c513e6b8b94740, whereby variation selectors 
not directly used by the cmap (i.e. by a format 14 subtable) are mapped 
separately via the standard cmap subtable so that GSUB processing can 
handle them.

This will work fine for fonts that actually expect to handle VS codes in 
this way. The trouble is that in many cases, the VS codepoint will -not- 
be mapped to a suitable glyph code by the cmap; it will simply map to 
the .notdef glyph, which will then appear as a box or similar unwanted 
artifact in the output. (It would also -block- GSUB/GPOS rules from 
applying as expected to the base and any following glyphs.)

So in the case where the font does not actually map the VS codepoint to 
a glyph, we should simply delete it from the buffer.

Maybe something like the attached patch...?

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

diff --git a/gfx/harfbuzz/src/hb-ot-shape-normalize.cc b/gfx/harfbuzz/src/hb-ot-shape-normalize.cc
--- a/gfx/harfbuzz/src/hb-ot-shape-normalize.cc
+++ b/gfx/harfbuzz/src/hb-ot-shape-normalize.cc
@@ -218,21 +218,28 @@ handle_variation_selector_cluster (const
     if (unlikely (buffer->unicode->is_variation_selector (buffer->cur(+1).codepoint))) {
       /* The next two lines are some ugly lines... But work. */
       if (c->font->get_glyph (buffer->cur().codepoint, buffer->cur(+1).codepoint, &buffer->cur().glyph_index()))
       {
 	buffer->replace_glyphs (2, 1, &buffer->cur().codepoint);
       }
       else
       {
-        /* Just pass on the two characters separately, let GSUB do its magic. */
 	set_glyph (buffer->cur(), c->font);
-	buffer->next_glyph ();
-	set_glyph (buffer->cur(), c->font);
-	buffer->next_glyph ();
+	/* Check whether the font supports the variation selector as a separate glyph */
+	hb_codepoint_t vs_glyph;
+	if (c->font->get_glyph (buffer->cur(+1).codepoint, 0, &vs_glyph))
+	{
+	  /* Just pass on the two characters separately, let GSUB do its magic. */
+	  buffer->next_glyph ();
+	  buffer->cur().glyph_index() = vs_glyph;
+	  buffer->next_glyph ();
+	} else {
+	  buffer->replace_glyphs (2, 1, &buffer->cur().codepoint);
+	}
       }
       /* Skip any further variation selectors. */
       while (buffer->idx < end && unlikely (buffer->unicode->is_variation_selector (buffer->cur().codepoint)))
       {
 	set_glyph (buffer->cur(), c->font);
 	buffer->next_glyph ();
       }
     } else {