[shape-plan] Turn hb_shape_plan_proposal_t into hb_shape_plan_key_t

And include it in hb_shape_plan_t itself.

src/hb-shape-plan.cc

@@ -45,41 +45,26 @@
 
 
 static void
-hb_shape_plan_choose_shaper (hb_shape_plan_t    *shape_plan,
-			     const hb_feature_t *user_features,
-			     unsigned int        num_user_features,
-			     const int          *coords,
-			     unsigned int        num_coords,
-			     const char * const *shaper_list)
+hb_shape_plan_key_choose_shaper (hb_shape_plan_key_t *key,
+				 hb_face_t           *face,
+				 const hb_feature_t  *user_features,
+				 unsigned int         num_user_features,
+				 const int           *coords,
+				 unsigned int         num_coords,
+				 const char * const  *shaper_list)
 {
-  DEBUG_MSG_FUNC (SHAPE_PLAN, shape_plan,
-		  "num_features=%d num_coords=%d shaper_list=%p",
-		  num_user_features,
-		  num_coords,
-		  shaper_list);
-
-  const hb_shaper_pair_static_t *shapers = _hb_shapers_get ();
-
 #define HB_SHAPER_PLAN(shaper) \
 	HB_STMT_START { \
-	  if (hb_##shaper##_shaper_face_data_ensure (shape_plan->face_unsafe)) \
+	  if (hb_##shaper##_shaper_face_data_ensure (face)) \
 	  { \
-	    shape_plan->shaper_func = _hb_##shaper##_shape; \
-	    shape_plan->shaper_name = #shaper; \
+	    key->shaper_func = _hb_##shaper##_shape; \
+	    key->shaper_name = #shaper; \
 	    return; \
 	  } \
 	} HB_STMT_END
 
-  if (likely (!shaper_list)) {
-    for (unsigned int i = 0; i < HB_SHAPERS_COUNT; i++)
-      if (false)
-	;
-#define HB_SHAPER_IMPLEMENT(shaper) \
-      else if (shapers[i].func == _hb_##shaper##_shape) \
-	HB_SHAPER_PLAN (shaper);
-#include "hb-shaper-list.hh"
-#undef HB_SHAPER_IMPLEMENT
-  } else {
+  if (unlikely (shaper_list))
+  {
     for (; *shaper_list; shaper_list++)
       if (false)
 	;
@@ -90,6 +75,16 @@ hb_shape_plan_choose_shaper (hb_shape_plan_t    *shape_plan,
 #undef HB_SHAPER_IMPLEMENT
   }
 
+  const hb_shaper_pair_static_t *shapers = _hb_shapers_get ();
+  for (unsigned int i = 0; i < HB_SHAPERS_COUNT; i++)
+    if (false)
+      ;
+#define HB_SHAPER_IMPLEMENT(shaper) \
+    else if (shapers[i].func == _hb_##shaper##_shape) \
+      HB_SHAPER_PLAN (shaper);
+#include "hb-shaper-list.hh"
+#undef HB_SHAPER_IMPLEMENT
+
 #undef HB_SHAPER_PLAN
 }
 
@@ -142,6 +137,8 @@ hb_shape_plan_create2 (hb_face_t                     *face,
 		  num_coords,
 		  shaper_list);
 
+  assert (props->direction != HB_DIRECTION_INVALID);
+
   hb_shape_plan_t *shape_plan;
   hb_feature_t *features = nullptr;
   int *coords = nullptr;
@@ -152,30 +149,32 @@ hb_shape_plan_create2 (hb_face_t                     *face,
     goto bail;
   if (num_coords && !(coords = (int *) calloc (num_coords, sizeof (int))))
     goto bail;
+
   if (!(shape_plan = hb_object_create<hb_shape_plan_t> ()))
     goto bail;
 
-  assert (props->direction != HB_DIRECTION_INVALID);
-
   if (unlikely (!face))
     face = hb_face_get_empty ();
   hb_face_make_immutable (face);
-
   shape_plan->face_unsafe = face;
-  shape_plan->props = *props;
-  shape_plan->num_user_features = num_user_features;
-  shape_plan->user_features = features;
-  if (num_user_features)
-    memcpy (features, user_features, num_user_features * sizeof (hb_feature_t));
-  shape_plan->num_coords = num_coords;
-  shape_plan->coords = coords;
-  if (num_coords)
-    memcpy (coords, orig_coords, num_coords * sizeof (int));
-  shape_plan->custom_shaper_list = shaper_list;
-  hb_shape_plan_choose_shaper (shape_plan,
-			       user_features, num_user_features,
-			       coords, num_coords,
-			       shaper_list);
+
+  {
+    hb_shape_plan_key_t *key = &shape_plan->key;
+    key->props = *props;
+    key->num_user_features = num_user_features;
+    key->user_features = features;
+    if (num_user_features)
+      memcpy (features, user_features, num_user_features * sizeof (hb_feature_t));
+    key->num_coords = num_coords;
+    key->coords = coords;
+    if (num_coords)
+      memcpy (coords, orig_coords, num_coords * sizeof (int));
+    hb_shape_plan_key_choose_shaper (key,
+				     face,
+				     user_features, num_user_features,
+				     coords, num_coords,
+				     shaper_list);
+  }
 
   if (unlikely (!shape_plan->ot.init0 (face,
 				       props,
@@ -239,8 +238,8 @@ hb_shape_plan_destroy (hb_shape_plan_t *shape_plan)
 
   shape_plan->ot.fini ();
 
-  free (shape_plan->user_features);
-  free (shape_plan->coords);
+  free ((void *) shape_plan->key.user_features);
+  free ((void *) shape_plan->key.coords);
 
   free (shape_plan);
 }
@@ -312,8 +311,8 @@ hb_shape_plan_execute (hb_shape_plan_t    *shape_plan,
   DEBUG_MSG_FUNC (SHAPE_PLAN, shape_plan,
 		  "num_features=%d shaper_func=%p, shaper_name=%s",
 		  num_features,
-		  shape_plan->shaper_func,
-		  shape_plan->shaper_name);
+		  shape_plan->key.shaper_func,
+		  shape_plan->key.shaper_name);
 
   if (unlikely (!buffer->len))
     return true;
@@ -325,7 +324,7 @@ hb_shape_plan_execute (hb_shape_plan_t    *shape_plan,
     return false;
 
   assert (shape_plan->face_unsafe == font->face);
-  assert (hb_segment_properties_equal (&shape_plan->props, &buffer->props));
+  assert (hb_segment_properties_equal (&shape_plan->key.props, &buffer->props));
 
 #define HB_SHAPER_EXECUTE(shaper) \
 	HB_STMT_START { \
@@ -336,7 +335,7 @@ hb_shape_plan_execute (hb_shape_plan_t    *shape_plan,
   if (false)
     ;
 #define HB_SHAPER_IMPLEMENT(shaper) \
-  else if (shape_plan->shaper_func == _hb_##shaper##_shape) \
+  else if (shape_plan->key.shaper_func == _hb_##shaper##_shape) \
     HB_SHAPER_EXECUTE (shaper);
 #include "hb-shaper-list.hh"
 #undef HB_SHAPER_IMPLEMENT
@@ -351,67 +350,36 @@ hb_shape_plan_execute (hb_shape_plan_t    *shape_plan,
  * caching
  */
 
-#if 0
-static unsigned int
-hb_shape_plan_hash (const hb_shape_plan_t *shape_plan)
-{
-  return hb_segment_properties_hash (&shape_plan->props) +
-	 shape_plan->custom_shaper_list ? (intptr_t) shape_plan->shaper_func : 0;
-}
-#endif
-
-/* User-feature caching is currently somewhat dumb:
- * it only finds matches where the feature array is identical,
- * not cases where the feature lists would be compatible for plan purposes
- * but have different ranges, for example.
- */
-struct hb_shape_plan_proposal_t
-{
-  const hb_segment_properties_t  props;
-  const hb_feature_t            *user_features;
-  unsigned int                   num_user_features;
-  const int                     *coords;
-  unsigned int                   num_coords;
-  bool                           custom_shaper_list;
-  hb_shape_func_t               *shaper_func;
-};
-
-static inline hb_bool_t
-hb_shape_plan_user_features_match (const hb_shape_plan_t          *shape_plan,
-				   const hb_shape_plan_proposal_t *proposal)
+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 (proposal->num_user_features != shape_plan->num_user_features)
+  if (key1->num_user_features != key2->num_user_features)
     return false;
-  for (unsigned int i = 0, n = proposal->num_user_features; i < n; i++)
-    if (proposal->user_features[i].tag   != shape_plan->user_features[i].tag   ||
-        proposal->user_features[i].value != shape_plan->user_features[i].value ||
-        proposal->user_features[i].start != shape_plan->user_features[i].start ||
-        proposal->user_features[i].end   != shape_plan->user_features[i].end)
-      return false;
-  return true;
+  return 0 == hb_memcmp(key1->user_features,
+			key2->user_features,
+			key1->num_user_features * sizeof (key1->user_features[0]));
 }
 
-static inline hb_bool_t
-hb_shape_plan_coords_match (const hb_shape_plan_t          *shape_plan,
-			    const hb_shape_plan_proposal_t *proposal)
+static inline bool
+hb_shape_plan_key_coords_equal (const hb_shape_plan_key_t *key2,
+				const hb_shape_plan_key_t *key1)
 {
-  if (proposal->num_coords != shape_plan->num_coords)
+  if (key1->num_coords != key2->num_coords)
     return false;
-  for (unsigned int i = 0, n = proposal->num_coords; i < n; i++)
-    if (proposal->coords[i] != shape_plan->coords[i])
-      return false;
-  return true;
+  return 0 == hb_memcmp(key1->coords,
+			key2->coords,
+			key1->num_coords * sizeof (key1->coords[0]));
 }
 
-static hb_bool_t
-hb_shape_plan_matches (const hb_shape_plan_t          *shape_plan,
-		       const hb_shape_plan_proposal_t *proposal)
+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 (&shape_plan->props, &proposal->props) &&
-	 hb_shape_plan_user_features_match (shape_plan, proposal) &&
-	 hb_shape_plan_coords_match (shape_plan, proposal) &&
-	 ((!shape_plan->custom_shaper_list && !proposal->custom_shaper_list) ||
-	  (shape_plan->shaper_func == proposal->shaper_func));
+  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 hb_bool_t
@@ -476,38 +444,20 @@ hb_shape_plan_create_cached2 (hb_face_t                     *face,
 		  num_user_features,
 		  shaper_list);
 
-  hb_shape_plan_proposal_t proposal =
+  hb_shape_plan_key_t key =
   {
     *props,
     user_features,
     num_user_features,
     coords,
     num_coords,
-    shaper_list,
     nullptr
   };
-
-  if (shaper_list)
-  {
-    /* Choose shaper.  Adapted from hb_shape_plan_choose_shaper().
-     * Must choose shaper exactly the same way as that function. */
-    for (const char * const *shaper_item = shaper_list; *shaper_item; shaper_item++)
-      if (false)
-	;
-#define HB_SHAPER_IMPLEMENT(shaper) \
-      else if (0 == strcmp (*shaper_item, #shaper) && \
-	       hb_##shaper##_shaper_face_data_ensure (face)) \
-      { \
-	proposal.shaper_func = _hb_##shaper##_shape; \
-	break; \
-      }
-#include "hb-shaper-list.hh"
-#undef HB_SHAPER_IMPLEMENT
-
-    if (unlikely (!proposal.shaper_func))
-      return hb_shape_plan_get_empty ();
-  }
-
+  hb_shape_plan_key_choose_shaper (&key,
+				   face,
+				   user_features, num_user_features,
+				   coords, num_coords,
+				   shaper_list);
 
 retry:
   hb_face_t::plan_node_t *cached_plan_nodes = face->shape_plans;
@@ -515,7 +465,7 @@ retry:
   /* Don't look for plan in the cache if there were variation coordinates XXX Fix me. */
   if (!hb_coords_present (coords, num_coords))
     for (hb_face_t::plan_node_t *node = cached_plan_nodes; node; node = node->next)
-      if (hb_shape_plan_matches (node->shape_plan, &proposal))
+      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);
@@ -569,5 +519,5 @@ retry:
 const char *
 hb_shape_plan_get_shaper (hb_shape_plan_t *shape_plan)
 {
-  return shape_plan->shaper_name;
+  return shape_plan->key.shaper_name;
 }

src/hb-shape-plan.hh

@@ -32,24 +32,25 @@
 #include "hb-ot-shape.hh"
 
 
-struct hb_shape_plan_t
+struct hb_shape_plan_key_t
 {
-  hb_object_header_t header;
-
-  hb_face_t *face_unsafe; /* We don't carry a reference to face. */
-
-  hb_segment_properties_t props;
+  hb_segment_properties_t  props;
 
-  hb_feature_t *user_features;
-  unsigned int num_user_features;
+  const hb_feature_t      *user_features;
+  unsigned int             num_user_features;
 
-  int *coords;
-  unsigned int num_coords;
+  const int               *coords;
+  unsigned int             num_coords;
 
-  bool custom_shaper_list;
-  hb_shape_func_t *shaper_func;
-  const char *shaper_name;
+  hb_shape_func_t         *shaper_func;
+  const char              *shaper_name;
+};
 
+struct hb_shape_plan_t
+{
+  hb_object_header_t header;
+  hb_face_t *face_unsafe; /* We don't carry a reference to face. */
+  hb_shape_plan_key_t key;
   hb_ot_shape_plan_t ot;
 };