diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 2fd36f6..505e16f 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -5,6 +5,39 @@ they can be judged and reversed later. Detail lives in RUST.md (and IRIS.md for iris API changes); this file is only the summary. Newest first. Items marked **DEFERRED** are ones the agent chose not to decide alone. +## 2026-09-06 (how a markdown block looks, P1a) + +- **A table is drawn as padded monospace columns, not as a grid.** Your + call to reverse. Compose draws a real grid: cells on a tint, each + column with a 136dp floor, scrolling sideways when there are too many. + iris has no grid widget, and building one would be a widget per + markdown feature -- which is the thing the block model exists to avoid. + In a monospace face a character count *is* a pixel width, so padding + each cell to its column's width is alignment, the widths are still + measured from the cells, and a table that is too wide pans sideways + through the same mechanism a code fence already uses. The header is + bold with a rule under it, and a long cell wraps inside its column + (capped at 28 characters, which is what fits three columns across a + phone). **What it trades:** no cell borders, and a table looks like + code rather than like a table. If you want the grid, it is a new widget + and it is a day's work. +- **Three block frames, and only three.** A heading, paragraph and list + are plain text with spans; a fence and a table are a rounded panel that + does not wrap; a quote is a bar with the text padded past it. + Everything else markdown says is expressed in span styles, which cost + no widgets and no layout nodes. So a new markdown feature is a span, + not a widget. +- **A list's marker is part of the text, so a wrapped item's second line + returns to the left margin.** Compose keeps it indented by giving the + marker its own column. Doing the same here needs per-line indent in + iris's text attributes; it is written down rather than done, because + the list items in a real reply are usually one line. +- **A link opens on a tap and not on the end of a drag.** A press that + panned the transcript past a link, or that held long enough to start a + selection, does not follow it -- decided by the same gesture machine + that decides pan-versus-select, so there is one rule rather than two + that can disagree. + ## 2026-09-06 (composer scroll and the streaming block model) - **A streamed message becomes a column of per-block widgets.** Decided by diff --git a/docs/IRIS.md b/docs/IRIS.md index 1d6d230..8f1cbe7 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -8,6 +8,55 @@ capability that moved. Small and trivial changes do not go here. An entry gives the date, what changed, why, and a short before/after where it helps judge the change without the session that made it. Newest first. +## 2026-09-06: a tap is its own gesture outcome, and opening a URL is a backend capability + +Three related additions, all for following a markdown link. + +**`iris::platform::OpenUrl`** is a new trait beside `attr::FocusHost`, and +has the same shape: declared in `iris`, implemented once per backend (a +detached `xdg-open`/`open`/`start` on the desktop, an `ACTION_VIEW` intent +on Android, deferred to the next view callback exactly the way +`pending_show_keyboard` is). A widget asks for the capability by bound -- +`Rsc::State: FocusHost + OpenUrl` -- instead of a caller threading a +callback down through every builder. One method, not a general "run an +intent": a narrower capability is a narrower thing to get wrong. Nothing +is returned; the platform either shows a browser or does not, and both +are outside the process. + +**`GestureOutcome::Tapped`** is new. `Released(None)` used to mean both +"the press ended having selected something" and "the press ended having +done nothing at all", and only the second is a tap. Any caller that acts +on a tap -- following a link -- must not also act when the finger was +panning the list past that link, so the distinction is made once, in the +gesture machine every widget already shares, rather than timed again per +widget. `DragArbiter::is_undecided()` is what answers it. +`Selection::drag` returns the outcome now instead of `()`. + +**`DragArbiter`/`DragGesture` take an axis** (`::on(Axis)`; `::new()` is +still vertical). A code fence pans across its own long lines exactly the +way a transcript pans down its rows, and the two were the same state +machine with `dx` and `dy` swapped. `WidgetLike::scrollable_on(axis)` +joins `scrollable()` for the same reason. Before this, a horizontal +`Scroll` existed but could not be dragged by a finger at all -- its +arbiter only ever committed on the vertical axis. + +Two smaller ones in the same pass. **`TextEditCtx::byte_at(pos, size)`** +answers which byte of the text a tap landed on, doing the same +region-relative transform `select` does, without handing out the parley +layout a caller could shape against stale text. And **`Rect::radius` now +takes a `Len`**, so a corner can be written in `dp` and come out the same +physical size on every display; a bare number still means physical pixels. + +**One behaviour change worth knowing about**: `Rect::is_size_independent()` +answers `false` now. It answered `true`, and a `Rect` fills whatever +region it is given -- so `draw_inner`'s fast path, which rewrites a +widget's primitives in place instead of redrawing it, could not reproduce +what `draw` would have done. A `.background(rect(..))` behind +variable-height content kept the size of the provisional pass its parent +`Span` had drawn it at, which on the transcript screen meant one code +block's panel covering every block below it. Costs one primitive's redraw +when a rect is resized. + ## 2026-09-06: a transcript row is a column of blocks, and a block is the selection unit `transcript-ui`'s row builder used to make **one** `TextEdit` per message. diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index 9229326..366e429 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -535,22 +535,31 @@ agent ticks it here with the evidence. `row.rs`'s `build_text_row` is where one would go, keyed to something stable per row (its sender + a short excerpt, matching what a screen reader announcing a chat message would say). - - [ ] **A tappable link and a background chip behind inline code.** - Both need per-range glyph geometry that `TextEditCtx` does not expose - outside `iris::widget::text` (`edit.rs`'s `layout()` helper is - private) — see `markdown.rs`'s module doc for the exact shape the fix - would take (the same primitive `TextEdit::draw`'s own selection - highlight already uses internally, - `iris/src/widget/text/edit.rs:99`). + - [x] **A tappable link** — done 2026-09-06 (P1a). `TextEditCtx:: + byte_at(pos, size)` answers which byte a tap landed on without + handing out the parley layout, `GestureOutcome::Tapped` says the + press committed to neither a pan nor a selection, and + `iris::platform::OpenUrl` is the capability each backend implements + (`xdg-open`/`open`/`start`; an `ACTION_VIEW` intent on Android, + deferred to `after_input` the way `pending_show_keyboard` is). + - [ ] **A background chip behind inline code.** Still needs per-range + glyph *geometry* — a run's boxes, not one offset — which + `TextEditCtx` does not expose outside `iris::widget::text` + (`edit.rs`'s `layout()` helper is private). The same primitive + `TextEdit::draw`'s own selection highlight uses internally, + `iris/src/widget/text/edit.rs:99`. `byte_at` above deliberately did + not open that up: a tap needs one offset and a chip needs the run. - [ ] **`Selection`'s anchor-row shortcut.** The row a drag started in is selected in full (`select_all`) the moment the drag leaves it, rather than "from the click point to whichever edge points away from the drag" — needs the same private `layout()` access as the item above. `selection.rs`'s module doc has the exact reasoning. - - [ ] **No syntax highlighting inside a fenced code block.** - `client_core::highlight` exists (built for the file explorer) and - could feed per-token `SpanStyle`s into a code block's span; wiring it - in was not attempted this pass. + - [x] **Syntax highlighting inside a fenced code block** — done + 2026-09-06 (P1a). `client_core::highlight::spans_of` by language, + converted from its char indices to `SpanStyle`'s byte offsets, in + the same Catppuccin palette `Theme.kt` uses. A language the scanner + has no rules for stays plain rather than being coloured by the + nearest one's. - [ ] **Masks defined relative to each other.** Wanted: mask A multiplies by something *and also* applies mask B — a mask can reference a parent @@ -570,6 +579,48 @@ agent ticks it here with the evidence. everything, the same way input is**. Whatever the mechanism, a widget that does not animate must pay nothing and import nothing for it. +## Found by P1a (2026-09-06) + +- [x] **`Rect` claimed to be size-independent, and it is not.** A `Rect` + fills whatever region it is handed, so `draw_inner`'s size-independent + fast path -- which rewrites primitives with + `r.outside(&from).within(®ion)` rather than redrawing -- could not + reproduce its `draw`, and a `.background(rect(..))` kept the size of + the *provisional* full-region pass `Span` does in phase 1. One fenced + code block's panel covered every block below it and every row below + that. Fixed in `iris/src/widget/rect.rs`; the reason is written at the + definition. Suspect the same cause for anything else tinted with a + background rect. +- [ ] **A wrapped transcript row trips `reposition`'s debug assert.** + *"widget ... is both moved by its parent's own layout (`mov`) and + repositioned within it"*, raised from `List::place`. Repro: change + `.wrap(!verbatim)` to `.wrap(true)` in `transcript-ui/src/row.rs`'s + `build_block` and run `iris/run-headless.sh transcript --shot + /tmp/x.png -- -p transcript-ui`. Survives the `Rect` fix above and is + not specific to any block kind -- it appears once the row is tall + enough. The shipping configuration does not reach it (verbatim blocks + do not wrap) and the Android bench runs clean with assertions live, + but it is a real disagreement about who owns a widget's move slot and + should be settled before more of P1 leans on `List`. +- [ ] **Desktop colours are washed out: the winit surface is sRGB and + the shader writes the palette's bytes as linear.** Mocha Crust + (17,17,27) is drawn as (73,73,91), measured off + `run-headless.sh --shot`. Android is correct, so this is the surface + format rather than the palette -- but it makes the desktop build + useless as a colour reference, which is exactly what P1a needed it for + when the emulator could not draw glyphs. +- [ ] **The emulator cannot draw iris's glyphs.** Under `-gpu host` with + Vulkan disabled (Mesa 26.2.2 / virgl -- what `emu` does on this + machine) every character renders as a solid filled box: the atlas + sample's alpha reads 1, which is what an incomplete GL texture returns + (0,0,0,1). Not new (`20303e0` does it too) and not the platform's + (Compose draws text perfectly on the same AVD in the same minute). + Enabling host Vulkan still dies at boot in gfxstream, and + `EMU_GPU=software` gives SwiftShader Vulkan on which iris **SIGSEGVs + in `surface_changed`**. Either of the last two would restore + appearance testing on Android; today it has to be done on the desktop + backend or on Iris's phone. + ## Build (for the port) Widgets `RUST.md`'s "The port, in order (decided 2026-09-05)" needs and diff --git a/docs/RUST.md b/docs/RUST.md index 3489614..be2062c 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -5465,17 +5465,141 @@ device. comparison fair. **Sub-order, decided by the design agent, by what the bench fixture exercises and Compose already draws** (each is one agent; tick and date in place): - - [ ] **P1a — markdown block rendering parity.** Now that a row is - a column of per-block widgets (`e1030d6`), draw each block - the way Compose does: headings at their sizes, fences in a - mono face with `client-core`'s `highlight` spans and a - distinct background, bullet/numbered lists with indent, - tables as aligned columns, block quotes, links as tappable - spans (opening through the platform — the "tappable link" - primitive in IRIS_TODO's "Build (for the port)"). Against - `app/bench-fixture/`, screenshot beside the Compose bench - build on the same emulator. Pure parts (block → style - mapping) tested in `client-core`/`transcript-ui`. + - [x] **P1a — markdown block rendering parity.** Done 2026-09-06. + Each top-level block is drawn in one of **three frames** + (`transcript-ui::markdown::BlockFrame`, mapped from + `BlockKind` by the pure `frame_of`): `Plain` (a paragraph, + heading, list or rule -- text and spans, no extra widget), + `Verbatim { fill }` (a fence or a table -- a rounded panel + that does not wrap and pans sideways, `CodeFence.kt`'s + `horizontalScroll`), and `Quote` (a bar behind text padded + past it). Everything else markdown can say is expressed in + `SpanStyle`s, which cost no widgets. + **What each block looks like now, against `Markdown.kt`:** + - *Headings* -- Material's own ladder, the six sizes + `markdownTypography` picks (24/22/16/14/12/11 at a 16pt + body), bold. Was a three-step 28/24/21/19. + - *Fences* -- monospace on Mocha Crust, rounded, with + `client_core::highlight`'s spans by language in the same + Catppuccin palette `Theme.kt`'s `catppuccinSyntax()` uses. + An unknown language is plain rather than coloured by the + nearest one. A fence being streamed into re-renders only + the last block (`RowBlocks::apply_delta`), so earlier + fences are never re-scanned. + - *Lists* -- the bullet ladder `MarkdownPieces.kt` draws + (disc/ring/square by depth) and ordered lists counting from + the number written, markers in Lavender. + - *Tables* -- padded monospace columns measured from the + cells, header bold, a rule under it, on Surface 0. A real + grid was rejected; docs/DECISIONS.md, 2026-09-06, has why. + - *Quotes* -- a Surface 2 bar down the left, text one shade + back from body. + - *Links* -- coloured and underlined as before, and now + **tappable**: `GestureOutcome::Tapped` (a press that + committed to neither a pan nor a selection), + `TextEditCtx::byte_at` for which byte, and + `iris::platform::OpenUrl` for the platform (`xdg-open`/ + `open`/`start`; on Android an `ACTION_VIEW` intent deferred + to `after_input`, the shape `pending_show_keyboard` uses). + **Screenshots: `docs/bench/p1a-2026-09-06/`.** + `compose-heading-fence-table.png` and + `compose-fence-table.png` are the Compose `bench` build on + this checkout's AVD against `app/bench-fixture/`; + `iris-blocks.png` is iris rendering the same heading, + paragraph, link, fence and table source (plus a list and a + quote, which the fixture has neither of) from + `transcript-ui`'s own `transcript` example. + **Why the iris half is not from the emulator**, which the + pass condition asked for: **the emulator cannot draw iris's + glyphs at all.** Every character comes out as a solid filled + box of the right width -- `iris-emulator-gles-glyphs.png`. + Established as *not* this change's doing and not the app's: + the previous commit (`20303e0`) draws the same boxes, and the + Compose bench build on the same AVD in the same minute draws + text perfectly. The atlas sample's alpha reads as 1 under + `-gpu host` + `-feature -Vulkan` (Mesa 26.2.2 / virgl), which + is what an *incomplete* GL texture returns (0,0,0,1). Both + ways out were tried and both fail: `GPU_HOST_FEATURES=" "` + still dies at boot with gfxstream's documented "Format + VK_FORMAT_R8G8B8A8_UNORM is not supported ... Failed to find + memory type for ColorBuffers", and `EMU_GPU=software` does + give the guest SwiftShader Vulkan but iris **SIGSEGVs inside + `surface_changed`** on it. So the appearance half of this box + is taken on the desktop/winit backend, which renders on the + host's real GPU through `iris/run-headless.sh`. + **What still differs, pair by pair:** + 1. *Colour, on the desktop shot only.* The winit surface is + sRGB and the shader writes the palette's bytes as linear, + so every fill reads ~4x lighter: Crust (17,17,27) comes out + (73,73,91), measured. Not a palette error and not present + on Android, where the previous pass measured the composer + bar at rgb(41,40,49) for a declared (40,40,46). Worth its + own item; it makes the desktop build a poor colour + reference until fixed. + 2. *A list's wrapped line.* Compose lays an item out as a + marker column beside a text column, so a second line stays + indented; iris writes the marker into the same buffer, so + a wrapped line returns to the left margin. Needs per-line + indent in `TextAttrs`. + 3. *A table.* Compose draws a real grid, cells wrapping at a + 136dp floor; iris draws padded monospace columns. Same + information, different picture. + 4. *Inline code.* Compose draws a chip behind it; iris gives + the range a monospace face and the code colour. Unchanged + by this box -- still blocked on per-range glyph geometry + (IRIS_TODO). + 5. *A user message.* Compose draws it in a rounded card; + iris draws a sender label above plain text. That is the + row's own styling, P1's rather than P1a's. + **One real defect found and fixed on the way**, and it is + not a small one: **`Rect::is_size_independent()` answered + `true`.** A `Rect` fills whatever region it is given, so its + content *is* the region -- and `draw_inner`'s + size-independent fast path, which rewrites a widget's + primitives with `r.outside(&from).within(®ion)` instead of + redrawing, cannot reproduce that remap once a region carries + both `rel` and `abs`. The visible result: a fenced block's + background kept the height of the **provisional full-region + draw** `Span` does in its first phase, so one fence's panel + covered every block below it *and every row below that*, + while the text underneath was laid out correctly. It answers + `false` now (`iris/src/widget/rect.rs`, with the account at + the definition). This is very likely the same family as this + file's older "the composer bar's grey background is not + drawn" note and any other `.background(rect(..))` tint. + **One defect found and left open**, with its repro: + `UiRenderState::reposition`'s debug assert -- *"widget ... is + both moved by its parent's own layout (`mov`) and + repositioned within it"* -- fires from `List::place` when a + transcript row's blocks **wrap**. Reproduce in one line: + change `.wrap(!verbatim)` to `.wrap(true)` in + `transcript-ui/src/row.rs`'s `build_block` and run + `iris/run-headless.sh transcript --shot /tmp/x.png -- -p + transcript-ui`. It is *not* caused by the `Rect` fix above + (it survives it) and not by any one block kind (bisected: it + appears once the row is tall enough). The shipping + configuration does not reach it -- verbatim blocks do not + wrap -- and neither does the Android bench, which ran clean + with the assertions live. It should be the next thing looked + at under P1, because it is a real inconsistency about who + owns a widget's move slot, not a false alarm. + **Bench, stream phase, this checkout's AVD, debug x86_64 + `force-gles`, assertions live, no abort:** + `stream: 294 frames over 21.0s, late 283 (96.3%), p50 53.0ms + p90 108.6ms p99 132.0ms` against the pre-P1a + `p50 52.8ms p90 108.1ms p99 137.3ms` -- unchanged, which is + the point: block styling is span work, not layout work. The + `worst` figure is the one number that moved and it does not + reproduce: 567.3ms, 140.1ms and 664.5ms across three runs of + the same build, against 148.9ms before. Unexplained; it is a + single frame in 294 and the percentiles are flat, so it reads + as an emulator hiccup rather than a cost, but it is written + down rather than rounded off. + **Checks**: `cargo fmt --all --check` clean in both + workspaces; `cargo clippy -p iris -p iris-core -p + transcript-ui -p desktop-app -p tabs-ui --all-targets` + warning-free; `cargo test` 85 (iris, +4) + 13 (iris-core) + + 31 (transcript-ui, +11) + 123 (client-core). - [ ] **P1b — tool-call cards and grouping.** `ToolRows.kt`/ `ToolInput.kt`'s cards: a collapsed row per call with name and a one-line summary, expand to input and output, runs of diff --git a/docs/bench/p1a-2026-09-06/compose-fence-table.png b/docs/bench/p1a-2026-09-06/compose-fence-table.png new file mode 100644 index 0000000..01c84fa Binary files /dev/null and b/docs/bench/p1a-2026-09-06/compose-fence-table.png differ diff --git a/docs/bench/p1a-2026-09-06/compose-heading-fence-table.png b/docs/bench/p1a-2026-09-06/compose-heading-fence-table.png new file mode 100644 index 0000000..be26df5 Binary files /dev/null and b/docs/bench/p1a-2026-09-06/compose-heading-fence-table.png differ diff --git a/docs/bench/p1a-2026-09-06/iris-blocks.png b/docs/bench/p1a-2026-09-06/iris-blocks.png new file mode 100644 index 0000000..6757a1d Binary files /dev/null and b/docs/bench/p1a-2026-09-06/iris-blocks.png differ diff --git a/docs/bench/p1a-2026-09-06/iris-emulator-gles-glyphs.png b/docs/bench/p1a-2026-09-06/iris-emulator-gles-glyphs.png new file mode 100644 index 0000000..c175a71 Binary files /dev/null and b/docs/bench/p1a-2026-09-06/iris-emulator-gles-glyphs.png differ diff --git a/iris/src/widget/rect.rs b/iris/src/widget/rect.rs index 77f93b7..28ea415 100644 --- a/iris/src/widget/rect.rs +++ b/iris/src/widget/rect.rs @@ -47,8 +47,30 @@ impl Widget for Rect { Size::REST // fills whatever it was given -- used == available } + /// **No** -- despite drawing one primitive and nothing else. + /// + /// `is_size_independent` asks whether the widget's *content* is + /// unaffected by how big a region it was given, so that + /// `draw_inner` may keep the primitives it already has and rewrite + /// their regions in place. A `Rect`'s content **is** its region: it + /// returns `Size::REST` and fills whatever it was handed, so the fast + /// path's `r.outside(&from).within(®ion)` remap has to reproduce + /// the whole of `draw` -- and it does not, because a region carries + /// `rel` and `abs` components that the round trip cannot recover + /// separately. + /// + /// What that looked like: a fenced code block's background + /// (`transcript-ui`'s `BlockFrame::Verbatim`, a `Rect` behind a + /// `Pad` in a `Stack`) kept the height of the *provisional* full- + /// region draw `Span` does in its first phase, so one fence's panel + /// covered every block below it -- and every row below that -- while + /// the text itself was laid out correctly. Visible in + /// `docs/bench/p1a-2026-09-06/`'s history and reproduced by this + /// crate's `transcript` example. Answering `false` costs a redraw of + /// one primitive when a rect is resized, which is what the fast path + /// was saving. fn is_size_independent(&self) -> bool { - true // content never depends on region size + false } } diff --git a/iris/transcript-ui/examples/transcript.rs b/iris/transcript-ui/examples/transcript.rs index 67e5989..c5c2e10 100644 --- a/iris/transcript-ui/examples/transcript.rs +++ b/iris/transcript-ui/examples/transcript.rs @@ -91,14 +91,48 @@ fn synthetic_rows() -> Vec { }, ]), msg(6, true, "Looks good, thanks!"), - msg( - 7, - false, - "You're welcome. Let me know if you'd like anything else.", - ), + // Every block kind `client_core::markdown_blocks` names, in one + // row, so P1a's appearance can be looked at against the Compose + // app's without a server (docs/RUST.md's P1a box). The heading, + // paragraph, fence and table are the *same source* the bench + // fixture carries (`app/bench-fixture/generate.py`), so the two + // screenshots differ only in the renderer; the list and the quote + // are extra, because the fixture has neither. + msg(7, false, BLOCK_SAMPLER), ] } +/// One of each markdown block, for the P1a screenshot pair. See +/// [`synthetic_rows`]. +const BLOCK_SAMPLER: &str = "\ +## What changed + +Iris **fold** render measure session window anchor context transcript \ +iris measure iris scroll call transcript layout *cursor* context, and a \ +[bench](https://example.com/bench) link. + +```rust +fn fold_event(items: Vec, seq: u64) -> Vec { + // a comment worth keeping: this is the fold the app's own screen runs + let mut out = items; + out.push(Item::new(seq)); + out +} +``` + +| column | value | +|---|---| +| a | measure place draw tool call token context window anchor | + +- one bullet +- another, with `inline code` + - nested one level +1. first numbered +2. second numbered + +> A quoted line, to show the bar and the indent. +"; + impl DefaultAppState for Client { fn new( mut ui_state: DefaultUiState, diff --git a/iris/transcript-ui/src/row.rs b/iris/transcript-ui/src/row.rs index 0862e0c..9256a1b 100644 --- a/iris/transcript-ui/src/row.rs +++ b/iris/transcript-ui/src/row.rs @@ -273,12 +273,20 @@ where .width(rest(1)) .add_strong(rsc) .any(), - BlockFrame::Quote => ( - rect(crate::markdown::QUOTE_BAR_COLOR).width(dp(QUOTE_BAR_DP)), - field.width(rest(1)), - ) - .span(Dir::RIGHT) - .gap(dp(FRAME_PAD_DP)) + // A `Stack` (through `background`) rather than a two-child + // `Span(Dir::RIGHT)`: the bar is drawn behind text padded past + // it, which is the same picture with one widget fewer and + // without `Span`'s provisional full-region pass. That pass is + // also what first surfaced the `mov`-then-`reposition` assert + // docs/RUST.md's P1a box records as still open, so the shape + // with fewer passes is the one to prefer here. + BlockFrame::Quote => field + .width(rest(1)) + .pad(Padding { + left: dp(QUOTE_BAR_DP + FRAME_PAD_DP), + ..Padding::ZERO + }) + .background(rect(crate::markdown::QUOTE_BAR_COLOR).width(dp(QUOTE_BAR_DP))) .width(rest(1)) .add_strong(rsc) .any(),