[subset] fix bug in (Chain)ContextFormat2

Only keep rulesets for glyphs class numbers that survived in coverage

11 files changed, 103 insertions, 9 deletions

diff --git a/src/hb-ot-layout-common.hh b/src/hb-ot-layout-common.hh
index a43f5f8c0..5d98278be 100644
--- a/src/hb-ot-layout-common.hh
+++ b/src/hb-ot-layout-common.hh
@@ -1336,7 +1336,7 @@ struct Lookup
       outMarkFilteringSet = markFilteringSet;
     }
 
-    return_trace (true);
+    return_trace (out->subTable.len);
   }
 
   template <typename TSubTable>
@@ -2081,6 +2081,22 @@ struct ClassDefFormat1
         intersect_glyphs->add (startGlyph + i);
   }
 
+  void intersected_classes (const hb_set_t *glyphs, hb_set_t *intersect_classes) const
+  {
+    if (glyphs->is_empty ()) return;
+    hb_codepoint_t end_glyph = startGlyph + classValue.len - 1;
+    if (glyphs->get_min () < startGlyph ||
+        glyphs->get_max () > end_glyph)
+      intersect_classes->add (0);
+
+    for (const auto& _ : + hb_enumerate (classValue))
+    {
+      hb_codepoint_t g = startGlyph + _.first;
+      if (glyphs->has (g))
+        intersect_classes->add (_.second);
+    }
+  }
+
   protected:
   HBUINT16	classFormat;	/* Format identifier--format = 1 */
   HBGlyphID16	startGlyph;	/* First GlyphID of the classValueArray */
@@ -2314,6 +2330,31 @@ struct ClassDefFormat2
     }
   }
 
+  void intersected_classes (const hb_set_t *glyphs, hb_set_t *intersect_classes) const
+  {
+    if (glyphs->is_empty ()) return;
+
+    unsigned count = rangeRecord.len;
+    hb_codepoint_t g = HB_SET_VALUE_INVALID;
+    for (unsigned int i = 0; i < count; i++)
+    {
+      if (!hb_set_next (glyphs, &g))
+        break;
+      if (g < rangeRecord[i].first)
+      {
+        intersect_classes->add (0);
+        break;
+      }
+      g = rangeRecord[i].last;
+    }
+    if (g != HB_SET_VALUE_INVALID && hb_set_next (glyphs, &g))
+      intersect_classes->add (0);
+
+    for (const RangeRecord& record : rangeRecord.iter ())
+      if (record.intersects (glyphs))
+        intersect_classes->add (record.value);
+  }
+
   protected:
   HBUINT16	classFormat;	/* Format identifier--format = 2 */
   SortedArray16Of<RangeRecord>
@@ -2466,6 +2507,16 @@ struct ClassDef
     }
   }
 
+  void intersected_classes (const hb_set_t *glyphs, hb_set_t *intersect_classes) const
+  {
+    switch (u.format) {
+    case 1: return u.format1.intersected_classes (glyphs, intersect_classes);
+    case 2: return u.format2.intersected_classes (glyphs, intersect_classes);
+    default:return;
+    }
+  }
+
+
   protected:
   union {
   HBUINT16		format;		/* Format identifier */
diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh
index 59abdf737..2dddb6da2 100644
--- a/src/hb-ot-layout-gsubgpos.hh
+++ b/src/hb-ot-layout-gsubgpos.hh
@@ -1950,12 +1950,20 @@ struct ContextFormat2
       &class_def
     };
 
+    hb_set_t retained_coverage_glyphs;
+    (this+coverage).intersected_coverage_glyphs (glyphs, &retained_coverage_glyphs);
+
+    hb_set_t coverage_glyph_classes;
+    class_def.intersected_classes (&retained_coverage_glyphs, &coverage_glyph_classes);
+
+
     return
     + hb_iter (ruleSet)
     | hb_map (hb_add (this))
     | hb_enumerate
     | hb_map ([&] (const hb_pair_t<unsigned, const RuleSet &> p)
 	      { return class_def.intersects_class (glyphs, p.first) &&
+		       coverage_glyph_classes.has (p.first) &&
 		       p.second.intersects (glyphs, lookup_context); })
     | hb_any
     ;
@@ -2076,9 +2084,16 @@ struct ContextFormat2
     hb_map_t klass_map;
     out->classDef.serialize_subset (c, classDef, this, &klass_map);
 
+    const hb_set_t* glyphset = c->plan->glyphset_gsub ();
+    hb_set_t retained_coverage_glyphs;
+    (this+coverage).intersected_coverage_glyphs (glyphset, &retained_coverage_glyphs);
+    
+    hb_set_t coverage_glyph_classes;
+    (this+classDef).intersected_classes (&retained_coverage_glyphs, &coverage_glyph_classes);
+
     const hb_map_t *lookup_map = c->table_tag == HB_OT_TAG_GSUB ? c->plan->gsub_lookups : c->plan->gpos_lookups;
     bool ret = true;
-    int non_zero_index = 0, index = 0;
+    int non_zero_index = -1, index = 0;
     for (const auto& _ : + hb_enumerate (ruleSet)
 			 | hb_filter (klass_map, hb_first))
     {
@@ -2089,13 +2104,14 @@ struct ContextFormat2
 	break;
       }
 
-      if (o->serialize_subset (c, _.second, this, lookup_map, &klass_map))
+      if (coverage_glyph_classes.has (_.first) &&
+	  o->serialize_subset (c, _.second, this, lookup_map, &klass_map))
 	non_zero_index = index;
 
       index++;
     }
 
-    if (!ret) return_trace (ret);
+    if (!ret || non_zero_index == -1) return_trace (false);
 
     //prune empty trailing ruleSets
     --index;
@@ -2910,12 +2926,19 @@ struct ChainContextFormat2
        &lookahead_class_def}
     };
 
+    hb_set_t retained_coverage_glyphs;
+    (this+coverage).intersected_coverage_glyphs (glyphs, &retained_coverage_glyphs);
+
+    hb_set_t coverage_glyph_classes;
+    input_class_def.intersected_classes (&retained_coverage_glyphs, &coverage_glyph_classes);
+
     return
     + hb_iter (ruleSet)
     | hb_map (hb_add (this))
     | hb_enumerate
     | hb_map ([&] (const hb_pair_t<unsigned, const ChainRuleSet &> p)
 	      { return input_class_def.intersects_class (glyphs, p.first) &&
+		       coverage_glyph_classes.has (p.first) &&
 		       p.second.intersects (glyphs, lookup_context); })
     | hb_any
     ;
@@ -3070,13 +3093,19 @@ struct ChainContextFormat2
 						   lookahead_klass_map)))
       return_trace (false);
 
+    const hb_set_t* glyphset = c->plan->glyphset_gsub ();
+    hb_set_t retained_coverage_glyphs;
+    (this+coverage).intersected_coverage_glyphs (glyphset, &retained_coverage_glyphs);
+
+    hb_set_t coverage_glyph_classes;
+    (this+inputClassDef).intersected_classes (&retained_coverage_glyphs, &coverage_glyph_classes);
+
     int non_zero_index = -1, index = 0;
     bool ret = true;
     const hb_map_t *lookup_map = c->table_tag == HB_OT_TAG_GSUB ? c->plan->gsub_lookups : c->plan->gpos_lookups;
     auto last_non_zero = c->serializer->snapshot ();
-    for (const Offset16To<ChainRuleSet>& _ : + hb_enumerate (ruleSet)
-					   | hb_filter (input_klass_map, hb_first)
-					   | hb_map (hb_second))
+    for (const auto& _ : + hb_enumerate (ruleSet)
+			 | hb_filter (input_klass_map, hb_first))
     {
       auto *o = out->ruleSet.serialize_append (c->serializer);
       if (unlikely (!o))
@@ -3084,7 +3113,8 @@ struct ChainContextFormat2
 	ret = false;
 	break;
       }
-      if (o->serialize_subset (c, _, this,
+      if (coverage_glyph_classes.has (_.first) &&
+          o->serialize_subset (c, _.second, this,
 			       lookup_map,
 			       &backtrack_klass_map,
 			       &input_klass_map,
@@ -3097,7 +3127,7 @@ struct ChainContextFormat2
       index++;
     }
 
-    if (!ret) return_trace (ret);
+    if (!ret || non_zero_index == -1) return_trace (false);
 
     // prune empty trailing ruleSets
     if (index > non_zero_index) {
diff --git a/test/subset/data/Makefile.am b/test/subset/data/Makefile.am
index 15061ef1c..a69005ab9 100644
--- a/test/subset/data/Makefile.am
+++ b/test/subset/data/Makefile.am
@@ -26,6 +26,7 @@ EXTRA_DIST += \
 	expected/layout.gpos9 \
 	expected/layout.gsub3 \
 	expected/layout.gsub5 \
+	expected/layout.gsub5_format2 \
 	expected/layout.gsub6 \
 	expected/layout.gsub8 \
 	expected/layout.khmer \
diff --git a/test/subset/data/Makefile.sources b/test/subset/data/Makefile.sources
index dedcef03a..c1a70ba90 100644
--- a/test/subset/data/Makefile.sources
+++ b/test/subset/data/Makefile.sources
@@ -28,6 +28,7 @@ TESTS = \
 	tests/layout.gpos9.tests \
 	tests/layout.gsub3.tests \
 	tests/layout.gsub5.tests \
+	tests/layout.gsub5_format2.tests \
 	tests/layout.gsub6.tests \
 	tests/layout.gsub8.tests \
 	tests/layout.khmer.tests \
diff --git a/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test-retain-gids.268,301,302,324.ttf b/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test-retain-gids.268,301,302,324.ttf
new file mode 100644
index 000000000..82cc95370
--- /dev/null
+++ b/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test-retain-gids.268,301,302,324.ttf
diff --git a/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test-retain-gids.retain-all-codepoint.ttf b/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test-retain-gids.retain-all-codepoint.ttf
new file mode 100644
index 000000000..06d1fad2f
--- /dev/null
+++ b/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test-retain-gids.retain-all-codepoint.ttf
diff --git a/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test.268,301,302,324.ttf b/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test.268,301,302,324.ttf
new file mode 100644
index 000000000..ceafb38ab
--- /dev/null
+++ b/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test.268,301,302,324.ttf
diff --git a/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test.retain-all-codepoint.ttf b/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test.retain-all-codepoint.ttf
new file mode 100644
index 000000000..895a035ef
--- /dev/null
+++ b/test/subset/data/expected/layout.gsub5_format2/Molengo-Regular.layout-test.retain-all-codepoint.ttf
diff --git a/test/subset/data/fonts/Molengo-Regular.ttf b/test/subset/data/fonts/Molengo-Regular.ttf
new file mode 100644
index 000000000..da82a52ac
--- /dev/null
+++ b/test/subset/data/fonts/Molengo-Regular.ttf
diff --git a/test/subset/data/tests/layout.gsub5_format2.tests b/test/subset/data/tests/layout.gsub5_format2.tests
new file mode 100644
index 000000000..8b3d68ea0
--- /dev/null
+++ b/test/subset/data/tests/layout.gsub5_format2.tests
@@ -0,0 +1,10 @@
+FONTS:
+Molengo-Regular.ttf
+
+PROFILES:
+layout-test.txt
+layout-test-retain-gids.txt
+
+SUBSETS:
+U+268,U+301,U+302,U+324
+*
diff --git a/test/subset/meson.build b/test/subset/meson.build
index 8f7ed8158..729aa3bfc 100644
--- a/test/subset/meson.build
+++ b/test/subset/meson.build
@@ -18,6 +18,7 @@ tests = [
   'layout.gpos9',
   'layout.gsub3',
   'layout.gsub5',
+  'layout.gsub5_format2',
   'layout.gsub6',
   'layout.gsub8',
   'layout.gdef',