[shape-plan] Cache shape plans with variations based on variation indices

src/hb-aat-map.cc

@@ -51,9 +51,7 @@ void hb_aat_map_builder_t::add_feature (hb_tag_t tag,
 }
 
 void
-hb_aat_map_builder_t::compile (hb_aat_map_t  &m,
-			       const int    *coords HB_UNUSED,
-			       unsigned int  num_coords HB_UNUSED)
+hb_aat_map_builder_t::compile (hb_aat_map_t  &m)
 {
   /* Sort features and merge duplicates */
   if (features.len)

src/hb-aat-map.hh

@@ -60,9 +60,7 @@ struct hb_aat_map_builder_t
 
   HB_INTERNAL void add_feature (hb_tag_t tag, unsigned int value=1);
 
-  HB_INTERNAL void compile (hb_aat_map_t  &m,
-			    const int    *coords,
-			    unsigned int  num_coords);
+  HB_INTERNAL void compile (hb_aat_map_t  &m);
 
   public:
   struct feature_info_t

src/hb-ot-map.cc

@@ -27,7 +27,7 @@
  */
 
 #include "hb-ot-map.hh"
-
+#include "hb-ot-shape.hh"
 #include "hb-ot-layout.hh"
 
 
@@ -143,9 +143,8 @@ void hb_ot_map_builder_t::add_pause (unsigned int table_index, hb_ot_map_t::paus
 }
 
 void
-hb_ot_map_builder_t::compile (hb_ot_map_t  &m,
-			      const int    *coords,
-			      unsigned int  num_coords)
+hb_ot_map_builder_t::compile (hb_ot_map_t                  &m,
+			      const hb_ot_shape_plan_key_t &key)
 {
   static_assert ((!(HB_GLYPH_FLAG_DEFINED & (HB_GLYPH_FLAG_DEFINED + 1))), "");
   unsigned int global_bit_mask = HB_GLYPH_FLAG_DEFINED + 1;
@@ -282,13 +281,6 @@ hb_ot_map_builder_t::compile (hb_ot_map_t  &m,
   {
     /* Collect lookup indices for features */
 
-    unsigned int variations_index;
-    hb_ot_layout_table_find_feature_variations (face,
-						table_tags[table_index],
-						coords,
-						num_coords,
-						&variations_index);
-
     unsigned int stage_index = 0;
     unsigned int last_num_lookups = 0;
     for (unsigned stage = 0; stage < current_stage[table_index]; stage++)
@@ -297,14 +289,14 @@ hb_ot_map_builder_t::compile (hb_ot_map_t  &m,
 	  required_feature_stage[table_index] == stage)
 	add_lookups (m, table_index,
 		     required_feature_index[table_index],
-		     variations_index,
+		     key.variations_index[table_index],
 		     global_bit_mask);
 
       for (unsigned i = 0; i < m.features.len; i++)
         if (m.features[i].stage[table_index] == stage)
 	  add_lookups (m, table_index,
 		       m.features[i].index[table_index],
-		       variations_index,
+		       key.variations_index[table_index],
 		       m.features[i].mask,
 		       m.features[i].auto_zwnj,
 		       m.features[i].auto_zwj,

src/hb-ot-map.hh

@@ -188,6 +188,7 @@ struct hb_ot_map_feature_t
   hb_ot_map_feature_flags_t flags;
 };
 
+struct hb_ot_shape_plan_key_t;
 
 struct hb_ot_map_builder_t
 {
@@ -218,9 +219,8 @@ struct hb_ot_map_builder_t
   inline void add_gpos_pause (hb_ot_map_t::pause_func_t pause_func)
   { add_pause (1, pause_func); }
 
-  HB_INTERNAL void compile (hb_ot_map_t  &m,
-			    const int    *coords,
-			    unsigned int  num_coords);
+  HB_INTERNAL void compile (hb_ot_map_t                  &m,
+			    const hb_ot_shape_plan_key_t &key);
 
   private:
 

src/hb-ot-shape.cc

@@ -79,15 +79,14 @@ hb_ot_shape_planner_t::hb_ot_shape_planner_t (hb_face_t                     *fac
 							hb_ot_shape_complex_categorize (this)) {}
 
 void
-hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan,
-				const int          *coords,
-				unsigned int        num_coords)
+hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t           &plan,
+				const hb_ot_shape_plan_key_t &key)
 {
   plan.props = props;
   plan.shaper = shaper;
-  map.compile (plan.map, coords, num_coords);
+  map.compile (plan.map, key);
   if (apply_morx)
-    aat_map.compile (plan.aat_map, coords, num_coords);
+    aat_map.compile (plan.aat_map);
 
   plan.frac_mask = plan.map.get_1_mask (HB_TAG ('f','r','a','c'));
   plan.numr_mask = plan.map.get_1_mask (HB_TAG ('n','u','m','r'));
@@ -160,9 +159,7 @@ hb_ot_shape_plan_t::init0 (hb_face_t                     *face,
 				key->user_features,
 				key->num_user_features);
 
-  planner.compile (*this,
-		   key->coords,
-		   key->num_coords);
+  planner.compile (*this, key->ot);
 
   if (shaper->data_create)
   {

src/hb-ot-shape.hh

@@ -33,6 +33,29 @@
 #include "hb-aat-map.hh"
 
 
+struct hb_ot_shape_plan_key_t
+{
+  unsigned int variations_index[2];
+
+  inline void init (hb_face_t   *face,
+		    const int   *coords,
+		    unsigned int num_coords)
+  {
+    for (unsigned int table_index = 0; table_index < 2; table_index++)
+      hb_ot_layout_table_find_feature_variations (face,
+						  table_tags[table_index],
+						  coords,
+						  num_coords,
+						  &variations_index[table_index]);
+  }
+
+  inline bool equal (const hb_ot_shape_plan_key_t *other)
+  {
+    return 0 == memcmp (this, other, sizeof (*this));
+  }
+};
+
+
 struct hb_shape_plan_key_t;
 
 struct hb_ot_shape_plan_t
@@ -95,9 +118,8 @@ struct hb_ot_shape_planner_t
   HB_INTERNAL hb_ot_shape_planner_t (hb_face_t                     *face,
 				     const hb_segment_properties_t *props);
 
-  HB_INTERNAL void compile (hb_ot_shape_plan_t &plan,
-			    const int          *coords,
-			    unsigned int        num_coords);
+  HB_INTERNAL void compile (hb_ot_shape_plan_t           &plan,
+			    const hb_ot_shape_plan_key_t &key);
 };
 
 

src/hb-shape-plan.cc

@@ -54,28 +54,22 @@ hb_shape_plan_key_t::init (bool                           copy,
 			   const hb_segment_properties_t *props,
 			   const hb_feature_t            *user_features,
 			   unsigned int                   num_user_features,
-			   const int                     *orig_coords,
+			   const int                     *coords,
 			   unsigned int                   num_coords,
 			   const char * const            *shaper_list)
 {
   hb_feature_t *features = nullptr;
-  int *coords = nullptr;
   if (copy && num_user_features && !(features = (hb_feature_t *) calloc (num_user_features, sizeof (hb_feature_t))))
     goto bail;
-  if (copy && num_coords && !(coords = (int *) calloc (num_coords, sizeof (int))))
-    goto bail;
 
   this->props = *props;
   this->num_user_features = num_user_features;
   this->user_features = copy ? features : user_features;
   if (copy && num_user_features)
     memcpy (features, user_features, num_user_features * sizeof (hb_feature_t));
-  this->num_coords = num_coords;
-  this->coords = copy ? coords : orig_coords;
-  if (copy && num_coords)
-    memcpy (coords, orig_coords, num_coords * sizeof (int));
   this->shaper_func = nullptr;
   this->shaper_name = nullptr;
+  this->ot.init (face, coords, num_coords);
 
   /*
    * Choose shaper.
@@ -117,7 +111,6 @@ hb_shape_plan_key_t::init (bool                           copy,
 #undef HB_SHAPER_PLAN
 
 bail:
-  ::free (coords);
   ::free (features);
   return false;
 }
@@ -378,38 +371,6 @@ hb_shape_plan_execute (hb_shape_plan_t    *shape_plan,
  * Caching
  */
 
-static inline bool
-hb_shape_plan_key_user_features_equal (const hb_shape_plan_key_t *key1,
-				       const hb_shape_plan_key_t *key2)
-{
-  if (key1->num_user_features != key2->num_user_features)
-    return false;
-  return 0 == hb_memcmp(key1->user_features,
-			key2->user_features,
-			key1->num_user_features * sizeof (key1->user_features[0]));
-}
-
-static inline bool
-hb_shape_plan_key_coords_equal (const hb_shape_plan_key_t *key2,
-				const hb_shape_plan_key_t *key1)
-{
-  if (key1->num_coords != key2->num_coords)
-    return false;
-  return 0 == hb_memcmp(key1->coords,
-			key2->coords,
-			key1->num_coords * sizeof (key1->coords[0]));
-}
-
-static bool
-hb_shape_plan_key_equal (const hb_shape_plan_key_t *key1,
-			 const hb_shape_plan_key_t *key2)
-{
-  return hb_segment_properties_equal (&key1->props, &key2->props) &&
-	 hb_shape_plan_key_user_features_equal (key1, key2) &&
-	 hb_shape_plan_key_coords_equal (key1, key2) &&
-	 key1->shaper_func == key2->shaper_func;
-}
-
 static inline bool
 _has_non_global_user_features (const hb_feature_t *user_features,
 			       unsigned int        num_user_features)
@@ -425,21 +386,13 @@ _has_non_global_user_features (const hb_feature_t *user_features,
   return false;
 }
 
-static inline bool
-_has_coords (const int    *coords,
-	     unsigned int  num_coords)
-{
-  return num_coords;
-}
-
 static inline bool
 _dont_cache (const hb_feature_t *user_features,
 	     unsigned int        num_user_features,
 	     const int          *coords,
 	     unsigned int        num_coords)
 {
-  return _has_non_global_user_features (user_features, num_user_features) ||
-	 _has_coords (coords, num_coords);
+  return _has_non_global_user_features (user_features, num_user_features);
 }
 
 /**
@@ -505,7 +458,7 @@ retry:
       return hb_shape_plan_get_empty ();
 
     for (hb_face_t::plan_node_t *node = cached_plan_nodes; node; node = node->next)
-      if (hb_shape_plan_key_equal (&node->shape_plan->key, &key))
+      if (node->shape_plan->key.equal (&key))
       {
         DEBUG_MSG_FUNC (SHAPE_PLAN, node->shape_plan, "fulfilled from cache");
         return hb_shape_plan_reference (node->shape_plan);

src/hb-shape-plan.hh

@@ -39,8 +39,7 @@ struct hb_shape_plan_key_t
   const hb_feature_t      *user_features;
   unsigned int             num_user_features;
 
-  const int               *coords;
-  unsigned int             num_coords;
+  hb_ot_shape_plan_key_t   ot;
 
   hb_shape_func_t         *shaper_func;
   const char              *shaper_name;
@@ -50,14 +49,30 @@ struct hb_shape_plan_key_t
 				const hb_segment_properties_t *props,
 				const hb_feature_t            *user_features,
 				unsigned int                   num_user_features,
-				const int                     *orig_coords,
+				const int                     *coords,
 				unsigned int                   num_coords,
 				const char * const            *shaper_list);
 
   HB_INTERNAL inline void free (void)
   {
     ::free ((void *) user_features);
-    ::free ((void *) coords);
+  }
+
+  inline bool user_features_match (const hb_shape_plan_key_t *other)
+  {
+    /* TODO Implement non-exact matching. */
+    if (this->num_user_features != other->num_user_features)
+      return false;
+    return 0 == hb_memcmp(this->user_features, other->user_features,
+			  this->num_user_features * sizeof (this->user_features[0]));
+  }
+
+  inline bool equal (const hb_shape_plan_key_t *other)
+  {
+    return hb_segment_properties_equal (&this->props, &other->props) &&
+	   this->user_features_match (other) &&
+	   this->ot.equal (&other->ot) &&
+	   this->shaper_func == other->shaper_func;
   }
 };