diff --git a/iris/src/android/mod.rs b/iris/src/android/mod.rs index f92f80d..a347426 100644 --- a/iris/src/android/mod.rs +++ b/iris/src/android/mod.rs @@ -23,7 +23,7 @@ mod view; pub use insets::Insets; pub use render::AndroidRenderer; pub use view::{ - AndroidAppState, AndroidRsc, AndroidUiState, HasAndroidUiState, IrisViewPeer, LogicalInsets, + AndroidAppState, AndroidRsc, AndroidUiState, HasAndroidUiState, IrisViewPeer, WindowInsets, new_peer, }; diff --git a/iris/src/android/render.rs b/iris/src/android/render.rs index 5cdd85e..3b04648 100644 --- a/iris/src/android/render.rs +++ b/iris/src/android/render.rs @@ -208,14 +208,14 @@ impl AndroidRenderer { surface.configure(&device, &config); let encoder = Self::create_encoder(&device); - // 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) { + // Physical pixels, matching the swapchain's own `width`/`height` + // exactly -- see `android::view::AndroidUiState::content_scale`'s + // field comment for why this is no longer divided into a separate + // logical space (that stopgap is what made text blurry, RUST.md's + // P0 box). `Len::dp` folds the density in at layout time instead, + // so nothing here needs to know it at all. + let window_size = iris_core::util::Vec2::new(width as f32, height as f32); + let ui = match UiRenderNode::new(&device, &queue, &config, window_size) { Ok(ui) => ui, Err(wgpu_error) => return Err(Self::diagnostic(&adapter, &wgpu_error)), }; @@ -398,25 +398,27 @@ 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 + /// Physical pixels -- 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 { - iris_core::util::Vec2::new( - self.config.width as f32 / self.content_scale, - self.config.height as f32 / self.content_scale, - ) + iris_core::util::Vec2::new(self.config.width as f32, self.config.height as f32) } + /// Reconfigures the surface and rewrites the window uniform for a new + /// physical size -- deliberately the *only* two things this does. + /// `device`, `ui`'s atlas, buffers and bind groups are untouched, so a + /// call here (as opposed to a fresh `AndroidRenderer::new`) never + /// invalidates a glyph the CPU-side cache already placed in the atlas. + /// See `android::view::IrisViewPeer::surface_changed`'s doc comment for + /// why that distinction matters -- it is what keeps text on screen + /// across an IME resize. pub fn resize(&mut self, width: u32, height: u32) { self.config.width = width; self.config.height = height; self.surface.configure(&self.device, &self.config); - 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); + let size = iris_core::util::Vec2::new(width as f32, height as f32); + self.ui.resize(size, &self.queue); } } diff --git a/iris/src/android/view.rs b/iris/src/android/view.rs index 77a81da..403ccbb 100644 --- a/iris/src/android/view.rs +++ b/iris/src/android/view.rs @@ -70,19 +70,31 @@ 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. + /// `DisplayMetrics.density` (`new_peer`'s doc comment): physical pixels + /// per dp on this device, read once at view construction and carried + /// on `UiRenderState::density` (`render.set_density`, `new_peer`) from + /// then on -- every `Len::dp` in the widget tree resolves against it at + /// layout time (`Len::dp`'s field doc, IRIS_TODO.md's + /// "density-independent length unit" item, 2026-09-06). + /// + /// **Everything else in this module is physical pixels, matching the + /// real wgpu surface/swapchain resolution** -- window size, touch + /// coordinates, insets. That is a correction from an earlier version + /// of this comment, which had `window_size`/`surface_changed`'s + /// `UiRenderState::resize` call divide by `content_scale` into a + /// *logical* coordinate space instead, as a global stopgap for + /// RUST.md's P0 box's phone report ("text is far too small"). That + /// stopgap fixed the size but not the *sharpness*: dividing to logical + /// units meant a `16.0`-sized glyph rasterised at 16 physical px and + /// then implicitly upscaled ~3x by the NDC mapping onto the real + /// physical framebuffer -- the exact "blurry ... glyphs drawn at + /// logical size and stretched by the scale" Iris reported next. + /// Resolving `dp` at layout time replaces it: a widget author writes + /// `dp(16)` for a size that should look the same physical size on any + /// density, and everything downstream (layout, hit-testing, the window + /// uniform, and the font size handed to the text shaper) works in the + /// display's own physical pixels throughout, so nothing is + /// rasterised at one resolution and displayed at another. pub content_scale: f32, /// The last insets `render()` saw -- compared each frame so /// `AndroidAppState::on_insets_changed` fires only when they actually @@ -154,21 +166,26 @@ pub trait AndroidAppState: HasAndroidUiState { /// (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. + /// `insets` is in the same physical-pixel units everything else in the + /// tree now uses (`AndroidUiState::content_scale`'s field comment), so + /// a widget can add it to a layout size directly -- `dp(...) + + /// abs(insets.top)` if the widget wants a density-independent size + /// plus the system bar's own (already-physical) height. 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) {} + fn on_insets_changed(&mut self, rsc: &mut AndroidRsc, insets: WindowInsets) {} } -/// `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. +/// `insets::Insets` as `f32`, for the widget-facing callback above -- a +/// distinct type from `insets::Insets` so a caller of `on_insets_changed` +/// is not coupled to that module's own (`i32`, JNI-shaped) representation. +/// Both are physical pixels; this used to divide by `content_scale` into a +/// separate *logical* unit (hence the old name, `LogicalInsets`), back when +/// the rest of layout was logical too -- see `AndroidUiState::content_scale`'s +/// field comment for why that stopgap is gone. #[derive(Clone, Copy, Default, Debug, PartialEq)] -pub struct LogicalInsets { +pub struct WindowInsets { pub left: f32, pub top: f32, pub right: f32, @@ -176,14 +193,14 @@ pub struct LogicalInsets { pub ime_bottom: f32, } -impl LogicalInsets { - fn from_physical(insets: Insets, content_scale: f32) -> Self { +impl WindowInsets { + fn from_physical(insets: Insets) -> 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, + left: insets.left as f32, + top: insets.top as f32, + right: insets.right as f32, + bottom: insets.bottom as f32, + ime_bottom: insets.ime_bottom as f32, } } } @@ -343,10 +360,9 @@ impl IrisViewPeer { 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); + let physical = WindowInsets::from_physical(current_insets); self.state.android_state_mut().last_insets = current_insets; - self.state.on_insets_changed(&mut self.rsc, logical); + self.state.on_insets_changed(&mut self.rsc, physical); } let ui_state = self.state.android_state(); @@ -504,16 +520,10 @@ impl ViewPeer for IrisViewPeer { ) -> bool { self.drain_tasks(); let action = event.action_masked(&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; + // Device (physical) pixels, same space layout now uses throughout + // -- see `AndroidUiState::content_scale`'s field comment. + let x = event.x(&mut ctx.env); + let y = event.y(&mut ctx.env); let ui_state = self.state.android_state_mut(); match action { MotionAction::Down => { @@ -564,7 +574,6 @@ impl ViewPeer for IrisViewPeer { height: i32, ) { self.drain_tasks(); - let window = holder.surface(&mut ctx.env).to_native_window(&mut ctx.env); // The layout engine's own notion of the canvas size is separate // from the wgpu surface's -- winit's backend sets it from // `WindowEvent::Resized`, and there is no equivalent automatic @@ -574,29 +583,55 @@ impl ViewPeer for IrisViewPeer { // whatever size `UiRenderState::new` starts at instead of the // surface's real one. // - // **Logical, not physical** -- `content_scale`'s field comment on - // `AndroidUiState`. This call sets `UiRenderState::output_size`, - // which is what every widget's absolute `PixelRegion` (a fixed - // `.height(56)`, in particular) is computed against; `AndroidRenderer`'s - // own `size()`/`resize()`/`new()` already report logical dimensions - // to the *shader*'s window uniform, so leaving this call on raw - // physical `width`/`height` split the two into different units -- - // layout placed a "56"-unit-tall row in an ~2219-tall physical - // canvas (an absolute, correctly-56-unit box), the shader then - // divided that same 56 by a ~845-unit *logical* window dimension, - // and the row rendered far too short rather than too tall or - // right, because a fixed-size item's absolute unit value never - // adapts to the mismatch the way a `rest(n)`-proportional one - // does. Found by measuring a fresh install's top button row at - // ~40 physical px instead of the ~147px `56 * content_scale` - // predicts, immediately after the density fix below was added. - let content_scale = self.state.android_state().content_scale; - self.render - .resize((width as f32 / content_scale, height as f32 / content_scale)); - // Drop the old renderer (and the surface it owns) before building - // one from the new window -- see `AndroidRenderer`'s doc comment. - let ui_state = self.state.android_state_mut(); - ui_state.renderer = None; + // **Physical pixels, matching `AndroidRenderer`'s own + // `size()`/`resize()`/`new()`** -- `AndroidUiState::content_scale`'s + // field comment. This call sets `UiRenderState::output_size`, which + // every `rel`/`rest` length resolves against and every `abs` + // pixel-region compares to directly; a `dp(56)` height now folds + // in the density at `Len::apply_rest` time instead of this call + // dividing the whole window into a separate logical space, which + // is what used to make every `abs`-unit size (a fixed `.height(56)` + // in particular) mean something different from a `rest`-based one. + self.render.resize((width as f32, height as f32)); + + // **Reuse the existing renderer (device, atlas, buffers, bind + // groups) when one is already live -- only reconfigure the + // surface.** `surfaceChanged` fires on *every* size or format + // change, not only on a genuinely new `Surface`/window: showing + // the IME under `adjustResize` resizes the same `SurfaceView` and + // is reported through this exact callback. Rebuilding the whole + // `AndroidRenderer` here used to mean a fresh `UiRenderNode::new` + // -- a brand-new, empty glyph atlas and fresh GPU buffers -- while + // `iris_core`'s CPU-side glyph cache (`primitive/text.rs`) kept the + // atlas coordinates it had already handed out against the *old* + // atlas. Every glyph then drew from a UV rectangle that pointed + // into a texture that had just been recreated empty, so text + // vanished on the first keyboard open while rects (which never go + // through the atlas) kept drawing -- exactly the "rectangles stay, + // glyphs disappear" Iris reported. Confirmed by reading this path + // end to end (no fresh-atlas rebuild anywhere in `resize()` below, + // only in `AndroidRenderer::new`) before changing anything, per + // AGENTS.md's "verify before finishing". + // + // `AndroidRenderer::resize` only reconfigures the wgpu surface and + // rewrites the window uniform -- device, atlas, buffers and bind + // groups are untouched, so the glyph cache's coordinates stay + // valid. A genuinely new surface (after `surface_destroyed`, e.g. + // backgrounding) still goes through `AndroidRenderer::new` below, + // since `renderer` is `None` in that case. + let already_live = self.state.android_state().renderer.is_some(); + if already_live { + let ui_state = self.state.android_state_mut(); + ui_state + .renderer + .as_mut() + .expect("checked Some above") + .resize(width as u32, height as u32); + self.render(ctx); + return; + } + + let window = holder.surface(&mut ctx.env).to_native_window(&mut ctx.env); // `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 @@ -607,6 +642,11 @@ 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. + // + // `content_scale` reaches `AndroidRenderer` only for the + // Diagnostics page's report text now -- window size and the + // shader's window uniform are physical pixels throughout (see the + // `resize` call above), not divided by it. let content_scale = self.state.android_state().content_scale; match AndroidRenderer::new(window, width as u32, height as u32, content_scale) { Ok(renderer) => { @@ -771,15 +811,22 @@ pub fn new_peer<'local, State: AndroidAppState>( state: Default::default(), _state: PhantomData, }; + // See `TextData::density`'s field doc for why this is set alongside + // `render.set_density` below rather than read from there. + rsc.ui.text.density = content_scale; let shared = Rc::new(RefCell::new(Shared::default())); 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(); state.platform_ready(&mut rsc, platform_vm, platform_view); + let mut render = UiRenderState::new(); + // Every `Len::dp` in the tree resolves against this from now on -- see + // `UiRenderState::density`'s field doc and `Len::dp`'s. + render.set_density(content_scale); let peer = IrisViewPeer { rsc, - render: UiRenderState::new(), + render, state, task_recv, };