From e6409d3905d8801d1be647d505524f71230c6ca1 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sun, 5 Jun 2022 06:57:37 -0600 Subject: [PATCH] Revert "[layout] Use a cache for main input ClassDef of (Chain)ContextLookups" This reverts commit 57d1c08739d0acd94b96da2f9d5dd6c0ff3b3722. Err. This was an accident. --- src/hb-ot-layout-common.hh | 13 -- src/hb-ot-layout-gsubgpos.hh | 275 ++++++----------------------------- src/hb-ot-layout.cc | 10 +- 3 files changed, 43 insertions(+), 255 deletions(-) diff --git a/src/hb-ot-layout-common.hh b/src/hb-ot-layout-common.hh index d34380534..015180778 100644 --- a/src/hb-ot-layout-common.hh +++ b/src/hb-ot-layout-common.hh @@ -2001,8 +2001,6 @@ struct ClassDefFormat1 return_trace (c->check_struct (this) && classValue.sanitize (c)); } - unsigned cost () const { return 1; } - template bool collect_coverage (set_t *glyphs) const { @@ -2239,8 +2237,6 @@ struct ClassDefFormat2 return_trace (rangeRecord.sanitize (c)); } - unsigned cost () const { return hb_bit_storage ((unsigned) rangeRecord.len); /* bsearch cost */ } - template bool collect_coverage (set_t *glyphs) const { @@ -2481,15 +2477,6 @@ struct ClassDef } } - unsigned cost () const - { - switch (u.format) { - case 1: return u.format1.cost (); - case 2: return u.format2.cost (); - default:return 0u; - } - } - /* Might return false if array looks unsorted. * Used for faster rejection of corrupt data. */ template diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index 17a1c878b..7f57adfb2 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -394,6 +394,7 @@ struct hb_collect_coverage_context_t : set_t *set; }; + struct hb_ot_apply_context_t : hb_dispatch_context_t { @@ -409,7 +410,7 @@ struct hb_ot_apply_context_t : match_func (nullptr), match_data (nullptr) {} - typedef bool (*match_func_t) (hb_glyph_info_t &info, const HBUINT16 &value, const void *data); + typedef bool (*match_func_t) (hb_codepoint_t glyph_id, const HBUINT16 &value, const void *data); void set_ignore_zwnj (bool ignore_zwnj_) { ignore_zwnj = ignore_zwnj_; } void set_ignore_zwj (bool ignore_zwj_) { ignore_zwj = ignore_zwj_; } @@ -427,7 +428,7 @@ struct hb_ot_apply_context_t : MATCH_MAYBE }; - may_match_t may_match (hb_glyph_info_t &info, + may_match_t may_match (const hb_glyph_info_t &info, const HBUINT16 *glyph_data) const { if (!(info.mask & mask) || @@ -435,7 +436,7 @@ struct hb_ot_apply_context_t : return MATCH_NO; if (match_func) - return match_func (info, *glyph_data, match_data) ? MATCH_YES : MATCH_NO; + return match_func (info.codepoint, *glyph_data, match_data) ? MATCH_YES : MATCH_NO; return MATCH_MAYBE; } @@ -523,7 +524,7 @@ struct hb_ot_apply_context_t : while (idx + num_items < end) { idx++; - hb_glyph_info_t &info = c->buffer->info[idx]; + const hb_glyph_info_t &info = c->buffer->info[idx]; matcher_t::may_skip_t skip = matcher.may_skip (c, info); if (unlikely (skip == matcher_t::SKIP_YES)) @@ -556,7 +557,7 @@ struct hb_ot_apply_context_t : while (idx > num_items - 1) { idx--; - hb_glyph_info_t &info = c->buffer->out_info[idx]; + const hb_glyph_info_t &info = c->buffer->out_info[idx]; matcher_t::may_skip_t skip = matcher.may_skip (c, info); if (unlikely (skip == matcher_t::SKIP_YES)) @@ -638,7 +639,6 @@ struct hb_ot_apply_context_t : bool per_syllable = false; bool random = false; uint32_t random_state = 1; - unsigned new_syllables = (unsigned) -1; hb_ot_apply_context_t (unsigned int table_index_, hb_font_t *font_, @@ -736,9 +736,6 @@ struct hb_ot_apply_context_t : bool ligature = false, bool component = false) const { - if (new_syllables != (unsigned) -1) - buffer->cur().syllable() = new_syllables; - unsigned int props = _hb_glyph_info_get_glyph_props (&buffer->cur()); props |= HB_OT_LAYOUT_GLYPH_PROPS_SUBSTITUTED; if (ligature) @@ -803,60 +800,15 @@ struct hb_accelerate_subtables_context_t : return typed_obj->apply (c); } - template - static inline auto apply_cached_ (const T *obj, OT::hb_ot_apply_context_t *c, hb_priority<1>) HB_RETURN (bool, obj->apply_cached (c) ) - template - static inline auto apply_cached_ (const T *obj, OT::hb_ot_apply_context_t *c, hb_priority<0>) HB_RETURN (bool, obj->apply (c) ) - template - static inline bool apply_cached_to (const void *obj, OT::hb_ot_apply_context_t *c) - { - const Type *typed_obj = (const Type *) obj; - return apply_cached_ (typed_obj, c, hb_prioritize); - } - - template - static inline auto cache_enter_ (const T *obj, OT::hb_ot_apply_context_t *c, hb_priority<1>) HB_RETURN (void, obj->cache_enter (c) ) - template - static inline void cache_enter_ (const T *obj, OT::hb_ot_apply_context_t *c, hb_priority<0>) {} - template - static inline void cache_enter_to (const void *obj, OT::hb_ot_apply_context_t *c) - { - const Type *typed_obj = (const Type *) obj; - return cache_enter_ (typed_obj, c, hb_prioritize); - } - - template - static inline auto cache_leave_ (const T *obj, OT::hb_ot_apply_context_t *c, hb_priority<1>) HB_RETURN (void, obj->cache_leave (c) ) - template - static inline void cache_leave_ (const T *obj, OT::hb_ot_apply_context_t *c, hb_priority<0>) {} - template - static inline void cache_leave_to (const void *obj, OT::hb_ot_apply_context_t *c) - { - const Type *typed_obj = (const Type *) obj; - return cache_leave_ (typed_obj, c, hb_prioritize); - } - typedef bool (*hb_apply_func_t) (const void *obj, OT::hb_ot_apply_context_t *c); - typedef void (*hb_cache_enter_func_t) (const void *obj, OT::hb_ot_apply_context_t *c); - typedef void (*hb_cache_leave_func_t) (const void *obj, OT::hb_ot_apply_context_t *c); struct hb_applicable_t { - friend struct hb_accelerate_subtables_context_t; - friend struct hb_ot_layout_lookup_accelerator_t; - template - void init (const T &obj_, - hb_apply_func_t apply_func_, - hb_apply_func_t apply_cached_func_, - hb_cache_enter_func_t cache_enter_func_, - hb_cache_leave_func_t cache_leave_func_) + void init (const T &obj_, hb_apply_func_t apply_func_) { obj = &obj_; apply_func = apply_func_; - apply_cached_func = apply_cached_func_; - cache_enter_func = cache_enter_func_; - cache_leave_func = cache_leave_func_; digest.init (); obj_.get_coverage ().collect_coverage (&digest); } @@ -865,59 +817,21 @@ struct hb_accelerate_subtables_context_t : { return digest.may_have (c->buffer->cur().codepoint) && apply_func (obj, c); } - bool apply_cached (OT::hb_ot_apply_context_t *c) const - { - return digest.may_have (c->buffer->cur().codepoint) && apply_cached_func (obj, c); - } - - void cache_enter (OT::hb_ot_apply_context_t *c) const - { - cache_enter_func (obj, c); - } - void cache_leave (OT::hb_ot_apply_context_t *c) const - { - cache_leave_func (obj, c); - } private: const void *obj; hb_apply_func_t apply_func; - hb_apply_func_t apply_cached_func; - hb_cache_enter_func_t cache_enter_func; - hb_cache_leave_func_t cache_leave_func; hb_set_digest_t digest; }; typedef hb_vector_t array_t; - template - auto cache_cost (const T &obj, hb_priority<1>) HB_AUTO_RETURN ( obj.cache_cost () ) - - template - auto cache_cost (const T &obj, hb_priority<0>) HB_AUTO_RETURN ( 0u ) - /* Dispatch interface. */ template return_t dispatch (const T &obj) { - hb_applicable_t entry; - - entry.init (obj, - apply_to, - apply_cached_to, - cache_enter_to, - cache_leave_to); - - array.push (entry); - - // Cache handling - unsigned cost = cache_cost (obj, hb_prioritize); - if (cost > cache_user_cost && !array.in_error ()) - { - cache_user_idx = array.length - 1; - cache_user_cost = cost; - } - + hb_applicable_t *entry = array.push(); + entry->init (obj, apply_to); return hb_empty_t (); } static return_t default_return_value () { return hb_empty_t (); } @@ -926,15 +840,15 @@ struct hb_accelerate_subtables_context_t : array (array_) {} array_t &array; - unsigned cache_user_idx = (unsigned) -1; - unsigned cache_user_cost = 0; }; + + typedef bool (*intersects_func_t) (const hb_set_t *glyphs, const HBUINT16 &value, const void *data); typedef void (*intersected_glyphs_func_t) (const hb_set_t *glyphs, const void *data, unsigned value, hb_set_t *intersected_glyphs); typedef void (*collect_glyphs_func_t) (hb_set_t *glyphs, const HBUINT16 &value, const void *data); -typedef bool (*match_func_t) (hb_glyph_info_t &info, const HBUINT16 &value, const void *data); +typedef bool (*match_func_t) (hb_codepoint_t glyph_id, const HBUINT16 &value, const void *data); struct ContextClosureFuncs { @@ -949,10 +863,6 @@ struct ContextApplyFuncs { match_func_t match; }; -struct ChainContextApplyFuncs -{ - match_func_t match[3]; -}; static inline bool intersects_glyph (const hb_set_t *glyphs, const HBUINT16 &value, const void *data HB_UNUSED) @@ -1029,30 +939,19 @@ static inline void collect_array (hb_collect_glyphs_context_t *c HB_UNUSED, } -static inline bool match_glyph (hb_glyph_info_t &info, const HBUINT16 &value, const void *data HB_UNUSED) +static inline bool match_glyph (hb_codepoint_t glyph_id, const HBUINT16 &value, const void *data HB_UNUSED) { - return info.codepoint == value; + return glyph_id == value; } -static inline bool match_class (hb_glyph_info_t &info, const HBUINT16 &value, const void *data) +static inline bool match_class (hb_codepoint_t glyph_id, const HBUINT16 &value, const void *data) { const ClassDef &class_def = *reinterpret_cast(data); - return class_def.get_class (info.codepoint) == value; + return class_def.get_class (glyph_id) == value; } -static inline bool match_class_cached (hb_glyph_info_t &info, const HBUINT16 &value, const void *data) -{ - unsigned klass = info.syllable(); - if (klass < 255) - return klass == value; - const ClassDef &class_def = *reinterpret_cast(data); - klass = class_def.get_class (info.codepoint); - if (likely (klass < 255)) - info.syllable() = klass; - return klass == value; -} -static inline bool match_coverage (hb_glyph_info_t &info, const HBUINT16 &value, const void *data) +static inline bool match_coverage (hb_codepoint_t glyph_id, const HBUINT16 &value, const void *data) { const Offset16To &coverage = (const Offset16To&)value; - return (data+coverage).get_coverage (info.codepoint) != NOT_COVERED; + return (data+coverage).get_coverage (glyph_id) != NOT_COVERED; } static inline bool would_match_input (hb_would_apply_context_t *c, @@ -1065,12 +964,8 @@ static inline bool would_match_input (hb_would_apply_context_t *c, return false; for (unsigned int i = 1; i < count; i++) - { - hb_glyph_info_t info; - info.codepoint = c->glyphs[i]; - if (likely (!match_func (info, input[i - 1], match_data))) + if (likely (!match_func (c->glyphs[i], input[i - 1], match_data))) return false; - } return true; } @@ -2230,47 +2125,19 @@ struct ContextFormat2 const Coverage &get_coverage () const { return this+coverage; } - unsigned cache_cost () const - { - unsigned c = (this+classDef).cost () * ruleSet.len; - return c >= 4 ? c : 0; - } - void cache_enter (hb_ot_apply_context_t *c) const - { - auto &info = c->buffer->info; - unsigned count = c->buffer->len; - for (unsigned i = 0; i < count; i++) - info[i].syllable() = 255; - c->new_syllables = 255; - } - void cache_leave (hb_ot_apply_context_t *c) const - { - c->new_syllables = (unsigned) -1; - } - bool apply_cached (hb_ot_apply_context_t *c) const { return apply (c, true); } - - bool apply (hb_ot_apply_context_t *c, bool cached = false) const + bool apply (hb_ot_apply_context_t *c) const { TRACE_APPLY (this); unsigned int index = (this+coverage).get_coverage (c->buffer->cur().codepoint); if (likely (index == NOT_COVERED)) return_trace (false); const ClassDef &class_def = this+classDef; - + index = class_def.get_class (c->buffer->cur().codepoint); + const RuleSet &rule_set = this+ruleSet[index]; struct ContextApplyLookupContext lookup_context = { - {cached ? match_class_cached : match_class}, + {match_class}, &class_def }; - - if (cached && c->buffer->cur().syllable() < 255) - index = c->buffer->cur().syllable (); - else - { - index = class_def.get_class (c->buffer->cur().codepoint); - if (cached && index < 255) - c->buffer->cur().syllable() = index; - } - const RuleSet &rule_set = this+ruleSet[index]; return_trace (rule_set.apply (c, lookup_context)); } @@ -2544,7 +2411,7 @@ struct ChainContextCollectGlyphsLookupContext struct ChainContextApplyLookupContext { - ChainContextApplyFuncs funcs; + ContextApplyFuncs funcs; const void *match_data[3]; }; @@ -2632,7 +2499,7 @@ static inline bool chain_context_would_apply_lookup (hb_would_apply_context_t *c return (c->zero_context ? !backtrackCount && !lookaheadCount : true) && would_match_input (c, inputCount, input, - lookup_context.funcs.match[1], lookup_context.match_data[1]); + lookup_context.funcs.match, lookup_context.match_data[1]); } static inline bool chain_context_apply_lookup (hb_ot_apply_context_t *c, @@ -2651,11 +2518,11 @@ static inline bool chain_context_apply_lookup (hb_ot_apply_context_t *c, unsigned match_positions[HB_MAX_CONTEXT_LENGTH]; if (!(match_input (c, inputCount, input, - lookup_context.funcs.match[1], lookup_context.match_data[1], + lookup_context.funcs.match, lookup_context.match_data[1], &match_end, match_positions) && (end_index = match_end) && match_lookahead (c, lookaheadCount, lookahead, - lookup_context.funcs.match[2], lookup_context.match_data[2], + lookup_context.funcs.match, lookup_context.match_data[2], match_end, &end_index))) { c->buffer->unsafe_to_concat (c->buffer->idx, end_index); @@ -2665,7 +2532,7 @@ static inline bool chain_context_apply_lookup (hb_ot_apply_context_t *c, unsigned start_index = c->buffer->out_len; if (!match_backtrack (c, backtrackCount, backtrack, - lookup_context.funcs.match[0], lookup_context.match_data[0], + lookup_context.funcs.match, lookup_context.match_data[0], &start_index)) { c->buffer->unsafe_to_concat_from_outbuffer (start_index, end_index); @@ -3067,7 +2934,7 @@ struct ChainContextFormat1 { const ChainRuleSet &rule_set = this+ruleSet[(this+coverage).get_coverage (c->glyphs[0])]; struct ChainContextApplyLookupContext lookup_context = { - {{match_glyph, match_glyph, match_glyph}}, + {match_glyph}, {nullptr, nullptr, nullptr} }; return rule_set.would_apply (c, lookup_context); @@ -3083,7 +2950,7 @@ struct ChainContextFormat1 const ChainRuleSet &rule_set = this+ruleSet[index]; struct ChainContextApplyLookupContext lookup_context = { - {{match_glyph, match_glyph, match_glyph}}, + {match_glyph}, {nullptr, nullptr, nullptr} }; return_trace (rule_set.apply (c, lookup_context)); @@ -3267,7 +3134,7 @@ struct ChainContextFormat2 unsigned int index = input_class_def.get_class (c->glyphs[0]); const ChainRuleSet &rule_set = this+ruleSet[index]; struct ChainContextApplyLookupContext lookup_context = { - {{match_class, match_class, match_class}}, + {match_class}, {&backtrack_class_def, &input_class_def, &lookahead_class_def} @@ -3277,26 +3144,7 @@ struct ChainContextFormat2 const Coverage &get_coverage () const { return this+coverage; } - unsigned cache_cost () const - { - unsigned c = (this+inputClassDef).cost () * ruleSet.len; - return c >= 4 ? c : 0; - } - void cache_enter (hb_ot_apply_context_t *c) const - { - auto &info = c->buffer->info; - unsigned count = c->buffer->len; - for (unsigned i = 0; i < count; i++) - info[i].syllable() = 255; - c->new_syllables = 255; - } - void cache_leave (hb_ot_apply_context_t *c) const - { - c->new_syllables = (unsigned) -1; - } - bool apply_cached (hb_ot_apply_context_t *c) const { return apply (c, true); } - - bool apply (hb_ot_apply_context_t *c, bool cached = false) const + bool apply (hb_ot_apply_context_t *c) const { TRACE_APPLY (this); unsigned int index = (this+coverage).get_coverage (c->buffer->cur().codepoint); @@ -3306,24 +3154,14 @@ struct ChainContextFormat2 const ClassDef &input_class_def = this+inputClassDef; const ClassDef &lookahead_class_def = this+lookaheadClassDef; + index = input_class_def.get_class (c->buffer->cur().codepoint); + const ChainRuleSet &rule_set = this+ruleSet[index]; struct ChainContextApplyLookupContext lookup_context = { - {{cached && &backtrack_class_def == &input_class_def ? match_class_cached : match_class, - cached ? match_class_cached : match_class, - cached && &lookahead_class_def == &input_class_def ? match_class_cached : match_class}}, + {match_class}, {&backtrack_class_def, &input_class_def, &lookahead_class_def} }; - - if (cached && c->buffer->cur().syllable() < 255) - index = c->buffer->cur().syllable (); - else - { - index = input_class_def.get_class (c->buffer->cur().codepoint); - if (cached && index < 255) - c->buffer->cur().syllable() = index; - } - const ChainRuleSet &rule_set = this+ruleSet[index]; return_trace (rule_set.apply (c, lookup_context)); } @@ -3521,7 +3359,7 @@ struct ChainContextFormat3 const Array16OfOffset16To &lookahead = StructAfter> (input); const Array16Of &lookup = StructAfter> (lookahead); struct ChainContextApplyLookupContext lookup_context = { - {{match_coverage, match_coverage, match_coverage}}, + {match_coverage}, {this, this, this} }; return chain_context_would_apply_lookup (c, @@ -3548,7 +3386,7 @@ struct ChainContextFormat3 const Array16OfOffset16To &lookahead = StructAfter> (input); const Array16Of &lookup = StructAfter> (lookahead); struct ChainContextApplyLookupContext lookup_context = { - {{match_coverage, match_coverage, match_coverage}}, + {match_coverage}, {this, this, this} }; return_trace (chain_context_apply_lookup (c, @@ -3787,54 +3625,23 @@ struct hb_ot_layout_lookup_accelerator_t subtables.init (); OT::hb_accelerate_subtables_context_t c_accelerate_subtables (subtables); lookup.dispatch (&c_accelerate_subtables); - cache_user_idx = c_accelerate_subtables.cache_user_idx; - for (unsigned i = 0; i < subtables.length; i++) - if (i != cache_user_idx) - subtables[i].apply_cached_func = subtables[i].apply_func; } void fini () { subtables.fini (); } bool may_have (hb_codepoint_t g) const { return digest.may_have (g); } - bool apply (hb_ot_apply_context_t *c, bool use_cache) const + bool apply (hb_ot_apply_context_t *c) const { - if (use_cache) - { - for (unsigned int i = 0; i < subtables.length; i++) - if (subtables[i].apply_cached (c)) - return true; - } - else - { - for (unsigned int i = 0; i < subtables.length; i++) - if (subtables[i].apply (c)) - return true; - } + for (unsigned int i = 0; i < subtables.length; i++) + if (subtables[i].apply (c)) + return true; return false; } - bool cache_enter (OT::hb_ot_apply_context_t *c) const - { - if (cache_user_idx == (unsigned) -1 || - !HB_BUFFER_TRY_ALLOCATE_VAR (c->buffer, syllable)) - return false; - - subtables[cache_user_idx].cache_enter (c); - - return true; - } - void cache_leave (OT::hb_ot_apply_context_t *c) const - { - subtables[cache_user_idx].cache_leave (c); - HB_BUFFER_DEALLOCATE_VAR (c->buffer, syllable); - } - - private: hb_set_digest_t digest; hb_accelerate_subtables_context_t::array_t subtables; - unsigned cache_user_idx = (unsigned) -1; }; struct GSUBGPOS diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index 183bbc94c..35e887ba3 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -1827,8 +1827,6 @@ static inline bool apply_forward (OT::hb_ot_apply_context_t *c, const OT::hb_ot_layout_lookup_accelerator_t &accel) { - bool use_cache = accel.cache_enter (c); - bool ret = false; hb_buffer_t *buffer = c->buffer; while (buffer->idx < buffer->len && buffer->successful) @@ -1838,7 +1836,7 @@ apply_forward (OT::hb_ot_apply_context_t *c, (buffer->cur().mask & c->lookup_mask) && c->check_glyph_property (&buffer->cur(), c->lookup_props)) { - applied = accel.apply (c, use_cache); + applied = accel.apply (c); } if (applied) @@ -1846,10 +1844,6 @@ apply_forward (OT::hb_ot_apply_context_t *c, else (void) buffer->next_glyph (); } - - if (use_cache) - accel.cache_leave (c); - return ret; } @@ -1864,7 +1858,7 @@ apply_backward (OT::hb_ot_apply_context_t *c, if (accel.may_have (buffer->cur().codepoint) && (buffer->cur().mask & c->lookup_mask) && c->check_glyph_property (&buffer->cur(), c->lookup_props)) - ret |= accel.apply (c, false); + ret |= accel.apply (c); /* The reverse lookup doesn't "advance" cursor (for good reason). */ buffer->idx--;