diff --git a/docs/IRIS.md b/docs/IRIS.md index fdaeffd..230d9c0 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -583,3 +583,46 @@ inset bugs the emulator never showed). Explicit `Arc`-backed value passed to the callback and kept on `AndroidRenderer`, not a global — a caller wanting one on desktop builds its own the same way. + +## 2026-09-06: `Len::dp`, physical pixels throughout, the keyboard glyph wipe + +Iris's phone report on build a9232ac (screenshots): text now the right +size but blurry; the keyboard still wipes every glyph; the header buttons +have nothing behind them. All three are fixed; this entry is the public +API side. docs/LAYOUT.md has the layout-side writeup, docs/RUST.md's P0 +box has the full investigation and the phone verification still to do. + +- **The keyboard wipe was `surface_changed` rebuilding the whole renderer + on every resize**, including an IME-driven one — a fresh, empty glyph + atlas while the CPU-side glyph cache kept UV coordinates from the old + one. `surface_changed` now calls `AndroidRenderer::resize` (reconfigures + the surface and window uniform only) when a renderer is already live, + and only builds a new one when there genuinely isn't one yet. +- **`Len` has a third field, `dp`** (Android's dp / CSS's reference pixel, + 1/160in), beside the existing `abs` (now explicitly *physical* pixels) + and `rel`/`rest`. `len_fns::dp`/`Len::dp` construct one, used exactly + like `abs`/`rel`/`rest` — `dp(16)` instead of a bare `16` wherever a + size should look the same physical size on any density. This is the + unit IRIS_TODO.md's "density-independent length unit" item asked for; + it replaces the previous stopgap (the whole rendered scene divided by + `content_scale` then implicitly stretched back up), which is also what + made text blurry — a glyph rasterised at the small, pre-stretch size and + then upscaled onto the real framebuffer. +- **`UiRenderState`/`Painter` gained `density()`/`set_density()`** (physical + pixels per dp). Every place a length resolves (`Len::apply_rest`, + `Size::to_uivec2`) now takes it; `Span::gap` and `Padding`'s four sides + moved from a bare `f32` to `Len` so they take `dp(...)` too. A bare + number anywhere is unaffected — still `abs`, physical pixels. +- **Text is rasterised at physical resolution now.** `TextBuffer::shape` + takes `density` and multiplies `font_size`/`line_height` (and any span + override) by it before handing them to parley, so the atlas holds a + bitmap at the size it is actually shown at rather than a low-resolution + one stretched afterward. +- **Everything at the Android boundary is physical pixels now** — window + size, touch coordinates, insets (`LogicalInsets` renamed + `WindowInsets`). The previous "logical" division by `content_scale` is + gone; `content_scale` now feeds `set_density` instead. +- Not yet verified on Iris's actual phone (this pass had no device) — + built and checked on this checkout's emulator only. RUST.md's P0 box + says what she should check for: crisp text at two densities, the + keyboard no longer wiping, and the header's background. diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index 5a0159d..d6ec842 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -421,8 +421,19 @@ do not duplicate it there. ## Build (asked for by Iris, 2026-09-06): a density-independent length unit -- [ ] **A third length kind beside relative and pixels, so display scales - "just work".** Iris's words: "another length type similar to absolute & +- [x] **A third length kind beside relative and pixels, so display scales + "just work".** Done 2026-09-06 — `Len::dp`/`len_fns::dp`, resolved + against `UiRenderState`/`Painter::density()` at `apply_rest` time; text + additionally rasterises at the resolved (physical) size instead of + scaling a low-resolution bitmap afterward, which was making text blurry. + `Span::gap`/`Padding` moved from `f32` to `Len` so they take `dp(...)` + too; transcript-ui's row/composer padding and one example migrated. + `em` was not added — nothing in this pass needed a text-relative unit, + and `dp`'s own doc says why it and physical pixels are kept as separate + fields rather than one the caller pre-multiplies. Not yet verified on + Iris's own phone at two densities (this pass had no device) — see + docs/RUST.md's P0 box and docs/IRIS.md's 2026-09-06 entry for what to + check. Iris's words: "another length type similar to absolute & relative, so instead there would be relative, pixels, and another unit like em or whatever is standard. That way different display scales should just work." Today a length is either a fraction of the parent diff --git a/docs/LAYOUT.md b/docs/LAYOUT.md index dfa5b84..f13af43 100644 --- a/docs/LAYOUT.md +++ b/docs/LAYOUT.md @@ -861,6 +861,58 @@ unspecified rather than getting them wrong: conditions, so the remaining slack was accepted rather than chased further. +## Density: `Len::dp`, resolved at `apply_rest` time (2026-09-06) + +Iris asked for a third length kind beside `abs` (physical pixels) and +`rel`/`rest` (a fraction of the parent) — IRIS_TODO.md's "density- +independent length unit" — after the P0 phone pass found 16px text +drawing at roughly a third size on a real phone. The fix that shipped +first (RUST.md's P0 box) was a global stopgap: divide the whole window +into a "logical" coordinate space (physical ÷ `content_scale`) and let +the shader's NDC mapping stretch it back up onto the real framebuffer. +That fixed the *size* but not the *sharpness* — a glyph rasterised at the +small, pre-stretch size and then stretched onto more physical pixels than +it has texels for is blurry, which is exactly what Iris's next report +said. + +**The fix**: `Len` gained a `dp` field, resolved against a `density: f32` +(physical pixels per dp) at the one place a `Len` becomes a `UiScalar` +(`Len::apply_rest`) — `abs + dp * density`. `density` lives on +`UiRenderState` (`set_density`/`density()`) and `Painter` (`density()`), +set once from `DisplayMetrics.density` in `android::view::new_peer`; the +desktop backend has no per-monitor density wired up yet and stays at +`1.0`. Every layout call site that used to call `.apply_rest()`/ +`.to_uivec2()` now passes `painter.density()` (nine call sites — `Span`, +`Sized`, `MaxSize`, `Aligned`, `Scroll`, `List::place`, and +`UiRenderState::reposition` itself). This also meant the Android +boundary's global logical-space stopgap could come out entirely: window +size, touch coordinates and insets are physical pixels again, matching +`AndroidRenderer`'s own swapchain resolution, with `dp` doing the +per-length work the global divide used to do for everything at once. + +**Text is the case that needed more than the `Len` plumbing.** A widget's +`font_size`/`line_height` are plain `f32`, not routed through `Len` at +all (there is no sensible `rel`/`rest` for a font size). `TextBuffer:: +shape` now takes `density` directly and multiplies `font_size`/ +`line_height` (and any span override) by it before handing them to +parley — so the size that reaches both the line-breaker and the +rasteriser (`TextData::place`, which reads back whatever `shape` set) is +the display's *physical* size, and the glyph atlas holds a bitmap at the +resolution it is actually shown at. `GlyphKey.size` already keys on the +resolved size, so a cache entry is naturally per-physical-size with no +further change. The one caller with no `Painter` to read density from +(`TextEditCtx::layout`, cursor movement and hit-testing) reads a second +copy kept directly on `TextData` (`TextData::density`) instead — an +accepted duplication rather than threading a `Painter` into every input +handler for one field, the same tradeoff `AndroidRenderer::content_scale` +already makes for the Diagnostics page. + +**What did not change**: `rel`/`rest` are unaffected (already +resolution-independent, a fraction of the parent). `Span::gap` and +`Padding`'s four sides moved from bare `f32` to `Len` so `dp(...)` works +on them the same as any other size; a bare number is still `abs`, +physical pixels, unchanged. + ## For IRIS.md When this lands, copy this entry into `IRIS.md` (newest first): diff --git a/docs/RUST.md b/docs/RUST.md index 6754c86..52f7e85 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -4438,6 +4438,129 @@ device. apk/release/iris-bench-arm64.apk`; that repo's own README gained a dated entry. Still not confirmed on Iris's actual phone. + **Redelivered again, 2026-09-06, a later pass.** Iris's report on + build a9232ac, with screenshots: text now the right size but + **blurry**; opening the keyboard still **wipes every glyph** + (rects stay, only text disappears); the **header buttons have + nothing behind them and overlap the transcript text**. + + **1. The keyboard wipe.** Hypothesis (given in the task, confirmed + by reading the path before changing anything, per AGENTS.md): + `android::view::IrisViewPeer::surface_changed` fires on *every* + `SurfaceView` size/format change, not only a genuinely new + `Surface` -- showing the IME under `adjustResize` resizes the same + surface through this exact callback. The handler unconditionally + set `renderer = None` and called `AndroidRenderer::new`, which + builds a fresh, empty glyph atlas and fresh GPU buffers via + `UiRenderNode::new`, while `iris_core`'s CPU-side glyph cache + (`primitive/text.rs`) kept the atlas UV coordinates it had already + handed out against the *old* atlas -- every glyph then drew from a + rectangle pointing into a texture that had just been recreated + empty. Confirmed by reading `AndroidRenderer::resize` (already + existed, already did none of that -- only `surface.configure` and + the window uniform) against what `surface_changed` was actually + calling instead. **Fix**: `surface_changed` now calls + `AndroidRenderer::resize` when a renderer is already live, and only + builds a new one when `surface_changed` finds `renderer` still + `None` (a genuinely new surface -- after `surface_destroyed`, e.g. + backgrounding). Not independently re-verified against a forced IME + resize on this pass's emulator (no display keyboard exercised + end-to-end here); the reasoning is a direct code read plus the + existing `resize` path already being surface-only, not a + screenshot diff -- **the next agent with emulator time should do + the before/after screenshot this box originally asked for.** + + **2. The blur.** Root cause: the P0 fix that made text the right + *size* (dividing the whole window into a "logical" space, then + letting the shader's NDC mapping stretch it back onto the real + framebuffer) rasterised each glyph at the small, pre-stretch size + and then displayed it stretched onto more physical pixels than it + had texels for. **Fix, and the density-independent length unit + Iris asked for the same day (IRIS_TODO.md) turned out to be the + same fix**: `Len::dp`, resolved against a `density` now carried on + `UiRenderState`/`Painter`, replaces the global stretch -- window + size, touch and insets are physical pixels throughout again + (`WindowInsets`, renamed from `LogicalInsets`), and + `TextBuffer::shape` multiplies `font_size`/`line_height` by density + before handing them to parley, so the atlas rasterises at the + display's real physical resolution. Full design in docs/LAYOUT.md's + "Density: `Len::dp`" section and the public-API summary in + docs/IRIS.md's 2026-09-06 entry. + + **3. The header.** Only each button's own `rect(...)` painted + anything, so the gaps between/around them and the status-bar strip + above showed `CLEAR_COLOR` (black) one layer back, and the row's + reserved height was three `abs` (now-physical-pixel) button boxes + -- smaller than the dp-correct size the transcript below uses, + which is what read as "overlap" once the two disagreed. Fixed with + a `HEADER_SURFACE` rect stacked behind the whole row and every + header size moved onto `dp(...)`. + + **4. Keyboard diagnostics, so Iris can report back even if a + keyboard-triggered regression persists.** `on_insets_changed` now + edge-triggers ~500ms after `ime_bottom` becomes non-zero, capturing + the same report the on-screen Diagnostics button produces, logging + it, copying it to the clipboard unprompted, and showing it in a new + plain-view overlay (`IrisView.showDiagnosticsOverlay`, Copy/Close) + that draws independently of iris's own renderer. + + **Verified this pass**: `cargo fmt --all`, `cargo clippy --workspace + --all-targets` and `cargo clippy` on `android-app` (both `-D + warnings`, zero beyond the pre-existing `tabs-ui` unused-dependency + and wgpu future-incompat notices), `cargo test --workspace` (all + passing), `cargo ndk -t arm64-v8a check`/`clippy` for both the + `transcript-screen bench` feature set. + + **Then run on this checkout's own emulator** (x86_64 debug, + `--features "transcript-screen force-gles bench"` -- this AVD has no + Vulkan adapter under a plain `-gpu host` boot, matching every prior + emulator finding in this file): `run-bench.sh` end to end, no crash, + `frames=534 janky%=79.03 ... cpu_p50=1.3ms`, 24/24 swipes, 400/400 + streamed events -- unchanged in shape from prior readings, so the + diff cost nothing on the success path. **Header background**: + screenshot confirms the `HEADER_SURFACE` panel now sits behind all + three buttons (`/tmp/bench-after-run.png` this pass). **Keyboard + wipe**: forced a real `surface_changed` two ways -- `adb shell wm + size 1080x1900` (screenshot before/after, text intact) and actually + opening the soft keyboard via `settings put secure + show_ime_with_hard_keyboard 1` + tapping the message field + (ui-trace confirmed a real resize, elements moved -547px; keyboard + visible in the screenshot, text still fully rendered, not wiped). + Both are real evidence the reuse-renderer fix works, though neither + is the literal before/after diff this box originally asked for -- + **still worth a deliberate side-by-side screenshot pair in a future + pass.** + + **Found during this same verification, not fixed, needs a follow-up + pass**: after the keyboard-triggered resize, the top button row + appeared to render a **second time**, well below its real position, + inside the transcript's scroll area (same colours/text, unmistakably + the same three buttons) -- and a tap aimed at the composer's + "Message" field landed on "Run benchmark" instead (a second + benchmark run started, visible in logcat as two `iris bench report:` + lines from one session). Only seen after a resize with the keyboard + genuinely open; the plain `wm size` resize screenshot pair did not + show it, nor did the fresh-install screenshot before either resize. + **Not root-caused this pass** -- time ran out before isolating + whether this is the `Span::DOWN` two-phase draw (LAYOUT.md's + provisional-then-real placement) leaving a phase-1 primitive + retained somewhere it should have been moved from, something + specific to the keyboard's `on_insets_changed` rebuild racing a + redraw, or unrelated to this pass's changes entirely (not verified + against a build predating this session's commits, so do not treat + "caused by this pass" as established -- MACHINE.md's pinned rule + about not attributing without measuring applies here too). Also + noteworthy: `capture_keyboard_diagnostics` never fired in this + session (no "iris keyboard diagnostics" log line) despite the + keyboard visibly opening -- `on_insets_changed`'s `ime_bottom` may + not be populated the way expected on this emulator/API level, or + the duplicate-row state above interfered; **also needs a follow-up + pass** before relying on the auto-capture on a real phone. + + **Not verified this pass**: anything on Iris's real phone, the + two-density crispness check IRIS_TODO.md's unit item asks for, and + the two open items just above. + **Fling and jitter, 2026-09-06.** The two `IRIS_TODO.md` "From the phone" items this box's own text names as follow-ups are fixed -- `List::fling`/`VelocityTracker`/`FlingCalculator` (IRIS.md's 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 9516e43..ef53bfc 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,8 +1,15 @@ package dev.iris.android.demo; import android.app.Activity; +import android.content.ClipData; +import android.content.ClipboardManager; import android.content.Context; import android.view.Gravity; +import android.view.View; +import android.view.ViewGroup; +import android.widget.Button; +import android.widget.FrameLayout; +import android.widget.LinearLayout; import android.widget.ScrollView; import android.widget.TextView; @@ -68,4 +75,81 @@ public final class IrisView extends RustView { scroll.addView(text); activity.setContentView(scroll); } + + private static final String DIAGNOSTICS_OVERLAY_TAG = "iris-diagnostics-overlay"; + + /** + * The bench build's keyboard diagnostics capture + * (`bench_client.rs`'s `on_insets_changed` / + * `capture_keyboard_diagnostics`, via `bench_jni.rs`'s + * `PlatformHandle::show_diagnostics_overlay`): unlike + * `showRendererError` above, this adds a panel *over* this view + * (`MainActivity`'s `FrameLayout` still holds `IrisView` underneath, + * running) rather than replacing the activity's content, and gives it + * a Copy button and a Close that removes the panel -- so it draws + * (and can be read) whether or not iris itself is still putting + * anything on screen, without abandoning the session that produced + * it. Runs on the UI thread regardless of which thread calls it, + * since the call comes from a background task (a delayed capture + * after the keyboard opens), and touching the view tree off the UI + * thread is undefined. + */ + void showDiagnosticsOverlay(String report) { + Context context = getContext(); + if (!(context instanceof Activity)) { + return; + } + Activity activity = (Activity) context; + activity.runOnUiThread(() -> { + ViewGroup parent = (ViewGroup) getParent(); + if (parent == null) { + return; + } + View existing = parent.findViewWithTag(DIAGNOSTICS_OVERLAY_TAG); + if (existing != null) { + parent.removeView(existing); + } + + float density = activity.getResources().getDisplayMetrics().density; + int pad = (int) (16 * density); + + LinearLayout overlay = new LinearLayout(activity); + overlay.setTag(DIAGNOSTICS_OVERLAY_TAG); + overlay.setOrientation(LinearLayout.VERTICAL); + overlay.setBackgroundColor(0xEE000000); + overlay.setPadding(pad, pad, pad, pad); + + TextView text = new TextView(activity); + text.setText(report); + text.setTextIsSelectable(true); + text.setTextColor(0xFFFFFFFF); + ScrollView scroll = new ScrollView(activity); + scroll.addView(text); + overlay.addView(scroll, new LinearLayout.LayoutParams( + LinearLayout.LayoutParams.MATCH_PARENT, 0, 1f)); + + LinearLayout buttonRow = new LinearLayout(activity); + buttonRow.setOrientation(LinearLayout.HORIZONTAL); + buttonRow.setPadding(0, pad, 0, 0); + + Button copy = new Button(activity); + copy.setText("Copy"); + copy.setOnClickListener(v -> { + ClipboardManager clipboard = + (ClipboardManager) activity.getSystemService(Context.CLIPBOARD_SERVICE); + if (clipboard != null) { + clipboard.setPrimaryClip(ClipData.newPlainText("iris diagnostics", report)); + } + }); + Button close = new Button(activity); + close.setText("Close"); + close.setOnClickListener(v -> parent.removeView(overlay)); + buttonRow.addView(copy); + buttonRow.addView(close); + overlay.addView(buttonRow); + + parent.addView(overlay, new FrameLayout.LayoutParams( + FrameLayout.LayoutParams.MATCH_PARENT, FrameLayout.LayoutParams.MATCH_PARENT)); + }); + } } diff --git a/iris/android-app/src/bench_client.rs b/iris/android-app/src/bench_client.rs index 2e98beb..b35da88 100644 --- a/iris/android-app/src/bench_client.rs +++ b/iris/android-app/src/bench_client.rs @@ -74,6 +74,13 @@ pub struct BenchClient { platform: Option>, last_report: Option, running: bool, + /// Edge-triggers the keyboard diagnostics capture below -- set on the + /// first `on_insets_changed` where `ime_bottom > 0.0`, cleared on the + /// first where it is not, so opening the keyboard fires this once + /// rather than on every insets update while it stays open (a rotation + /// or a status-bar change with the keyboard already up would otherwise + /// re-fire it). + keyboard_was_visible: bool, } impl HasAndroidUiState for BenchClient { @@ -184,7 +191,7 @@ impl AndroidAppState for BenchClient { let tree = ( top_bar, content.height(rest(2)), - report_display.height(rest(1)).pad(8), + report_display.height(rest(1)).pad(dp(8)), ) .span(Dir::DOWN) .add_strong(rsc) @@ -219,6 +226,7 @@ impl AndroidAppState for BenchClient { platform: None, last_report: None, running: false, + keyboard_was_visible: false, }; let (backlog, stream_tail) = parse_fixture(); @@ -246,19 +254,79 @@ impl AndroidAppState for BenchClient { /// Pads the top button row by the status-bar inset -- see `top_bar`'s /// field comment. Rebuilds the row rather than mutating a stored /// `Padding` in place, since nothing here holds a handle to one. - fn on_insets_changed(&mut self, rsc: &mut AndroidRsc, insets: iris::android::LogicalInsets) { + /// + /// **Also the trigger for the keyboard diagnostics capture** (RUST.md's + /// P0 box): the IME resizing the surface is exactly the case the + /// previous commit found wiped text, and Iris needs a way to get a + /// report off the phone even if that (or some other keyboard-triggered + /// regression) is still happening on the build she is holding -- + /// `capture_keyboard_diagnostics` below fires ~500ms after the + /// keyboard becomes visible, once per keyboard opening, and shows its + /// report in a plain overlay view that draws independently of + /// whatever iris itself is doing. + fn on_insets_changed( + &mut self, + rsc: &mut AndroidRsc, + insets: iris::android::WindowInsets, + ) { let controls = bench_controls(rsc, insets.top); (self.top_bar)(rsc).set(controls); + + let ime_visible = insets.ime_bottom > 0.0; + if ime_visible && !self.keyboard_was_visible { + self.keyboard_was_visible = true; + let redraw = rsc.tasks.redraw_handle(); + rsc.spawn_task(async move |mut ctx| { + tokio::time::sleep(Duration::from_millis(KEYBOARD_DIAGNOSTICS_DELAY_MS)).await; + ctx.update(|state: &mut BenchClient, rsc| { + state.capture_keyboard_diagnostics(rsc); + }); + redraw.request_redraw(); + }); + } else if !ime_visible { + self.keyboard_was_visible = false; + } } } +/// How long to wait after the keyboard becomes visible before capturing +/// diagnostics -- long enough that the resize, the reported wipe (if it is +/// still happening) and a couple of frames have all had time to land, per +/// AGENTS.md's "so that operations that finish in milliseconds have states +/// on the way that nothing can observe" reasoning applied the other way: +/// this wants to observe the state *after* the transition settles, not +/// mid-flight. +const KEYBOARD_DIAGNOSTICS_DELAY_MS: u64 = 500; + type Rsc = AndroidRsc; -/// `top_pad` is the status-bar inset in logical units (0.0 until +/// The header row's own backdrop -- see `bench_controls`'s doc comment on +/// why it needs one at all. A dark neutral rather than pure black +/// (`android::render::CLEAR_COLOR`) so the row reads as a distinct panel +/// instead of a hole in the background the buttons happen to float in. +const HEADER_SURFACE: UiColor = UiColor::new(28, 28, 34, 255); + +/// `top_pad` is the status-bar inset in physical pixels (0.0 until /// `on_insets_changed` has run once) -- folded in here, rather than /// exposing the unadded builder for a caller to `.pad()` itself, because /// naming that builder's type at each call site is more machinery than a /// top-of-screen padding number is worth. +/// +/// **Backed by an opaque rect the full size of the row, not just the three +/// buttons.** Iris's phone report (docs/RUST.md's P0 box, screenshots on +/// build a9232ac): "the header buttons have nothing behind them and +/// overlap the transcript text" -- before this, only each button's own +/// `rect(...)` painted anything, so the gaps between and around them (and +/// the status-bar strip above them) showed whatever was one layer back +/// (`CLEAR_COLOR`, black), and the row's true height was three +/// physical-pixel-sized (`abs`, not `dp`) button boxes rather than the +/// density-correct size the transcript below was already using post-P0 -- +/// exactly what reads as "overlap" once the two disagree. Fixed two ways +/// together: a `HEADER_SURFACE` rect stacked behind the whole row (this +/// function), and every size below moved from a bare number (physical +/// pixels) to `dp(...)` (IRIS_TODO.md's density-independent length unit), +/// so the row's reserved height in the outer `Span::DOWN` +/// (`AndroidAppState::new`) matches what is actually painted. fn bench_controls(rsc: &mut Rsc, top_pad: f32) -> StrongWidget { let run_rect = rect(Color::rgb(40, 70, 40)) .on( @@ -273,7 +341,7 @@ fn bench_controls(rsc: &mut Rsc, top_pad: f32) -> StrongWidget { wtext("Run benchmark").size(18).text_align(Align::CENTER), ) .stack() - .pad(8) + .pad(dp(8)) .add(rsc); let copy_rect = rect(Color::rgb(50, 50, 60)) @@ -289,7 +357,7 @@ fn bench_controls(rsc: &mut Rsc, top_pad: f32) -> StrongWidget { wtext("Copy report").size(18).text_align(Align::CENTER), ) .stack() - .pad(8) + .pad(dp(8)) .add(rsc); let diag_rect = rect(Color::rgb(60, 45, 70)) @@ -305,12 +373,14 @@ fn bench_controls(rsc: &mut Rsc, top_pad: f32) -> StrongWidget { wtext("Diagnostics").size(18).text_align(Align::CENTER), ) .stack() - .pad(8) + .pad(dp(8)) .add(rsc); - (run, copy, diagnostics) - .span(Dir::RIGHT) - .height(56) + let buttons = (run, copy, diagnostics).span(Dir::RIGHT).add(rsc); + + (rect(HEADER_SURFACE), buttons) + .stack() + .height(dp(56)) .pad(Padding::top(top_pad)) .add_strong(rsc) .any() @@ -350,6 +420,36 @@ impl BenchClient { self.last_report = Some(report); } + /// The keyboard's own diagnostics capture -- see `on_insets_changed`'s + /// doc comment. Reuses `show_diagnostics`'s exact report (so it is the + /// same text the on-screen `Diagnostics` button produces, plus the + /// per-frame log `FrameReport` already keeps around the resize -- + /// `frame_report.report()` above covers "the frames around the + /// resize" without a second accounting mechanism), then does three + /// things the button does not: logs it (so a `logcat` pull gets it + /// even if nothing on screen does), copies it to the clipboard + /// unprompted, and shows it in the shell's plain overlay view, which + /// draws independently of iris's own renderer -- the whole point, + /// since the renderer is exactly what might be in the wiped state + /// this exists to report on. + fn capture_keyboard_diagnostics(&mut self, rsc: &mut Rsc) { + self.show_diagnostics(rsc); + let Some(report) = self.last_report.clone() else { + return; + }; + log::info!("iris keyboard diagnostics:\n{report}"); + let Some(platform) = &self.platform else { + log::info!("iris keyboard diagnostics: no platform handle, can't reach the shell"); + return; + }; + if platform.copy_to_clipboard("iris keyboard diagnostics", &report) { + log::info!("iris keyboard diagnostics: copied to clipboard"); + } else { + log::info!("iris keyboard diagnostics: clipboard copy failed"); + } + platform.show_diagnostics_overlay(&report); + } + fn copy_report(&mut self) { let Some(report) = &self.last_report else { log::info!("iris bench report: nothing to copy -- run the benchmark first"); diff --git a/iris/android-app/src/bench_jni.rs b/iris/android-app/src/bench_jni.rs index 0a5ae4b..b05312e 100644 --- a/iris/android-app/src/bench_jni.rs +++ b/iris/android-app/src/bench_jni.rs @@ -131,4 +131,31 @@ impl PlatformHandle { .ok()?; Some(()) } + + /// Shows `report` in the shell's plain-view diagnostics overlay + /// (`IrisView.showDiagnosticsOverlay`) -- a real `TextView` plus Copy + /// and Close controls, added over whatever iris itself is drawing + /// rather than replacing it (unlike `android::view::show_renderer_error`, + /// which exists for the case the renderer can never recover from and + /// intentionally never returns). Called from a background task after + /// the keyboard-open delay (`bench_client.rs`'s `on_insets_changed`), + /// so the Java side hops onto the UI thread itself before touching the + /// view tree -- see that method's own comment. + pub fn show_diagnostics_overlay(&self, report: &str) -> bool { + self.try_show_diagnostics_overlay(report).is_some() + } + + fn try_show_diagnostics_overlay(&self, report: &str) -> Option<()> { + let mut guard = self.vm.attach_current_thread().ok()?; + let env: &mut JNIEnv = &mut guard; + let jreport = env.new_string(report).ok()?; + env.call_method( + self.view.as_obj(), + "showDiagnosticsOverlay", + "(Ljava/lang/String;)V", + &[JValue::Object(jreport.as_ref())], + ) + .ok()?; + Some(()) + } } diff --git a/iris/core/src/orientation/len.rs b/iris/core/src/orientation/len.rs index 8725211..84c194d 100644 --- a/iris/core/src/orientation/len.rs +++ b/iris/core/src/orientation/len.rs @@ -9,7 +9,31 @@ pub struct Size { #[derive(Debug, Clone, Copy, PartialEq)] pub struct Len { + /// Physical pixels -- a raw device pixel, unaffected by the display's + /// density. Rare to want directly (a hairline border is the usual + /// case); most sizes should be `dp` instead. See `dp`'s own doc for why + /// the two are kept separate rather than one field a caller has to + /// remember to pre-multiply. pub abs: f32, + /// Density-independent pixels -- Android's `dp` / CSS's reference pixel + /// (1 unit = 1/160in), resolved against the display's density at + /// layout time (`apply_rest`'s `density` parameter) rather than at the + /// point a widget is built, since density is a property of the device + /// this ends up running on, not of the widget tree. This is the unit + /// IRIS_TODO.md's "a density-independent length unit" item asked for, + /// 2026-09-06: before it existed, every size in the tree was `abs` + /// (physical pixels), and the only way to make a 16px design draw at + /// the right *size* on a denser display was a single global multiply + /// applied to the whole rendered scene after layout -- which is also + /// what made text blurry (RUST.md's P0 box, "blurry ... glyphs drawn + /// at logical size and stretched by the scale"): a glyph rasterised at + /// 16 physical px and then stretched 3x by that global multiply is a + /// 48px area sampled from a 16px bitmap. Resolving `dp` per-length at + /// layout time instead means the font size handed to the text shaper + /// is already the physical size (`16.0.dp() * 3.0`), so the glyph + /// atlas rasterises at the display's real resolution and nothing + /// downstream needs to stretch anything. + pub dp: f32, pub rel: f32, pub rest: f32, } @@ -67,10 +91,10 @@ impl Size { } } - pub fn to_uivec2(self) -> UiVec2 { + pub fn to_uivec2(self, density: f32) -> UiVec2 { UiVec2 { - x: self.x.apply_rest(), - y: self.y.apply_rest(), + x: self.x.apply_rest(density), + y: self.y.apply_rest(density), } } @@ -98,26 +122,43 @@ impl Size { impl Len { pub const ZERO: Self = Self { abs: 0.0, + dp: 0.0, rel: 0.0, rest: 0.0, }; pub const REST: Self = Self { abs: 0.0, + dp: 0.0, rel: 0.0, rest: 1.0, }; - pub fn apply_rest(&self) -> UiScalar { + /// Resolves to a `UiScalar`, folding `dp` into `abs` pixels against + /// `density` (physical pixels per dp -- 1.0 on a desktop or an + /// unscaled display, `content_scale` on Android; see `dp`'s field + /// doc). Every other component of `Len` is already resolution- + /// independent (`rel` is a fraction of the parent; `rest` becomes a + /// fraction too, below), so `density` only ever touches this one term. + pub fn apply_rest(&self, density: f32) -> UiScalar { UiScalar { rel: self.rel + if self.rest > 0.0 { 1.0 } else { 0.0 }, - abs: self.abs, + abs: self.abs + self.dp * density, } } pub fn abs(abs: impl UiNum) -> Self { Self { abs: abs.to_f32(), + dp: 0.0, + rel: 0.0, + rest: 0.0, + } + } + pub fn dp(dp: impl UiNum) -> Self { + Self { + abs: 0.0, + dp: dp.to_f32(), rel: 0.0, rest: 0.0, } @@ -125,6 +166,7 @@ impl Len { pub fn rel(rel: impl UiNum) -> Self { Self { abs: 0.0, + dp: 0.0, rel: rel.to_f32(), rest: 0.0, } @@ -132,6 +174,7 @@ impl Len { pub fn rest(ratio: impl UiNum) -> Self { Self { abs: 0.0, + dp: 0.0, rel: 0.0, rest: ratio.to_f32(), } @@ -144,6 +187,15 @@ pub mod len_fns { pub fn abs(abs: impl UiNum) -> Len { Len { abs: abs.to_f32(), + dp: 0.0, + rel: 0.0, + rest: 0.0, + } + } + pub fn dp(dp: impl UiNum) -> Len { + Len { + abs: 0.0, + dp: dp.to_f32(), rel: 0.0, rest: 0.0, } @@ -151,6 +203,7 @@ pub mod len_fns { pub fn rel(rel: impl UiNum) -> Len { Len { abs: 0.0, + dp: 0.0, rel: rel.to_f32(), rest: 0.0, } @@ -158,14 +211,15 @@ pub mod len_fns { pub fn rest(ratio: impl UiNum) -> Len { Len { abs: 0.0, + dp: 0.0, rel: 0.0, rest: ratio.to_f32(), } } } -impl_op!(Len Add add; abs rel rest); -impl_op!(Len Sub sub; abs rel rest); +impl_op!(Len Add add; abs dp rel rest); +impl_op!(Len Sub sub; abs dp rel rest); impl_op!(Size Add add; x y); impl_op!(Size Sub sub; x y); @@ -187,6 +241,9 @@ impl std::fmt::Display for Len { if self.abs != 0.0 { write!(f, "{} abs;", self.abs)?; } + if self.dp != 0.0 { + write!(f, "{} dp;", self.dp)?; + } if self.rel != 0.0 { write!(f, "{} rel;", self.rel)?; } diff --git a/iris/core/src/primitive/text.rs b/iris/core/src/primitive/text.rs index 1480d43..2b67b81 100644 --- a/iris/core/src/primitive/text.rs +++ b/iris/core/src/primitive/text.rs @@ -66,6 +66,17 @@ pub struct TextData { pub layout_cx: LayoutContext, scale_cx: ScaleContext, pub atlas: GlyphAtlas, + /// Physical pixels per dp -- a second copy of + /// `UiRenderState::density`, kept here too because `TextEditCtx::layout` + /// (cursor movement and hit-testing, `widget/text/edit.rs`) shapes text + /// from an event callback that has a `TextData` but no `Painter`, so it + /// has nowhere else to read the display's density from. Both copies are + /// set together, from the one place either backend learns the real + /// value (`android::view::new_peer`); this is the same accepted + /// duplication as `AndroidRenderer::content_scale`; a single source of + /// truth would mean carrying a `Painter` (or output size) into every + /// input handler for the sake of one field. + pub density: f32, } impl Default for TextData { @@ -75,6 +86,7 @@ impl Default for TextData { layout_cx: LayoutContext::new(), scale_cx: ScaleContext::new(), atlas: GlyphAtlas::default(), + density: 1.0, }; data.register_bundled_fonts(); data @@ -363,7 +375,7 @@ pub struct TextBuffer { /// `set_spans` forces `shaped` to `None` directly, the same way `edit` /// does, since spans change far less often than a naive equality check /// on the whole `Vec` would cost to compute every frame. - shaped: Option<(TextAttrs, Option)>, + shaped: Option<(TextAttrs, Option, f32)>, } impl TextBuffer { @@ -419,19 +431,42 @@ impl TextBuffer { Vec2::new(self.layout.width(), self.layout.height()) } - /// Lay the text out, unless it is already laid out for these attributes and - /// this width. - pub fn shape(&mut self, data: &mut TextData, attrs: &TextAttrs, width: Option) { - if self.shaped.as_ref() == Some(&(attrs.clone(), width)) { + /// Lay the text out, unless it is already laid out for these + /// attributes, this width and this density. + /// + /// **`attrs.font_size`/`line_height` and every span's own `font_size` + /// are density-independent (dp) units, multiplied by `density` here -- + /// the one place text crosses from the widget tree's dp sizes into the + /// physical pixels the shaper and rasteriser (`TextData::place`) both + /// then work in.** This is what makes glyphs sharp on a dense display: + /// before this existed, `font_size` was already a physical-pixel value + /// (RUST.md's P0 box's global-scale stopgap resolved density by + /// stretching the whole rendered frame afterward instead), so a glyph + /// was rasterised small and then upscaled by whatever the display's + /// scale factor was -- exactly the blur Iris's report described. + /// Multiplying here instead means the font size hitting `ScaleContext` + /// in `place` below is already the display's real physical size, so + /// the atlas holds a bitmap at the resolution it is actually shown at. + /// `GlyphKey.size` already keys on that resolved `font_size` + /// (`(font_size * 16.0).round()`), so a cache entry is naturally per + /// physical size with no change needed there. + pub fn shape( + &mut self, + data: &mut TextData, + attrs: &TextAttrs, + width: Option, + density: f32, + ) { + if self.shaped.as_ref() == Some(&(attrs.clone(), width, density)) { return; } let mut builder = data .layout_cx .ranged_builder(&mut data.font_cx, &self.text, 1.0, true); builder.push_default(StyleProperty::FontFamily(attrs.family.family())); - builder.push_default(StyleProperty::FontSize(attrs.font_size)); + builder.push_default(StyleProperty::FontSize(attrs.font_size * density)); builder.push_default(StyleProperty::LineHeight(LineHeight::Absolute( - attrs.line_height, + attrs.line_height * density, ))); builder.push_default(StyleProperty::Brush(attrs.color)); for span in &self.spans { @@ -443,7 +478,7 @@ impl TextBuffer { builder.push(StyleProperty::FontFamily(family.family()), range.clone()); } if let Some(size) = span.font_size { - builder.push(StyleProperty::FontSize(size), range.clone()); + builder.push(StyleProperty::FontSize(size * density), range.clone()); } if span.bold { builder.push(StyleProperty::FontWeight(FontWeight::BOLD), range.clone()); @@ -459,7 +494,7 @@ impl TextBuffer { self.layout.break_all_lines(width); self.layout .align(Alignment::Start, AlignmentOptions::default()); - self.shaped = Some((attrs.clone(), width)); + self.shaped = Some((attrs.clone(), width, density)); } } @@ -576,8 +611,9 @@ impl TextData { attrs: &TextAttrs, width: Option, textures: &mut Textures, + density: f32, ) -> RenderedText { - buffer.shape(self, attrs, width); + buffer.shape(self, attrs, width, density); let glyphs = self.place(buffer, textures); RenderedText { glyphs: std::sync::Arc::new(glyphs), diff --git a/iris/core/src/ui/painter.rs b/iris/core/src/ui/painter.rs index 801f279..09cd0cf 100644 --- a/iris/core/src/ui/painter.rs +++ b/iris/core/src/ui/painter.rs @@ -165,8 +165,10 @@ impl<'a> Painter<'a> { attrs: &TextAttrs, width: Option, ) -> RenderedText { + let density = self.state.density; let ui = self.rsc.ui_mut(); - ui.text.render(buffer, attrs, width, &mut ui.textures) + ui.text + .render(buffer, attrs, width, &mut ui.textures, density) } /// Draw a laid-out string: one quad per glyph, all sampling the atlas. @@ -210,6 +212,12 @@ impl<'a> Painter<'a> { self.state.output_size } + /// Physical pixels per `dp` -- see `UiRenderState::density`'s field + /// doc. What `Len::dp`'s `apply_rest` call resolves against. + pub fn density(&self) -> f32 { + self.state.density + } + pub fn px_size(&mut self) -> Vec2 { self.region.size().to_abs(self.state.output_size) } diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 31f1183..a799091 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -9,6 +9,12 @@ pub struct UiRenderState { pub active: HashMap, pub layers: PrimitiveLayers, pub(super) output_size: Vec2, + /// Physical pixels per `dp` -- see `Len::dp`'s field doc. `1.0` (an + /// unscaled display) until a backend that knows its own density calls + /// `set_density` (Android's `content_scale`, read at `surface_changed` + /// time); the winit backend has no analogous per-monitor value wired up + /// yet and stays at the default. + pub(super) density: f32, old_root: Option, resized: bool, @@ -35,6 +41,7 @@ impl UiRenderState { active: Default::default(), layers: Default::default(), output_size: Vec2::ZERO, + density: 1.0, old_root: None, resized: false, draw_started: Default::default(), @@ -60,6 +67,20 @@ impl UiRenderState { self.resized = true; } + /// Sets the physical-pixels-per-dp ratio every `Len::dp` in the tree + /// resolves against from the next layout pass on -- see `density`'s + /// field doc. Not folded into `resize` because the two change on + /// different triggers (a surface resize on every rotation or keyboard + /// open; a density change only if the app follows the display to a + /// different screen, which Android surfaces separately). + pub fn set_density(&mut self, density: f32) { + self.density = density; + } + + pub fn density(&self) -> f32 { + self.density + } + pub fn update<'a>(&mut self, root: impl Into>, rsc: &mut dyn UiRsc) { // safety mechanism for memory leaks; might wanna return a result instead so user can // decide whether to panic or not @@ -311,7 +332,7 @@ impl UiRenderState { }; let from = active .size - .to_uivec2() + .to_uivec2(self.density) .align(RegionAlign::TOP_LEFT) .within(&active.region); let slot = active.move_slot; diff --git a/iris/examples/message_list.rs b/iris/examples/message_list.rs index f8a303a..6eb7237 100644 --- a/iris/examples/message_list.rs +++ b/iris/examples/message_list.rs @@ -68,12 +68,15 @@ fn build_row(rsc: &mut Rsc, i: usize) -> StrongWidget { let mut span = Span::empty(Dir::DOWN); span.push(text); span.push(img); - span.pad(8.0).background(rect(tint)).add_strong(rsc).any() + span.pad(dp(8.0)) + .background(rect(tint)) + .add_strong(rsc) + .any() } else { wtext(row_text(i)) .wrap(true) .color(text_color) - .pad(8.0) + .pad(dp(8.0)) .background(rect(tint)) .add_strong(rsc) .any() 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, }; diff --git a/iris/src/widget/list.rs b/iris/src/widget/list.rs index 3f4187c..5732666 100644 --- a/iris/src/widget/list.rs +++ b/iris/src/widget/list.rs @@ -734,9 +734,10 @@ impl List { let axis = self.axis; let output_len = painter.output_size().axis(axis); let container_len = painter.region().axis(axis).len(); + let density = painter.density(); let resolve = move |used: Size| -> f32 { used.axis(axis) - .apply_rest() + .apply_rest(density) .within_len(container_len) .to_abs(output_len) }; diff --git a/iris/src/widget/position/align.rs b/iris/src/widget/position/align.rs index 6581a9c..feb3c06 100644 --- a/iris/src/widget/position/align.rs +++ b/iris/src/widget/position/align.rs @@ -17,14 +17,15 @@ impl Widget for Aligned { // already-resolved region double-applies that composition and is // wrong for any widget nested below the root. let used = painter.widget(&self.inner); + let density = painter.density(); let region = match self.align.tuple() { - (Some(x), Some(y)) => used.to_uivec2().align(RegionAlign { x, y }), + (Some(x), Some(y)) => used.to_uivec2(density).align(RegionAlign { x, y }), (Some(x), None) => { - let x = used.x.apply_rest().align(x); + let x = used.x.apply_rest(density).align(x); UiRegion::new(x, UiSpan::FULL) } (None, Some(y)) => { - let y = used.y.apply_rest().align(y); + let y = used.y.apply_rest(density).align(y); UiRegion::new(UiSpan::FULL, y) } (None, None) => UiRegion::FULL, diff --git a/iris/src/widget/position/max_size.rs b/iris/src/widget/position/max_size.rs index bd56f57..45fed7f 100644 --- a/iris/src/widget/position/max_size.rs +++ b/iris/src/widget/position/max_size.rs @@ -9,12 +9,12 @@ pub struct MaxSize { impl MaxSize { /// Caps a reported length at `max`, comparing in pixels since `Len`'s /// rel/abs/rest components are not otherwise comparable. - fn clamp(len: Len, max: Option, output: f32) -> Len { + fn clamp(len: Len, max: Option, output: f32, density: f32) -> Len { let Some(max) = max else { return len; }; - let len_px = len.apply_rest().to_abs(output); - let max_px = max.apply_rest().to_abs(output); + let len_px = len.apply_rest(density).to_abs(output); + let max_px = max.apply_rest(density).to_abs(output); if len_px > max_px { max } else { len } } @@ -24,11 +24,11 @@ impl MaxSize { /// start, if it does not. Needed so the child is never painted bigger /// than the size this widget reports for it -- see the identical /// requirement noted on `Sized::draw`. - fn clamp_region(offered_px: f32, max: Option, output: f32) -> UiSpan { + fn clamp_region(offered_px: f32, max: Option, output: f32, density: f32) -> UiSpan { let Some(max) = max else { return UiSpan::FULL; }; - let max_scalar = max.apply_rest(); + let max_scalar = max.apply_rest(density); let max_px = max_scalar.to_abs(output); if offered_px > max_px { max_scalar.align(AxisAlign::Neg) @@ -41,15 +41,16 @@ impl MaxSize { impl Widget for MaxSize { fn draw(&mut self, painter: &mut Painter) -> Size { let output = painter.output_size(); + let density = painter.density(); let offered = painter.px_size(); let region = UiRegion { - x: Self::clamp_region(offered.x, self.x, output.x), - y: Self::clamp_region(offered.y, self.y, output.y), + x: Self::clamp_region(offered.x, self.x, output.x, density), + y: Self::clamp_region(offered.y, self.y, output.y, density), }; let used = painter.widget_within(&self.inner, region); Size { - x: Self::clamp(used.x, self.x, output.x), - y: Self::clamp(used.y, self.y, output.y), + x: Self::clamp(used.x, self.x, output.x, density), + y: Self::clamp(used.y, self.y, output.y, density), } } } diff --git a/iris/src/widget/position/pad.rs b/iris/src/widget/position/pad.rs index 065fce8..8e2b3ec 100644 --- a/iris/src/widget/position/pad.rs +++ b/iris/src/widget/position/pad.rs @@ -7,9 +7,12 @@ pub struct Pad { impl Widget for Pad { fn draw(&mut self, painter: &mut Painter) -> Size { - let used = painter.widget_within(&self.inner, self.padding.region()); - let width = self.padding.left + self.padding.right; - let height = self.padding.top + self.padding.bottom; + let density = painter.density(); + let used = painter.widget_within(&self.inner, self.padding.region(density)); + let width = + self.padding.left.apply_rest(density).abs + self.padding.right.apply_rest(density).abs; + let height = + self.padding.top.apply_rest(density).abs + self.padding.bottom.apply_rest(density).abs; Size { x: used.x + Len::abs(width), y: used.y + Len::abs(height), @@ -17,23 +20,29 @@ impl Widget for Pad { } } +/// Each side is a `Len`, not a bare `f32`, so `.pad(dp(10))` resolves +/// against the display's density the same way any other size does -- see +/// `Len::dp`'s field doc. `.pad(10)` (a bare number) still works via +/// `From` below, unchanged: it becomes an `abs` (physical-pixel) +/// `Len`, exactly as a bare number always has meant elsewhere in this +/// crate. pub struct Padding { - pub left: f32, - pub right: f32, - pub top: f32, - pub bottom: f32, + pub left: Len, + pub right: Len, + pub top: Len, + pub bottom: Len, } impl Padding { pub const ZERO: Self = Self { - left: 0.0, - right: 0.0, - top: 0.0, - bottom: 0.0, + left: Len::ZERO, + right: Len::ZERO, + top: Len::ZERO, + bottom: Len::ZERO, }; - pub fn uniform(amt: impl UiNum) -> Self { - let amt = amt.to_f32(); + pub fn uniform(amt: impl Into) -> Self { + let amt = amt.into(); Self { left: amt, right: amt, @@ -41,80 +50,84 @@ impl Padding { bottom: amt, } } - pub fn region(&self) -> UiRegion { + pub fn region(&self, density: f32) -> UiRegion { let mut region = UiRegion::FULL; - region.x.start.abs += self.left; - region.y.start.abs += self.top; - region.x.end.abs -= self.right; - region.y.end.abs -= self.bottom; + region.x.start.abs += self.left.apply_rest(density).abs; + region.y.start.abs += self.top.apply_rest(density).abs; + region.x.end.abs -= self.right.apply_rest(density).abs; + region.y.end.abs -= self.bottom.apply_rest(density).abs; region } - pub fn x(amt: impl UiNum) -> Self { - let amt = amt.to_f32(); + pub fn x(amt: impl Into) -> Self { + let amt = amt.into(); Self { left: amt, right: amt, - top: 0.0, - bottom: 0.0, + top: Len::ZERO, + bottom: Len::ZERO, } } - pub fn y(amt: impl UiNum) -> Self { - let amt = amt.to_f32(); + pub fn y(amt: impl Into) -> Self { + let amt = amt.into(); Self { - left: 0.0, - right: 0.0, + left: Len::ZERO, + right: Len::ZERO, top: amt, bottom: amt, } } - pub fn top(amt: impl UiNum) -> Self { + pub fn top(amt: impl Into) -> Self { let mut s = Self::ZERO; - s.top = amt.to_f32(); + s.top = amt.into(); s } - pub fn bottom(amt: impl UiNum) -> Self { + pub fn bottom(amt: impl Into) -> Self { let mut s = Self::ZERO; - s.bottom = amt.to_f32(); + s.bottom = amt.into(); s } - pub fn left(amt: impl UiNum) -> Self { + pub fn left(amt: impl Into) -> Self { let mut s = Self::ZERO; - s.left = amt.to_f32(); + s.left = amt.into(); s } - pub fn right(amt: impl UiNum) -> Self { + pub fn right(amt: impl Into) -> Self { let mut s = Self::ZERO; - s.right = amt.to_f32(); + s.right = amt.into(); s } - pub fn with_top(mut self, amt: impl UiNum) -> Self { - self.top = amt.to_f32(); + pub fn with_top(mut self, amt: impl Into) -> Self { + self.top = amt.into(); self } - pub fn with_bottom(mut self, amt: impl UiNum) -> Self { - self.bottom = amt.to_f32(); + pub fn with_bottom(mut self, amt: impl Into) -> Self { + self.bottom = amt.into(); self } - pub fn with_left(mut self, amt: impl UiNum) -> Self { - self.left = amt.to_f32(); + pub fn with_left(mut self, amt: impl Into) -> Self { + self.left = amt.into(); self } - pub fn with_right(mut self, amt: impl UiNum) -> Self { - self.right = amt.to_f32(); + pub fn with_right(mut self, amt: impl Into) -> Self { + self.right = amt.into(); self } } -impl From for Padding { +/// Covers both a bare number (`.pad(8)`, via `Len`'s own `From` +/// blanket -- an `abs`/physical-pixel `Len`) and a `Len` directly +/// (`.pad(dp(10))`) with the one impl, since `Len: Into` is the +/// reflexive case of the same bound. +impl> From for Padding { fn from(amt: T) -> Self { - Self::uniform(amt.to_f32()) + Self::uniform(amt.into()) } } diff --git a/iris/src/widget/position/scroll.rs b/iris/src/widget/position/scroll.rs index 4c837e5..e35f8d2 100644 --- a/iris/src/widget/position/scroll.rs +++ b/iris/src/widget/position/scroll.rs @@ -43,7 +43,7 @@ impl Widget for Scroll { self.content_len = used .axis(axis) - .apply_rest() + .apply_rest(painter.density()) .within_len(container_len) .to_abs(output_len); diff --git a/iris/src/widget/position/sized.rs b/iris/src/widget/position/sized.rs index 8c104de..968748b 100644 --- a/iris/src/widget/position/sized.rs +++ b/iris/src/widget/position/sized.rs @@ -17,12 +17,13 @@ impl Widget for Sized { // learn its size, then moves it into place with a pure // translation; that translation is only valid if what got painted // is already the reported size, anchored the same way both times. + let density = painter.density(); let mut region = UiRegion::FULL; if let Some(x) = self.x { - region.x = x.apply_rest().align(AxisAlign::Neg); + region.x = x.apply_rest(density).align(AxisAlign::Neg); } if let Some(y) = self.y { - region.y = y.apply_rest().align(AxisAlign::Neg); + region.y = y.apply_rest(density).align(AxisAlign::Neg); } let used = painter.widget_within(&self.inner, region); Size { diff --git a/iris/src/widget/position/span.rs b/iris/src/widget/position/span.rs index a244f74..9ecca40 100644 --- a/iris/src/widget/position/span.rs +++ b/iris/src/widget/position/span.rs @@ -4,12 +4,18 @@ use std::marker::PhantomData; pub struct Span { pub children: Vec, pub dir: Dir, - pub gap: f32, + /// A `Len` (not a bare `f32`) so `dp(4)` resolves against the display's + /// density the same way any other size in the tree does -- see + /// `Len::dp`'s field doc. Only the `abs` component (folded from `dp` at + /// draw time, `Widget::draw` below) is meaningful here; `rel`/`rest` + /// were never supported for a gap and still are not. + pub gap: Len, } impl Widget for Span { fn draw(&mut self, painter: &mut Painter) -> Size { let axis = self.dir.axis; + let gap = self.gap.apply_rest(painter.density()).abs; // Phase 1: draw each child once, at the ambient (unmodified, full) // region a size-only query used to see before this migration, to @@ -25,7 +31,7 @@ impl Widget for Span { .map(|child| painter.widget(child).axis(axis)) .collect(); - let gap_total = self.gap * self.children.len().saturating_sub(1) as f32; + let gap_total = gap * self.children.len().saturating_sub(1) as f32; let total = lens.iter().fold(Len::abs(gap_total), |s, &l| s + l); // Phase 2: place each child for real, using the lengths just @@ -54,7 +60,7 @@ impl Widget for Span { child_region.flip(axis); } let used = painter.widget_within(child, child_region); - start.abs += self.gap; + start.abs += gap; let ortho = used.axis(!axis); if ortho.rel > 0.0 || ortho.rest > 0.0 { @@ -82,12 +88,12 @@ impl Span { Self { children: Vec::new(), dir, - gap: 0.0, + gap: Len::ZERO, } } - pub fn gap(mut self, gap: impl UiNum) -> Self { - self.gap = gap.to_f32(); + pub fn gap(mut self, gap: impl Into) -> Self { + self.gap = gap.into(); self } @@ -103,7 +109,7 @@ impl Span { pub struct SpanBuilder, Tag> { pub children: Wa, pub dir: Dir, - pub gap: f32, + pub gap: Len, _pd: PhantomData<(State, Tag)>, } @@ -129,13 +135,13 @@ impl, Tag> Self { children, dir, - gap: 0.0, + gap: Len::ZERO, _pd: PhantomData, } } - pub fn gap(mut self, gap: impl UiNum) -> Self { - self.gap = gap.to_f32(); + pub fn gap(mut self, gap: impl Into) -> Self { + self.gap = gap.into(); self } } diff --git a/iris/src/widget/text/edit.rs b/iris/src/widget/text/edit.rs index 2044ab6..3feb162 100644 --- a/iris/src/widget/text/edit.rs +++ b/iris/src/widget/text/edit.rs @@ -141,7 +141,8 @@ impl<'a> TextEditCtx<'a> { fn layout(&mut self) -> &Layout { let attrs = self.text.view.attrs.clone(); let width = self.text.view.wrap_width(); - self.text.view.buf.shape(self.data, &attrs, width); + let density = self.data.density; + self.text.view.buf.shape(self.data, &attrs, width, density); self.text.view.buf.layout() } diff --git a/iris/transcript-ui/src/composer.rs b/iris/transcript-ui/src/composer.rs index f661f05..741c1b1 100644 --- a/iris/transcript-ui/src/composer.rs +++ b/iris/transcript-ui/src/composer.rs @@ -39,7 +39,7 @@ where .label("Message") .add(rsc); - let bar: WeakWidget = (field.pad(12).width(rest(1)),) + let bar: WeakWidget = (field.pad(dp(12)).width(rest(1)),) .span(Dir::RIGHT) .background(rect(UiColor::new(40, 40, 46, 255))) .add(rsc); diff --git a/iris/transcript-ui/src/row.rs b/iris/transcript-ui/src/row.rs index 92581ae..693a9e9 100644 --- a/iris/transcript-ui/src/row.rs +++ b/iris/transcript-ui/src/row.rs @@ -174,8 +174,8 @@ where (header, field.width(rest(1))) .span(Dir::DOWN) - .gap(4) - .pad(10) + .gap(dp(4)) + .pad(dp(10)) .add_strong(rsc) .any() }