gtk: implement GtkAccessibleText for the terminal surface - #13779
gtk: implement GtkAccessibleText for the terminal surface#13779alex19EP wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Implements initial AT-SPI text accessibility for the GTK terminal surface by exposing terminal viewport text, caret position, hit-testing, per-line extents, and incremental change events through GtkAccessibleText, backed by GTK-free snapshot/diff logic that is unit-tested and benchmarked.
Changes:
- Add GTK-independent viewport snapshot builder (
a11y_text) and offset/diff math (a11y_offsets), with extensive unit tests. - Implement
GtkAccessibleText(+GtkAccessibleoverride) onGhosttySurface, including cached snapshots, change probing, and AT-SPI update signals. - Add an
a11y-textbenchmark and seed app/window accessibility identity and roles for better screen reader discovery.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/build/SharedDeps.zig | Adds graphene pkg-config mapping needed for graphene.Point/Rect usage in accessibility APIs. |
| src/benchmark/main.zig | Exposes the new A11yText benchmark module. |
| src/benchmark/cli.zig | Registers the a11y-text benchmark action and wiring. |
| src/benchmark/A11yText.zig | Adds benchmark for snapshot build vs per-frame probe paths. |
| src/apprt/gtk/ui/1.2/surface.blp | Moves focus/key controllers to GhosttySurface to make the accessible terminal object the focus target. |
| src/apprt/gtk/class/window.zig | Seeds accessible-role=window at construct time for correct AT-SPI role reporting. |
| src/apprt/gtk/class/surface.zig | Implements GtkAccessibleText and associated caching/probing/diff-based update emissions on the surface. |
| src/apprt/gtk/class/application.zig | Seeds prgname/application name early and ensures quick-terminal windows also get accessible-role=window. |
| src/apprt/a11y_text.zig | New GTK-free viewport snapshot builder (text + optional per-codepoint cell widths) with unit tests. |
| src/apprt/a11y_offsets.zig | New GTK-free offset math + minimal diff selection with extensive tests and fuzz-style round-trip validation. |
| src/apprt.zig | Includes new modules in the test build. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
1ee991c to
31b3678
Compare
|
hello. i addressed copilot revues and fixed typos found by CI. can someone approve workflow? |
Is the description of this written by you or AI? Looks suspicious and violating our AI policy. Checks workflow approved. |
| _ = runtime; | ||
| _ = action; | ||
| _ = structs; | ||
| _ = @import("apprt/a11y_text.zig"); |
There was a problem hiding this comment.
I don't think we need to import them here. I also had this with ai generated changes before.
You can move the related files down below with the benefit of zig0.16
There was a problem hiding this comment.
yep tested will remove them.
You can move the related files down below with the benefit of zig0.16
This I don't understand Do you mean that I should change these files hierarchy? move them to another directory?
Hello. by me. But I am a Russian speaker so I sometime using translator (deepl) service to translate my Russian text to English and fix some grammar. i can stop Doing that if it is suspicious. |
pluiedev
left a comment
There was a problem hiding this comment.
I don't know. I haven't come close to reviewing all of this but so far I'm pretty disappointed by the quality. I love that we're finally trying to get Ghostty working with Assistive Technology but this PR isn't really up to my standards.
|
|
||
| action_group: ?*gio.SimpleActionGroup = null, | ||
|
|
||
| // Accessibility state for GtkAccessibleText |
There was a problem hiding this comment.
IMO there's too much scattered state within the surface's private fields that should logically belong together. Make a new struct with all of the accessibility state and move all the related logic there
There was a problem hiding this comment.
Definitely agree this needs to be moved to a new struct. surface.zig is already too big.
| // Accessibility state. These use `std.heap.c_allocator` rather | ||
| // than `alloc` above because their contents are handed to GTK, | ||
| // which may still be holding them when we get here. | ||
| priv.ax_probe_buf.deinit(std.heap.c_allocator); |
There was a problem hiding this comment.
Don't use std.heap.c_allocator when we already have access to the application's overall allocator.
There was a problem hiding this comment.
switched to Application.default.allocator
| // `get_extents` and `get_offset` are 4.16 additions, and they are | ||
| // trailing slots. The guard is on the *runtime* version, not the | ||
| // compile-time one: our bindings always declare these fields, but | ||
| // `G_DEFINE_INTERFACE` allocates the vtable at the running GTK's | ||
| // own `sizeof(GtkAccessibleTextInterface)`. Against a 4.14 | ||
| // runtime the two slots are not part of that allocation and | ||
| // writing them scribbles past the end of it. | ||
| // | ||
| // Leaving them unset on 4.14 costs per-range geometry, so a | ||
| // screen reader's flat review has no coordinates to group rows by | ||
| // and falls back to reading the object as a whole. Degraded, but | ||
| // it reads. |
There was a problem hiding this comment.
Far too much LLM yapping.
| // `get_extents` and `get_offset` are 4.16 additions, and they are | |
| // trailing slots. The guard is on the *runtime* version, not the | |
| // compile-time one: our bindings always declare these fields, but | |
| // `G_DEFINE_INTERFACE` allocates the vtable at the running GTK's | |
| // own `sizeof(GtkAccessibleTextInterface)`. Against a 4.14 | |
| // runtime the two slots are not part of that allocation and | |
| // writing them scribbles past the end of it. | |
| // | |
| // Leaving them unset on 4.14 costs per-range geometry, so a | |
| // screen reader's flat review has no coordinates to group rows by | |
| // and falls back to reading the object as a whole. Degraded, but | |
| // it reads. | |
| // `get_extents` and `get_offset` are 4.16 additions. | |
| // While they are accessible with current bindings, we need to make sure | |
| // we do not access invalid memory when running with older GTK versions | |
| // by writing to fields that don't yet exist. |
There was a problem hiding this comment.
done.
I tried to strike a balance between the comments that are already in the codebase—which really help me, as a beginner, read the Zig code—and the fact that the AI does generate unnecessarily long comments. I addressed your specific comment and reviewed the rest of the comments again. I made changes where I thought it was appropriate.
| end: c_uint, | ||
| extents: *graphene.Rect, | ||
| ) callconv(.c) c_int { | ||
| const self: *Self = @ptrCast(@alignCast(self_opaque)); |
There was a problem hiding this comment.
Do a safe check instead.
| const self: *Self = @ptrCast(@alignCast(self_opaque)); | |
| const self = gobject.ext.cast(Self, self_opaque) orelse return 0; |
There was a problem hiding this comment.
done. Thank you for recommendation
| /// | ||
| /// `core_surface.size` is in device pixels, since the renderer draws in | ||
| /// them | ||
| /// and `scaledCoordinates` multiplies pointer input up into them before | ||
| /// handing it to the core. GTK's accessibility coordinates are widget | ||
| /// space, so anything we derive from the cell metrics has to come back | ||
| /// down by the same factor. Mirroring `scaledCoordinates` rather than | ||
| /// using `getContentScale` is deliberate: the latter folds in the | ||
| /// gtk-xft-dpi font scale, which changes how large a cell *is* but not | ||
| /// which coordinate space it is measured in. | ||
| /// | ||
| /// At 1x the two spaces coincide, which is what kept this hidden: every | ||
| /// rect we reported was already correct, and the test harness runs under | ||
| /// Xvfb at 1x. On a scaled display an unconverted rect is inflated by the | ||
| /// scale factor, and Orca intersects each line's rect with the widget's | ||
| /// allocation to decide which lines are on screen. Inflated rows run out | ||
| /// of the box early, so flat review silently loses the bottom of the | ||
| /// viewport, including the row the prompt is on. |
There was a problem hiding this comment.
Wayyyyyyyyyyyyy too much yapping. You need to tell the LLM to output less text and condense the logic more.
There was a problem hiding this comment.
fixed. and treamed the other comments too
|
|
||
| const core_surface = priv.core_surface orelse return null; | ||
|
|
||
| const alloc = std.heap.c_allocator; |
There was a problem hiding this comment.
Again use application's allocator and not the C allocator
Two GTK-free modules that the GTK apprt will build on. They are separate from the apprt so the viewport walk and the offset arithmetic can be unit tested directly, without standing up a live surface and an AT-SPI bus. `a11y_text` walks a terminal viewport into a flat UTF-8 buffer: one line per row, rows joined by '\n', trailing blanks on a row dropped and interior blanks kept as spaces so columns still line up. Alongside the text it records how many screen columns each codepoint occupies, which is the only point where that is knowable — a double-width character is one codepoint across two cells and a combining mark is a codepoint across none, and neither can be recovered from the text afterwards without guessing at the terminal's configured grapheme-width method. `a11y_offsets` navigates that snapshot: byte/codepoint conversion, grid and widget-space mapping, and the minimal diff between two snapshots. Every offset an AT client sees is in codepoints rather than bytes, which is the source of every historical crash in this area, so the arithmetic lives in one tested place and the C integer types stay at the boundary. The diff is the load-bearing part. An AT client does not re-read the buffer when it changes; it applies the insert/remove events to the copy it already holds. So the property that matters is not that each event is plausible but that replaying them reproduces the new text exactly, and that is what the round-trip tests assert — by name for the shapes we emit deliberately, and over 3000 generated viewports containing multi-byte rows, characters that share leading bytes, and emoji. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The GTK apprt rebuilds this snapshot while holding the renderer mutex, so
like ScreenClone it is a lock holder that costs the IO thread directly.
Two modes mirror the two real per-frame paths: `probe` is the change gate
every rendered frame pays, and `text` is the full rebuild only a frame
that actually changed pays. `noop` iterates the same rows building
nothing, to separate iteration overhead from the work.
On this machine, ReleaseFast, best of three:
probe 4.0us at 30x80 12.3us at 60x200
text 5.4us at 30x80 15.6us at 60x200
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two independent reasons a screen reader currently cannot find a Ghostty window at all. The AT-SPI bridge reads g_get_prgname() for the application object's Name property and g_get_application_name() for its Description. We set neither, so we appear as "Unnamed" and cannot be located by name. Set both before any GTK or libadwaita initialization, since g_set_application_name is one-shot and the earliest setter wins. Separately, the accessible role has to be passed as a construct-time property. Setting it with gtk_widget_class_set_accessible_role does not propagate to the AT context, which goes on reporting the default `widget` role — AT-SPI "filler". Screen readers locate a window by looking for a frame, so a filler at the top of the tree leaves them nothing to descend into. Both are visible in accerciser without a screen reader installed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Implement the read side of GtkAccessibleText on GhosttySurface, which is what lets Orca and other AT-SPI clients read a Ghostty terminal at all. The surface serves its viewport as text, reports where the cursor is, resolves points to offsets and offsets to on-screen rectangles, and emits change events as the terminal produces output. Focus and key handling move from the inner GLArea onto GhosttySurface. A screen reader's flat review reads the focused object, and the bare GLArea has role `panel` and carries no text interface, so focusing it gave clients nothing to read. Moving focus up means the focused object is the one with role `terminal` and the text interface on it. This is the part of the change most likely to affect sighted users and wants a look from someone who can see the pointer: click-to-focus, click-drag selection, middle-click paste, and focusing a split by clicking it. Change events are gated twice, because getting this wrong is the difference between a usable terminal and an unusable one. A cheap probe rebuilds only the text and compares it against the last notified snapshot, so an unchanged frame does no work beyond that; and when something did change, the diff is reduced to the smallest remove/insert pair that describes it. Emitting the whole viewport per frame makes a screen reader spin in a read-interrupt-read loop and never finish a sentence, and emitting it per keystroke makes Orca's terminal script read typing echo as command output. Extents are reported per row in widget coordinates. Flat review groups zones into lines by their Y coordinate, so rows that report the same Y collapse into one line; and Ghostty draws in device pixels, so on a scaled display the numbers have to be converted or every row claims to be larger than it is and rows fall outside the widget and get dropped. Selection, text attributes and links are not implemented here. GTK's default_init installs implementations for those slots that decline, and the two it does not cover are null-checked by their public wrappers, so they read to a client as "this object has no selection / no attributes" rather than crashing the bridge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
31b3678 to
3a3c172
Compare
|
i think i Addressed all the review feedback for now. please tell me if I overlooked something. I'm also updated pull request description to be more transparent about AI usage. |
This PR is a first step toward making Ghostty usable for blind users on Linux.
Screen reader accessibility was requested in
#2351. I opened a vouch
request for this work in
#13746, which jcollie
granted. The staged PR order described there starts with this
one.
What this PR does
With this change the Orca screen reader can read the terminal viewport: the
visible text, the cursor position, hit-testing between screen points and text
offsets, per-row rectangles so flat review works, and change events as output
arrives so Orca can announce what changed.
What it contains
src/apprt/a11y_text.zigandsrc/apprt/a11y_offsets.zig: the snapshotwalk and the offset math. Both are GTK-free and unit tested.
src/benchmark/A11yText.zig: a benchmark for the snapshot walk and for theper-frame change probe.
GtkAccessibleTextimplementation onGhosttySurface.a name and the right role instead of as an unnamed object.
Performance
The per-frame work is a cheap change probe, not a rebuild: 6.7us at 30x80 and
17.6us at 60x200. A full snapshot rebuild costs 9.5us and 23.2us respectively
and only runs when the probe reports that the contents actually changed.
To reproduce, against a ReleaseFast build:
Those figures are hyperfine medians over 10 runs after 3 warmups.
The probe only runs once an AT client has queried the surface at least once,
and only on frames that surface actually renders, so an idle terminal and a
terminal nobody is inspecting both cost nothing.
I deliberately did not gate this on focus. With splits, that would silence a
build running in the other half of the window. Whether to speak an unfocused
terminal is the AT client's policy call, not ours to pre-empt by withholding
the events. There is a comment in
glareaRenderexplaining this, and whyorg.a11y.Status.IsEnabledis not the gate it looks like either.GTK versions
get_extentsandget_offsetwere added toGtkAccessibleTextInterfaceinGTK 4.16.
G_DEFINE_INTERFACEallocates the vtable at the running GTK'ssizeof, so writing those two slots unconditionally would write past theallocation on an older runtime. They are installed only when the running GTK
is 4.16 or newer. Everything else in the interface is 4.14, and the public
wrappers for those two null-check, so on 4.14 they simply do not participate.
Testing
48 unit tests across the two new modules, 42 of them in
a11y_offsets.zig.The one that matters most is a 3000 iteration round-trip fuzz test. It replays
the change events we emit against a model of the AT client's own copy of the
text and asserts the two never diverge. That test is why I trust the diff.
Beyond the unit tests I have been driving this daily with Orca and a braille
display.
One thing I cannot test myself
I am blind, so I cannot verify mouse behaviour.
The change I would most like a sighted reviewer to look at is
src/apprt/gtk/ui/1.2/surface.blp: focus and both event controllers move fromthe inner
GLAreaup toGhosttySurface. That move is what makes the surfacean accessible focus target, but it is also the part of this diff most likely
to break things for sighted users. The four things need checking:
What is not in this PR
Selection, text attributes, and links, in that order.
Test harness
I have an end-to-end harness on my
gtk-accessibility-pocbranch: pytestdriving a real Ghostty window over AT-SPI under Xvfb. 41 tests, 36 of them
passing against this branch; the 5 failures are all features deferred to the
later PRs (attributes, selection, set_caret).
I left it out here because it needs a display server, so it would have to be
opt-in and disabled in CI, the same shape as the existing macOS XCUITest
suite. Happy to submit it separately if you want it.
AI Usage Disclosure
This change was developed with heavy assistance from Claude Code (Anthropic). I direct the design and behavior and Claude Code writes most of the Zig under my review. All code was reviewed, built, and tested by me before submission, including daily use with Orca and a braille display.