iris: turn the phone bind-group-layout crash into a diagnostic, drop force-gles from phone builds
UiRenderNode::new used to let a wgpu validation error reach the default
uncaptured-error handler and panic, which is what aborted the P0 bench APK
on Iris's phone in AndroidRenderer::new with only "wgpu error: Validation
Error" surviving into the truncated crash report. It now wraps creation in
wgpu error scopes and returns Result<Self, String>; the Android backend
turns a failure into the adapter's identity, the limits/downlevel flags a
layout validates against, and wgpu's own error chain, logged as one logcat
line and shown on screen (IrisView.showRendererError) instead of crashing.
Auditing every bind-group-layout entry against wgpu-core's own validation
source names the likely cause: masks_layout's move_offsets storage buffer
is visible to the vertex stage, which Vulkan grants unconditionally but
GLES gates on the driver's own vertex-stage SSBO support -- and the
delivered APK was built with force-gles, a flag meant only to force the
*emulator* onto GLES for one frame-time measurement, that build-apk.sh's
default feature list applied to every arm64 build regardless of target.
Its default no longer includes force-gles.
Testing the diagnostic (by inducing an artificial validation error) also
found and fixed a real reentrancy bug: calling Activity.setContentView
synchronously from inside a ViewPeer callback re-enters the same peer's
RefCell borrow through onFocusChanged, aborting with "RefCell already
borrowed". Deferred through the same push_dynamic_deferred_callback
mechanism raise_if_enabled already uses.
Full audit, verification, and the named hypothesis are in RUST.md's P0
box ("iris bench crash on the phone, 2026-09-06"); the API change is in
IRIS.md. Nobody on this session has the phone, so this is unconfirmed
against real hardware -- the point of (1) is that the next run says so
either way.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
1 parent
a27fbdb029
commit
46246ea511
11 files changed
+508
-13
No files matched your search
@@ -8,6 +8,35 @@ 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: `UiRenderNode::new` returns `Result`, not `Self` (RUST.md's P0 box, phone-crash fix)
|
||||
|
||||
`iris_core::UiRenderNode::new(device, queue, config)` now returns
|
||||
`Result<Self, String>` instead of `Self`. Why: it used to let a bind-group-
|
||||
layout validation failure reach wgpu's default error handler, which panics
|
||||
with no way for a caller to intervene -- exactly what aborted the P0 bench
|
||||
APK on Iris's phone with the crash report truncated to "wgpu error:
|
||||
Validation Error" and nothing else recoverable. It now runs its creation
|
||||
calls inside wgpu error scopes and returns the full error text (wgpu's own
|
||||
"Caused by" chain) as `Err` instead.
|
||||
|
||||
Both callers changed to match: `android::render::AndroidRenderer::new`
|
||||
itself now returns `Result<Self, String>` too, building a fuller report
|
||||
(adapter identity, the limits/downlevel flags a layout validates against,
|
||||
then wgpu's text) on failure -- its caller,
|
||||
`android::view::IrisViewPeer::surface_changed`, logs that report as one
|
||||
logcat line and shows it on screen (a new `IrisView.showRendererError`,
|
||||
called via an ordinary JNI method call rather than a new `native fn`)
|
||||
instead of letting the process abort. `default::render::UiRenderer::new`
|
||||
(the winit/desktop backend) still panics on failure -- there is no
|
||||
on-screen fallback there -- but the panic message is now the same full
|
||||
text rather than whatever wgpu's own handler would have printed.
|
||||
|
||||
No change for an app that never constructs a `UiRenderNode` directly (every
|
||||
current one goes through `AndroidRenderer`/`UiRenderer`), but anyone who
|
||||
does needs an `?`/`.expect()`/`match` at the call site now. Full audit and
|
||||
the named hypothesis for what actually failed on the phone are in
|
||||
RUST.md's P0 box, "iris bench crash on the phone, 2026-09-06."
|
||||
|
||||
## 2026-09-05: `AndroidAppState::platform_ready` (RUST.md's P0 box, iris half)
|
||||
|
||||
Added a second, optional lifecycle method to `iris::android::AndroidAppState`
|
||||
|
||||
+229
@@ -3906,6 +3906,235 @@ device.
|
||||
a one-line build-date/commit note so Iris can tell which build she
|
||||
has.
|
||||
|
||||
**iris bench crash on the phone, 2026-09-06.** The delivered APK
|
||||
(`dev.iris.android.demo.bench`, arm64, release) aborted on Iris's own
|
||||
phone (a Pixel, GrapheneOS, Mali GPU) on the very first
|
||||
`surface_changed`: `AndroidRenderer::new` -> `UiRenderNode::new` ->
|
||||
`create_bind_group_layout` -> wgpu's `default_error_handler` panics
|
||||
with `wgpu error: Validation Error`, and Android's crash report
|
||||
truncated the message right there, so the actual validation failure
|
||||
was unknown. Ran fine on the emulator's Vulkan (SwiftShader) and GLES
|
||||
(`force-gles`/virgl) and on the desktop GPU. Nobody on this session
|
||||
has the phone or `adb` access to it; this pass worked from the crash
|
||||
report alone plus reading wgpu-core's own validation source
|
||||
(`wgpu-core-28.0.0/src/{binding_model,device/resource}.rs`, the
|
||||
version this workspace pins).
|
||||
|
||||
**1. Diagnostic, not guesswork -- what actually happens is now
|
||||
visible.** `iris_core::UiRenderNode::new` (`core/src/render/mod.rs`)
|
||||
wraps every `create_bind_group_layout`/pipeline call in three nested
|
||||
wgpu error scopes (one per `ErrorFilter`: `OutOfMemory`,
|
||||
`Validation`, `Internal`), pops them in reverse once creation is
|
||||
done, and returns `Result<Self, String>` -- the `String` is wgpu's
|
||||
own `Display` text for whichever scope caught something, which is
|
||||
already wgpu-core's `format_error` output (`"Validation Error\n\n
|
||||
Caused by:\n ..."`, confirmed by reading
|
||||
`wgpu-28.0.0/src/backend/wgpu_core.rs`'s `format_error` -- the exact
|
||||
text the panic would have printed, just no longer thrown away).
|
||||
`android::render::AndroidRenderer::new` turns a failure into a full
|
||||
report: the adapter's name/backend/driver
|
||||
(`Adapter::get_info`), the limits bind-group-layout validation
|
||||
checks a storage/texture binding against
|
||||
(`max_storage_buffers_per_shader_stage`,
|
||||
`max_sampled_textures_per_shader_stage`, `max_bind_groups`,
|
||||
`max_bindings_per_bind_group`, `max_storage_buffer_binding_size`,
|
||||
`min_storage_buffer_offset_alignment`), and
|
||||
`DownlevelCapabilities.flags` (`Adapter::get_downlevel_capabilities`)
|
||||
-- then wgpu's own error text. `android::view::IrisViewPeer::
|
||||
surface_changed` logs it as one logcat line (`iris renderer init
|
||||
failed: ...`, newlines replaced with ` | `) and shows the full
|
||||
multi-line text on screen: a new `IrisView.showRendererError(String)`
|
||||
(an ordinary instance method Rust calls into via JNI, not a `native`
|
||||
one -- the direction is Rust reaching into Java, the opposite of
|
||||
every `native fn` this view already declares) swaps the activity's
|
||||
whole content for a plain, selectable, scrollable `TextView`, opening
|
||||
with "Copy this text and send it to Iris" (UI_RULES.md: a failure is
|
||||
reported where it happened and says what to do next). Desktop's
|
||||
`UiRenderer::new` keeps panicking on failure (no on-screen fallback
|
||||
exists there) but now with wgpu's full chain as the message, since it
|
||||
no longer relies on wgpu's own handler getting there first.
|
||||
|
||||
**A real, separate reentrancy bug turned up while testing this, and
|
||||
is fixed alongside it.** Calling `Activity::setContentView` directly
|
||||
from inside `surface_changed` deadlocked -- not literally, but hit
|
||||
Rust's `RefCell already borrowed` abort: `setContentView` tears the
|
||||
old view hierarchy down synchronously, which fires `IrisView`'s own
|
||||
`onFocusChanged` *before* `setContentView` returns, straight back
|
||||
into the same `IrisViewPeer` through `on_focus_changed` while
|
||||
android-view's own dispatch (`with_peer` in its `view.rs`) still
|
||||
holds this peer's `RefCell` borrow for the `surface_changed` call in
|
||||
progress. Found by deliberately inducing a validation error (see
|
||||
below) and watching it abort a different way than the crash this
|
||||
pass was fixing. Fixed by moving the Java call into
|
||||
`ctx.push_dynamic_deferred_callback`, which android-view already
|
||||
runs only after dropping the borrow (confirmed by reading
|
||||
`with_peer`'s body) -- the same mechanism `raise_if_enabled` (this
|
||||
file's AccessKit push) already relies on for the identical reason.
|
||||
Left as a comment at the call site rather than only here, since the
|
||||
next thing that reaches into Java from inside a `ViewPeer` callback
|
||||
needs the same warning.
|
||||
|
||||
**2. The audit -- every bind-group-layout entry, checked against
|
||||
wgpu-core's actual validation, not guessed.** `CreateBindGroupLayoutError`
|
||||
(`wgpu-core::binding_model`) has seven variants; the ones a static,
|
||||
no-`count`, no-feature layout like this crate's can hit are
|
||||
`Entry { error: MissingDownlevelFlags(_) }` and
|
||||
`Entry { error: MissingFeatures(_) }`. Walked every entry in
|
||||
`uniform_layout`, `primitive_layout`, `rsc_layout`, `masks_layout`
|
||||
(all four in `UiRenderNode::new`):
|
||||
- `uniform_layout` (group 0): one uniform buffer, `VERTEX|FRAGMENT`.
|
||||
Uniform buffers need no downlevel flag or feature at any
|
||||
visibility. Not it.
|
||||
- `primitive_layout` (group 1, `rects`/`glyphs`): two storage
|
||||
buffers, both `FRAGMENT`-only (confirmed against `shader.wgsl`:
|
||||
`rects`/`glyphs` are read only in `fs_main`). `FRAGMENT`-visible
|
||||
storage buffers need `DownlevelFlags::FRAGMENT_STORAGE`, which
|
||||
every backend in wgpu-hal grants unconditionally for a
|
||||
non-write-only binding (`ty: Storage { read_only: true }` here).
|
||||
Not it.
|
||||
- `rsc_layout` (group 2, atlas/image/sampler): a `D2Array` texture,
|
||||
a `D2` texture, a `NonFiltering` sampler, all `FRAGMENT`, no
|
||||
`count`. `Bt::Texture`'s only feature requirement
|
||||
(`TEXTURE_BINDING_ARRAY`) gates on `count.is_some()`, which none
|
||||
of these set. Not it.
|
||||
- `masks_layout` (group 3, `masks`/`move_offsets`): `masks` is
|
||||
`FRAGMENT`-only (read only in `fs_main`'s mask lookup). But
|
||||
`move_offsets` is `VERTEX | FRAGMENT` -- `shader.wgsl`'s
|
||||
`resolve_move` is called from both `vs_main` (a primitive's own
|
||||
corners) and `fs_main` (a mask's move chain) -- and it is a
|
||||
storage buffer, which is exactly what
|
||||
`wgpu-core/src/device/resource.rs` gates on
|
||||
`DownlevelFlags::VERTEX_STORAGE` whenever `visibility` contains
|
||||
`VERTEX` (confirmed by reading that check directly, not inferring
|
||||
it from the flag's name). **This is the one entry among all four
|
||||
layouts whose validity is device-dependent rather than static.**
|
||||
|
||||
**Named hypothesis: the delivered build forced GLES, and GLES's
|
||||
`VERTEX_STORAGE` is not unconditional the way Vulkan's is.**
|
||||
Read `wgpu-hal-28.0.0`'s two backends' own downlevel-flag
|
||||
construction (`vulkan/adapter.rs`, `gles/adapter.rs`):
|
||||
- **Vulkan** grants `Df::VERTEX_STORAGE` unconditionally for any
|
||||
Vulkan 1.0 device, alongside `COMPUTE_SHADERS`/`FRAGMENT_STORAGE`
|
||||
and others in one unconditional `Df::empty() | ... ` -- there is
|
||||
no `.set(VERTEX_STORAGE, <device check>)` call anywhere in that
|
||||
file. This is why the emulator's SwiftShader-Vulkan run and the
|
||||
desktop's real Vulkan both pass: **on Vulkan, this exact layout
|
||||
cannot fail this check, on any conforming device.**
|
||||
- **GLES** computes it explicitly:
|
||||
`downlevel_flags.set(VERTEX_STORAGE, max_storage_block_size != 0
|
||||
&& max_storage_buffers_per_shader_stage != 0 &&
|
||||
(vertex_shader_storage_blocks != 0 || vertex_ssbo_false_zero))` --
|
||||
i.e. it depends on the driver actually reporting a nonzero
|
||||
`GL_MAX_VERTEX_SHADER_STORAGE_BLOCKS`. This is a known-weak spot
|
||||
on Android GLES drivers specifically (vertex-stage SSBO support
|
||||
lags fragment-stage support even on ES 3.1+ hardware), and this
|
||||
exact document already found the adjacent failure mode once this
|
||||
pass: SwiftShader's ES 3.0 GL path reports zero storage-buffer
|
||||
capacity at all (`max_storage_buffer_binding_size: 0`, this box's
|
||||
item 2 under "The three remaining I5 verifications"). A phone
|
||||
that negotiates a GLES context with no (or driver-buggy)
|
||||
vertex-stage SSBO support hits precisely this: `masks_layout`'s
|
||||
`move_offsets` entry, `MissingDownlevelFlags(VERTEX_STORAGE)`.
|
||||
|
||||
**And the delivered build does force GLES.** `~/repos/
|
||||
ai-app-bench`'s own README (committed alongside the P0 APKs)
|
||||
says so directly: "`force-gles` matches I5's own ... finding: the
|
||||
default Vulkan backend has no adapter under a plain `-gpu host`
|
||||
boot on this AVD" -- true and reasoned correctly *for this VM's
|
||||
emulator*, but `build-apk.sh`'s default `FEATURES` applied the
|
||||
same flag to every arm64 build regardless of target, so the exact
|
||||
same cfg-locked GLES-only path that was a deliberate, documented
|
||||
emulator workaround shipped to a real Mali phone with no way to
|
||||
turn it back to Vulkan short of a rebuild. `force-gles`'s own doc
|
||||
comment (`iris/Cargo.toml`) only ever talks about the emulator
|
||||
("the same build can be measured against SwiftShader's software
|
||||
Vulkan ICD ... or virgl's GLES path") -- real hardware was never
|
||||
the case it was written for.
|
||||
|
||||
**What would confirm or kill this, from the on-screen report
|
||||
alone**: `backend: Gl` (confirms GLES was in fact what ran) and
|
||||
`downlevel flags: DownlevelFlags(...)` *not* containing
|
||||
`VERTEX_STORAGE` in the list. If a future report instead shows
|
||||
`backend: Vulkan` with `VERTEX_STORAGE` present, this hypothesis is
|
||||
wrong and the "Caused by" chain in that same report names the real
|
||||
one directly -- which is the entire point of doing (1) first.
|
||||
|
||||
**Fix applied**: `build-apk.sh`'s default `--features` dropped
|
||||
`force-gles` (now `"transcript-screen bench"`, was
|
||||
`"transcript-screen force-gles bench"`), with a comment explaining
|
||||
why and telling a future emulator-isolation run to pass it back
|
||||
explicitly. This is a build/delivery fix, not a shader rewrite --
|
||||
the shader itself is untouched, since moving `move_offsets` off a
|
||||
storage buffer is real scope this document already declined once
|
||||
this pass for the adjacent SwiftShader-ES-3.0 finding, and the
|
||||
actual defect here is that a debug-only backend override reached a
|
||||
real device, not that the shader's design is wrong. The rebuilt
|
||||
arm64 APK (below) uses the default backend, i.e. Vulkan on a real
|
||||
phone -- if it still fails, (1)'s on-screen report is what comes
|
||||
back this time, not a truncated abort.
|
||||
|
||||
**Verified, this checkout's emulator, cold `emu up`
|
||||
(`ai-app-2`):**
|
||||
- GLES (`force-gles`, matching every prior P0 GLES reading's
|
||||
backend): `run-bench.sh` end to end, no crash, report unchanged
|
||||
in shape from the pre-fix readings above
|
||||
(`frames=691 janky%=58.90 p50=19.2ms p90=43.7ms p99=62.4ms
|
||||
worst=69.3ms cpu_p50=4.7ms gpu_wait_p50=12.6ms`, 24/24 swipes,
|
||||
400/400 streamed events) -- the diagnostic wrapper adds no
|
||||
measurable cost or behaviour change on the success path.
|
||||
- **Induced failure, confirmed the fix works end to end**: added a
|
||||
temporary `count: Some(NonZeroU32::new(2))` to `uniform_layout`'s
|
||||
one entry (an artificial `ArrayUnsupported`/`MissingFeatures`,
|
||||
chosen because it is guaranteed to fail on every backend rather
|
||||
than depending on this VM's flaky adapter enumeration), rebuilt,
|
||||
installed, launched: logcat showed the full one-line report
|
||||
(adapter `Android Emulator OpenGL ES Translator (virgl ...)`,
|
||||
every named limit, the downlevel flags, and wgpu's "Caused by"
|
||||
chain naming `Binding 0 entry is invalid` / the missing
|
||||
`BUFFER_BINDING_ARRAY` feature) and `ui-trace elements` confirmed
|
||||
a `TextView` labelled with that exact report text was on screen
|
||||
-- process alive, no abort. This is also what caught the
|
||||
reentrancy bug above (the first attempt aborted a different way,
|
||||
`RefCell already borrowed`, fixed, then reproduced clean). The
|
||||
temporary `count: Some(...)` was reverted before anything else.
|
||||
- **Vulkan (no `force-gles`) could not be re-verified this pass**:
|
||||
this cold `emu up` enumerates zero Vulkan adapters
|
||||
(`wgpu_core::instance: enabled backend 'Vulkan' has no adapters`,
|
||||
the *unrelated*, already-panicking `.expect("Could not get
|
||||
adapter!")` path this fix does not touch) even with the default
|
||||
`-gpu host` boot the same README quoted above once relied on --
|
||||
matching this document's own prior notes that Vulkan
|
||||
availability on this VM's emulator is flaky across cold boots,
|
||||
not something this pass's diff caused (confirmed by checking the
|
||||
panic message is byte-for-byte the pre-existing
|
||||
`RequestAdapterError` shape, not a new one). Not chased further:
|
||||
it is orthogonal to the crash this pass fixes, and the real test
|
||||
of "does Vulkan work" is the phone itself, not this VM.
|
||||
|
||||
**Checks, all clean**: `cargo fmt --all -- --check` and
|
||||
`cargo clippy --workspace --all-targets` (iris workspace, zero
|
||||
warnings beyond the pre-existing wgpu/winit future-incompat
|
||||
notice), `cargo test --workspace` (unchanged counts, all passing --
|
||||
nothing here touched logic under test), `cargo ndk -t x86_64 -P 26
|
||||
clippy --features "transcript-screen force-gles bench" --lib -- -D
|
||||
warnings` (clean), `cargo ndk -t arm64-v8a -P 26 build --release
|
||||
--features "transcript-screen bench"` (clean, arm64-only
|
||||
`jniLibs`).
|
||||
|
||||
**Redelivered, 2026-09-06.** New arm64 APK (Vulkan, no
|
||||
`force-gles`), same `dev.iris.android.demo.bench` id, same
|
||||
`CN=ai-app` signing cert, copied to `~/host/bench/
|
||||
iris-bench-arm64.apk` and `~/repos/ai-app-bench/iris/build/outputs/
|
||||
apk/release/iris-bench-arm64.apk`; that repo's own README gained a
|
||||
dated entry explaining both changes (the diagnostic and the
|
||||
`force-gles` removal) so Iris can tell this build apart from the
|
||||
one that crashed. **Not done this pass**: confirming the fix on
|
||||
Iris's actual phone -- nobody on this session has it or `adb`
|
||||
access to it, so this is read from the crash report and wgpu's
|
||||
source, verified as far as this VM's tooling reaches, and handed
|
||||
back with a diagnostic that will say the real story on the next
|
||||
run either way.
|
||||
|
||||
- [ ] **P1 — session screen parity.** History paging backward (with the
|
||||
page-boundary healing `client-core` does not have yet, below),
|
||||
`TranscriptSource`-backed cache/server stitching, jump-to-latest,
|
||||
|
||||
Reference in new issue
Block a user