[subset] GDEF Variation Store: step 2

do subset based on variation indices collected in step 1

src/hb-ot-layout-common.hh

@@ -721,6 +721,12 @@ struct FeatureParamsSize
       return_trace (true);
   }
 
+  bool subset (hb_subset_context_t *c) const
+  {
+    TRACE_SUBSET (this);
+    return_trace ((bool) c->serializer->embed (*this));
+  }
+
   HBUINT16	designSize;	/* Represents the design size in 720/inch
 				 * units (decipoints).  The design size entry
 				 * must be non-zero.  When there is a design
@@ -771,6 +777,12 @@ struct FeatureParamsStylisticSet
     return_trace (c->check_struct (this));
   }
 
+  bool subset (hb_subset_context_t *c) const
+  {
+    TRACE_SUBSET (this);
+    return_trace ((bool) c->serializer->embed (*this));
+  }
+
   HBUINT16	version;	/* (set to 0): This corresponds to a “minor”
 				 * version number. Additional data may be
 				 * added to the end of this Feature Parameters
@@ -804,6 +816,15 @@ struct FeatureParamsCharacterVariants
 		  characters.sanitize (c));
   }
 
+  unsigned get_size () const
+  { return min_size + characters.len * HBUINT24::static_size; }
+
+  bool subset (hb_subset_context_t *c) const
+  {
+    TRACE_SUBSET (this);
+    return_trace ((bool) c->serializer->embed (*this));
+  }
+
   HBUINT16	format;			/* Format number is set to 0. */
   NameID	featUILableNameID;	/* The ‘name’ table name ID that
 					 * specifies a string (or strings,
@@ -853,6 +874,19 @@ struct FeatureParams
     return_trace (true);
   }
 
+  bool subset (hb_subset_context_t *c, const Tag* tag) const
+  {
+    TRACE_SUBSET (this);
+    if (!tag) return_trace (false);
+    if (*tag == HB_TAG ('s','i','z','e'))
+      return_trace (u.size.subset (c));
+    if ((*tag & 0xFFFF0000u) == HB_TAG ('s','s','\0','\0')) /* ssXX */
+      return_trace (u.stylisticSet.subset (c));
+    if ((*tag & 0xFFFF0000u) == HB_TAG ('c','v','\0','\0')) /* cvXX */
+      return_trace (u.characterVariants.subset (c));
+    return_trace (false);
+  }
+
 #ifndef HB_NO_LAYOUT_FEATURE_PARAMS
   const FeatureParamsSize& get_size_params (hb_tag_t tag) const
   {
@@ -911,7 +945,7 @@ struct Feature
     auto *out = c->serializer->start_embed (*this);
     if (unlikely (!out || !c->serializer->extend_min (out))) return_trace (false);
 
-    out->featureParams = 0; /* TODO(subset) FeatureParams. */
+    bool subset_featureParams = out->featureParams.serialize_subset (c, featureParams, this, tag);
 
     auto it =
     + hb_iter (lookupIndex)
@@ -920,7 +954,8 @@ struct Feature
     ;
 
     out->lookupIndex.serialize (c->serializer, l, it);
-    return_trace (bool (it) || (tag && *tag == HB_TAG ('p', 'r', 'e', 'f')));
+    return_trace (bool (it) || subset_featureParams
+                  || (tag && *tag == HB_TAG ('p', 'r', 'e', 'f')));
   }
 
   bool sanitize (hb_sanitize_context_t *c,
@@ -2462,6 +2497,41 @@ struct VariationStore
     return_trace (true);
   }
 
+  bool subset (hb_subset_context_t *c) const
+  {
+    TRACE_SUBSET (this);
+
+    VariationStore *varstore_prime = c->serializer->start_embed<VariationStore> ();
+    if (unlikely (!varstore_prime)) return_trace (false);
+
+    const hb_set_t *variation_indices = c->plan->layout_variation_indices;
+    if (variation_indices->is_empty ()) return_trace (false);
+
+    hb_vector_t<hb_inc_bimap_t> inner_maps;
+    inner_maps.resize ((unsigned) dataSets.len);
+    for (unsigned i = 0; i < inner_maps.length; i++)
+      inner_maps[i].init ();
+
+    for (unsigned idx : c->plan->layout_variation_indices->iter ())
+    {
+      uint16_t major = idx >> 16;
+      uint16_t minor = idx & 0xFFFF;
+
+      if (major >= inner_maps.length)
+      {
+        for (unsigned i = 0; i < inner_maps.length; i++)
+          inner_maps[i].fini ();
+        return_trace (false);
+      }
+      inner_maps[major].add (minor);
+    }
+    varstore_prime->serialize (c->serializer, this, inner_maps.as_array ());
+
+    for (unsigned i = 0; i < inner_maps.length; i++)
+      inner_maps[i].fini ();
+    return_trace (bool (varstore_prime->dataSets));
+  }
+
   unsigned int get_region_index_count (unsigned int ivs) const
   { return (this+dataSets[ivs]).get_region_index_count (); }
 
@@ -2932,10 +3002,24 @@ struct VariationDevice
   hb_position_t get_y_delta (hb_font_t *font, const VariationStore &store) const
   { return font->em_scalef_y (get_delta (font, store)); }
 
-  VariationDevice* copy (hb_serialize_context_t *c) const
+  VariationDevice* copy (hb_serialize_context_t *c, const hb_map_t *layout_variation_idx_map) const
   {
     TRACE_SERIALIZE (this);
-    return_trace (c->embed<VariationDevice> (this));
+    auto snap = c->snapshot ();
+    auto *out = c->embed (this);
+    if (unlikely (!out)) return_trace (nullptr);
+    if (!layout_variation_idx_map || layout_variation_idx_map->is_empty ()) return_trace (out);
+
+    unsigned org_idx = (outerIndex << 16) + innerIndex;
+    if (!layout_variation_idx_map->has (org_idx))
+    {
+      c->revert (snap);
+      return_trace (nullptr);
+    }
+    unsigned new_idx = layout_variation_idx_map->get (org_idx);
+    out->outerIndex = new_idx >> 16;
+    out->innerIndex = new_idx & 0xFFFF;
+    return_trace (out);
   }
 
   void record_variation_index (hb_set_t *layout_variation_indices) const
@@ -3029,7 +3113,7 @@ struct Device
     }
   }
 
-  Device* copy (hb_serialize_context_t *c) const
+  Device* copy (hb_serialize_context_t *c, const hb_map_t *layout_variation_idx_map=nullptr) const
   {
     TRACE_SERIALIZE (this);
     switch (u.b.format) {
@@ -3041,7 +3125,7 @@ struct Device
 #endif
 #ifndef HB_NO_VAR
     case 0x8000:
-      return_trace (reinterpret_cast<Device *> (u.variation.copy (c)));
+      return_trace (reinterpret_cast<Device *> (u.variation.copy (c, layout_variation_idx_map)));
 #endif
     default:
       return_trace (nullptr);

src/hb-ot-layout-gdef-table.hh

@@ -173,7 +173,8 @@ struct CaretValueFormat3
     auto *out = c->serializer->embed (this);
     if (unlikely (!out)) return_trace (false);
 
-    return_trace (out->deviceTable.serialize_copy (c->serializer, deviceTable, this));
+    return_trace (out->deviceTable.serialize_copy (c->serializer, deviceTable, this, c->serializer->to_bias (out),
+                                                   hb_serialize_context_t::Head, c->plan->layout_variation_idx_map));
   }
   
   void collect_variation_indices (hb_set_t *layout_variation_indices) const
@@ -610,22 +611,31 @@ struct GDEF
     auto *out = c->serializer->embed (*this);
     if (unlikely (!out)) return_trace (false);
 
-    out->glyphClassDef.serialize_subset (c, glyphClassDef, this);
+    bool subset_glyphclassdef = out->glyphClassDef.serialize_subset (c, glyphClassDef, this);
     out->attachList = 0;//TODO(subset) serialize_subset (c, attachList, this);
-    out->ligCaretList.serialize_subset (c, ligCaretList, this);
-    out->markAttachClassDef.serialize_subset (c, markAttachClassDef, this);
+    bool subset_ligcaretlist = out->ligCaretList.serialize_subset (c, ligCaretList, this);
+    bool subset_markattachclassdef = out->markAttachClassDef.serialize_subset (c, markAttachClassDef, this);
 
+    bool subset_markglyphsetsdef = true;
     if (version.to_int () >= 0x00010002u)
     {
-      if (!out->markGlyphSetsDef.serialize_subset (c, markGlyphSetsDef, this) &&
+      subset_markglyphsetsdef = out->markGlyphSetsDef.serialize_subset (c, markGlyphSetsDef, this);
+      if (!subset_markglyphsetsdef &&
           version.to_int () == 0x00010002u)
         out->version.minor = 0;
     }
 
+    bool subset_varstore = true;
     if (version.to_int () >= 0x00010003u)
-      out->varStore = 0;// TODO(subset) serialize_subset (c, varStore, this);
+    {
+      subset_varstore = out->varStore.serialize_subset (c, varStore, this);
+      if (!subset_varstore && version.to_int () == 0x00010003u)
+        out->version.minor = 2;
+    }
 
-    return_trace (true);
+    return_trace (subset_glyphclassdef || subset_ligcaretlist || subset_markattachclassdef ||
+                  (out->version.to_int () >= 0x00010002u && subset_markglyphsetsdef) ||
+                  (out->version.to_int () >= 0x00010003u && subset_varstore));
   }
 
   bool sanitize (hb_sanitize_context_t *c) const

src/hb-ot-layout-gpos-table.hh

@@ -160,7 +160,8 @@ struct ValueFormat : HBUINT16
     return ret;
   }
 
-  void serialize_copy (hb_serialize_context_t *c, const void *base, const Value *values) const
+  void serialize_copy (hb_serialize_context_t *c, const void *base,
+                       const Value *values, const hb_map_t *layout_variation_idx_map) const
   {
     unsigned int format = *this;
     if (!format) return;
@@ -170,10 +171,10 @@ struct ValueFormat : HBUINT16
     if (format & xAdvance)   c->copy (*values++);
     if (format & yAdvance)   c->copy (*values++);
 
-    if (format & xPlaDevice) copy_device (c, base, values++);
-    if (format & yPlaDevice) copy_device (c, base, values++);
-    if (format & xAdvDevice) copy_device (c, base, values++);
-    if (format & yAdvDevice) copy_device (c, base, values++);
+    if (format & xPlaDevice) copy_device (c, base, values++, layout_variation_idx_map);
+    if (format & yPlaDevice) copy_device (c, base, values++, layout_variation_idx_map);
+    if (format & xAdvDevice) copy_device (c, base, values++, layout_variation_idx_map);
+    if (format & yAdvDevice) copy_device (c, base, values++, layout_variation_idx_map);
   }
 
   void collect_variation_indices (hb_collect_variation_indices_context_t *c,
@@ -241,7 +242,8 @@ struct ValueFormat : HBUINT16
     return *static_cast<const OffsetTo<Device> *> (value);
   }
 
-  bool copy_device (hb_serialize_context_t *c, const void *base, const Value *src_value) const
+  bool copy_device (hb_serialize_context_t *c, const void *base,
+                    const Value *src_value, const hb_map_t *layout_variation_idx_map) const
   {
     Value	*dst_value = c->copy (*src_value);
 
@@ -250,7 +252,7 @@ struct ValueFormat : HBUINT16
 
     *dst_value = 0;
     c->push ();
-    if ((base + get_device (src_value)).copy (c))
+    if ((base + get_device (src_value)).copy (c, layout_variation_idx_map))
     {
       c->add_link (*dst_value, c->pop_pack ());
       return true;
@@ -321,7 +323,8 @@ template<typename Iterator>
 static void SinglePos_serialize (hb_serialize_context_t *c,
 				 const void *src,
 				 Iterator it,
-				 ValueFormat valFormat);
+				 ValueFormat valFormat,
+                                 const hb_map_t *layout_variation_idx_map);
 
 
 struct AnchorFormat1
@@ -420,14 +423,17 @@ struct AnchorFormat3
     return_trace (c->check_struct (this) && xDeviceTable.sanitize (c, this) && yDeviceTable.sanitize (c, this));
   }
 
-  AnchorFormat3* copy (hb_serialize_context_t *c) const
+  AnchorFormat3* copy (hb_serialize_context_t *c,
+		       const hb_map_t *layout_variation_idx_map) const
   {
     TRACE_SERIALIZE (this);
+    if (!layout_variation_idx_map) return_trace (nullptr);
+
     auto *out = c->embed<AnchorFormat3> (this);
     if (unlikely (!out)) return_trace (nullptr);
 
-    out->xDeviceTable.serialize_copy (c, xDeviceTable, this);
-    out->yDeviceTable.serialize_copy (c, yDeviceTable, this);
+    out->xDeviceTable.serialize_copy (c, xDeviceTable, this, 0, hb_serialize_context_t::Head, layout_variation_idx_map);
+    out->yDeviceTable.serialize_copy (c, yDeviceTable, this, 0, hb_serialize_context_t::Head, layout_variation_idx_map);
     return_trace (out);
   }
 
@@ -479,13 +485,13 @@ struct Anchor
     }
   }
 
-  Anchor* copy (hb_serialize_context_t *c) const
+  Anchor* copy (hb_serialize_context_t *c, const hb_map_t *layout_variation_idx_map) const
   {
     TRACE_SERIALIZE (this);
     switch (u.format) {
     case 1: return_trace (reinterpret_cast<Anchor *> (u.format1.copy (c)));
     case 2: return_trace (reinterpret_cast<Anchor *> (u.format2.copy (c)));
-    case 3: return_trace (reinterpret_cast<Anchor *> (u.format3.copy (c)));
+    case 3: return_trace (reinterpret_cast<Anchor *> (u.format3.copy (c, layout_variation_idx_map)));
     default:return_trace (nullptr);
     }
   }
@@ -539,6 +545,7 @@ struct AnchorMatrix
   bool serialize (hb_serialize_context_t *c,
 		  unsigned                num_rows,
 		  AnchorMatrix const     *offset_matrix,
+                  const hb_map_t         *layout_variation_idx_map,
 		  Iterator                index_iter)
   {
     TRACE_SERIALIZE (this);
@@ -550,7 +557,10 @@ struct AnchorMatrix
     {
       auto *offset = c->embed (offset_matrix->matrixZ[i]);
       if (!offset) return_trace (false);
-      offset->serialize_copy (c, offset_matrix->matrixZ[i], offset_matrix, c->to_bias (this));
+      offset->serialize_copy (c, offset_matrix->matrixZ[i],
+                              offset_matrix, c->to_bias (this),
+                              hb_serialize_context_t::Head,
+                              layout_variation_idx_map);
     }
 
     return_trace (true);
@@ -588,15 +598,18 @@ struct MarkRecord
     return_trace (c->check_struct (this) && markAnchor.sanitize (c, base));
   }
 
-  MarkRecord *copy (hb_serialize_context_t *c, const void *base,
-		    unsigned dst_bias, const hb_map_t *klass_mapping) const
+  MarkRecord *copy (hb_serialize_context_t *c,
+		    const void             *src_base,
+		    unsigned                dst_bias,
+		    const hb_map_t         *klass_mapping,
+		    const hb_map_t         *layout_variation_idx_map) const
   {
     TRACE_SERIALIZE (this);
     auto *out = c->embed (this);
     if (unlikely (!out)) return_trace (nullptr);
 
     out->klass = klass_mapping->get (klass);
-    out->markAnchor.serialize_copy (c, markAnchor, base, dst_bias);
+    out->markAnchor.serialize_copy (c, markAnchor, src_base, dst_bias, hb_serialize_context_t::Head, layout_variation_idx_map);
     return_trace (out);
   }
 
@@ -655,13 +668,14 @@ struct MarkArray : ArrayOf<MarkRecord>	/* Array of MarkRecords--in Coverage orde
 	   hb_requires (hb_is_source_of (Iterator, MarkRecord))>
   bool serialize (hb_serialize_context_t *c,
 		  const hb_map_t         *klass_mapping,
+                  const hb_map_t         *layout_variation_idx_map,
 		  const void             *base,
 		  Iterator                it)
   {
     TRACE_SERIALIZE (this);
     if (unlikely (!c->extend_min (*this))) return_trace (false);
     if (unlikely (!c->check_assign (len, it.len ()))) return_trace (false);
-    c->copy_all (it, base, c->to_bias (this), klass_mapping);
+    c->copy_all (it, base, c->to_bias (this), klass_mapping, layout_variation_idx_map);
     return_trace (true);
   }
 
@@ -717,7 +731,8 @@ struct SinglePosFormat1
   void serialize (hb_serialize_context_t *c,
 		  const void *src,
 		  Iterator it,
-		  ValueFormat valFormat)
+		  ValueFormat valFormat,
+                  const hb_map_t *layout_variation_idx_map)
   {
     auto out = c->extend_min (*this);
     if (unlikely (!out)) return;
@@ -726,7 +741,7 @@ struct SinglePosFormat1
     + it
     | hb_map (hb_second)
     | hb_apply ([&] (hb_array_t<const Value> _)
-		{ valFormat.serialize_copy (c, src, &_); })
+		{ valFormat.serialize_copy (c, src, &_, layout_variation_idx_map); })
     ;
 
     auto glyphs =
@@ -751,7 +766,7 @@ struct SinglePosFormat1
     ;
 
     bool ret = bool (it);
-    SinglePos_serialize (c->serializer, this, it, valueFormat);
+    SinglePos_serialize (c->serializer, this, it, valueFormat, c->plan->layout_variation_idx_map);
     return_trace (ret);
   }
 
@@ -830,7 +845,8 @@ struct SinglePosFormat2
   void serialize (hb_serialize_context_t *c,
 		  const void *src,
 		  Iterator it,
-		  ValueFormat valFormat)
+		  ValueFormat valFormat,
+                  const hb_map_t *layout_variation_idx_map)
   {
     auto out = c->extend_min (*this);
     if (unlikely (!out)) return;
@@ -840,7 +856,7 @@ struct SinglePosFormat2
     + it
     | hb_map (hb_second)
     | hb_apply ([&] (hb_array_t<const Value> _)
-		{ valFormat.serialize_copy (c, src, &_); })
+		{ valFormat.serialize_copy (c, src, &_, layout_variation_idx_map); })
     ;
 
     auto glyphs =
@@ -872,7 +888,7 @@ struct SinglePosFormat2
     ;
 
     bool ret = bool (it);
-    SinglePos_serialize (c->serializer, this, it, valueFormat);
+    SinglePos_serialize (c->serializer, this, it, valueFormat, c->plan->layout_variation_idx_map);
     return_trace (ret);
   }
 
@@ -920,7 +936,8 @@ struct SinglePos
   void serialize (hb_serialize_context_t *c,
 		  const void *src,
 		  Iterator glyph_val_iter_pairs,
-		  ValueFormat valFormat)
+		  ValueFormat valFormat,
+                  const hb_map_t *layout_variation_idx_map)
   {
     if (unlikely (!c->extend_min (u.format))) return;
     unsigned format = 2;
@@ -929,9 +946,9 @@ struct SinglePos
 
     u.format = format;
     switch (u.format) {
-    case 1: u.format1.serialize (c, src, glyph_val_iter_pairs, valFormat);
+    case 1: u.format1.serialize (c, src, glyph_val_iter_pairs, valFormat, layout_variation_idx_map);
 	    return;
-    case 2: u.format2.serialize (c, src, glyph_val_iter_pairs, valFormat);
+    case 2: u.format2.serialize (c, src, glyph_val_iter_pairs, valFormat, layout_variation_idx_map);
 	    return;
     default:return;
     }
@@ -962,8 +979,9 @@ static void
 SinglePos_serialize (hb_serialize_context_t *c,
 		     const void *src,
 		     Iterator it,
-		     ValueFormat valFormat)
-{ c->start_embed<SinglePos> ()->serialize (c, src, it, valFormat); }
+		     ValueFormat valFormat,
+                     const hb_map_t *layout_variation_idx_map)
+{ c->start_embed<SinglePos> ()->serialize (c, src, it, valFormat, layout_variation_idx_map); }
 
 
 struct PairValueRecord
@@ -979,6 +997,7 @@ struct PairValueRecord
     const ValueFormat	*valueFormats;
     unsigned		len1; /* valueFormats[0].get_len() */
     const hb_map_t 	*glyph_map;
+    const hb_map_t      *layout_variation_idx_map;
   };
 
   bool serialize (hb_serialize_context_t *c,
@@ -990,8 +1009,8 @@ struct PairValueRecord
 
     out->secondGlyph = (*closure->glyph_map)[secondGlyph];
 
-    closure->valueFormats[0].serialize_copy (c, closure->base, &values[0]);
-    closure->valueFormats[1].serialize_copy (c, closure->base, &values[closure->len1]);
+    closure->valueFormats[0].serialize_copy (c, closure->base, &values[0], closure->layout_variation_idx_map);
+    closure->valueFormats[1].serialize_copy (c, closure->base, &values[closure->len1], closure->layout_variation_idx_map);
 
     return_trace (true);
   }
@@ -1122,7 +1141,8 @@ struct PairSet
       this,
       valueFormats,
       len1,
-      &glyph_map
+      &glyph_map,
+      c->plan->layout_variation_idx_map
     };
 
     const PairValueRecord *record = &firstPairValueRecord;
@@ -1414,8 +1434,8 @@ struct PairPosFormat2
                   | hb_apply ([&] (const unsigned class2_idx)
                               {
                                 unsigned idx = (class1_idx * (unsigned) class2Count + class2_idx) * (len1 + len2);
-				valueFormat1.serialize_copy (c->serializer, this, &values[idx]);
-				valueFormat2.serialize_copy (c->serializer, this, &values[idx + len1]);
+                                valueFormat1.serialize_copy (c->serializer, this, &values[idx], c->plan->layout_variation_idx_map);
+                                valueFormat2.serialize_copy (c->serializer, this, &values[idx + len1], c->plan->layout_variation_idx_map);
                               })
                   ;
                 })
@@ -1524,14 +1544,17 @@ struct EntryExitRecord
     (src_base+exitAnchor).collect_variation_indices (c);
   }
 
-  EntryExitRecord* copy (hb_serialize_context_t *c, const void *base) const
+  EntryExitRecord* copy (hb_serialize_context_t *c,
+			 const void *src_base,
+			 const void *dst_base,
+                         const hb_map_t *layout_variation_idx_map) const
   {
     TRACE_SERIALIZE (this);
     auto *out = c->embed (this);
     if (unlikely (!out)) return_trace (nullptr);
 
-    out->entryAnchor.serialize_copy (c, entryAnchor, base);
-    out->exitAnchor.serialize_copy (c, exitAnchor, base);
+    out->entryAnchor.serialize_copy (c, entryAnchor, src_base, c->to_bias (dst_base), hb_serialize_context_t::Head, layout_variation_idx_map);
+    out->exitAnchor.serialize_copy (c, exitAnchor, src_base, c->to_bias (dst_base), hb_serialize_context_t::Head, layout_variation_idx_map);
     return_trace (out);
   }
 
@@ -1675,7 +1698,10 @@ struct CursivePosFormat1
 
   template <typename Iterator,
 	    hb_requires (hb_is_iterator (Iterator))>
-  void serialize (hb_serialize_context_t *c, Iterator it, const void *base)
+  void serialize (hb_serialize_context_t *c,
+		  Iterator it,
+		  const void *src_base,
+                  const hb_map_t *layout_variation_idx_map)
   {
     if (unlikely (!c->extend_min ((*this)))) return;
     this->format = 1;
@@ -1683,7 +1709,7 @@ struct CursivePosFormat1
 
     for (const EntryExitRecord& entry_record : + it
 					       | hb_map (hb_second))
-      c->copy (entry_record, base);
+      c->copy (entry_record, src_base, this, layout_variation_idx_map);
 
     auto glyphs =
     + it
@@ -1710,7 +1736,7 @@ struct CursivePosFormat1
     ;
 
     bool ret = bool (it);
-    out->serialize (c->serializer, it, this);
+    out->serialize (c->serializer, it, this, c->plan->layout_variation_idx_map);
     return_trace (ret);
   }
 
@@ -1900,7 +1926,7 @@ struct MarkBasePosFormat1
       return_trace (false);
 
     out->markArray.serialize (c->serializer, out)
-		  .serialize (c->serializer, &klass_mapping, &(this+markArray), + mark_iter
+		  .serialize (c->serializer, &klass_mapping, c->plan->layout_variation_idx_map, &(this+markArray), + mark_iter
 										                                   | hb_map (hb_second));
 
     unsigned basecount = (this+baseArray).rows;
@@ -1931,7 +1957,7 @@ struct MarkBasePosFormat1
       ;
     }
     out->baseArray.serialize (c->serializer, out)
-		  .serialize (c->serializer, base_iter.len (), &(this+baseArray), base_indexes.iter ());
+		  .serialize (c->serializer, base_iter.len (), &(this+baseArray), c->plan->layout_variation_idx_map, base_indexes.iter ());
 
     return_trace (true);
   }
@@ -2277,7 +2303,7 @@ struct MarkMarkPosFormat1
       return_trace (false);
 
     out->mark1Array.serialize (c->serializer, out)
-		   .serialize (c->serializer, &klass_mapping, &(this+mark1Array), + mark1_iter
+		   .serialize (c->serializer, &klass_mapping, c->plan->layout_variation_idx_map, &(this+mark1Array), + mark1_iter
 										                                     | hb_map (hb_second));
     
     unsigned mark2count = (this+mark2Array).rows;
@@ -2308,7 +2334,7 @@ struct MarkMarkPosFormat1
       ;
     }
     out->mark2Array.serialize (c->serializer, out)
-		   .serialize (c->serializer, mark2_iter.len (), &(this+mark2Array), mark2_indexes.iter ());
+		   .serialize (c->serializer, mark2_iter.len (), &(this+mark2Array), c->plan->layout_variation_idx_map, mark2_indexes.iter ());
 
     return_trace (true);
   }

src/hb-ot-layout-gsubgpos.hh

@@ -3303,7 +3303,8 @@ struct GSUBGPOS
     unsigned int feature_count = hb_min (get_feature_count (), (unsigned) HB_MAX_FEATURES);
     for (unsigned i = 0; i < feature_count; i++)
     {
-      if (get_feature (i).intersects_lookup_indexes (lookup_indexes))
+      const Feature& f = get_feature (i);
+      if ((!f.featureParams.is_null ()) || f.intersects_lookup_indexes (lookup_indexes))
         feature_indexes->add (i);
     }
 #ifndef HB_NO_VAR

src/hb-ot-layout.cc

@@ -318,38 +318,6 @@ hb_ot_layout_get_glyphs_in_class (hb_face_t                  *face,
   return face->table.GDEF->table->get_glyphs_in_class (klass, glyphs);
 }
 
-#ifndef HB_NO_VAR
-/**
- * hb_ot_layout_collect_variation_indices:
- * @face: The #hb_face_t to work on
- * @layout_variation_indices: (inout): The #hb_set_t set of var indexes in all Device
- * tables
- *
- * Fetch and remap a list of variation indices in all Device tables referenced
- * in the specified face's GDEF table and GPOS tables that are going to be
- * retained in the final subset
- *
- * Since: REPLACEME
- **/
-void
-hb_ot_layout_collect_variation_indices (hb_face_t      *face,
-					const hb_set_t *glyphset,
-					const hb_map_t *gpos_lookups,
-					hb_set_t       *layout_variation_indices /* OUT */,
-					hb_map_t       *layout_variation_idx_map /* OUT */)
-{
-  if (!face->table.GDEF->table->has_data ()) return;
-  OT::hb_collect_variation_indices_context_t c (layout_variation_indices, glyphset, gpos_lookups);
-  face->table.GDEF->table->collect_variation_indices (&c);
-
-  if (hb_ot_layout_has_positioning (face))
-    face->table.GPOS->table->collect_variation_indices (&c);
-
-  face->table.GDEF->table->remap_layout_variation_indices (layout_variation_indices, layout_variation_idx_map);
-}
-#endif
-
-
 #ifndef HB_NO_LAYOUT_UNUSED
 /**
  * hb_ot_layout_get_attach_points:

src/hb-ot-layout.h

@@ -121,13 +121,6 @@ hb_ot_layout_get_glyphs_in_class (hb_face_t                  *face,
 				  hb_ot_layout_glyph_class_t  klass,
 				  hb_set_t                   *glyphs /* OUT */);
 
-HB_EXTERN void
-hb_ot_layout_collect_variation_indices (hb_face_t *face,
-					const hb_set_t *glyphset,
-					const hb_map_t *gpos_lookups,
-					hb_set_t  *layout_variation_indices /* INOUT */,
-					hb_map_t  *layout_variation_idx_map /* INOUT */);
-
 /* Not that useful.  Provides list of attach points for a glyph that a
  * client may want to cache */
 HB_EXTERN unsigned int

src/hb-subset-plan.cc

@@ -30,6 +30,8 @@
 
 #include "hb-ot-cmap-table.hh"
 #include "hb-ot-glyf-table.hh"
+#include "hb-ot-layout-gdef-table.hh"
+#include "hb-ot-layout-gpos-table.hh"
 #include "hb-ot-cff1-table.hh"
 #include "hb-ot-color-colr-table.hh"
 #include "hb-ot-var-fvar-table.hh"
@@ -137,7 +139,25 @@ static inline void
 				     hb_set_t  *layout_variation_indices,
 				     hb_map_t  *layout_variation_idx_map)
 {
-  hb_ot_layout_collect_variation_indices (face, glyphset, gpos_lookups, layout_variation_indices, layout_variation_idx_map);
+  hb_blob_ptr_t<OT::GDEF> gdef = hb_sanitize_context_t ().reference_table<OT::GDEF> (face);
+  hb_blob_ptr_t<OT::GPOS> gpos = hb_sanitize_context_t ().reference_table<OT::GPOS> (face);
+
+  if (!gdef->has_data ())
+  {
+    gdef.destroy ();
+    gpos.destroy ();
+    return;
+  }
+  OT::hb_collect_variation_indices_context_t c (layout_variation_indices, glyphset, gpos_lookups);
+  gdef->collect_variation_indices (&c);
+
+  if (hb_ot_layout_has_positioning (face))
+    gpos->collect_variation_indices (&c);
+
+  gdef->remap_layout_variation_indices (layout_variation_indices, layout_variation_idx_map);
+  
+  gdef.destroy ();
+  gpos.destroy ();
 }
 #endif
 

test/subset/data/Makefile.am

@@ -22,6 +22,7 @@ EXTRA_DIST += \
 	expected/layout.gsub6 \
 	expected/layout.gdef \
 	expected/layout.context \
+	expected/layout.gdef-varstore \
 	expected/cmap \
 	expected/cmap14 \
 	expected/sbix \

test/subset/data/Makefile.sources

@@ -22,6 +22,7 @@ DISABLED_TESTS = \
 	tests/layout.gsub6.tests \
 	tests/layout.gdef.tests \
 	tests/layout.context.tests \
+	tests/layout.gdef-varstore.tests \
 	$(NULL)
 
 XFAIL_TESTS = \

test/subset/data/tests/layout.gdef-varstore.tests

@@ -0,0 +1,16 @@
+FONTS:
+AdobeVFPrototype.otf
+
+PROFILES:
+default.txt
+drop-hints.txt
+keep-gdef-gpos.txt
+
+SUBSETS:
+A
+AB
+ABC
+BW
+AVW
+ABCW
+