Hey Behdad,
Turns out that for fonts such as Noto Devanagari and Gujarati, we're
spending an inordinate amount of time under lookup.is_inplace
(&inplace_c) in apply_string().
We can get a big win for these fonts if we cache the is_inplace result
in the lookup-accelerator, instead of recursing down through the
sub-lookups every time.
With the attached patch, I see the total run time for a test such as
hb-shape NotoSansDevanagari-Regular.ttf --text-file hi.txt > /dev/null
drop from 22 seconds to 8 sec. And that includes the time to read the
file, populate and serialize the buffer, etc., so once those constant
factors are discounted, the actual shaping itself is over 3x faster. :)
NotoSansGujarati shows a similar gain; and Amiri with ar.txt runs in 45
sec instead of 1 min+, for a speedup of over 25%.
(For fonts that don't have lots of complex contextual lookups, of
course, there's no significant difference.)
See what you think. An alternative implementation might be to initialize
the is_inplace flag during hb_ot_layout_lookup_accelerator_t::init()
(then no need for the _initialized flag, hence taking an if() out of the
hot path, and the hb_ot_layout_lookup_accelerator_t references could
remain const), but it looked like this would involve rather more
rearrangement of code, so I took the simplest approach for now.
JK
diff --git a/src/hb-ot-layout-private.hh b/src/hb-ot-layout-private.hh
index 139e33f..ae5bb99 100644
--- a/src/hb-ot-layout-private.hh
+++ b/src/hb-ot-layout-private.hh
@@ -86,7 +86,7 @@ namespace OT {
HB_INTERNAL void
hb_ot_layout_substitute_lookup (OT::hb_apply_context_t *c,
const OT::SubstLookup &lookup,
- const hb_ot_layout_lookup_accelerator_t &accel);
+ hb_ot_layout_lookup_accelerator_t &accel);
/* Should be called after all the substitute_lookup's are done */
@@ -124,6 +124,7 @@ struct hb_ot_layout_lookup_accelerator_t
{
digest.init ();
lookup.add_coverage (&digest);
+ is_inplace_initialized = false;
}
template <typename TLookup>
@@ -132,6 +133,8 @@ struct hb_ot_layout_lookup_accelerator_t
}
hb_set_digest_t digest;
+ bool is_inplace;
+ bool is_inplace_initialized;
};
struct hb_ot_layout_t
diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc
index bd8ef08..be17701 100644
--- a/src/hb-ot-layout.cc
+++ b/src/hb-ot-layout.cc
@@ -790,7 +790,7 @@ struct GSUBProxy
accels (hb_ot_layout_from_face (face)->gsub_accels) {}
const OT::GSUB &table;
- const hb_ot_layout_lookup_accelerator_t *accels;
+ hb_ot_layout_lookup_accelerator_t *accels;
};
struct GPOSProxy
@@ -803,7 +803,7 @@ struct GPOSProxy
accels (hb_ot_layout_from_face (face)->gpos_accels) {}
const OT::GPOS &table;
- const hb_ot_layout_lookup_accelerator_t *accels;
+ hb_ot_layout_lookup_accelerator_t *accels;
};
@@ -820,11 +820,9 @@ template <typename Proxy>
static inline bool
apply_string (OT::hb_apply_context_t *c,
const typename Proxy::Lookup &lookup,
- const hb_ot_layout_lookup_accelerator_t &accel)
+ hb_ot_layout_lookup_accelerator_t &accel)
{
bool ret = false;
- OT::hb_is_inplace_context_t inplace_c (c->face);
- bool inplace = lookup.is_inplace (&inplace_c);
hb_buffer_t *buffer = c->buffer;
if (unlikely (!buffer->len || !c->lookup_mask))
@@ -850,6 +848,18 @@ apply_string (OT::hb_apply_context_t *c,
}
if (ret)
{
+ bool inplace;
+ if (accel.is_inplace_initialized)
+ {
+ inplace = accel.is_inplace;
+ }
+ else
+ {
+ accel.is_inplace_initialized = true;
+ OT::hb_is_inplace_context_t inplace_c (c->face);
+ inplace = accel.is_inplace = lookup.is_inplace (&inplace_c);
+ }
+
if (!inplace)
buffer->swap_buffers ();
else
@@ -924,7 +934,7 @@ void hb_ot_map_t::position (const hb_ot_shape_plan_t *plan,
hb_font_t *font, hb_
HB_INTERNAL void
hb_ot_layout_substitute_lookup (OT::hb_apply_context_t *c,
const OT::SubstLookup &lookup,
- const hb_ot_layout_lookup_accelerator_t &accel)
+ hb_ot_layout_lookup_accelerator_t &accel)
{
apply_string<GSUBProxy> (c, lookup, accel);
}
_______________________________________________
HarfBuzz mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/harfbuzz
