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 <beh...@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 <beh...@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 <beh...@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 <jfkth...@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 <beh...@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); _______________________________________________ HarfBuzz mailing list HarfBuzz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/harfbuzz