diff --git a/docs/changelog.rst b/docs/changelog.rst index 91ad4d0fd..dcc4974d2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -204,6 +204,8 @@ Detailed list of changes - Thai/Lao: Render the vowel sign AM (U+0E33/U+0EB3) correctly by widening the cell it combines into, matching wcwidth() based programs +- :doc:`Text sizing protocol `: A cell whose width was set explicitly with the ``w`` key is no longer narrowed by a subsequent ``U+FE0E`` variation selector + - Support for :doc:`/custom-shaders` for adding various graphical effects (:iss:`10344`) - Various throughput performance improvements for a 15-35% real world improvement depending on workload diff --git a/docs/text-sizing-protocol.rst b/docs/text-sizing-protocol.rst index 83f9ef5a1..4a78be636 100644 --- a/docs/text-sizing-protocol.rst +++ b/docs/text-sizing-protocol.rst @@ -414,8 +414,10 @@ For each decoded code point: cell and processing of the code point is finished. See the :ref:`var_select` section below for handling of Unicode Variation selectors. If the code point has ``Grapheme_Cluster_Break=SpacingMark`` and a non-zero width and the previous - cell is one cell wide, the previous cell becomes two cells wide. This affects - only :code:`U+0E33 THAI CHARACTER SARA AM` and :code:`U+0EB3 LAO VOWEL SIGN AM`. + cell is a single, unscaled cell whose width was not set explicitly using the + ``w`` key of this protocol, the previous cell becomes two cells wide. This + affects only :code:`U+0E33 THAI CHARACTER SARA AM` and + :code:`U+0EB3 LAO VOWEL SIGN AM`. #. If there is a boundary, but the width of the current code point is zero, it is added to the previous cell and processing is finished. @@ -469,17 +471,23 @@ by changing their presentation between ``Emoji_Presentation`` and ``Text_Present When adding a code point to the previous cell these have to be handled specially. ``U+FE0E`` - Variation Selector 15 - When the previous cell has width two and the last code point in the previous + When the previous cell is two cells wide, its width was not set explicitly using + the ``w`` key of this protocol, and the last code point in the previous cell is one of the ``Basic_Emoji`` code points from the *Wide emoji* rule above that is *not* followed by ``FEOF`` then the width of the previous cell is decreased to one. ``U+FE0F`` - Variation Selector 16 - When the previous cell has width one and the last code point in the previous - cell is one of the ``Basic_Emoji`` code points from the *Wide emoji* rule above - that is followed by ``FEOF`` then the width of the + When the previous cell is a single, unscaled cell whose width was not set + explicitly using the ``w`` key of this protocol, and the last code point in the + previous cell is one of the ``Basic_Emoji`` code points from the *Wide emoji* + rule above that is followed by ``FEOF`` then the width of the previous cell is increased to two. +In all three of these cases the width the application requested with the ``w`` key +wins: a cell whose width was set explicitly is never re-sized by a variation +selector or a spacing mark. + Note that the rule for ``U+FE0E`` is particularly problematic for terminals as it means that the width of a string cannot be determined without knowing the width of the screen it will be rendered on. This is because when there is only diff --git a/gen/wcwidth.py b/gen/wcwidth.py index 4bd6c57a4..9e61ab5a0 100755 --- a/gen/wcwidth.py +++ b/gen/wcwidth.py @@ -1182,6 +1182,18 @@ def gen_char_props() -> None: ) for x in prop_array ) + # Both kitty's wcswidth() and its text drawing code widen the cell a SpacingMark + # with non-zero width combines into. Only these two code points have that + # combination (they are Lo not Mc, so they get a width of one), and the + # docs/text-sizing-protocol.rst spec says so, hence the assertion. + spacing_marks_with_width = frozenset(ch for ch, p in enumerate(prop_array) if p.grapheme_break == 'SpacingMark' and p.width > 0) + if spacing_marks_with_width != {0x0E33, 0x0EB3}: + raise SystemExit( + 'The set of non-zero width SpacingMarks has changed to: ' + f'{sorted(map(hex, spacing_marks_with_width))}. Update screen.c, wcswidth.c,' + ' wcswidth.go and docs/text-sizing-protocol.rst to match.' + ) + test_grapheme_segmentation(partial(split_into_graphemes, gsprops)) gseg_results = tuple(GraphemeSegmentationKey.from_int(i).result() for i in range(1 << 16)) diff --git a/kitty/screen.c b/kitty/screen.c index 72a4947d8..c8cedde89 100644 --- a/kitty/screen.c +++ b/kitty/screen.c @@ -1218,17 +1218,23 @@ draw_combining_char(Screen *self, text_loop_state *s, char_type ch) { } } else if (ch == VS15) { const CPUCell *cpu_cell = cp + xpos; - if (self->lc->chars[base_pos + 1] == VS15 && cpu_cell->is_multicell && cpu_cell->width == 2 && is_emoji_presentation_base(self->lc->chars[base_pos])) { + // natural_width is false when the width was set explicitly by the text sizing + // protocol, in which case the application's width wins over the variation selector, + // matching how VS16 and Thai/Lao SARA AM leave such cells alone. + if (self->lc->chars[base_pos + 1] == VS15 && cpu_cell->is_multicell && cpu_cell->natural_width && cpu_cell->width == 2 && + is_emoji_presentation_base(self->lc->chars[base_pos])) { index_type deltax = (cpu_cell->scale * cpu_cell->width) / 2; if (halve_multicell_width(self, xpos, s->prev.y)) { self->cursor->x -= deltax; init_segmentation_state(self, s); } } - } else { - // Thai/Lao SARA AM is a SpacingMark with non-zero width, it widens a narrow base cell - CharProps ch_props = char_props_for(ch); - if (ch_props.grapheme_break == GBP_SpacingMark && wcwidth_std(ch_props) > 0 && !cp[xpos].is_multicell) widen_cell_to_two(self, s, cp, gp, xpos); + } else if (s->seg.grapheme_break == GBP_SpacingMark && !cp[xpos].is_multicell && wcwidth_std(char_props_for(ch)) > 0) { + // Thai/Lao SARA AM is a SpacingMark with non-zero width, it widens a narrow base cell. + // s->seg was just stepped with ch, and grapheme_segmentation_step() copies the + // grapheme break property of ch into it, so no extra char_props_for() lookup is + // needed to reject the overwhelmingly common case of a zero width combining char. + widen_cell_to_two(self, s, cp, gp, xpos); } } diff --git a/kitty_tests/screen.py b/kitty_tests/screen.py index fd81a7a86..1a9b40226 100644 --- a/kitty_tests/screen.py +++ b/kitty_tests/screen.py @@ -1067,6 +1067,45 @@ class TestScreen(BaseTest): s.reset() s.draw('\N{HEAVY EXCLAMATION MARK SYMBOL}' + 4500 * '\N{VARIATION SELECTOR-16}') + def test_explicit_width_is_not_changed_by_combining_chars(self): + # A width set explicitly via the text sizing protocol wins over the implicit + # re-sizing done by the variation selectors and Thai/Lao SARA AM + def mcd(text, width, follow): + s = self.create_screen(cols=8) + draw_multicell(s, text, width=width) + before = s.cpu_cells(0, 0)['mcd'] + self.ae(before['width'], width) + self.assertFalse(before['natural_width']) + s.draw(follow) + return s, s.cpu_cells(0, 0)['mcd'] + + # VS15 must not narrow an explicitly widened cell + s, c = mcd('\u25b6', 2, '\ufe0e') + self.ae(c['width'], 2) + self.ae(s.cursor.x, 2) + # VS16 must not widen an explicitly narrowed cell + s, c = mcd('\u26d4', 1, '\ufe0f') + self.ae(c['width'], 1) + self.ae(s.cursor.x, 1) + # SARA AM must not widen an explicitly narrowed cell + s, c = mcd('\u0e08', 1, '\u0e33') + self.ae(c['width'], 1) + self.ae(s.cursor.x, 1) + + # and the natural width cases still re-size, so the tests above are meaningful + s = self.create_screen(cols=8) + s.draw('\U0001f610') + self.ae(s.cursor.x, 2) + self.assertTrue(s.cpu_cells(0, 0)['mcd']['natural_width']) + s.draw('\ufe0e') + self.ae(s.cursor.x, 1) + self.ae(s.cpu_cells(0, 0)['mcd']['width'], 1) + s = self.create_screen(cols=8) + s.draw('\u26d4') + self.ae(s.cursor.x, 2) + s.draw('\u0e08\u0e33') + self.ae(s.cursor.x, 4) + def test_spacing_mark_widens_narrow_cell(self): # Thai SARA AM and Lao AM are SpacingMarks with width 1 that widen the cell they combine into s = self.create_screen(cols=5) diff --git a/tools/wcswidth/wcswidth_test.go b/tools/wcswidth/wcswidth_test.go index 3ab81cee5..b58a5c6cb 100644 --- a/tools/wcswidth/wcswidth_test.go +++ b/tools/wcswidth/wcswidth_test.go @@ -8,6 +8,26 @@ import ( "github.com/google/go-cmp/cmp" ) +func TestNonZeroWidthSpacingMarks(t *testing.T) { + // The cell a SpacingMark with non-zero width combines into is widened, both here and + // in kitty's C code. Only U+0E33 and U+0EB3 have that combination of properties, which + // docs/text-sizing-protocol.rst specifies, so guard against a Unicode data refresh + // silently changing the set. + expected := map[rune]bool{0x0e33: true, 0x0eb3: true} + for r := rune(0); r <= MAX_UNICODE; r++ { + cp := CharPropsFor(r) + if cp.Grapheme_break() == uint8(GBP_SpacingMark) && cp.Width() > 0 { + if !expected[r] { + t.Fatalf("U+%04X is an unexpected non-zero width SpacingMark of width: %d", r, cp.Width()) + } + delete(expected, r) + } + } + for r := range expected { + t.Fatalf("U+%04X is no longer a non-zero width SpacingMark", r) + } +} + func TestWCSWidth(t *testing.T) { wcswidth := func(text string, expected int) {