From efc0b830cf5a4430adfa48be9a874086f974a427 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 28 Sep 2018 15:34:14 +0200 Subject: [PATCH 1/8] Handle multiple spaces after PUA glyphs --- kitty/fonts.c | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/kitty/fonts.c b/kitty/fonts.c index 80323759c..2806ce374 100644 --- a/kitty/fonts.c +++ b/kitty/fonts.c @@ -901,12 +901,12 @@ shape_run(CPUCell *first_cpu_cell, GPUCell *first_gpu_cell, index_type num_cells static inline void merge_groups_for_pua_space_ligature() { - if (G(group_idx) == 1) { + while (G(group_idx) > 0) { Group *g = G(groups), *g1 = G(groups) + 1; g->num_cells += g1->num_cells; g->num_glyphs += g1->num_glyphs; g->num_glyphs = MIN(g->num_glyphs, MAX_NUM_EXTRA_GLYPHS + 1); - G(group_idx) = 0; + G(group_idx)--; } } @@ -1006,21 +1006,33 @@ render_line(FONTS_DATA_HANDLE fg_, Line *line) { CPUCell *cpu_cell = line->cpu_cells + i; GPUCell *gpu_cell = line->gpu_cells + i; ssize_t cell_font_idx = font_for_cell(fg, cpu_cell, gpu_cell); - if (is_private_use(cpu_cell->ch) && i + 1 < line->xnum && (line->cpu_cells[i+1].ch == ' ' || line->cpu_cells[i+1].ch == 0) && cell_font_idx != BOX_FONT && cell_font_idx != MISSING_FONT) { - // We have a private use char followed by a space char, render it as a two cell ligature. - GPUCell *space_cell = line->gpu_cells + i+1; - // Ensure the space cell uses the foreground colors from the PUA cell - // This is needed because there are stupid applications like - // powerline that use PUA+space with different foreground colors - // for the space and the PUA. See for example: https://github.com/kovidgoyal/kitty/issues/467 - space_cell->fg = gpu_cell->fg; space_cell->decoration_fg = gpu_cell->decoration_fg; - RENDER; - render_run(fg, line->cpu_cells + i, line->gpu_cells + i, 2, cell_font_idx, true); - run_font_idx = NO_FONT; - first_cell_in_run = i + 2; - prev_width = line->gpu_cells[i+1].attrs & WIDTH_MASK; - i++; - continue; + if (is_private_use(cpu_cell->ch) + && cell_font_idx != BOX_FONT + && cell_font_idx != MISSING_FONT) { + int j = 0; + while ((line->cpu_cells[i+1+j].ch == ' ' || line->cpu_cells[i+1+j].ch == 0) + && j < MAX_NUM_EXTRA_GLYPHS + && i + 1 + j < line->xnum) { + j++; + // We have a private use char followed by space(s), render it as a multi-cell ligature. + GPUCell *space_cell = line->gpu_cells + i+j; + // Ensure the space cell uses the foreground/background colors from the PUA cell. + // This is needed because there are stupid applications like + // powerline that use PUA+space with different foreground colors + // for the space and the PUA. See for example: https://github.com/kovidgoyal/kitty/issues/467 + space_cell->fg = gpu_cell->fg; + space_cell->bg = gpu_cell->bg; + space_cell->decoration_fg = gpu_cell->decoration_fg; + } + if (j) { + RENDER; + render_run(fg, line->cpu_cells + i, line->gpu_cells + i, j + 1, cell_font_idx, true); + run_font_idx = NO_FONT; + first_cell_in_run = i + 1 + j; + prev_width = gpu_cell->attrs & WIDTH_MASK; + i += j; + continue; + } } prev_width = gpu_cell->attrs & WIDTH_MASK; if (run_font_idx == NO_FONT) run_font_idx = cell_font_idx; From d52434c0aa8a1e2493dcfb14f14acdcb491746e2 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 28 Sep 2018 23:01:28 +0200 Subject: [PATCH 2/8] fixup! Handle multiple spaces after PUA glyphs [ci skip] --- kitty/fonts.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kitty/fonts.c b/kitty/fonts.c index 2806ce374..7c0f6ec68 100644 --- a/kitty/fonts.c +++ b/kitty/fonts.c @@ -1010,12 +1010,12 @@ render_line(FONTS_DATA_HANDLE fg_, Line *line) { && cell_font_idx != BOX_FONT && cell_font_idx != MISSING_FONT) { int j = 0; - while ((line->cpu_cells[i+1+j].ch == ' ' || line->cpu_cells[i+1+j].ch == 0) + while ((line->cpu_cells[i+j+1].ch == ' ' || line->cpu_cells[i+j+1].ch == 0) && j < MAX_NUM_EXTRA_GLYPHS - && i + 1 + j < line->xnum) { + && i + j + 1 < line->xnum) { j++; // We have a private use char followed by space(s), render it as a multi-cell ligature. - GPUCell *space_cell = line->gpu_cells + i+j; + GPUCell *space_cell = line->gpu_cells + i + j; // Ensure the space cell uses the foreground/background colors from the PUA cell. // This is needed because there are stupid applications like // powerline that use PUA+space with different foreground colors @@ -1028,8 +1028,8 @@ render_line(FONTS_DATA_HANDLE fg_, Line *line) { RENDER; render_run(fg, line->cpu_cells + i, line->gpu_cells + i, j + 1, cell_font_idx, true); run_font_idx = NO_FONT; - first_cell_in_run = i + 1 + j; - prev_width = gpu_cell->attrs & WIDTH_MASK; + first_cell_in_run = i + j + 1; + prev_width = line->gpu_cells[i+1].attrs & WIDTH_MASK; i += j; continue; } From 1d50a188c4f40f4770d18587b3040f5cdf8128b4 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sat, 29 Sep 2018 16:19:53 +0200 Subject: [PATCH 3/8] render_glyphs_in_cells: skip centering glyph for PUA --- kitty/fonts.c | 13 +++++++------ kitty/fonts.h | 2 +- kitty/freetype.c | 15 ++++++++------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/kitty/fonts.c b/kitty/fonts.c index 7c0f6ec68..772793a38 100644 --- a/kitty/fonts.c +++ b/kitty/fonts.c @@ -635,6 +635,11 @@ extract_cell_from_canvas(FontGroup *fg, unsigned int i, unsigned int num_cells) return ans; } +static inline bool +is_private_use(char_type ch) { + return (0xe000 <= ch && ch <= 0xf8ff) || (0xF0000 <= ch && ch <= 0xFFFFF) || (0x100000 <= ch && ch <= 0x10FFFF); +} + static inline void render_group(FontGroup *fg, unsigned int num_cells, unsigned int num_glyphs, CPUCell *cpu_cells, GPUCell *gpu_cells, hb_glyph_info_t *info, hb_glyph_position_t *positions, Font *font, glyph_index glyph, ExtraGlyphs *extra_glyphs) { static SpritePosition* sprite_position[16]; @@ -651,7 +656,8 @@ render_group(FontGroup *fg, unsigned int num_cells, unsigned int num_glyphs, CPU clear_canvas(fg); bool was_colored = (gpu_cells->attrs & WIDTH_MASK) == 2 && is_emoji(cpu_cells->ch); - render_glyphs_in_cells(font->face, font->bold, font->italic, info, positions, num_glyphs, fg->canvas, fg->cell_width, fg->cell_height, num_cells, fg->baseline, &was_colored, (FONTS_DATA_HANDLE)fg); + bool center_glyph = !is_private_use(cpu_cells->ch); + render_glyphs_in_cells(font->face, font->bold, font->italic, info, positions, num_glyphs, fg->canvas, fg->cell_width, fg->cell_height, num_cells, fg->baseline, &was_colored, (FONTS_DATA_HANDLE)fg, center_glyph); if (PyErr_Occurred()) PyErr_Print(); for (unsigned int i = 0; i < num_cells; i++) { @@ -989,11 +995,6 @@ render_run(FontGroup *fg, CPUCell *first_cpu_cell, GPUCell *first_gpu_cell, inde } } -static inline bool -is_private_use(char_type ch) { - return (0xe000 <= ch && ch <= 0xf8ff) || (0xF0000 <= ch && ch <= 0xFFFFF) || (0x100000 <= ch && ch <= 0x10FFFF); -} - void render_line(FONTS_DATA_HANDLE fg_, Line *line) { #define RENDER if (run_font_idx != NO_FONT && i > first_cell_in_run) render_run(fg, line->cpu_cells + first_cell_in_run, line->gpu_cells + first_cell_in_run, i - first_cell_in_run, run_font_idx, false); diff --git a/kitty/fonts.h b/kitty/fonts.h index 0e5a7aacb..1a05f1b0c 100644 --- a/kitty/fonts.h +++ b/kitty/fonts.h @@ -21,7 +21,7 @@ bool is_glyph_empty(PyObject *, glyph_index); hb_font_t* harfbuzz_font_for_face(PyObject*); bool set_size_for_face(PyObject*, unsigned int, bool, FONTS_DATA_HANDLE); void cell_metrics(PyObject*, unsigned int*, unsigned int*, unsigned int*, unsigned int*, unsigned int*); -bool render_glyphs_in_cells(PyObject *f, bool bold, bool italic, hb_glyph_info_t *info, hb_glyph_position_t *positions, unsigned int num_glyphs, pixel *canvas, unsigned int cell_width, unsigned int cell_height, unsigned int num_cells, unsigned int baseline, bool *was_colored, FONTS_DATA_HANDLE); +bool render_glyphs_in_cells(PyObject *f, bool bold, bool italic, hb_glyph_info_t *info, hb_glyph_position_t *positions, unsigned int num_glyphs, pixel *canvas, unsigned int cell_width, unsigned int cell_height, unsigned int num_cells, unsigned int baseline, bool *was_colored, FONTS_DATA_HANDLE, bool center_glyph); PyObject* create_fallback_face(PyObject *base_face, CPUCell* cell, bool bold, bool italic, bool emoji_presentation, FONTS_DATA_HANDLE fg); PyObject* specialize_font_descriptor(PyObject *base_descriptor, FONTS_DATA_HANDLE); PyObject* face_from_path(const char *path, int index, FONTS_DATA_HANDLE); diff --git a/kitty/freetype.c b/kitty/freetype.c index a0aca4a13..42229aa2f 100644 --- a/kitty/freetype.c +++ b/kitty/freetype.c @@ -483,7 +483,7 @@ place_bitmap_in_canvas(pixel *cell, ProcessedBitmap *bm, size_t cell_width, size static const ProcessedBitmap EMPTY_PBM = {.factor = 1}; bool -render_glyphs_in_cells(PyObject *f, bool bold, bool italic, hb_glyph_info_t *info, hb_glyph_position_t *positions, unsigned int num_glyphs, pixel *canvas, unsigned int cell_width, unsigned int cell_height, unsigned int num_cells, unsigned int baseline, bool *was_colored, FONTS_DATA_HANDLE fg) { +render_glyphs_in_cells(PyObject *f, bool bold, bool italic, hb_glyph_info_t *info, hb_glyph_position_t *positions, unsigned int num_glyphs, pixel *canvas, unsigned int cell_width, unsigned int cell_height, unsigned int num_cells, unsigned int baseline, bool *was_colored, FONTS_DATA_HANDLE fg, bool center_glyph) { Face *self = (Face*)f; bool is_emoji = *was_colored; *was_colored = is_emoji && self->has_color; float x = 0.f, y = 0.f, x_offset = 0.f; @@ -507,12 +507,13 @@ render_glyphs_in_cells(PyObject *f, bool bold, bool italic, hb_glyph_info_t *inf if (bm.needs_free) free(bm.buf); } - // center the glyphs in the canvas - unsigned int right_edge = (unsigned int)x, delta; - // x_advance is wrong for colored bitmaps that have been downsampled - if (*was_colored) right_edge = num_glyphs == 1 ? bm.right_edge : canvas_width; - if (num_cells > 1 && right_edge < canvas_width && (delta = (canvas_width - right_edge) / 2) && delta > 1) { - right_shift_canvas(canvas, canvas_width, cell_height, delta); + if (center_glyph) { + unsigned int right_edge = (unsigned int)x, delta; + // x_advance is wrong for colored bitmaps that have been downsampled + if (*was_colored) right_edge = num_glyphs == 1 ? bm.right_edge : canvas_width; + if (num_cells > 1 && right_edge < canvas_width && (delta = (canvas_width - right_edge) / 2) && delta > 1) { + right_shift_canvas(canvas, canvas_width, cell_height, delta); + } } return true; } From fec5a80b895750608370052f27b7fd2f4e9a327e Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sat, 29 Sep 2018 16:20:08 +0200 Subject: [PATCH 4/8] Init bm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not sure why this became necessary though?! Fixes: Linking kitty/fast_data_types ... kitty/freetype.c: In function ‘render_glyphs_in_cells’: kitty/freetype.c:514:82: warning: ‘bm.right_edge’ may be used uninitialized in this function [-Wmaybe-uninitialized] if (num_cells > 1 && right_edge < canvas_width && (delta = (canvas_width - right_edge) / 2) && delta > 1) { ^ kitty/freetype.c:490:21: note: ‘bm.right_edge’ was declared here ProcessedBitmap bm; ^ --- kitty/freetype.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kitty/freetype.c b/kitty/freetype.c index 42229aa2f..e844d062f 100644 --- a/kitty/freetype.c +++ b/kitty/freetype.c @@ -487,7 +487,7 @@ render_glyphs_in_cells(PyObject *f, bool bold, bool italic, hb_glyph_info_t *inf Face *self = (Face*)f; bool is_emoji = *was_colored; *was_colored = is_emoji && self->has_color; float x = 0.f, y = 0.f, x_offset = 0.f; - ProcessedBitmap bm; + ProcessedBitmap bm = EMPTY_PBM; unsigned int canvas_width = cell_width * num_cells; for (unsigned int i = 0; i < num_glyphs; i++) { bm = EMPTY_PBM; From 8e09e809aa47fce2e32b0ea5affdb4d3eeac9f6c Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sat, 29 Sep 2018 16:23:16 +0200 Subject: [PATCH 5/8] fixup! render_glyphs_in_cells: skip centering glyph for PUA --- kitty/core_text.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kitty/core_text.m b/kitty/core_text.m index e13536726..28079309f 100644 --- a/kitty/core_text.m +++ b/kitty/core_text.m @@ -383,7 +383,7 @@ render_glyphs(CTFontRef font, unsigned int width, unsigned int height, unsigned } static inline bool -do_render(CTFontRef ct_font, bool bold, bool italic, hb_glyph_info_t *info, hb_glyph_position_t *hb_positions, unsigned int num_glyphs, pixel *canvas, unsigned int cell_width, unsigned int cell_height, unsigned int num_cells, unsigned int baseline, bool *was_colored, bool allow_resize, FONTS_DATA_HANDLE fg) { +do_render(CTFontRef ct_font, bool bold, bool italic, hb_glyph_info_t *info, hb_glyph_position_t *hb_positions, unsigned int num_glyphs, pixel *canvas, unsigned int cell_width, unsigned int cell_height, unsigned int num_cells, unsigned int baseline, bool *was_colored, bool allow_resize, FONTS_DATA_HANDLE fg, bool center_glyph) { unsigned int canvas_width = cell_width * num_cells; CGRect br = CTFontGetBoundingRectsForGlyphs(ct_font, kCTFontOrientationHorizontal, glyphs, boxes, num_glyphs); if (allow_resize) { @@ -394,7 +394,7 @@ do_render(CTFontRef ct_font, bool bold, bool italic, hb_glyph_info_t *info, hb_g CGFloat sz = CTFontGetSize(ct_font); sz *= canvas_width / right; CTFontRef new_font = CTFontCreateCopyWithAttributes(ct_font, sz, NULL, NULL); - bool ret = do_render(new_font, bold, italic, info, hb_positions, num_glyphs, canvas, cell_width, cell_height, num_cells, baseline, was_colored, false, fg); + bool ret = do_render(new_font, bold, italic, info, hb_positions, num_glyphs, canvas, cell_width, cell_height, num_cells, baseline, was_colored, false, fg, center_glyph); CFRelease(new_font); return ret; } @@ -420,10 +420,10 @@ do_render(CTFontRef ct_font, bool bold, bool italic, hb_glyph_info_t *info, hb_g } bool -render_glyphs_in_cells(PyObject *s, bool bold, bool italic, hb_glyph_info_t *info, hb_glyph_position_t *hb_positions, unsigned int num_glyphs, pixel *canvas, unsigned int cell_width, unsigned int cell_height, unsigned int num_cells, unsigned int baseline, bool *was_colored, FONTS_DATA_HANDLE fg) { +render_glyphs_in_cells(PyObject *s, bool bold, bool italic, hb_glyph_info_t *info, hb_glyph_position_t *hb_positions, unsigned int num_glyphs, pixel *canvas, unsigned int cell_width, unsigned int cell_height, unsigned int num_cells, unsigned int baseline, bool *was_colored, FONTS_DATA_HANDLE fg, bool center_glyph) { CTFace *self = (CTFace*)s; for (unsigned i=0; i < num_glyphs; i++) glyphs[i] = info[i].codepoint; - return do_render(self->ct_font, bold, italic, info, hb_positions, num_glyphs, canvas, cell_width, cell_height, num_cells, baseline, was_colored, true, fg); + return do_render(self->ct_font, bold, italic, info, hb_positions, num_glyphs, canvas, cell_width, cell_height, num_cells, baseline, was_colored, true, fg, center_glyph); } From c64e8fc4343ab7ecfefa8b6ab9f6bce4faf22db6 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sat, 29 Sep 2018 16:49:40 +0200 Subject: [PATCH 6/8] MAX_NUM_EXTRA_GLYPHS_PUA=4 --- kitty/fonts.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kitty/fonts.c b/kitty/fonts.c index 772793a38..2e964e178 100644 --- a/kitty/fonts.c +++ b/kitty/fonts.c @@ -14,6 +14,7 @@ #define MISSING_GLYPH 4 #define MAX_NUM_EXTRA_GLYPHS 8 #define CELLS_IN_CANVAS ((MAX_NUM_EXTRA_GLYPHS + 1) * 3) +#define MAX_NUM_EXTRA_GLYPHS_PUA 4 typedef void (*send_sprite_to_gpu_func)(FONTS_DATA_HANDLE fg, unsigned int, unsigned int, unsigned int, pixel*); send_sprite_to_gpu_func current_send_sprite_to_gpu = NULL; @@ -1012,7 +1013,7 @@ render_line(FONTS_DATA_HANDLE fg_, Line *line) { && cell_font_idx != MISSING_FONT) { int j = 0; while ((line->cpu_cells[i+j+1].ch == ' ' || line->cpu_cells[i+j+1].ch == 0) - && j < MAX_NUM_EXTRA_GLYPHS + && j < MAX_NUM_EXTRA_GLYPHS_PUA && i + j + 1 < line->xnum) { j++; // We have a private use char followed by space(s), render it as a multi-cell ligature. From 12cfe6d1d1546444c1d99025d9a36872f665b510 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sat, 29 Sep 2018 18:38:53 +0200 Subject: [PATCH 7/8] render_line: use cells based on bitmap width [ci skip] --- kitty/fonts.c | 13 +++++++++++++ kitty/fonts.h | 1 + kitty/freetype.c | 14 ++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/kitty/fonts.c b/kitty/fonts.c index 2e964e178..8cf99685c 100644 --- a/kitty/fonts.c +++ b/kitty/fonts.c @@ -1008,12 +1008,25 @@ render_line(FONTS_DATA_HANDLE fg_, Line *line) { CPUCell *cpu_cell = line->cpu_cells + i; GPUCell *gpu_cell = line->gpu_cells + i; ssize_t cell_font_idx = font_for_cell(fg, cpu_cell, gpu_cell); + if (is_private_use(cpu_cell->ch) && cell_font_idx != BOX_FONT && cell_font_idx != MISSING_FONT) { + int cells; + if (cell_font_idx > 0) { + Font *font = (fg->fonts + cell_font_idx); + glyph_index glyph_id = glyph_id_for_codepoint(font->face, cpu_cell->ch); + + int width = get_glyph_width(font->face, glyph_id); + cells = ceilf((float)width / fg->cell_width); + } else { + cells = 1; + } + int j = 0; while ((line->cpu_cells[i+j+1].ch == ' ' || line->cpu_cells[i+j+1].ch == 0) && j < MAX_NUM_EXTRA_GLYPHS_PUA + && j < cells && i + j + 1 < line->xnum) { j++; // We have a private use char followed by space(s), render it as a multi-cell ligature. diff --git a/kitty/fonts.h b/kitty/fonts.h index 1a05f1b0c..1ddc6c5ff 100644 --- a/kitty/fonts.h +++ b/kitty/fonts.h @@ -17,6 +17,7 @@ // API that font backends need to implement typedef uint16_t glyph_index; unsigned int glyph_id_for_codepoint(PyObject *, char_type); +int get_glyph_width(PyObject *, glyph_index); bool is_glyph_empty(PyObject *, glyph_index); hb_font_t* harfbuzz_font_for_face(PyObject*); bool set_size_for_face(PyObject*, unsigned int, bool, FONTS_DATA_HANDLE); diff --git a/kitty/freetype.c b/kitty/freetype.c index e844d062f..2937b9e40 100644 --- a/kitty/freetype.c +++ b/kitty/freetype.c @@ -294,6 +294,20 @@ is_glyph_empty(PyObject *s, glyph_index g) { #undef M } +int +get_glyph_width(PyObject *s, glyph_index g) { + Face *self = (Face*)s; + if (!load_glyph(self, g, FT_LOAD_DEFAULT)) { PyErr_Print(); return false; } + FT_Bitmap *bitmap = &self->face->glyph->bitmap; +#define M self->face->glyph->metrics +#define B self->face->glyph->bitmap + /* printf("glyph: %u bitmap.width: %d bitmap.rows: %d horiAdvance: %ld horiBearingX: %ld horiBearingY: %ld vertBearingX: %ld vertBearingY: %ld vertAdvance: %ld width: %ld height: %ld\n", */ + /* g, B.width, B.rows, M.horiAdvance, M.horiBearingX, M.horiBearingY, M.vertBearingX, M.vertBearingY, M.vertAdvance, M.width, M.height); */ + return bitmap->width; +#undef M +#undef B +} + hb_font_t* harfbuzz_font_for_face(PyObject *self) { return ((Face*)self)->harfbuzz_font; } From 3c4cb3c35b321e6f556c68c575d9be728367edd0 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sat, 29 Sep 2018 18:56:54 +0200 Subject: [PATCH 8/8] get_glyph_width for core text?! --- kitty/core_text.m | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/kitty/core_text.m b/kitty/core_text.m index 28079309f..560f6f83b 100644 --- a/kitty/core_text.m +++ b/kitty/core_text.m @@ -221,6 +221,15 @@ is_glyph_empty(PyObject *s, glyph_index g) { return bounds.size.width <= 0; } +int +get_glyph_width(PyObject *s, glyph_index g) { + CTFace *self = (CTFace*)s; + CGGlyph gg = g; + CGRect bounds; + CTFontGetBoundingRectsForGlyphs(self->ct_font, kCTFontOrientationHorizontal, &gg, &bounds, 1); + return bounds.size.width; +} + static inline float scaled_point_sz(FONTS_DATA_HANDLE fg) { return ((fg->logical_dpi_x + fg->logical_dpi_y) / 144.0) * fg->font_sz_in_pts;