[subset] COLR : only include glyphs after COLR closure

src/hb-ot-color-colr-table.hh

@@ -1025,7 +1025,7 @@ struct ClipList
     if (unlikely (!c->serializer->extend_min (out))) return_trace (false);
     if (!c->serializer->check_assign (out->format, format, HB_SERIALIZE_ERROR_INT_OVERFLOW)) return_trace (false);
 
-    const hb_set_t& glyphset = *c->plan->_glyphset;
+    const hb_set_t& glyphset = *c->plan->_glyphset_colred;
     const hb_map_t &glyph_map = *c->plan->glyph_map;
 
     hb_map_t new_gid_offset_map;
@@ -1193,7 +1193,7 @@ struct BaseGlyphList : SortedArray32Of<BaseGlyphPaintRecord>
     TRACE_SUBSET (this);
     auto *out = c->serializer->start_embed (this);
     if (unlikely (!c->serializer->extend_min (out)))  return_trace (false);
-    const hb_set_t* glyphset = c->plan->_glyphset;
+    const hb_set_t* glyphset = c->plan->_glyphset_colred;
 
     for (const auto& _ : as_array ())
     {
@@ -1411,10 +1411,9 @@ struct COLR
 
   const BaseGlyphRecord* get_base_glyph_record (hb_codepoint_t gid) const
   {
-    if ((unsigned int) gid == 0) // Ignore notdef.
-      return nullptr;
     const BaseGlyphRecord* record = &(this+baseGlyphsZ).bsearch (numBaseGlyphs, (unsigned int) gid);
-    if ((record && (hb_codepoint_t) record->glyphId != gid))
+    if (record == &Null (BaseGlyphRecord) ||
+        (record && (hb_codepoint_t) record->glyphId != gid))
       record = nullptr;
     return record;
   }
@@ -1432,9 +1431,16 @@ struct COLR
     TRACE_SUBSET (this);
 
     const hb_map_t &reverse_glyph_map = *c->plan->reverse_glyph_map;
+    const hb_set_t& glyphset = *c->plan->_glyphset_colred;
 
     auto base_it =
     + hb_range (c->plan->num_output_glyphs ())
+    | hb_filter ([&](hb_codepoint_t new_gid)
+		 {
+		    hb_codepoint_t old_gid = reverse_glyph_map.get (new_gid);
+		    if (glyphset.has (old_gid)) return true;
+		    return false;
+		 })
     | hb_map_retains_sorting ([&](hb_codepoint_t new_gid)
 			      {
 				hb_codepoint_t old_gid = reverse_glyph_map.get (new_gid);
@@ -1442,7 +1448,6 @@ struct COLR
 				const BaseGlyphRecord* old_record = get_base_glyph_record (old_gid);
 				if (unlikely (!old_record))
 				  return hb_pair_t<bool, BaseGlyphRecord> (false, Null (BaseGlyphRecord));
-
 				BaseGlyphRecord new_record = {};
 				new_record.glyphId = new_gid;
 				new_record.numLayers = old_record->numLayers;
@@ -1455,6 +1460,7 @@ struct COLR
     auto layer_it =
     + hb_range (c->plan->num_output_glyphs ())
     | hb_map (reverse_glyph_map)
+    | hb_filter (glyphset)
     | hb_map_retains_sorting ([&](hb_codepoint_t old_gid)
 			      {
 				const BaseGlyphRecord* old_record = get_base_glyph_record (old_gid);

src/hb-subset-plan.cc

@@ -248,7 +248,6 @@ static void _colr_closure (hb_face_t *face,
   unsigned glyphs_num;
   {
     glyphs_num = glyphs_colred->get_population ();
-
     // Collect all glyphs referenced by COLRv0
     hb_set_t glyphset_colrv0;
     for (hb_codepoint_t gid : glyphs_colred->iter ())
@@ -397,6 +396,7 @@ _populate_gids_to_retain (hb_subset_plan_t* plan,
   _colr_closure (plan->source, plan->colrv1_layers, plan->colr_palettes, &cur_glyphset);
   _remove_invalid_gids (&cur_glyphset, plan->source->get_num_glyphs ());
 
+  hb_set_set (plan->_glyphset_colred, &cur_glyphset);
   // Populate a full set of glyphs to retain by adding all referenced
   // composite glyphs.
   for (hb_codepoint_t gid : cur_glyphset.iter ())
@@ -511,6 +511,7 @@ hb_subset_plan_create (hb_face_t	 *face,
   plan->_glyphset = hb_set_create ();
   plan->_glyphset_gsub = hb_set_create ();
   plan->_glyphset_mathed = hb_set_create ();
+  plan->_glyphset_colred = hb_set_create ();
   plan->codepoint_to_glyph = hb_map_create ();
   plan->glyph_map = hb_map_create ();
   plan->reverse_glyph_map = hb_map_create ();
@@ -579,6 +580,7 @@ hb_subset_plan_destroy (hb_subset_plan_t *plan)
   hb_set_destroy (plan->_glyphset);
   hb_set_destroy (plan->_glyphset_gsub);
   hb_set_destroy (plan->_glyphset_mathed);
+  hb_set_destroy (plan->_glyphset_colred);
   hb_map_destroy (plan->gsub_lookups);
   hb_map_destroy (plan->gpos_lookups);
   hb_map_destroy (plan->gsub_features);

src/hb-subset-plan.hh

@@ -78,6 +78,7 @@ struct hb_subset_plan_t
   hb_set_t *_glyphset;
   hb_set_t *_glyphset_gsub;
   hb_set_t *_glyphset_mathed;
+  hb_set_t *_glyphset_colred;
 
   //active lookups we'd like to retain
   hb_map_t *gsub_lookups;

test/subset/data/Makefile.am

@@ -46,6 +46,7 @@ EXTRA_DIST += \
 	expected/cmap14 \
 	expected/sbix \
 	expected/colr \
+	expected/colr_glyphs \
 	expected/colrv1 \
 	expected/colr_with_components \
 	expected/cbdt \

test/subset/data/Makefile.sources

@@ -7,6 +7,7 @@ TESTS = \
 	tests/cmap.tests \
 	tests/cmap14.tests \
 	tests/colr.tests \
+	tests/colr_glyphs.tests \
 	tests/colrv1.tests \
 	tests/colr_with_components.tests \
 	tests/full-font.tests \

test/subset/data/tests/colr_glyphs.tests

@@ -0,0 +1,11 @@
+FONTS:
+BungeeColor-Regular.ttf
+
+PROFILES:
+default.txt
+drop-hints.txt
+drop-hints-retain-gids.txt
+retain-gids.txt
+
+SUBSETS:
+A

test/subset/meson.build

@@ -38,6 +38,7 @@ tests = [
   'cmap14',
   'sbix',
   'colr',
+  'colr_glyphs',
   'math',
   'math_coverage_offset',
 # TODO: re-enable once colrv1 subsetting is stabilized.