Cleanup previous PR and make VS15 on explicit width cell behavior consistent with VS16

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 </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

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

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))
 

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);
     }
 }
 

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)

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) {