diff --git a/kitty/fonts.c b/kitty/fonts.c index 4e7b59dbe..4b30172c6 100644 --- a/kitty/fonts.c +++ b/kitty/fonts.c @@ -1475,8 +1475,9 @@ ensure_group_capacity(size_t num_cells, size_t num_groups) { static bool restore_shaped_run(CPUCell *first_cpu_cell, GPUCell *first_gpu_cell, index_type num_cells, const ShapedRun *cached) { if (!ensure_group_capacity(num_cells, cached->num_groups)) return false; - zero_at_ptr_count(group_state.groups, group_state.groups_capacity); + // every group up to group_idx is overwritten below and nothing reads past it, so zeroing the whole array is unnecessary if (cached->num_groups) memcpy(group_state.groups, cached->groups, cached->num_groups * sizeof(Group)); + else zero_at_ptr(group_state.groups); if (cached->num_glyphs) { if (!ensure_group_glyph_storage(cached->num_glyphs)) return false; memcpy(group_state.info_storage, cached->info, cached->num_glyphs * sizeof(cached->info[0])); @@ -1899,20 +1900,7 @@ shape_run( hb_buffer_serialize_glyphs(harfbuzz_buffer, 0, group_state.num_glyphs, dbuf, sizeof(dbuf), NULL, harfbuzz_font_for_face(font->face), HB_BUFFER_SERIALIZE_FORMAT_TEXT, HB_BUFFER_SERIALIZE_FLAG_DEFAULT | HB_BUFFER_SERIALIZE_FLAG_GLYPH_EXTENTS); printf("\n%s\n", dbuf); #endif - shaped_run_put( - font->shaped_run_hash_table, - first_cpu_cell, - num_cells, - tc, - disable_ligature, - OPT(force_ltr), - key_scale, - subscale, - G(info), - G(positions), - (unsigned)G(num_glyphs), - G(groups), - (unsigned)(G(group_idx) + 1)); + shaped_run_put(font->shaped_run_hash_table, G(info), G(positions), (unsigned)G(num_glyphs), G(groups), (unsigned)(G(group_idx) + 1)); if (scale != 1.f) { apply_scale_to_font_group(fg, NULL); if (!face_apply_scaling(font->face, (FONTS_DATA_HANDLE)fg) && PyErr_Occurred()) PyErr_Print(); diff --git a/kitty/shaped-run-cache.c b/kitty/shaped-run-cache.c index 36af87ef4..980700eb5 100644 --- a/kitty/shaped-run-cache.c +++ b/kitty/shaped-run-cache.c @@ -7,6 +7,9 @@ #include "shaped-run-cache.h" +// Note that these limits apply to every Font, of which there are at least a +// dozen per FontGroup once fallback and symbol fonts are in use, so the total +// memory used by shaped run caches is a multiple of SHAPED_RUN_MAX_BYTES. #define SHAPED_RUN_MAX_CELLS 32u #define SHAPED_RUN_MAX_ENTRIES 2048u #define SHAPED_RUN_MAX_BYTES (1024u * 1024u) @@ -78,6 +81,7 @@ typedef struct HashTable { struct { ShapedRunKey *key; size_t capacity; + bool key_is_valid; // true when key describes the run from the last shaped_run_get() } scratch; } HashTable; @@ -161,11 +165,14 @@ shaped_run_get( ShapedRun *result) { HashTable *ht = (HashTable *)map; if (!ht) return false; + ht->scratch.key_is_valid = false; if (shaped_run_too_long(num_cells)) return false; if (!fill_key(ht, cells, num_cells, tc, disable_ligature, force_ltr, scale, subscale)) return false; -#define scratch ht->scratch.key - shaped_run_map_itr n = vt_get(&ht->table, scratch); - if (vt_is_end(n)) return false; + shaped_run_map_itr n = vt_get(&ht->table, ht->scratch.key); + if (vt_is_end(n)) { + ht->scratch.key_is_valid = true; // so that shaped_run_put() can re-use it + return false; + } ShapedRunValue *v = n.data->val; result->num_glyphs = v->num_glyphs; result->num_groups = v->num_groups; @@ -173,7 +180,6 @@ shaped_run_get( result->positions = v->num_glyphs ? val_positions(v) : NULL; result->groups = v->num_groups ? val_groups(v) : NULL; return true; -#undef scratch } static size_t @@ -215,31 +221,28 @@ shaped_run_drop(HashTable *ht) { void shaped_run_put( SHAPED_RUN_MAP_HANDLE map, - const CPUCell *cells, - index_type num_cells, - const TextCache *tc, - bool disable_ligature, - bool force_ltr, - uint8_t scale, - uint8_t subscale, const hb_glyph_info_t *info, const hb_glyph_position_t *positions, unsigned num_glyphs, const ShapeGroup *groups, unsigned num_groups) { HashTable *ht = (HashTable *)map; - if (!ht) return; - if (shaped_run_too_long(num_cells)) return; + // the key for this run was built by the shaped_run_get() call that missed, + // without it we have no way of knowing what run these glyphs belong to + if (!ht || !ht->scratch.key_is_valid) return; + ht->scratch.key_is_valid = false; if ((num_glyphs && (!info || !positions)) || (num_groups && !groups)) return; - if (!fill_key(ht, cells, num_cells, tc, disable_ligature, force_ltr, scale, subscale)) return; #define scratch ht->scratch.key - if (!vt_is_end(vt_get(&ht->table, scratch))) return; if (shaped_run_at_cap(ht)) shaped_run_drop(ht); ShapedRunKey *key = Key_get(&ht->keys, sizeof(ShapedRunKey) + scratch->keysz_in_bytes); - if (!key) return; + ShapedRunValue *val = key ? Val_get(&ht->vals, val_size(num_glyphs, num_groups)) : NULL; + // the arenas cannot free individual allocations, so on failure drop the + // whole cache rather than leaking what was allocated for this entry + if (!val) { + shaped_run_drop(ht); + return; + } memcpy(key, scratch, sizeof(scratch[0]) + scratch->keysz_in_bytes); - ShapedRunValue *val = Val_get(&ht->vals, val_size(num_glyphs, num_groups)); - if (!val) return; val->num_glyphs = num_glyphs; val->num_groups = num_groups; if (num_glyphs) { @@ -247,6 +250,6 @@ shaped_run_put( memcpy(val_positions(val), positions, num_glyphs * sizeof(positions[0])); } if (num_groups) memcpy(val_groups(val), groups, num_groups * sizeof(groups[0])); - if (vt_is_end(vt_insert(&ht->table, key, val))) return; + if (vt_is_end(vt_insert(&ht->table, key, val))) shaped_run_drop(ht); #undef scratch } diff --git a/kitty/shaped-run-cache.h b/kitty/shaped-run-cache.h index 9087c6710..5192079a4 100644 --- a/kitty/shaped-run-cache.h +++ b/kitty/shaped-run-cache.h @@ -41,15 +41,13 @@ bool shaped_run_get( uint8_t scale, uint8_t subscale, ShapedRun *result); +// Store the results of shaping the run that the immediately preceding +// shaped_run_get() call looked up and did not find. Building the key is not +// cheap, so it is re-used rather than computed a second time. Does nothing if +// there was no such call, i.e. if the run was found in the cache or is not +// cacheable at all. void shaped_run_put( SHAPED_RUN_MAP_HANDLE map, - const CPUCell *cells, - index_type num_cells, - const TextCache *tc, - bool disable_ligature, - bool force_ltr, - uint8_t scale, - uint8_t subscale, const hb_glyph_info_t *info, const hb_glyph_position_t *positions, unsigned num_glyphs,