From 3b80a88f3bf0b79a0ccfa56da72d4bc3e99eec84 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sat, 5 Sep 2026 23:40:30 -0400 Subject: [PATCH] iris/android: content_scale (density), per-frame diagnostics, insets hook Threads DisplayMetrics.density (read once in new_peer, via the Context android-view already hands the JNI entry point) through AndroidUiState as content_scale, and divides by it everywhere a raw device-pixel number used to reach layout unscaled: AndroidRenderer::size()/resize()/new() now report logical (physical / density) dimensions to UiRenderNode and to UiRenderState's own root-layout size, and on_touch_event divides the incoming MotionEvent coordinates the same way, so touch and layout agree on units again. This is the fix for RUST.md's P0 box, "text is far too small" -- a font_size: 16.0 was 16 raw device pixels on a ~3x-density phone, identical to the desktop fix in the previous commit. Installs Device::on_uncaptured_error on the Android device (wgpu's default handler is an unconditional panic outside UiRenderNode::new's own error scopes) into a new iris_core::WgpuErrorLog, and adds AndroidRenderer::diagnostics_report() combining adapter identity, font resolution, atlas view count and the error log into one string for a future Diagnostics screen. render() now logs a one-line diagnostic (masks/moves resized, atlas pages grown, image bind-group creates, wgpu error count) for the first 10 frames after each surface_changed -- the window RUST.md's P0 box says the glyph-wipe-on-first-touch happens in. Adds AndroidAppState::on_insets_changed(rsc, LogicalInsets), called from render() exactly when AndroidUiState::insets() changes (once at startup for the status bar, again on rotation/IME) -- nothing previously read insets().top at all, which is why RUST.md's P0 box found the bench screen's top buttons sitting under the status bar. Co-Authored-By: Claude Fable 5.1 --- iris/src/android/render.rs | 173 +++++++++++++++++++++++++++++++++++-- iris/src/android/view.rs | 133 +++++++++++++++++++++++++--- 2 files changed, 290 insertions(+), 16 deletions(-) diff --git a/iris/src/android/render.rs b/iris/src/android/render.rs index 4dbc68d..4ac6a60 100644 --- a/iris/src/android/render.rs +++ b/iris/src/android/render.rs @@ -48,6 +48,43 @@ pub struct AndroidRenderer { config: SurfaceConfiguration, encoder: CommandEncoder, pub ui: UiRenderNode, + /// The adapter identity, kept past `new()` for the Diagnostics page -- + /// `Adapter` itself is not `Clone`, so the three fields the page shows + /// are copied out once here rather than holding the adapter. + pub adapter_name: String, + pub adapter_backend: Backend, + pub adapter_driver: String, + /// Every uncaptured wgpu error since this renderer was created -- see + /// `iris_core::WgpuErrorLog`'s doc comment. Installed on `device` in + /// `new()`, kept here so the Diagnostics page and the per-frame log in + /// `update()` can both read it without a global. + pub wgpu_errors: iris_core::WgpuErrorLog, + /// Frames drawn on this surface -- what gates the first-10-frames log + /// `update()` writes (RUST.md's P0 box, "the first input frame" + /// investigation): a fresh surface is exactly what Iris's own report + /// says renders correctly at first, so the frames that matter are the + /// first several after each `surface_changed`, not an arbitrary window + /// during a long-running session. + frame_count: u64, + /// Physical pixels per dp -- see `android::view::AndroidUiState:: + /// content_scale`'s field comment for what this feeds. + content_scale: f32, +} + +/// One frame's worth of the counters `render/mod.rs`'s doc comments on +/// `FrameUpdateStats`/`take_image_bind_group_creates`/ +/// `take_atlas_pages_grown` describe -- assembled here because the three +/// live on two different calling conventions (`FrameUpdateStats` from this +/// exact `update()` call; the other two describe the *previous* frame, +/// same as `bench_images`' existing use of them) and a diagnostic reader +/// should not have to know that split. +#[derive(Clone, Copy, Debug, Default)] +pub struct FrameDiagnostics { + pub masks_resized: bool, + pub moves_resized: bool, + /// From the previous frame's `update()` -- see the struct doc. + pub atlas_pages_grown_prev: u64, + pub image_bind_group_creates_prev: u64, } impl AndroidRenderer { @@ -64,7 +101,12 @@ impl AndroidRenderer { /// 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 { + pub fn new( + window: NativeWindow, + width: u32, + height: u32, + content_scale: f32, + ) -> 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 @@ -121,6 +163,30 @@ impl AndroidRenderer { .block_on() .expect("Could not get device!"); + // wgpu's default handler for an error raised outside `UiRenderNode:: + // new`'s own error scopes (i.e. everything past device creation -- + // an ordinary frame's `update`/`draw`) is `panic!`, unconditionally, + // with no caller able to intervene: the same mechanism that aborted + // the P0 bench APK once already, just at a different call site. Log + // and record instead of letting that default stand -- RUST.md's P0 + // box, "every wgpu uncaptured error ... it must never panic in + // release". + let wgpu_errors = iris_core::WgpuErrorLog::default(); + let wgpu_errors_for_handler = wgpu_errors.clone(); + device.on_uncaptured_error(std::sync::Arc::new(move |error| { + log::error!("iris wgpu uncaptured error: {error}"); + wgpu_errors_for_handler.record(error); + })); + + let info = adapter.get_info(); + let adapter_name = info.name.clone(); + let adapter_backend = info.backend; + let adapter_driver = if info.driver_info.is_empty() { + info.driver.clone() + } else { + format!("{} {}", info.driver, info.driver_info) + }; + let surface_caps = surface.get_capabilities(&adapter); let surface_format = surface_caps .formats @@ -142,7 +208,16 @@ impl AndroidRenderer { surface.configure(&device, &config); let encoder = Self::create_encoder(&device); - let ui = match UiRenderNode::new(&device, &queue, &config) { + // Logical size (physical / `content_scale`) -- see + // `android::view::AndroidUiState::content_scale`'s field comment + // for why this crate now divides at all (RUST.md's P0 box, "text + // is far too small"). The swapchain above stays at the real + // physical `width`/`height` for a sharp framebuffer. + let logical_size = iris_core::util::Vec2::new( + width as f32 / content_scale, + height as f32 / content_scale, + ); + let ui = match UiRenderNode::new(&device, &queue, &config, logical_size) { Ok(ui) => ui, Err(wgpu_error) => return Err(Self::diagnostic(&adapter, &wgpu_error)), }; @@ -154,6 +229,12 @@ impl AndroidRenderer { config, encoder, ui, + adapter_name, + adapter_backend, + adapter_driver, + wgpu_errors, + frame_count: 0, + content_scale, }) } @@ -199,14 +280,84 @@ impl AndroidRenderer { ) } + /// The Diagnostics page's whole report: adapter identity, font + /// resolution, the atlas's own view count, every uncaptured wgpu error + /// so far, and the frame report -- RUST.md's P0 box, "a named + /// `Diagnostics` control ... adapter info, limits, fonts found, atlas + /// format/pages, wgpu errors so far, frame report". One string rather + /// than a struct the caller formats, since the only consumer is a + /// plain `TextView` with a "copy this and send it to Iris" affordance, + /// the same shape `surface_changed`'s crash report already uses + /// (UI_RULES.md: a failure -- or here, a state worth reporting -- + /// carries enough to act on where it's shown). + pub fn diagnostics_report( + &self, + font: &iris_core::FontDiagnostics, + frame_report: &str, + ) -> String { + let errors = self.wgpu_errors.snapshot(); + let errors_text = if errors.is_empty() { + "none".to_string() + } else { + errors.join("\n ") + }; + format!( + "iris diagnostics. Copy this text and send it to Iris.\n\n\ + adapter: {name} ({backend:?}), driver: {driver}\n\ + content_scale: {content_scale}\n\ + atlas format: Rgba8Unorm, views live: {views}\n\ + fonts: {families_found} families found, default={default_family:?} \ + mono={default_mono_family:?}\n\ + fonts resolved: regular={regular:?} bold={bold:?} italic={italic:?} \ + mono={mono:?}\n\ + wgpu errors since surface creation:\n {errors_text}\n\n\ + {frame_report}", + name = self.adapter_name, + backend = self.adapter_backend, + driver = self.adapter_driver, + content_scale = self.content_scale, + views = self.ui.view_count(), + families_found = font.families_found, + default_family = font.default_family, + default_mono_family = font.default_mono_family, + regular = font.regular_resolved, + bold = font.bold_resolved, + italic = font.italic_resolved, + mono = font.mono_resolved, + ) + } + fn create_encoder(device: &Device) -> CommandEncoder { device.create_command_encoder(&CommandEncoderDescriptor { label: Some("Render Encoder"), }) } - pub fn update(&mut self, ui: &mut UiData, render: &mut UiRenderState) { - self.ui.update(&self.device, &self.queue, ui, render); + /// Returns what changed this frame -- see `FrameDiagnostics`'s doc + /// comment for why two of its four fields describe the *previous* + /// frame rather than this one. `IrisViewPeer::render` logs this for + /// the first `DIAGNOSTIC_FRAMES` frames after each `surface_changed`, + /// per RUST.md's P0 box ("the first input frame" investigation): the + /// glyph-wipe Iris reported happens on the first tap or scroll after a + /// fresh surface, so that is exactly the window a report needs to + /// cover, not an arbitrary slice of a long session. + pub fn update(&mut self, ui: &mut UiData, render: &mut UiRenderState) -> FrameDiagnostics { + let atlas_pages_grown_prev = self.ui.take_atlas_pages_grown(); + let image_bind_group_creates_prev = self.ui.take_image_bind_group_creates(); + let stats = self.ui.update(&self.device, &self.queue, ui, render); + self.frame_count += 1; + FrameDiagnostics { + masks_resized: stats.masks_resized, + moves_resized: stats.moves_resized, + atlas_pages_grown_prev, + image_bind_group_creates_prev, + } + } + + /// Frames drawn on this surface so far -- see `frame_count`'s field + /// comment. + pub fn frame_count(&self) -> u64 { + self.frame_count } /// Draws and presents one frame, returning the time spent in @@ -249,15 +400,25 @@ impl AndroidRenderer { submit_start.elapsed() } + /// Logical size (physical / `content_scale`) -- the unit layout and + /// hit-testing use, matching the window uniform's own units. See + /// `android::view::AndroidUiState::content_scale`'s field comment. pub fn size(&self) -> iris_core::util::Vec2 { - (self.config.width, self.config.height).into() + iris_core::util::Vec2::new( + self.config.width as f32 / self.content_scale, + self.config.height as f32 / self.content_scale, + ) } pub fn resize(&mut self, width: u32, height: u32) { self.config.width = width; self.config.height = height; self.surface.configure(&self.device, &self.config); - self.ui.resize((width, height), &self.queue); + let logical = iris_core::util::Vec2::new( + width as f32 / self.content_scale, + height as f32 / self.content_scale, + ); + self.ui.resize(logical, &self.queue); } } diff --git a/iris/src/android/view.rs b/iris/src/android/view.rs index e8b205f..6a8ca54 100644 --- a/iris/src/android/view.rs +++ b/iris/src/android/view.rs @@ -33,6 +33,10 @@ use super::{ /// `Option` because a `SurfaceView`'s surface does not outlive backgrounding /// the way a winit `Window` does -- `surfaceDestroyed`/`surfaceCreated` can /// happen any number of times over the life of one `IrisViewPeer`. +/// How many frames after each `surface_changed` `render()` logs a full +/// diagnostic line for -- see the log site's own comment. +const DIAGNOSTIC_FRAMES: u64 = 10; + pub struct AndroidUiState { pub root: Option, pub renderer: Option, @@ -66,10 +70,29 @@ pub struct AndroidUiState { /// because `dumpsys gfxinfo` cannot see a `SurfaceView`'s own /// GPU-drawn frames at all. See `iris_core::FrameReport`'s own doc. pub frame_report: FrameReport, + /// `DisplayMetrics.density` (`new_peer`'s doc comment), read once at + /// view construction: physical pixels per dp on this device. Neither + /// this crate nor `default::` had ever divided by it before RUST.md's + /// P0 box's phone report ("text is far too small") -- `window_size` + /// below and `surface_changed`'s call into `UiRenderState::resize` both + /// report *logical* (physical / `content_scale`) dimensions now, which + /// is what makes a `font_size: 16.0` 16 dp rather than 16 raw device + /// pixels on a ~3x-density phone. The actual wgpu surface/swapchain + /// stays at the real physical resolution (`AndroidRenderer`'s own + /// `config.width/height`) for a sharp framebuffer; only the *logical* + /// coordinate system layout, hit-testing and the window uniform agree + /// on is scaled. Touch coordinates (`on_touch_event`) are divided by + /// this too, so they land in the same space layout is using. + pub content_scale: f32, + /// The last insets `render()` saw -- compared each frame so + /// `AndroidAppState::on_insets_changed` fires only when they actually + /// change (once at startup for the status bar, again if the device + /// rotates), not every frame. + last_insets: Insets, } impl AndroidUiState { - fn new(shared: Rc>) -> Self { + fn new(shared: Rc>, content_scale: f32) -> Self { Self { root: None, renderer: None, @@ -82,6 +105,8 @@ impl AndroidUiState { access_adapter: Default::default(), access: AccessTree::new(), frame_report: FrameReport::new(), + content_scale, + last_insets: Insets::default(), } } @@ -124,6 +149,43 @@ pub trait AndroidAppState: HasAndroidUiState { /// storing them has no effect on that mechanism. #[allow(unused_variables)] fn platform_ready(&mut self, rsc: &mut AndroidRsc, vm: JavaVM, view: GlobalRef) {} + /// Called from `render()` whenever `AndroidUiState::insets()` differs + /// from what it was last frame -- once at startup for the status bar + /// (RUST.md's P0 box: "the status-bar inset is not applied" reported + /// the two top buttons sitting under it, because nothing read `.top` + /// at all), and again on a rotation or the keyboard opening/closing. + /// `insets` is in the same *logical* units `content_scale` converts + /// everything else to (physical / `content_scale`), so a widget can add + /// it to a layout size directly. The default does nothing -- most + /// screens have no chrome that sits under a system bar. + #[allow(unused_variables)] + fn on_insets_changed(&mut self, rsc: &mut AndroidRsc, insets: LogicalInsets) {} +} + +/// `insets::Insets`, converted from physical to logical units -- see +/// `AndroidUiState::content_scale`'s field comment. A distinct type from +/// `insets::Insets` (rather than dividing in place) so a reader at the call +/// site can tell which unit a value is already in without checking where it +/// came from. +#[derive(Clone, Copy, Default, Debug, PartialEq)] +pub struct LogicalInsets { + pub left: f32, + pub top: f32, + pub right: f32, + pub bottom: f32, + pub ime_bottom: f32, +} + +impl LogicalInsets { + fn from_physical(insets: Insets, content_scale: f32) -> Self { + Self { + left: insets.left as f32 / content_scale, + top: insets.top as f32 / content_scale, + right: insets.right as f32 / content_scale, + bottom: insets.bottom as f32 / content_scale, + ime_bottom: insets.ime_bottom as f32 / content_scale, + } + } } /// The android-view analogue of `default::DefaultRsc` -- identical in @@ -270,10 +332,24 @@ impl IrisViewPeer { /// these in until that is root-caused; removing them loses the exact /// evidence a `logcat` capture needs to reproduce the state. fn render(&mut self, ctx: &mut CallbackCtx) { - let ui_state = self.state.android_state(); - if ui_state.renderer.is_none() { + if self.state.android_state().renderer.is_none() { return; } + // See `AndroidAppState::on_insets_changed`'s doc comment: fires + // exactly when insets actually differ from last frame, not every + // frame -- most frames this is one `Insets` equality check against + // a `Copy` struct. Done before `ui_state` is bound below, since + // `on_insets_changed` needs `&mut self.state`/`&mut self.rsc` both. + let ui_state = self.state.android_state(); + let current_insets = ui_state.insets(); + if current_insets != ui_state.last_insets { + let content_scale = ui_state.content_scale; + let logical = LogicalInsets::from_physical(current_insets, content_scale); + self.state.android_state_mut().last_insets = current_insets; + self.state.on_insets_changed(&mut self.rsc, logical); + } + + let ui_state = self.state.android_state(); log::debug!( "render(): root={:?} widgets={} active={} root_px={:?} out_size={:?}", ui_state.root.is_some(), @@ -298,7 +374,27 @@ impl IrisViewPeer { let Some(renderer) = &mut ui_state.renderer else { return; }; - renderer.update(&mut self.rsc.ui, &mut self.render); + let frame_diagnostics = renderer.update(&mut self.rsc.ui, &mut self.render); + // First `DIAGNOSTIC_FRAMES` frames after each `surface_changed` + // only -- RUST.md's P0 box, "the first input frame" investigation: + // the glyph-wipe Iris reported happens on the first tap or scroll + // after a fresh surface, so a report from that window is what + // would show whether an atlas grow, a masks/move_offsets resize, or + // a fresh wgpu error coincided with it. `frame_count()` was just + // incremented inside `update()`, so `<=` counts frame 1 through + // `DIAGNOSTIC_FRAMES` inclusive. + if renderer.frame_count() <= DIAGNOSTIC_FRAMES { + log::info!( + "iris frame diagnostics: frame={} masks_resized={} moves_resized={} \ + atlas_pages_grown_prev={} image_bind_group_creates_prev={} wgpu_errors={}", + renderer.frame_count(), + frame_diagnostics.masks_resized, + frame_diagnostics.moves_resized, + frame_diagnostics.atlas_pages_grown_prev, + frame_diagnostics.image_bind_group_creates_prev, + renderer.wgpu_errors.snapshot().len(), + ); + } let submit_to_present = renderer.draw(); self.state .android_state_mut() @@ -408,8 +504,16 @@ impl ViewPeer for IrisViewPeer { ) -> bool { self.drain_tasks(); let action = event.action_masked(&mut ctx.env); - let x = event.x(&mut ctx.env); - let y = event.y(&mut ctx.env); + // Device (physical) pixels, same as every other Android coordinate + // -- divided so a touch lands in the same *logical* space layout + // now uses (`AndroidUiState::content_scale`'s field comment). + // Without this, `window_size()` reporting logical dims while touch + // stayed physical would land every tap off by exactly the density + // factor on any phone denser than 1x. + let ui_state = self.state.android_state(); + let content_scale = ui_state.content_scale; + let x = event.x(&mut ctx.env) / content_scale; + let y = event.y(&mut ctx.env) / content_scale; let ui_state = self.state.android_state_mut(); match action { MotionAction::Down => { @@ -484,7 +588,8 @@ impl ViewPeer for IrisViewPeer { // 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) { + let content_scale = self.state.android_state().content_scale; + match AndroidRenderer::new(window, width as u32, height as u32, content_scale) { Ok(renderer) => { self.state.android_state_mut().renderer = Some(renderer); self.render(ctx); @@ -623,10 +728,18 @@ impl AccessibilityNodeProvider for IrisViewPeer { /// `register_view_class`, which wants a plain function pointer) -- see /// `iris/android-app/src/lib.rs`. pub fn new_peer<'local, State: AndroidAppState>( - env: JNIEnv<'local>, + mut env: JNIEnv<'local>, view: View<'local>, - _context: Context<'local>, + context: Context<'local>, ) -> android_view::jni::sys::jlong { + // `DisplayMetrics.density` -- physical pixels per dp on this device. + // Read once here, at the one point in this file already handed a + // `Context`, and carried on `AndroidUiState` from then on (see + // `content_scale`'s field comment for what depends on it). + let content_scale = context + .resources(&mut env) + .display_metrics(&mut env) + .density(&mut env); let vm = env.get_java_vm().unwrap(); let global_view = env.new_global_ref(&view.0).unwrap(); let redraw: Arc = Arc::new(AndroidRedrawHandle::new(vm, global_view)); @@ -639,7 +752,7 @@ pub fn new_peer<'local, State: AndroidAppState>( _state: PhantomData, }; let shared = Rc::new(RefCell::new(Shared::default())); - let ui_state = AndroidUiState::new(shared.clone()); + let ui_state = AndroidUiState::new(shared.clone(), content_scale); let mut state = State::new(ui_state, &mut rsc); let platform_vm = env.get_java_vm().unwrap(); let platform_view = env.new_global_ref(&view.0).unwrap();