[HarfBuzz] harfbuzz: Branch 'master' - 5 commits

Behdad Esfahbod behdad at kemper.freedesktop.org
Mon Dec 2 02:57:42 PST 2013


 src/hb-ot-shape-private.hh   |    2 -
 src/hb-shape-plan-private.hh |    5 ++-
 src/hb-shape-plan.cc         |   69 +++++++++++++++++++++++++++++++++++--------
 util/options.cc              |    5 ++-
 util/view-cairo.hh           |    2 -
 5 files changed, 67 insertions(+), 16 deletions(-)

New commits:
commit f47b9219546edcfdeb3991ee27f6d9ba455c3e08
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Mon Dec 2 05:57:27 2013 -0500

    Fix unsafe shape_plan->face dependency

diff --git a/src/hb-ot-shape-private.hh b/src/hb-ot-shape-private.hh
index 8171471..cbfab5b 100644
--- a/src/hb-ot-shape-private.hh
+++ b/src/hb-ot-shape-private.hh
@@ -66,7 +66,7 @@ struct hb_ot_shape_planner_t
   hb_ot_map_builder_t map;
 
   hb_ot_shape_planner_t (const hb_shape_plan_t *master_plan) :
-			 face (master_plan->face),
+			 face (master_plan->face_unsafe),
 			 props (master_plan->props),
 			 shaper (NULL),
 			 map (face, &props) {}
diff --git a/src/hb-shape-plan-private.hh b/src/hb-shape-plan-private.hh
index ee48767..e12b05f 100644
--- a/src/hb-shape-plan-private.hh
+++ b/src/hb-shape-plan-private.hh
@@ -39,7 +39,7 @@ struct hb_shape_plan_t
   ASSERT_POD ();
 
   hb_bool_t default_shaper_list;
-  hb_face_t *face;
+  hb_face_t *face_unsafe; /* We don't carry a reference to face. */
   hb_segment_properties_t props;
 
   hb_shape_func_t *shaper_func;
diff --git a/src/hb-shape-plan.cc b/src/hb-shape-plan.cc
index d2aa03b..e354f29 100644
--- a/src/hb-shape-plan.cc
+++ b/src/hb-shape-plan.cc
@@ -46,7 +46,7 @@ hb_shape_plan_plan (hb_shape_plan_t    *shape_plan,
 
 #define HB_SHAPER_PLAN(shaper) \
 	HB_STMT_START { \
-	  if (hb_##shaper##_shaper_face_data_ensure (shape_plan->face)) { \
+	  if (hb_##shaper##_shaper_face_data_ensure (shape_plan->face_unsafe)) { \
 	    HB_SHAPER_DATA (shaper, shape_plan) = \
 	      HB_SHAPER_DATA_CREATE_FUNC (shaper, shape_plan) (shape_plan, user_features, num_user_features); \
 	    shape_plan->shaper_func = _hb_##shaper##_shape; \
@@ -122,7 +122,7 @@ hb_shape_plan_create (hb_face_t                     *face,
 
   hb_face_make_immutable (face);
   shape_plan->default_shaper_list = shaper_list == NULL;
-  shape_plan->face = hb_face_reference (face);
+  shape_plan->face_unsafe = face;
   shape_plan->props = *props;
   shape_plan->num_user_features = num_user_features;
   shape_plan->user_features = features;
@@ -202,7 +202,6 @@ hb_shape_plan_destroy (hb_shape_plan_t *shape_plan)
 #include "hb-shaper-list.hh"
 #undef HB_SHAPER_IMPLEMENT
 
-  hb_face_destroy (shape_plan->face);
   free (shape_plan->user_features);
 
   free (shape_plan);
@@ -277,7 +276,7 @@ hb_shape_plan_execute (hb_shape_plan_t    *shape_plan,
 		hb_object_is_inert (buffer)))
     return false;
 
-  assert (shape_plan->face == font->face);
+  assert (shape_plan->face_unsafe == font->face);
   assert (hb_segment_properties_equal (&shape_plan->props, &buffer->props));
 
 #define HB_SHAPER_EXECUTE(shaper) \
@@ -444,11 +443,6 @@ retry:
     goto retry;
   }
 
-  /* Release our reference on face. */
-  /* XXX This is unsafe, since the face can be freed before us,
-   * and we will call hb_face_destroy() in our destroy()! */
-  hb_face_destroy (face);
-
   return hb_shape_plan_reference (shape_plan);
 }
 
commit c704a8700e169885f1d9cbab93544d85aa4358e9
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Mon Dec 2 05:42:04 2013 -0500

    [util] Fix uninitialized memory access

diff --git a/util/view-cairo.hh b/util/view-cairo.hh
index 2c504c1..7fe217c 100644
--- a/util/view-cairo.hh
+++ b/util/view-cairo.hh
@@ -36,7 +36,7 @@ struct view_cairo_t
   view_cairo_t (option_parser_t *parser)
 	       : output_options (parser, helper_cairo_supported_formats),
 		 view_options (parser),
-		 lines (0), scale (1.0) {}
+		 lines (0), scale (1.0), direction (HB_DIRECTION_INVALID) {}
   ~view_cairo_t (void) {
     if (debug)
       cairo_debug_reset_static_data ();
commit 260a3198f44a4ece60864b6f6caab2ee756ad762
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Mon Dec 2 05:39:39 2013 -0500

    [util] Plug leak

diff --git a/util/options.cc b/util/options.cc
index 66b5e15..3ae2454 100644
--- a/util/options.cc
+++ b/util/options.cc
@@ -369,11 +369,12 @@ void
 output_options_t::add_options (option_parser_t *parser)
 {
   const char *text;
+  char *text_free = NULL;
 
   if (NULL == supported_formats)
     text = "Set output format";
   else
-    text = g_strdup_printf ("Set output format\n\n    Supported formats are: %s", supported_formats);
+    text = text_free = g_strdup_printf ("Set output format\n\n    Supported formats are: %s", supported_formats);
 
   GOptionEntry entries[] =
   {
@@ -386,6 +387,8 @@ output_options_t::add_options (option_parser_t *parser)
 		     "Output options:",
 		     "Options controlling the output",
 		     this);
+
+  g_free (text_free);
 }
 
 
commit ca8d96c8ba33ce581684cbc07936a3696b6c83d9
Author: Jonathan Kew <jfkthame at gmail.com>
Date:   Mon Dec 2 05:22:00 2013 -0500

    cache shape plans even if (global) user features are set

diff --git a/src/hb-shape-plan-private.hh b/src/hb-shape-plan-private.hh
index dd014e3..ee48767 100644
--- a/src/hb-shape-plan-private.hh
+++ b/src/hb-shape-plan-private.hh
@@ -45,6 +45,9 @@ struct hb_shape_plan_t
   hb_shape_func_t *shaper_func;
   const char *shaper_name;
 
+  hb_feature_t *user_features;
+  unsigned int num_user_features;
+
   struct hb_shaper_data_t shaper_data;
 };
 
diff --git a/src/hb-shape-plan.cc b/src/hb-shape-plan.cc
index ab79262..d2aa03b 100644
--- a/src/hb-shape-plan.cc
+++ b/src/hb-shape-plan.cc
@@ -107,18 +107,27 @@ hb_shape_plan_create (hb_face_t                     *face,
   assert (props->direction != HB_DIRECTION_INVALID);
 
   hb_shape_plan_t *shape_plan;
+  hb_feature_t *features = NULL;
 
   if (unlikely (!face))
     face = hb_face_get_empty ();
   if (unlikely (!props || hb_object_is_inert (face)))
     return hb_shape_plan_get_empty ();
-  if (!(shape_plan = hb_object_create<hb_shape_plan_t> ()))
+  if (num_user_features && !(features = (hb_feature_t *) malloc (num_user_features * sizeof (hb_feature_t))))
     return hb_shape_plan_get_empty ();
+  if (!(shape_plan = hb_object_create<hb_shape_plan_t> ())) {
+    free (features);
+    return hb_shape_plan_get_empty ();
+  }
 
   hb_face_make_immutable (face);
   shape_plan->default_shaper_list = shaper_list == NULL;
   shape_plan->face = hb_face_reference (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));
 
   hb_shape_plan_plan (shape_plan, user_features, num_user_features, shaper_list);
 
@@ -147,6 +156,9 @@ hb_shape_plan_get_empty (void)
     NULL, /* shaper_func */
     NULL, /* shaper_name */
 
+    NULL, /* user_features */
+    0,    /* num_user_featurs */
+
     {
 #define HB_SHAPER_IMPLEMENT(shaper) HB_SHAPER_DATA_INVALID,
 #include "hb-shaper-list.hh"
@@ -191,6 +203,7 @@ hb_shape_plan_destroy (hb_shape_plan_t *shape_plan)
 #undef HB_SHAPER_IMPLEMENT
 
   hb_face_destroy (shape_plan->face);
+  free (shape_plan->user_features);
 
   free (shape_plan);
 }
@@ -301,23 +314,55 @@ hb_shape_plan_hash (const hb_shape_plan_t *shape_plan)
 }
 #endif
 
-/* TODO no user-feature caching for now. */
+/* 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 char * const            *shaper_list;
+  const hb_feature_t            *user_features;
+  unsigned int                   num_user_features;
   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)
+{
+  if (proposal->num_user_features != shape_plan->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;
+}
+
 static hb_bool_t
 hb_shape_plan_matches (const hb_shape_plan_t          *shape_plan,
 		       const hb_shape_plan_proposal_t *proposal)
 {
   return hb_segment_properties_equal (&shape_plan->props, &proposal->props) &&
+	 hb_shape_plan_user_features_match (shape_plan, proposal) &&
 	 ((shape_plan->default_shaper_list && proposal->shaper_list == NULL) ||
 	  (shape_plan->shaper_func == proposal->shaper_func));
 }
 
+static inline hb_bool_t
+hb_non_global_user_features_present (const hb_feature_t *user_features,
+				     unsigned int        num_user_features)
+{
+  while (num_user_features)
+    if (user_features->start != 0 || user_features->end != (unsigned int) -1)
+      return true;
+    else
+      num_user_features--, user_features++;
+  return false;
+}
+
 /**
  * hb_shape_plan_create_cached:
  * @face: 
@@ -339,12 +384,11 @@ hb_shape_plan_create_cached (hb_face_t                     *face,
 			     unsigned int                   num_user_features,
 			     const char * const            *shaper_list)
 {
-  if (num_user_features)
-    return hb_shape_plan_create (face, props, user_features, num_user_features, shaper_list);
-
   hb_shape_plan_proposal_t proposal = {
     *props,
     shaper_list,
+    user_features,
+    num_user_features,
     NULL
   };
 
@@ -382,6 +426,11 @@ retry:
 
   hb_shape_plan_t *shape_plan = hb_shape_plan_create (face, props, user_features, num_user_features, shaper_list);
 
+  /* Don't add the plan to the cache if there were user features with non-global ranges */
+
+  if (hb_non_global_user_features_present (user_features, num_user_features))
+    return shape_plan;
+
   hb_face_t::plan_node_t *node = (hb_face_t::plan_node_t *) calloc (1, sizeof (hb_face_t::plan_node_t));
   if (unlikely (!node))
     return shape_plan;
commit 8ffa528f28a24ae85952ad1c1b0206e736bcfeab
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Mon Dec 2 05:17:14 2013 -0500

    Add note about unsafe shape_plan->face
    
    Will fix by removing shape_plan->face completely.

diff --git a/src/hb-shape-plan.cc b/src/hb-shape-plan.cc
index b44a9e2..ab79262 100644
--- a/src/hb-shape-plan.cc
+++ b/src/hb-shape-plan.cc
@@ -396,6 +396,8 @@ retry:
   }
 
   /* Release our reference on face. */
+  /* XXX This is unsafe, since the face can be freed before us,
+   * and we will call hb_face_destroy() in our destroy()! */
   hb_face_destroy (face);
 
   return hb_shape_plan_reference (shape_plan);



More information about the HarfBuzz mailing list