Commit Graph

46 Commits

Author SHA1 Message Date
Jacob Sandlund
6b2caf69db Merge remote-tracking branch 'upstream/main' into grapheme-width-changes 2026-01-27 09:44:55 -05:00
Mitchell Hashimoto
49b2b8d644 unicode: switch to uucode grapheme break to (mostly) match unicode spec (#9680)
This PR builds on https://github.com/ghostty-org/ghostty/pull/9678 ~so
the diff from there is included here (it's not possible to stack PRs
unless it's a PR against my own fork)--review that one first!~

This PR updates the `graphemeBreak` calculation to use `uucode`'s
`computeGraphemeBreakNoControl`, which has [tests in
uucode](215ff09730/src/x/grapheme.zig (L753))
that confirm it passes the `GraphemeBreakTest.txt` (minus some
exceptions).

Note that the `grapheme_break` (and `grapheme_break_no_control`)
property in `uucode` incorporates `emoji_modifier` and
`emoji_modifier_base`, diverging from UAX #29 but matching UTS #51. See
[this comment in
uucode](215ff09730/src/grapheme.zig (L420-L434))
for details.

The `grapheme_break_no_control` property and
`computeGraphemeBreakNoControl` both assume `control`, `cr`, and `lf`
have been filtered out, matching the current grapheme break logic in
Ghostty.

This PR keeps the `Precompute.data` logic mostly equivalent, since the
`uucode` `precomputedGraphemeBreak` lacks benchmarks in the `uucode`
repository (it was benchmarked in [the original PR adding `uucode` to
Ghostty](https://github.com/ghostty-org/ghostty/pull/8757)). Note
however, that due to `grapheme_break` being one bit larger than
`grapheme_boundary_class` and the new `BreakState` also being one bit
larger, the state jumps up by a factor of 8 (u10 -> u13), to 8KB.

## Benchmarks

~I benchmarked the old `main` version versus this PR for
`+grapheme-break` and surprisingly this PR is 2% faster (?). Looking at
the assembly though, I'm thinking something else might be causing that.
Once I get to the bottom of that I'll remove the below TODO and include
the benchmark results here.~

When seeing the speedup with `data.txt` and maybe a tiny speedup on
English wiki, I was surprised given the 1KB -> 8KB tables. Here's what
AI said when I asked it to inspect the assembly:
https://ampcode.com/threads/T-979b1743-19e7-47c9-8074-9778b4b2a61e, and
here's what it said when I asked it to predict the faster version:
https://ampcode.com/threads/T-3291dcd3-7a21-4d24-a192-7b3f6e18cd31

It looks like two loads got reordered and that put the load that
depended on stage1 -> stage2 -> stage3 second, "hiding memory latency".
So that makes the new one faster when looking up the `grapheme_break`
property. These gains go away with the Japanese and Arabic benchmarks,
which spend more time processing utf8, and may even have more grapheme
clusters too.

### with data.txt (200 MB ghostty-gen random utf8)

<img width="1822" height="464" alt="CleanShot 2025-11-26 at 08 42 03@2x"
src="https://github.com/user-attachments/assets/56d4ee98-21db-4eab-93ab-a0463a653883"
/>

### with English wiki dump

<img width="2012" height="506" alt="CleanShot 2025-11-26 at 08 43 15@2x"
src="https://github.com/user-attachments/assets/230fbfb7-272d-4a2a-93e7-7268962a9814"
/>

### with Japanese wiki dump

<img width="2008" height="518" alt="CleanShot 2025-11-26 at 08 43 49@2x"
src="https://github.com/user-attachments/assets/edb408c8-a604-4a8f-bd5b-80f19e3d65ee"
/>

### with Arabic wiki dump

<img width="2010" height="512" alt="CleanShot 2025-11-26 at 08 44 25@2x"
src="https://github.com/user-attachments/assets/81a29ac8-0586-4e82-8276-d7fa90c31c90"
/>


TODO:

* [x] Take a closer look at the assembly and understand why this PR (8
KB vs 1 KB table) is faster on my machine.
* [x] _(**edit**: checking this off because it seems unnecessary)_ If
this turns out to actually be unacceptably slower, one possibility is to
switch to `uucode`'s `precomputedGraphemeBreak` which uses a 1445 byte
table since it uses a dense table (indexed using multiplication instead
of bitCast, though, which did show up in the initial benchmarks from
https://github.com/ghostty-org/ghostty/pull/8757 a small amount.)

AI was used in some of the uucode changes in
https://github.com/ghostty-org/ghostty/pull/9678 (Amp--primarily for
tests), but everything was carefully vetted and much of it done by hand.
This PR was made without AI with the exception of consulting AI about
whether the "Prepend + ASCII" scenario is common (hopefully it's right
about that being uncommon).
2026-01-20 09:44:15 -08:00
Tommy D. Rossi
e2de0bfd93 fix: clarify error codes in comment 2026-01-03 13:44:29 +01:00
Tommy D. Rossi
4ea669562e fix: use flush instead of end on stdout in code generators for Windows compatibility 2026-01-03 13:43:27 +01:00
Jacob Sandlund
d240a194e1 Merge branch 'grapheme-break' into grapheme-width-changes 2025-11-24 11:51:08 -05:00
Jacob Sandlund
e52f0d233f Merge remote-tracking branch 'upstream/main' into grapheme-break 2025-11-24 11:44:47 -05:00
Jacob Sandlund
6afdf3400e unicode: change cell to wide when grapheme width changes 2025-11-24 04:47:23 -05:00
Jacob Sandlund
36c3295806 unicode: don't narrow invalid text presentation (VS15) sequences 2025-11-23 22:39:21 -05:00
Jacob Sandlund
97aff0743e unicode: switch to uucode grapheme break to match unicode 16 spec 2025-11-23 22:33:01 -05:00
Jacob Sandlund
97926ca307 Update uucode to the latest, for future width and grapheme break changes 2025-11-23 17:26:25 -05:00
Mitchell Hashimoto
3d58dc51c9 terminal: keypad variation sequences should respect VS16
This fixes the VS16 issues found in this test:
https://ucs-detect.readthedocs.io/sw_results/ghostty.html#ghostty
This is also a more robust way to handle VS15/16 in general. 

This commit also changes our propeties to be a packed struct which
reduces its size from 4 bytes to 1 and likewise drops our unicode table
size 4x.
2025-11-06 07:05:57 -08:00
Mitchell Hashimoto
913d2dfb23 unicode: fix lookup table generation 2025-10-03 07:10:43 -07:00
Mitchell Hashimoto
16deea2761 nuke ziglyph from orbit
Since we now use uucode, we don't need ziglyph anymore. Ziglyph was kept
around as a test-only dep so we can verify matching but this is
complicating our Zig 0.15 upgrade because ziglyph doesn't support Zig
0.15. Let's just drop it.
2025-09-30 12:17:10 -07:00
Jacob Sandlund
c7ad29ca91 move tests over to _uucode.zig files to avoid needing deps for vt tests 2025-09-23 10:49:59 -04:00
Jacob Sandlund
b5c6c044a7 Fix merge 2025-09-23 10:01:23 -04:00
Jacob Sandlund
b01770c21c Merge remote-tracking branch 'upstream/main' into jacob/uucode 2025-09-23 09:36:41 -04:00
Mitchell Hashimoto
10dc9353b7 unicode: delete props.zig and clean up symbols deps too
Follow up to #8810

Same reasoning.
2025-09-20 20:28:25 -07:00
Mitchell Hashimoto
bf1278deff unicode: isolate properties, tables, and ziglyph into separate files
This makes it cleaner to add new sources of table generation and also
avoids inadvertently depending on different modules (despite Zig's lazy
analysis). 

This also fixes up terminal to only use our look up tables which avoids
bringing ziglyph in for the terminal module.
2025-09-20 15:00:55 -07:00
Jacob Sandlund
7b0722bf16 Remove comment above test. it's not too slow 2025-09-19 01:26:17 -04:00
Jacob Sandlund
cf3b514efc pr feedback: get, remove todos for case_folding_simple 2025-09-19 01:24:13 -04:00
Jacob Sandlund
b83315cb81 set max for unicode grapheme executable 2025-09-18 14:26:04 -04:00
Jacob Sandlund
69594119c3 fix up diff from benchmarks, and add tests against ziglyph 2025-09-18 11:46:05 -04:00
Jacob Sandlund
3275903611 update uucode and cleanups 2025-09-18 09:26:09 -04:00
Jacob Sandlund
4d37853f6c benchmark sources 2025-09-11 10:30:01 -04:00
Jacob Sandlund
cffa52e658 changes after benchmarking 2025-09-09 11:38:10 -04:00
Jacob Sandlund
b0db51c45e fast getX(.is_symbol) 2025-09-06 15:01:29 -04:00
Jacob Sandlund
f86a3a9b50 Merge remote-tracking branch 'upstream/main' into jacob/uucode 2025-09-06 14:31:41 -04:00
Jacob Sandlund
2af08bdbe3 trying a bunch of things to get performance to match 2025-09-06 10:42:02 -04:00
Jeffrey C. Ollie
1ef220a679 render: address review feedback
1. `inline` the table get.
2. Delete unused functions on the LUT table.
3. Disable the isSymbol test under valgrind
2025-09-05 11:40:03 -05:00
Jeffrey C. Ollie
e024b77ad5 drop the new LUT type as no performance advantage detected 2025-09-05 07:58:05 -05:00
Jeffrey C. Ollie
a7da96faee add two LUT-based implementations of isSymbol 2025-09-05 07:58:01 -05:00
Jacob Sandlund
0444c614da update for new grapheme_break 2025-08-21 22:29:34 -04:00
Jacob Sandlund
e84d8535f5 removing all ziglyph imports (aside from unicode/grapheme.zig) 2025-08-17 21:24:27 -04:00
Jacob Sandlund
f5a036a6a0 update after refactor (string field config, etc) 2025-08-12 09:43:12 -04:00
Jacob Sandlund
0c393299b0 using just get 2025-08-05 23:59:30 -04:00
Qwerasd
2384bd69cc style: use decl literals
This commit changes a LOT of areas of the code to use decl literals
instead of redundantly referring to the type.

These changes were mostly driven by some regex searches and then manual
adjustment on a case-by-case basis.

I almost certainly missed quite a few places where decl literals could
be used, but this is a good first step in converting things, and other
instances can be addressed when they're discovered.

I tested GLFW+Metal and building the framework on macOS and tested a GTK
build on Linux, so I'm 99% sure I didn't introduce any syntax errors or
other problems with this. (fingers crossed)
2025-05-26 21:50:14 -06:00
Mitchell Hashimoto
0f4d2bb237 Lots of 0.14 changes 2025-03-12 09:55:52 -07:00
Ryan Liptak
2d3db866e6 unigen: Remove libc dependency, use ArenaAllocator
Not linking libc avoids potential problems when compiling from/for certain targets (see https://github.com/ghostty-org/ghostty/discussions/3218), and using an ArenaAllocator makes unigen run just as fast (in both release and debug modes) while also taking less memory.

Benchmark 1 (3 runs): ./zig-out/bin/unigen-release-c
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          1.75s  ± 15.8ms    1.73s  … 1.76s           0 ( 0%)        0%
  peak_rss           2.23MB ±    0      2.23MB … 2.23MB          0 ( 0%)        0%
  cpu_cycles         7.22G  ± 62.8M     7.16G  … 7.29G           0 ( 0%)        0%
  instructions       11.5G  ± 16.0      11.5G  … 11.5G           0 ( 0%)        0%
  cache_references    436M  ± 6.54M      430M  …  443M           0 ( 0%)        0%
  cache_misses        310K  ±  203K      134K  …  532K           0 ( 0%)        0%
  branch_misses      1.03M  ± 29.9K      997K  … 1.06M           0 ( 0%)        0%
Benchmark 2 (3 runs): ./zig-out/bin/unigen-release-arena
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          1.73s  ± 6.40ms    1.72s  … 1.73s           0 ( 0%)          -  1.0% ±  1.6%
  peak_rss           1.27MB ± 75.7KB    1.18MB … 1.31MB          0 ( 0%)        - 43.1% ±  5.4%
  cpu_cycles         7.16G  ± 26.5M     7.13G  … 7.18G           0 ( 0%)          -  0.9% ±  1.5%
  instructions       11.4G  ± 28.2      11.4G  … 11.4G           0 ( 0%)          -  0.8% ±  0.0%
  cache_references    441M  ± 2.89M      439M  …  444M           0 ( 0%)          +  1.2% ±  2.6%
  cache_misses        152K  ±  102K     35.2K  …  220K           0 ( 0%)          - 50.8% ± 117.8%
  branch_misses      1.05M  ± 13.4K     1.04M  … 1.06M           0 ( 0%)          +  2.0% ±  5.1%

Benchmark 1 (3 runs): ./zig-out/bin/unigen-debug-c
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          1.75s  ± 32.4ms    1.71s  … 1.77s           0 ( 0%)        0%
  peak_rss           2.23MB ±    0      2.23MB … 2.23MB          0 ( 0%)        0%
  cpu_cycles         7.23G  ±  136M     7.08G  … 7.34G           0 ( 0%)        0%
  instructions       11.5G  ± 37.9      11.5G  … 11.5G           0 ( 0%)        0%
  cache_references    448M  ± 1.03M      447M  …  449M           0 ( 0%)        0%
  cache_misses        148K  ± 42.6K     99.3K  …  180K           0 ( 0%)        0%
  branch_misses       987K  ± 5.27K      983K  …  993K           0 ( 0%)        0%
Benchmark 2 (3 runs): ./zig-out/bin/unigen-debug-arena
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          1.76s  ± 4.12ms    1.76s  … 1.76s           0 ( 0%)          +  0.4% ±  3.0%
  peak_rss           1.22MB ± 75.7KB    1.18MB … 1.31MB          0 ( 0%)        - 45.1% ±  5.4%
  cpu_cycles         7.27G  ± 17.1M     7.26G  … 7.29G           0 ( 0%)          +  0.6% ±  3.0%
  instructions       11.4G  ± 3.79      11.4G  … 11.4G           0 ( 0%)          -  0.8% ±  0.0%
  cache_references    440M  ± 4.52M      435M  …  444M           0 ( 0%)          -  1.7% ±  1.7%
  cache_misses       43.6K  ± 19.2K     26.5K  … 64.3K           0 ( 0%)        - 70.5% ± 50.8%
  branch_misses      1.04M  ± 2.25K     1.04M  … 1.05M           0 ( 0%)        💩+  5.8% ±  0.9%
2025-01-20 18:30:22 -08:00
Mitchell Hashimoto
fd1201323e unicode: emoji modifier requires emoji modifier base preceding to not break
Fixes #2941

This fixes the rendering of the text below. For those that can't see it,
it is the following in UTF-32: `0x22 0x1F3FF 0x22`.

```
"🏿"
```

`0x1F3FF` is the Fitzpatrick modifier for dark skin tone. It has the
Unicode property `Emoji_Modifier`. Emoji modifiers are defined in UTS
#51 and are only valid based on ED-13:

```
emoji_modifier_sequence := emoji_modifier_base emoji_modifier
emoji_modifier_base := \p{Emoji_Modifier_Base}
emoji_modifier := \p{Emoji_Modifier}
```

Additional quote from UTS #51:

> To have an effect on an emoji, an emoji modifier must immediately follow
> that base emoji character. Emoji presentation selectors are neither needed
> nor recommended for emoji characters when they are followed by emoji
> modifiers, and should not be used in newly generated emoji modifier
> sequences; the emoji modifier automatically implies the emoji presentation
> style.

Our precomputed grapheme break table was mistakingly not following this
rule. This commit fixes that by adding a check for that every
`Emoji_Modifier` character must be preceded by an `Emoji_Modifier_Base`.
This only has a cost during compilation (table generation). The runtime
cost is identical; the table size didn't increase since we had leftover
bits we could use.
2024-12-12 12:53:08 -08:00
Mitchell Hashimoto
004405ccf9 terminal: only apply VS15/16 to emoji
Fixes #1482
2024-02-10 17:26:45 -08:00
Mitchell Hashimoto
5275d44e7d unicode: precompute grapheme break data 2024-02-09 20:50:13 -08:00
Mitchell Hashimoto
132fbb3a46 unicode: use packed struct for break state 2024-02-09 20:29:36 -08:00
Mitchell Hashimoto
c47ad97f62 unicode: remove unused 2024-02-09 20:23:29 -08:00
Mitchell Hashimoto
5f3574a4bf unicode: direct port of ziglyph to start 2024-02-09 19:44:57 -08:00
Mitchell Hashimoto
0632410857 unicode: get grapheme boundary class 2024-02-09 12:22:23 -08:00
Mitchell Hashimoto
9755d0696e unicode: generate our own lookup tables 2024-02-08 21:01:11 -08:00