Merge remote-tracking branch 'origin/rustify' into worktree-agent-a673ba12761c025d9

This commit is contained in:
iris committed 2026-09-06 21:33:28 -04:00
commit b332873894
9 files changed
+298 -35

No files matched your search

+40 -22
View File
@@ -591,17 +591,23 @@ agent ticks it here with the evidence.
that. Fixed in `iris/src/widget/rect.rs`; the reason is written at the 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 definition. Suspect the same cause for anything else tinted with a
background rect. background rect.
- [ ] **A wrapped transcript row trips `reposition`'s debug assert.** - [x] **A wrapped transcript row tripped `reposition`'s debug assert.**
*"widget ... is both moved by its parent's own layout (`mov`) and Settled 2026-09-06 by giving the move slot one owner instead of two.
repositioned within it"*, raised from `List::place`. Repro: change `mov` accumulates a delta on it, `reposition` overwrote it, and both
`.wrap(!verbatim)` to `.wrap(true)` in `transcript-ui/src/row.rs`'s legitimately land on one widget in one frame: `List::place`'s
`build_block` and run `iris/run-headless.sh transcript --shot Bottom-known branch offers a row a same-size box that has *moved*
/tmp/x.png -- -p transcript-ui`. Survives the `Rect` fix above and is (`mov`), then corrects the placement inside it when the row's cached
not specific to any block kind -- it appears once the row is tall height no longer matches what the row reports (`reposition`). The
enough. The shipping configuration does not reach it (verbatim blocks slot now always means `move_applied + repositioned`
do not wrap) and the Android bench runs clean with assertions live, (`ActiveData::repositioned`, `iris/core/src/ui/render_state.rs`), so
but it is a real disagreement about who owns a widget's move slot and `reposition` adds the move rather than dropping it -- the assert is
should be settled before more of P1 leans on `List`. gone and the arithmetic is right. Test:
`a_widget_moved_by_its_parent_and_then_placed_inside_it_lands_at_the_placement`
in `layout_tests.rs`, which lands the child at the *offered* position
(-100px) instead of the placement (100px) without the fix, and a
`debug_assert_eq!` in `reposition` that nothing but those two ever
writes the slot. Verified with the `.wrap(true)` repro (draws, no
panic) and an emulator bench run with assertions live.
- [ ] **Desktop colours are washed out: the winit surface is sRGB and - [ ] **Desktop colours are washed out: the winit surface is sRGB and
the shader writes the palette's bytes as linear.** Mocha Crust the shader writes the palette's bytes as linear.** Mocha Crust
(17,17,27) is drawn as (73,73,91), measured off (17,17,27) is drawn as (73,73,91), measured off
@@ -609,17 +615,29 @@ agent ticks it here with the evidence.
format rather than the palette -- but it makes the desktop build format rather than the palette -- but it makes the desktop build
useless as a colour reference, which is exactly what P1a needed it for useless as a colour reference, which is exactly what P1a needed it for
when the emulator could not draw glyphs. when the emulator could not draw glyphs.
- [ ] **The emulator cannot draw iris's glyphs.** Under `-gpu host` with - [x] **Every glyph was a solid box on the GLES backend -- iris's bug,
Vulkan disabled (Mesa 26.2.2 / virgl -- what `emu` does on this not the emulator's.** Fixed 2026-09-06. The atlas is one
machine) every character renders as a solid filled box: the atlas `texture_2d_array` and `GpuTextures::new` created it with **one
sample's alpha reads 1, which is what an incomplete GL texture returns layer**; wgpu-hal picks the GL target from the descriptor
(0,0,0,1). Not new (`20303e0` does it too) and not the platform's (`(false, 1) => TEXTURE_2D`), so under GLES that array was a
(Compose draws text perfectly on the same AVD in the same minute). `GL_TEXTURE_2D` bound to the shader's `sampler2DArray`, the unit was
Enabling host Vulkan still dies at boot in gfxstream, and incomplete, every `textureSample` returned (0,0,0,1), and
`EMU_GPU=software` gives SwiftShader Vulkan on which iris **SIGSEGVs `draw_glyph`'s `color.a *= texel.a` filled the quad. `MIN_ARRAY_LAYERS
in `surface_changed`**. Either of the last two would restore = 2` in `iris/core/src/render/texture.rs`, with a `debug_assert!` at
appearance testing on Android; today it has to be done on the desktop `create_array_texture`. Vulkan (the phone, the desktop's default
backend or on Iris's phone. backend) was never affected. Reproduce the class in seconds without an
emulator: `iris`'s `force-gles` feature now switches the **desktop**
backend too -- `./run-headless.sh transcript --shot /tmp/x.png -- -p
transcript-ui --features iris/force-gles`.
- [ ] **The bench report pane draws over the transcript rows instead of
replacing them.** Visible on the emulator for the first time now that
glyphs render there (`/tmp/emu-final.png`, 2026-09-06): after a bench
run the report's lines and the transcript's occupy the same rows in the
top third of the screen, both legible, neither on top. Pre-existing --
the same overlap is in a screenshot taken before the move-slot fix -- so
it is its own item, most likely the report pane not masking or not
claiming its region.
## Build (for the port) ## Build (for the port)
+90 -3
View File
@@ -559,9 +559,12 @@ column above.
reconnect/backoff and cross-thread stream cancellation, which are reconnect/backoff and cross-thread stream cancellation, which are
runtime policy for whichever framework embeds this crate, not pure runtime policy for whichever framework embeds this crate, not pure
logic -- see `CLIENT_CORE.md`'s new section for the full account. logic -- see `CLIENT_CORE.md`'s new section for the full account.
- **Then**: redeliver `~/host/bench/iris-bench-arm64.apk` for Iris with - **Then**: redeliver the APK for Iris. **Delivery is a push to the
its README saying what changed, and record any choice she should see in `~/repos/ai-app-bench` repo** (`iris/build/outputs/apk/release/
`DECISIONS.md`. iris-bench-arm64.apk` plus a dated README section), which Dev Updater
on the host pulls -- **not** `~/host/bench/`, which nothing reads; two
builds on 2026-09-06 went there and never reached her phone. Record any
choice she should see in `DECISIONS.md`.
## Where things stand (2026-09-05) ## Where things stand (2026-09-05)
@@ -5600,6 +5603,90 @@ device.
transcript-ui -p desktop-app -p tabs-ui --all-targets` transcript-ui -p desktop-app -p tabs-ui --all-targets`
warning-free; `cargo test` 85 (iris, +4) + 13 (iris-core) + warning-free; `cargo test` 85 (iris, +4) + 13 (iris-core) +
31 (transcript-ui, +11) + 123 (client-core). 31 (transcript-ui, +11) + 123 (client-core).
**2026-09-06, after P1a: "the emulator cannot draw iris's glyphs"
was iris's bug, not the emulator's.** The finding recorded in the
box above -- that every glyph is a solid filled box under `-gpu
host` GLES and that this is what an incomplete GL texture returns
-- had the mechanism right and the attribution wrong. It is a real
defect on **any** adapter that is GL rather than Vulkan.
- **Reproduced off the emulator entirely**, which is what made it
cheap: `default/render.rs` now honours the same `force-gles`
feature `android/render.rs` did, so
`./run-headless.sh transcript --shot /tmp/x.png -- -p
transcript-ui --features iris/force-gles` draws the boxes on this
machine's own GPU in seconds. Two shader probes then said what
the sample was: `return vec4(texel.rgb, 1.0)` drew black boxes and
`return vec4(texel.a, texel.a, texel.a, 1.0)` drew white ones, so
the atlas sample was exactly (0, 0, 0, 1) -- GL's answer for an
**incomplete texture unit**, and not the null texture (which is
zeroed, alpha 0).
- **Root cause: the glyph atlas array was created with one layer.**
`GpuTextures::new` started `array_capacity` at 1 and `grow_array`
only doubles once a page needs a layer past it, so the ordinary
case -- one atlas page -- is a one-layer array. wgpu-hal picks the
GL texture target from the descriptor alone
(`gles::Texture::get_info_from_desc`: `(false, 1) => TEXTURE_2D`),
so that array is created as a `GL_TEXTURE_2D` and then bound to
the shader's `sampler2DArray`. wgpu has a name for this
(`log_failing_target_heuristics`, its issues #1614/#1574); the
result is an incomplete unit, `texel.a == 1`, and `draw_glyph`'s
`color.a *= texel.a` paints the whole glyph quad.
- **Fix**: `MIN_ARRAY_LAYERS = 2` in
`iris/core/src/render/texture.rs` -- the array is never created
with fewer, with the account at `create_array_texture` and a
`debug_assert!` there so a future capacity arithmetic change fails
at the mistake rather than as boxes on a screen. Cost: one page of
texture memory, which the next atlas page uses anyway.
- **Not a regression from `3e72a4e..20303e0`, and the bisect was not
run.** The defect is a function of the layer count, not of any
commit in that range: it has been there since the atlas became a
`texture_2d_array` (TEXTURES.md, 2026-09-04) and it reproduces at
HEAD and disappears at HEAD with the one-line capacity change. The
claimed "visible text at `3e72a4e`" is a misreading of its own
evidence -- `/tmp/final-typing.png`, the screenshot that entry
cites, is boxes; what the agent read was the `iris text render:
chars=5 glyphs=5` log line, which reports what **parley shaped**,
not what reached the screen. The earlier genuinely-good emulator
shots (`/tmp/after3.png`, 2026-09-05 19:56) predate the APK being
built with `force-gles` at all, so they were the Vulkan path.
- **The phone build is not affected and does not need withdrawing.**
`android-app/build-apk.sh`'s default features are deliberately
without `force-gles` (`d73db97`'s comment), so a phone build takes
`Backends::PRIMARY` -> Vulkan, where a one-layer array is an
ordinary one-layer array and glyphs draw correctly -- which is
also what Iris's phone reports have shown all along. The fix
matters for any device that falls back to GLES, which is why it is
not just an emulator convenience.
- **Appearance testing on Android is back.** `build-apk.sh debug
--abi x86_64 --features "transcript-screen bench force-gles"` on
this checkout's AVD draws the transcript legibly --
`docs/bench/p1a-2026-09-06/iris-emulator-gles-fixed.png` -- so
P1b onwards can be checked here rather than only on the desktop
backend or Iris's phone.
**2026-09-06: the move slot has one owner.** IRIS_TODO's open
"a wrapped transcript row trips `reposition`'s debug assert" is
fixed rather than suppressed. `mov` accumulates a delta on a
widget's move slot and `reposition` overwrote it, and both
legitimately land on one widget in one frame: `List::place`'s
Bottom-known branch offers a row a same-size box that has *moved*
(`mov`), then corrects the placement inside it when the row's
cached height no longer matches what the row reports
(`reposition`) -- measured with a probe on the `.wrap(true)` repro:
`h=1604.7 height=548.7`, the row's own draw having updated
`active.size` without the list's height cache. The slot now means
`move_applied + repositioned` (both on `ActiveData`), so
`reposition` adds the move instead of dropping it and stays
idempotent, and the old assert is replaced by a `debug_assert_eq!`
that nothing *but* those two ever wrote the slot. Test:
`a_widget_moved_by_its_parent_and_then_placed_inside_it_lands_at_the_placement`
(`layout_tests.rs`), which draws the child at the offered position
rather than the placement without the fix. Checks: `cargo fmt --all
--check` clean, `cargo clippy --workspace --all-targets`
warning-free, `cargo test --workspace` 86 (iris, +1) + 13
(iris-core) + 31 (transcript-ui), the `.wrap(true)` repro drawing
correctly, and an emulator bench run with assertions live and no
abort (`2438 frames over 147.7s, p50 27.2ms`).
- [ ] **P1b — tool-call cards and grouping.** `ToolRows.kt`/ - [ ] **P1b — tool-call cards and grouping.** `ToolRows.kt`/
`ToolInput.kt`'s cards: a collapsed row per call with name `ToolInput.kt`'s cards: a collapsed row per call with name
and a one-line summary, expand to input and output, runs of and a one-line summary, expand to input and output, runs of
Binary file not shown.

After

Width:  |  Height:  |  Size: 294 KiB

+5 -1
View File
@@ -66,7 +66,11 @@ send_wrapper = "0.6.0"
# default) or virgl's GLES path, without a second env-var plumbing path that # default) or virgl's GLES path, without a second env-var plumbing path that
# nothing on this machine can hand to an already-launched Android process # nothing on this machine can hand to an already-launched Android process
# (there is no `am start` environment and no system-property reader here to # (there is no `am start` environment and no system-property reader here to
# add one). Android-only; `android/render.rs` is the only reader. # add one). Read by `android/render.rs` and, so the GLES path can be
# reproduced on a machine with a real GPU rather than only in the emulator,
# by `default/render.rs`:
# ./run-headless.sh transcript --shot /tmp/x.png -- -p transcript-ui \
# --features iris/force-gles
force-gles = [] force-gles = []
[dev-dependencies] [dev-dependencies]
+24 -1
View File
@@ -5,6 +5,10 @@ use crate::{PatchRect, TextureKind, TextureUpdate, Textures};
use super::atlas::PAGE; use super::atlas::PAGE;
/// The fewest layers the glyph atlas array is ever created with. Two, not
/// one, for the GLES reason written on `create_array_texture`.
const MIN_ARRAY_LAYERS: u32 = 2;
/// What one texture slot is, GPU-side. Parallel to `Textures`' own slot /// What one texture slot is, GPU-side. Parallel to `Textures`' own slot
/// numbering (`TextureKind`'s `Image`/`Page`), so a slot's index means the /// numbering (`TextureKind`'s `Image`/`Page`), so a slot's index means the
/// same thing on both sides without a second map to keep in sync. /// same thing on both sides without a second map to keep in sync.
@@ -360,7 +364,26 @@ impl GpuTextures {
}) })
} }
/// The atlas is sampled as a `texture_2d_array`, and **a one-layer
/// array is not one on the GLES backend**: wgpu-hal picks the GL
/// texture target from the descriptor alone
/// (`gles::Texture::get_info_from_desc`, `(false, 1) => TEXTURE_2D`),
/// so a capacity of 1 creates a `GL_TEXTURE_2D` and binds it to the
/// shader's `sampler2DArray`. GL then treats that unit as incomplete
/// and every `textureSample` returns (0, 0, 0, 1) -- which, through
/// `draw_glyph`'s `color.a *= texel.a`, draws every glyph as a solid
/// filled box. That was iris's appearance on the emulator's GLES for
/// two days (RUST.md, "the emulator cannot draw iris's glyphs"), and
/// it is a real defect on any device whose adapter is GL rather than
/// Vulkan, not an emulator artifact. So the array never has fewer than
/// `MIN_ARRAY_LAYERS` layers; the second layer costs one page of
/// texture memory and is used by the next atlas page anyway.
fn create_array_texture(device: &Device, capacity: u32) -> Texture { fn create_array_texture(device: &Device, capacity: u32) -> Texture {
debug_assert!(
capacity >= MIN_ARRAY_LAYERS,
"glyph atlas array asked for {capacity} layers; fewer than {MIN_ARRAY_LAYERS} is a \
GL_TEXTURE_2D on the GLES backend and draws every glyph as a box"
);
device.create_texture(&TextureDescriptor { device.create_texture(&TextureDescriptor {
label: Some("glyph atlas array"), label: Some("glyph atlas array"),
size: Extent3d { size: Extent3d {
@@ -382,7 +405,7 @@ impl GpuTextures {
pub fn new(device: &Device, queue: &Queue) -> Self { pub fn new(device: &Device, queue: &Queue) -> Self {
let sampler = default_sampler(device); let sampler = default_sampler(device);
let null_view = null_texture_view(device); let null_view = null_texture_view(device);
let array_capacity = 1; let array_capacity = MIN_ARRAY_LAYERS;
let array_texture = Self::create_array_texture(device, array_capacity); let array_texture = Self::create_array_texture(device, array_capacity);
let array_view = array_texture.create_view(&TextureViewDescriptor { let array_view = array_texture.create_view(&TextureViewDescriptor {
dimension: Some(TextureViewDimension::D2Array), dimension: Some(TextureViewDimension::D2Array),
+13
View File
@@ -53,4 +53,17 @@ pub struct ActiveData {
/// after a finger pan (2026-09-06). Reset to zero whenever the widget /// after a finger pan (2026-09-06). Reset to zero whenever the widget
/// is really redrawn, since `draw_inner` zeroes the slot then too. /// is really redrawn, since `draw_inner` zeroes the slot then too.
pub move_applied: Vec2, pub move_applied: Vec2,
/// The offset the last `Painter::reposition` placed this widget's
/// content at *within* `region`, in window pixels. The move slot has
/// exactly one owner and one meaning:
/// `move_offsets[move_slot] == move_applied + repositioned`. `mov`
/// adds to the first, `reposition` overwrites the second (it
/// recomputes `from` afresh every call, so repeating it must land on
/// the same answer rather than drifting), and both then rewrite the
/// slot from the sum -- which is what lets a parent both move a child
/// with its own layout and place it inside that moved region in one
/// frame. `List::place`'s Bottom-known branch does exactly that once a
/// row's blocks wrap. Reset to zero on a real redraw, with
/// `move_applied` and the slot itself.
pub repositioned: Vec2,
} }
+23 -7
View File
@@ -380,6 +380,7 @@ impl UiRenderState {
move_slot, move_slot,
own_mask, own_mask,
move_applied: Vec2::ZERO, move_applied: Vec2::ZERO,
repositioned: Vec2::ZERO,
}; };
// remove old children that weren't kept // remove old children that weren't kept
@@ -453,12 +454,8 @@ impl UiRenderState {
let Some(active) = self.active.get(&id) else { let Some(active) = self.active.get(&id) else {
return; return;
}; };
debug_assert!( let move_applied = active.move_applied;
active.move_applied == Vec2::ZERO, let repositioned = active.repositioned;
"widget {id:?} is both moved by its parent's own layout (`mov`) and repositioned \
within it; the two write the same slot with different conventions -- see \
`ActiveData::move_applied`"
);
let from = active let from = active
.size .size
.to_uivec2(self.density) .to_uivec2(self.density)
@@ -468,8 +465,27 @@ impl UiRenderState {
let from_px = from.top_left().to_abs(self.output_size); let from_px = from.top_left().to_abs(self.output_size);
let to_px = to.top_left().to_abs(self.output_size); let to_px = to.top_left().to_abs(self.output_size);
let delta = to_px - from_px; let delta = to_px - from_px;
// Not `delta` alone: a parent may have `mov`ed this widget to a
// region that itself moved earlier in the same frame, and that
// part of the slot is `move_applied`'s, not this call's. Writing
// `delta` on its own dropped it and put the content back at the
// pre-move position. `from` is computed against `active.region`,
// which `mov` already updated, so `delta` is purely the placement
// inside the region and the two summands never overlap.
let entry = rsc.ui_mut().move_offsets.get_mut(slot); let entry = rsc.ui_mut().move_offsets.get_mut(slot);
entry.delta = [delta.x, delta.y]; debug_assert_eq!(
entry.delta,
[
move_applied.x + repositioned.x,
move_applied.y + repositioned.y
],
"widget {id:?}'s move slot was written by something other than `mov`/`reposition`; \
the slot is theirs and means `move_applied + repositioned` -- see `ActiveData`"
);
entry.delta = [move_applied.x + delta.x, move_applied.y + delta.y];
if let Some(active) = self.active.get_mut(&id) {
active.repositioned = delta;
}
self.mov_count += 1; self.mov_count += 1;
} }
+10 -1
View File
@@ -85,7 +85,16 @@ impl UiRenderer {
let size = window.inner_size(); let size = window.inner_size();
let instance = Instance::new(&InstanceDescriptor { let instance = Instance::new(&InstanceDescriptor {
backends: Backends::PRIMARY, // `force-gles` on the desktop too, not just on Android: the
// GLES backend has behaviour of its own (a one-layer array
// texture is a `GL_TEXTURE_2D` -- see
// `GpuTextures::create_array_texture`), and a machine with a
// real GPU is where that is cheap to reproduce and screenshot.
backends: if cfg!(feature = "force-gles") {
Backends::GL
} else {
Backends::PRIMARY
},
..Default::default() ..Default::default()
}); });
+93
View File
@@ -562,3 +562,96 @@ fn a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at(
after.top_left after.top_left
); );
} }
/// A parent that both `mov`s a child (its own layout moved the box it
/// offers) and `reposition`s it inside that box in the same frame -- what
/// `List::place`'s Bottom-known branch does once a row's cached height
/// stops matching what the row reports, which is reachable as soon as a
/// transcript row's blocks wrap (docs/IRIS_TODO.md's "Found by P1a").
struct MoveThenPlace {
inner: StrongWidget,
/// Where the child is *offered* a (constant-size) box, moved between
/// frames by the test.
offer_top: f32,
/// Where the child is then placed within this widget's own region.
place_top: f32,
}
impl Widget for MoveThenPlace {
fn draw(&mut self, painter: &mut Painter) -> Size {
let offer = UiRegion::new(
UiSpan::FULL,
UiSpan::new(
UiScalar::abs(self.offer_top),
UiScalar::abs(self.offer_top + 40.0),
),
);
painter.widget_within(&self.inner, offer);
let place = UiRegion::new(
UiSpan::FULL,
UiSpan::new(
UiScalar::abs(self.place_top),
UiScalar::abs(self.place_top + 40.0),
),
);
painter.reposition(&self.inner, place);
Size::default()
}
}
/// `mov` accumulates a delta onto a widget's move slot and `reposition`
/// overwrites it, and both can legitimately land on one widget in one
/// frame (see `MoveThenPlace`). `reposition` used to write its own delta
/// alone, which dropped the move and put the child back at the position
/// the offered box had *before* it moved; a `debug_assert!` that
/// `move_applied` was zero hid that behind a panic instead of fixing it.
/// The slot has one owner and one meaning now --
/// `move_applied + repositioned` -- so the child stays where it was
/// placed however its offered box moves. Fails at the offer's position
/// (200) rather than the placement's (100) without that.
#[test]
fn a_widget_moved_by_its_parent_and_then_placed_inside_it_lands_at_the_placement() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let rect = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let child = rsc.ui.widgets.add_strong(Sized {
inner: rect.any(),
x: None,
y: Some(Len::abs(40.0)),
});
let child_w = child.weak();
let parent = rsc.ui.widgets.add_strong(MoveThenPlace {
inner: child.any(),
offer_top: 0.0,
place_top: 100.0,
});
let parent_w = parent.weak();
let root = parent.any();
let mut render = UiRenderState::new();
render.resize((200.0, 400.0));
render.update(&root, &mut rsc);
let before = render.window_region(&child_w, &rsc).unwrap();
assert!(
(before.top_left.y - 100.0).abs() < 0.01,
"the child should be drawn where it was placed, not where it was offered: {before:?}"
);
// Move the offered box without changing its size (the `mov` fast path)
// and place the child at the same spot as before. Marking the parent
// dirty is what a real container's own content change does; the child
// itself is untouched, which is the case `mov` exists for.
{
let parent = rsc.ui.widgets.get_mut(&parent_w).unwrap();
parent.offer_top = 200.0;
}
rsc.ui.widgets.needs_redraw.insert(parent_w.id());
render.update(&root, &mut rsc);
let after = render.window_region(&child_w, &rsc).unwrap();
assert!(
(after.top_left.y - 100.0).abs() < 0.01,
"the placement did not change, so neither should the child: before={before:?} \
after={after:?}"
);
}