diff --git a/docs/IRIS.md b/docs/IRIS.md index 4610707..dace824 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -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` 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` 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` diff --git a/docs/RUST.md b/docs/RUST.md index e0a36b9..1ac022a 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -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` -- 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, )` 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, diff --git a/iris/Cargo.lock b/iris/Cargo.lock index 8d959ca..f013f3e 100644 --- a/iris/Cargo.lock +++ b/iris/Cargo.lock @@ -1757,6 +1757,7 @@ dependencies = [ "fxhash", "image", "parley", + "pollster", "swash", "wgpu", ] diff --git a/iris/android-app/Cargo.lock b/iris/android-app/Cargo.lock index 592dbfd..09fea5a 100644 --- a/iris/android-app/Cargo.lock +++ b/iris/android-app/Cargo.lock @@ -1785,6 +1785,7 @@ dependencies = [ "fxhash", "image", "parley", + "pollster", "swash", "wgpu", ] diff --git a/iris/android-app/app/src/main/java/dev/iris/android/demo/IrisView.java b/iris/android-app/app/src/main/java/dev/iris/android/demo/IrisView.java index 43a9e8d..9516e43 100644 --- a/iris/android-app/app/src/main/java/dev/iris/android/demo/IrisView.java +++ b/iris/android-app/app/src/main/java/dev/iris/android/demo/IrisView.java @@ -1,6 +1,10 @@ package dev.iris.android.demo; +import android.app.Activity; import android.content.Context; +import android.view.Gravity; +import android.widget.ScrollView; +import android.widget.TextView; import org.linebender.android.rustview.RustView; @@ -33,4 +37,35 @@ public final class IrisView extends RustView { unregisterInsetsNative(mViewPeer); super.onDetachedFromWindow(); } + + /** + * Called from the Rust side (iris/src/android/view.rs's + * `show_renderer_error`) when `AndroidRenderer::new` fails instead of + * drawing -- an ordinary instance method rather than a `native` one, + * since this call is Rust reaching into Java rather than the other + * direction. Replaces the whole activity content with plain, + * selectable, scrollable text rather than leaving the last frame (or a + * blank surface) on screen with no way to report what happened: + * UI_RULES.md's "a failure is reported where it happened, and says + * what to do next." No dialog and no styling beyond what is needed to + * read and copy the text -- this path exists for exactly the crash it + * replaces, so it must not depend on anything that could itself fail + * to render. + */ + void showRendererError(String report) { + Context context = getContext(); + if (!(context instanceof Activity)) { + return; + } + Activity activity = (Activity) context; + TextView text = new TextView(activity); + text.setText(report); + text.setTextIsSelectable(true); + text.setGravity(Gravity.TOP | Gravity.START); + int pad = (int) (16 * activity.getResources().getDisplayMetrics().density); + text.setPadding(pad, pad, pad, pad); + ScrollView scroll = new ScrollView(activity); + scroll.addView(text); + activity.setContentView(scroll); + } } diff --git a/iris/android-app/build-apk.sh b/iris/android-app/build-apk.sh index 97eb699..c4073da 100755 --- a/iris/android-app/build-apk.sh +++ b/iris/android-app/build-apk.sh @@ -12,13 +12,25 @@ # emulator stays on debug" rule -- pass `release` explicitly for a phone # build). --abi defaults to arm64-v8a (a phone/real device); pass # x86_64 for this checkout's own AVD. --features defaults to -# "transcript-screen force-gles bench", P0's exact combination. +# "transcript-screen bench" -- deliberately *without* `force-gles`, unlike +# an earlier version of this default. `force-gles` (`iris/Cargo.toml`'s +# own doc) exists only to force the emulator off its default software +# Vulkan and onto GLES for one specific measurement (RUST.md's I5, "Where +# iris's frame time goes") -- it was never meant to reach a real device, +# but this script's old default put it in every arm64 build regardless, +# so the P0 bench APK delivered to Iris's phone forced GLES there too. +# That is the named hypothesis in RUST.md's P0 box ("iris bench crash on +# the phone, 2026-09-06"): a real Vulkan driver is what a phone should +# run, and GLES is the backend the same box's own SwiftShader finding +# already flagged as the fragile one for this shader's storage buffers. +# Pass `--features "transcript-screen force-gles bench"` explicitly for +# an emulator backend-isolation run; never for a build meant for a phone. set -eu cd "$(dirname "$0")" BUILD_TYPE="debug" ABI="arm64-v8a" -FEATURES="transcript-screen force-gles bench" +FEATURES="transcript-screen bench" case "${1:-}" in debug|release) BUILD_TYPE="$1"; shift ;; esac diff --git a/iris/core/Cargo.toml b/iris/core/Cargo.toml index 5a546de..cfe77c1 100644 --- a/iris/core/Cargo.toml +++ b/iris/core/Cargo.toml @@ -5,6 +5,12 @@ edition.workspace = true [dependencies] wgpu = { workspace = true } +# Only for `UiRenderNode::new`'s `push_error_scope`/`pop_error_scope` pair +# (renderer-creation error reporting, RUST.md's P0 phone-crash box) -- +# `block_on` turns that one async pop into the same synchronous call shape +# `device_limits()`'s two callers already use for `request_adapter`/ +# `request_device`, rather than making this crate's one entry point async. +pollster = { workspace = true } bytemuck ={ workspace = true } image = { workspace = true } parley = { workspace = true } diff --git a/iris/core/src/render/mod.rs b/iris/core/src/render/mod.rs index ee30733..1eb4e79 100644 --- a/iris/core/src/render/mod.rs +++ b/iris/core/src/render/mod.rs @@ -4,6 +4,7 @@ use crate::{ util::{HashMap, Vec2}, }; use data::WindowUniform; +use pollster::FutureExt; use wgpu::{ util::{BufferInitDescriptor, DeviceExt}, *, @@ -251,7 +252,34 @@ impl UiRenderNode { queue.write_buffer(&self.window_buffer, 0, bytemuck::cast_slice(slice)); } - pub fn new(device: &Device, queue: &Queue, config: &SurfaceConfiguration) -> Self { + /// Builds every bind group layout, the pipeline, and the two storage + /// buffers this needs -- fallibly, since this is exactly the call that + /// aborted the process on Iris's phone in a release build with no + /// message beyond "wgpu error: Validation Error" (RUST.md's P0 box, + /// "iris bench crash on the phone, 2026-09-06"). wgpu's own default + /// behaviour for an uncaptured error is `panic!` with no caller able to + /// intervene, so every `create_bind_group_layout`/`create_render_pipeline` + /// call below runs inside three nested error scopes (one per + /// `ErrorFilter`) instead: whichever scope catches something, its + /// `wgpu::Error`'s `Display` is wgpu-core's own `format_error` output + /// (`"Validation Error\n\nCaused by:\n ..."`, the same text the panic + /// would have printed before Android's crash reporter truncated it) and + /// becomes this function's `Err`. Both callers + /// (`android::render::AndroidRenderer::new`, `default::render:: + /// UiRenderer::new`) already call `Device`-creation with + /// `pollster::block_on`, so returning a plain `Result` here rather than + /// making this `async fn` keeps that same synchronous shape. + pub fn new( + device: &Device, + queue: &Queue, + config: &SurfaceConfiguration, + ) -> Result { + // Popped in reverse of this order, once every creation call below + // has run -- `Device::push_error_scope`'s own contract. + let oom_scope = device.push_error_scope(ErrorFilter::OutOfMemory); + let validation_scope = device.push_error_scope(ErrorFilter::Validation); + let internal_scope = device.push_error_scope(ErrorFilter::Internal); + let shader = device.create_shader_module(ShaderModuleDescriptor { label: Some("UI Shape Shader"), source: ShaderSource::Wgsl(SHAPE_SHADER.into()), @@ -373,7 +401,18 @@ impl UiRenderNode { cache: None, }); - Self { + // Reverse of the push order above. Only one of these should ever be + // `Some` in practice -- three separate scopes exist to name *which* + // kind of error it was, not because more than one is expected at + // once. + let internal_err = internal_scope.pop().block_on(); + let validation_err = validation_scope.pop().block_on(); + let oom_err = oom_scope.pop().block_on(); + if let Some(err) = validation_err.or(oom_err).or(internal_err) { + return Err(err.to_string()); + } + + Ok(Self { uniform_group, primitive_layout, rsc_layout, @@ -387,7 +426,7 @@ impl UiRenderNode { move_offsets, masks_layout, masks_group, - } + }) } fn bind_group_0( diff --git a/iris/src/android/render.rs b/iris/src/android/render.rs index 05a1f2c..4dbc68d 100644 --- a/iris/src/android/render.rs +++ b/iris/src/android/render.rs @@ -51,7 +51,20 @@ pub struct AndroidRenderer { } impl AndroidRenderer { - pub fn new(window: NativeWindow, width: u32, height: u32) -> Self { + /// `Err` holds a full, human-readable report -- wgpu's own error text + /// (`UiRenderNode::new`'s doc comment) plus the adapter identity and + /// the limits/downlevel flags bind-group-layout validation checks + /// against -- rather than the panic wgpu's default error handler would + /// otherwise raise with no caller able to see it. This is what aborted + /// the P0 bench APK on Iris's phone with only "wgpu error: Validation + /// Error" surviving into the crash report (RUST.md's P0 box, "iris + /// bench crash on the phone, 2026-09-06"): `create_bind_group_layout` + /// validates against *this* adapter's downlevel capabilities and + /// limits, which a desktop GPU and the emulator's software renderers + /// never exercised. The caller (`android::view::IrisViewPeer:: + /// surface_changed`) logs this one-line-flattened and shows it on + /// screen instead of aborting the process. + pub fn new(window: NativeWindow, width: u32, height: u32) -> Result { // `force-gles` (RUST.md's I5 "Where iris's frame time goes") swaps // the software-Vulkan (SwiftShader) path for GLES/virgl on the same // build, to isolate whether the backend itself explains the frame @@ -84,6 +97,18 @@ impl AndroidRenderer { .block_on() .expect("Could not get adapter!"); + // Requesting the device itself still panics on failure: that is a + // `RequestDeviceError` (a limit or feature the adapter cannot grant + // at all), a different and already-diagnosable failure from the one + // this function now recovers from -- `RUST.md`'s "Software mode ... + // crashes for a third, different reason" is exactly that class, and + // its message already names the limit and the requested/allowed + // values with no truncation risk (it never reaches wgpu's + // uncaptured-error path). What this function's `Result` return + // covers is the *next* class of failure: the adapter grants the + // device, and validation only fails once a specific bind group + // layout is checked against it. + // Same request as the winit backend's `UiRenderer::new` -- no // binding-array features, see TEXTURES.md's "Recommended shape". // `iris_core::device_limits()` is shared between the two backends; @@ -117,16 +142,61 @@ impl AndroidRenderer { surface.configure(&device, &config); let encoder = Self::create_encoder(&device); - let ui = UiRenderNode::new(&device, &queue, &config); + let ui = match UiRenderNode::new(&device, &queue, &config) { + Ok(ui) => ui, + Err(wgpu_error) => return Err(Self::diagnostic(&adapter, &wgpu_error)), + }; - Self { + Ok(Self { surface, device, queue, config, encoder, ui, - } + }) + } + + /// The adapter identity plus every limit and downlevel flag + /// `create_bind_group_layout` validates a storage buffer or texture + /// binding against, followed by wgpu's own error text -- everything a + /// person reading this off a screenshot needs to tell "this adapter + /// lacks X" from "this is a bug in the layout." Named explicitly rather + /// than `{limits:?}`/`{flags:?}` wholesale, because `Limits` alone is + /// dozens of fields nobody asked for -- these are exactly the ones + /// `UiRenderNode::new`'s layouts (`rsc_layout`, `masks_layout`, + /// `primitive_layout`) can fail against, per `CreateBindGroupLayoutError` + /// (`wgpu-core::binding_model`) and its downlevel-flag checks + /// (`wgpu-core::device::resource`, `VERTEX_STORAGE` in particular -- + /// the one storage buffer here, `move_offsets`, that is visible to the + /// vertex stage). + fn diagnostic(adapter: &Adapter, wgpu_error: &str) -> String { + let info = adapter.get_info(); + let limits = adapter.limits(); + let downlevel = adapter.get_downlevel_capabilities(); + format!( + "iris could not start rendering. Copy this text and send it to Iris.\n\n\ + adapter: {name} ({backend:?}), driver: {driver} {driver_info}\n\ + limits: max_storage_buffers_per_shader_stage={max_storage_buffers} \ + max_sampled_textures_per_shader_stage={max_sampled_textures} \ + max_bind_groups={max_bind_groups} \ + max_bindings_per_bind_group={max_bindings} \ + max_storage_buffer_binding_size={max_storage_binding} \ + min_storage_buffer_offset_alignment={min_storage_align}\n\ + downlevel flags: {flags:?}\n\n\ + {wgpu_error}", + name = info.name, + backend = info.backend, + driver = info.driver, + driver_info = info.driver_info, + max_storage_buffers = limits.max_storage_buffers_per_shader_stage, + max_sampled_textures = limits.max_sampled_textures_per_shader_stage, + max_bind_groups = limits.max_bind_groups, + max_bindings = limits.max_bindings_per_bind_group, + max_storage_binding = limits.max_storage_buffer_binding_size, + min_storage_align = limits.min_storage_buffer_offset_alignment, + flags = downlevel.flags, + ) } fn create_encoder(device: &Device) -> CommandEncoder { diff --git a/iris/src/android/view.rs b/iris/src/android/view.rs index ed3128b..e8b205f 100644 --- a/iris/src/android/view.rs +++ b/iris/src/android/view.rs @@ -4,7 +4,11 @@ use accesskit_android::Adapter as AccessAdapter; use android_view::{ AccessibilityNodeInfo, AccessibilityNodeProvider, Bundle, CallbackCtx, Context, InputConnection, KeyEvent, MotionEvent, Rect, View, ViewPeer, - jni::{JNIEnv, JavaVM, objects::GlobalRef, sys::jint}, + jni::{ + JNIEnv, JavaVM, + objects::{GlobalRef, JValue}, + sys::jint, + }, ndk::event::{Keycode, MotionAction}, }; // `marker::Sized` explicitly: `crate::prelude::*` below also brings in the @@ -337,6 +341,31 @@ fn show_soft_input<'local>(env: &mut JNIEnv<'local>, view: &View<'local>) { imm.show_soft_input(env, view, 0); } +/// Replaces the activity's content with a plain, selectable, scrollable +/// text view holding `report` -- the on-screen half of `surface_changed`'s +/// renderer-failure path (UI_RULES.md: "a failure is reported where it +/// happened, and says what to do next," here "copy this and send it"). +/// Goes through an ordinary instance method on the Java side +/// (`IrisView.showRendererError`) rather than a new `native` method: this +/// call is Rust reaching *into* Java, the opposite direction from every +/// `native fn` android-view/`IrisView` declare, and an ordinary virtual +/// call resolves against `ctx.view`'s real runtime class (`IrisView`) the +/// same way any other JNI method call here does. Silently does nothing on +/// any JNI failure -- there is no more-fallback screen to fall back to, +/// and the `log::error!` in `surface_changed` already reached logcat +/// first. +fn show_renderer_error<'local>(env: &mut JNIEnv<'local>, view: &View<'local>, report: &str) { + let Ok(message) = env.new_string(report) else { + return; + }; + let _ = env.call_method( + &view.0, + "showRendererError", + "(Ljava/lang/String;)V", + &[JValue::Object(message.as_ref())], + ); +} + impl ViewPeer for IrisViewPeer { fn on_key_down<'local>( &mut self, @@ -445,8 +474,45 @@ impl ViewPeer for IrisViewPeer { // one from the new window -- see `AndroidRenderer`'s doc comment. let ui_state = self.state.android_state_mut(); ui_state.renderer = None; - ui_state.renderer = Some(AndroidRenderer::new(window, width as u32, height as u32)); - self.render(ctx); + // `AndroidRenderer::new` used to panic here through wgpu's own + // default uncaptured-error handler on a bind-group-layout + // validation failure -- exactly what aborted the P0 bench APK on + // Iris's phone with the message truncated to "wgpu error: + // Validation Error" and nothing else recoverable from the crash + // report (RUST.md's P0 box, "iris bench crash on the phone, + // 2026-09-06"). It now returns the full diagnostic instead; this is + // the one place in the app that can turn it into something a + // person can read, since `ctx.view`/`ctx.env` (needed to reach the + // Java side) are only in scope inside a `ViewPeer` callback. + match AndroidRenderer::new(window, width as u32, height as u32) { + Ok(renderer) => { + self.state.android_state_mut().renderer = Some(renderer); + self.render(ctx); + } + Err(report) => { + // One line for logcat (UI_RULES.md: "the full text for + // whoever can read the log" lives here), the multi-line + // original on screen -- `show_renderer_error` below. + log::error!("iris renderer init failed: {}", report.replace('\n', " | ")); + // Deferred, not called directly: `Activity::setContentView` + // tears the old view hierarchy down synchronously, which + // fires `IrisView`'s own `onFocusChanged` before + // `setContentView` returns -- straight back into this same + // `IrisViewPeer` through `on_focus_changed` while + // `with_peer` (android-view's dispatch, `view.rs` upstream) + // still holds this peer's `RefCell` borrow for the + // `surface_changed` call in progress. Found by inducing a + // validation error and hitting `RefCell already borrowed` + // at exactly that reentrant call (RUST.md's P0 box). + // `push_dynamic_deferred_callback` runs after `with_peer` + // drops the borrow, which is what every other callback in + // this file that reaches into Java already relies on + // (`raise_if_enabled`, above). + ctx.push_dynamic_deferred_callback(move |env, view| { + show_renderer_error(env, view, &report); + }); + } + } } fn surface_destroyed<'local>( diff --git a/iris/src/default/render.rs b/iris/src/default/render.rs index 1da993f..44e9f48 100644 --- a/iris/src/default/render.rs +++ b/iris/src/default/render.rs @@ -141,7 +141,14 @@ impl UiRenderer { let encoder = Self::create_encoder(&device); - let ui = UiRenderNode::new(&device, &queue, &config); + // Unlike the Android backend, the desktop backend has no on-screen + // fallback to show a diagnostic through, so a renderer-creation + // failure still panics here -- but now with wgpu's full "Caused + // by:" chain as the message, since `UiRenderNode::new` returns it + // rather than letting wgpu's own default handler panic first (see + // that function's doc comment). + let ui = UiRenderNode::new(&device, &queue, &config) + .expect("Could not create iris render node!"); Self { surface,