src/hb-aat-map.cc    |    4 --
 src/hb-aat-map.hh    |    4 --
 src/hb-ot-map.cc     |   18 ++-------
 src/hb-ot-map.hh     |    6 +--
 src/hb-ot-shape.cc   |   32 ++++++----------
 src/hb-ot-shape.hh   |   36 ++++++++++++++----
 src/hb-shape-plan.cc |   98 +++++++++++++--------------------------------------
 src/hb-shape-plan.hh |   23 +++++++++--
 8 files changed, 96 insertions(+), 125 deletions(-)

New commits:
commit cc8428756a1b18b0445c2c5fbb38e05453693dad
Author: Behdad Esfahbod <[email protected]>
Date:   Mon Nov 12 18:48:10 2018 -0500

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

diff --git a/src/hb-aat-map.cc b/src/hb-aat-map.cc
index 1ce1b12b..8bc1a0c6 100644
--- a/src/hb-aat-map.cc
+++ b/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)
diff --git a/src/hb-aat-map.hh b/src/hb-aat-map.hh
index 6fd3fa1e..07454b2c 100644
--- a/src/hb-aat-map.hh
+++ b/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
diff --git a/src/hb-ot-map.cc b/src/hb-ot-map.cc
index fd810995..95f794ab 100644
--- a/src/hb-ot-map.cc
+++ b/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,
diff --git a/src/hb-ot-map.hh b/src/hb-ot-map.hh
index fde85b1d..8e1f5aa8 100644
--- a/src/hb-ot-map.hh
+++ b/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:
 
diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc
index b94b0107..459e1229 100644
--- a/src/hb-ot-shape.cc
+++ b/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)
   {
diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh
index 956cc6df..b35f243e 100644
--- a/src/hb-ot-shape.hh
+++ b/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);
 };
 
 
diff --git a/src/hb-shape-plan.cc b/src/hb-shape-plan.cc
index fb026760..a4eacf50 100644
--- a/src/hb-shape-plan.cc
+++ b/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;
 }
@@ -379,38 +372,6 @@ hb_shape_plan_execute (hb_shape_plan_t    *shape_plan,
  */
 
 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)
 {
@@ -426,20 +387,12 @@ _has_non_global_user_features (const hb_feature_t 
*user_features,
 }
 
 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);
diff --git a/src/hb-shape-plan.hh b/src/hb-shape-plan.hh
index 8b34fa0a..739427b2 100644
--- a/src/hb-shape-plan.hh
+++ b/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;
   }
 };
 
commit 8284cb9fb3600268e06d8a2ba8400700510de7a5
Author: Behdad Esfahbod <[email protected]>
Date:   Mon Nov 12 18:18:20 2018 -0500

    [shape-plan] Refactor more

diff --git a/src/hb-shape-plan.cc b/src/hb-shape-plan.cc
index 70083c8b..fb026760 100644
--- a/src/hb-shape-plan.cc
+++ b/src/hb-shape-plan.cc
@@ -411,10 +411,9 @@ hb_shape_plan_key_equal (const hb_shape_plan_key_t *key1,
 }
 
 static inline bool
-hb_shape_plan_key_has_non_global_user_features (const hb_shape_plan_key_t *key)
+_has_non_global_user_features (const hb_feature_t *user_features,
+                              unsigned int        num_user_features)
 {
-  unsigned int num_user_features = key->num_user_features;
-  const hb_feature_t *user_features = key->user_features;
   while (num_user_features)
   {
     if (user_features->start != HB_FEATURE_GLOBAL_START ||
@@ -427,16 +426,20 @@ hb_shape_plan_key_has_non_global_user_features (const 
hb_shape_plan_key_t *key)
 }
 
 static inline bool
-hb_shape_plan_key_has_coords (const hb_shape_plan_key_t *key)
+_has_coords (const int    *coords,
+            unsigned int  num_coords)
 {
-  return key->num_coords;
+  return num_coords;
 }
 
 static inline bool
-hb_shape_plan_key_dont_cache (const hb_shape_plan_key_t *key)
+_dont_cache (const hb_feature_t *user_features,
+            unsigned int        num_user_features,
+            const int          *coords,
+            unsigned int        num_coords)
 {
-  return hb_shape_plan_key_has_non_global_user_features (key) ||
-        hb_shape_plan_key_has_coords (key);
+  return _has_non_global_user_features (user_features, num_user_features) ||
+        _has_coords (coords, num_coords);
 }
 
 /**
@@ -481,30 +484,33 @@ hb_shape_plan_create_cached2 (hb_face_t                   
  *face,
                  num_user_features,
                  shaper_list);
 
-  hb_shape_plan_key_t key;
-  if (!key.init (false,
-                face,
-                props,
-                user_features,
-                num_user_features,
-                coords,
-                num_coords,
-                shaper_list))
-    return hb_shape_plan_get_empty ();
-
 retry:
   hb_face_t::plan_node_t *cached_plan_nodes = face->shape_plans;
 
-  bool dont_cache = hb_shape_plan_key_dont_cache (&key) ||
+  bool dont_cache = _dont_cache (user_features, num_user_features,
+                                coords, num_coords) ||
                    hb_object_is_inert (face);
 
   if (!dont_cache)
+  {
+    hb_shape_plan_key_t key;
+    if (!key.init (false,
+                  face,
+                  props,
+                  user_features,
+                  num_user_features,
+                  coords,
+                  num_coords,
+                  shaper_list))
+      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))
       {
         DEBUG_MSG_FUNC (SHAPE_PLAN, node->shape_plan, "fulfilled from cache");
         return hb_shape_plan_reference (node->shape_plan);
       }
+  }
 
   hb_shape_plan_t *shape_plan = hb_shape_plan_create2 (face, props,
                                                       user_features, 
num_user_features,
commit 1082338525c96206f43785e283e41b3e959871fd
Author: Behdad Esfahbod <[email protected]>
Date:   Mon Nov 12 18:05:02 2018 -0500

    [shape-plan] Only use shape-plan key to initialize hb_ot_shape_plan_t
    
    Such that we don't accidentally use info not in the cache key.

diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc
index 2500bcb8..b94b0107 100644
--- a/src/hb-ot-shape.cc
+++ b/src/hb-ot-shape.cc
@@ -53,7 +53,6 @@
 
 static void
 hb_ot_shape_collect_features (hb_ot_shape_planner_t          *planner,
-                             const hb_segment_properties_t  *props,
                              const hb_feature_t             *user_features,
                              unsigned int                    
num_user_features);
 
@@ -150,21 +149,20 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan,
 
 bool
 hb_ot_shape_plan_t::init0 (hb_face_t                     *face,
-                          const hb_segment_properties_t *props,
-                          const hb_feature_t            *user_features,
-                          unsigned int                   num_user_features,
-                          const int                     *coords,
-                          unsigned int                   num_coords)
+                          const hb_shape_plan_key_t     *key)
 {
   map.init ();
   aat_map.init ();
 
-  hb_ot_shape_planner_t planner (face, props);
+  hb_ot_shape_planner_t planner (face,
+                                &key->props);
+  hb_ot_shape_collect_features (&planner,
+                               key->user_features,
+                               key->num_user_features);
 
-  hb_ot_shape_collect_features (&planner, props,
-                               user_features, num_user_features);
-
-  planner.compile (*this, coords, num_coords);
+  planner.compile (*this,
+                  key->coords,
+                  key->num_coords);
 
   if (shaper->data_create)
   {
@@ -211,7 +209,6 @@ horizontal_features[] =
 
 static void
 hb_ot_shape_collect_features (hb_ot_shape_planner_t          *planner,
-                             const hb_segment_properties_t  *props,
                              const hb_feature_t             *user_features,
                              unsigned int                    num_user_features)
 {
@@ -220,7 +217,7 @@ hb_ot_shape_collect_features (hb_ot_shape_planner_t         
 *planner,
   map->enable_feature (HB_TAG('r','v','r','n'));
   map->add_gsub_pause (nullptr);
 
-  switch (props->direction) {
+  switch (planner->props.direction) {
     case HB_DIRECTION_LTR:
       map->enable_feature (HB_TAG ('l','t','r','a'));
       map->enable_feature (HB_TAG ('l','t','r','m'));
@@ -259,7 +256,7 @@ hb_ot_shape_collect_features (hb_ot_shape_planner_t         
 *planner,
   for (unsigned int i = 0; i < ARRAY_LENGTH (common_features); i++)
     map->add_feature (common_features[i]);
 
-  if (HB_DIRECTION_IS_HORIZONTAL (props->direction))
+  if (HB_DIRECTION_IS_HORIZONTAL (planner->props.direction))
     for (unsigned int i = 0; i < ARRAY_LENGTH (horizontal_features); i++)
       map->add_feature (horizontal_features[i]);
   else
diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh
index 9753752a..956cc6df 100644
--- a/src/hb-ot-shape.hh
+++ b/src/hb-ot-shape.hh
@@ -33,6 +33,8 @@
 #include "hb-aat-map.hh"
 
 
+struct hb_shape_plan_key_t;
+
 struct hb_ot_shape_plan_t
 {
   hb_segment_properties_t props;
@@ -74,11 +76,7 @@ struct hb_ot_shape_plan_t
   inline void position (hb_font_t *font, hb_buffer_t *buffer) const { 
map.position (this, font, buffer); }
 
   HB_INTERNAL bool init0 (hb_face_t                     *face,
-                         const hb_segment_properties_t *props,
-                         const hb_feature_t            *user_features,
-                         unsigned int                   num_user_features,
-                         const int                     *coords,
-                         unsigned int                   num_coords);
+                         const hb_shape_plan_key_t     *key);
   HB_INTERNAL void fini (void);
 };
 
diff --git a/src/hb-shape-plan.cc b/src/hb-shape-plan.cc
index 4c3ae062..70083c8b 100644
--- a/src/hb-shape-plan.cc
+++ b/src/hb-shape-plan.cc
@@ -194,12 +194,7 @@ hb_shape_plan_create2 (hb_face_t                     *face,
                                       num_coords,
                                       shaper_list)))
     goto bail2;
-  if (unlikely (!shape_plan->ot.init0 (face,
-                                      props,
-                                      user_features,
-                                      num_user_features,
-                                      coords,
-                                      num_coords)))
+  if (unlikely (!shape_plan->ot.init0 (face, &shape_plan->key)))
     goto bail3;
 
   return shape_plan;
_______________________________________________
HarfBuzz mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/harfbuzz

Reply via email to