From ff1d6ea9329e7d610f7e04110a12292c9e8f595b Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 7 Sep 2026 21:03:43 -0400 Subject: [PATCH] iris: a degenerate fit has no solution, and the desktop follows a display's density Two of docs/REVIEW-2026-09-07.md's risks. **R7.** `poly_fit_least_squares` clamped a near-zero basis-vector norm (`1.0 / dot(..).sqrt().max(1e-6)`) where Compose's `polyFitLeastSquares` bails: below `0.000001f` the vectors are linearly dependent and there is no solution. Clamping reached the solve with a `q` row of zeros and a zero on `r`'s diagonal, produced `[NaN, NaN, NaN]`, and was rescued only by the caller's `is_finite` check -- working, but by accident, and not what the source it is transcribed from does. It returns `Option` now and `velocity()` answers 0 on `None`. `a_fit_through_linearly_dependent_points_has_no_solution` reports `Some([NaN, NaN, NaN])` with the clamp back in place. Three samples at one instant is exactly what the input clock produced before 2ec0fee, so this is the second half of the same fault. **R5.** `WindowEvent::ScaleFactorChanged` was unhandled, so dragging the window to a display with a different scale left every `Len::dp` and every rasterised glyph at the density the window opened on. It now re-reads `content_scale` -- through that function rather than off the event, so `IRIS_SCALE` still pins `--phone`'s density instead of following the monitor -- and sets both copies. `UiRenderState::set_density` marks the tree for a full redraw when the value actually changes, because `Text::shape` keys its cache on `(attrs, width, density)` and nothing else would ask for those glyphs again. Invisible on this machine (every display here is 1.0), which is why the review asked for it in writing. Verified: `cargo test --lib -p iris` (104), `cargo test -p transcript-fixture` (12), `cargo ndk check -p iris`, fmt and clippy clean, and layer 2 (`run-headless.sh phone --phone --replay flick-120hz.touch --shot`) still draws and still clips at the composer. Co-Authored-By: Claude Fable 5.1 --- iris/core/src/ui/render_state.rs | 9 +++++ iris/src/default/mod.rs | 16 +++++++++ iris/src/sense.rs | 57 +++++++++++++++++++++++++++----- 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 2ab34b9..d18c504 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -167,7 +167,16 @@ impl UiRenderState { /// 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). + /// + /// Marks the tree for a full redraw when the value actually changes: + /// every `Len::dp` already resolved and every glyph already shaped + /// (`Text::shape` keys its cache on `(attrs, width, density)`) belongs + /// to the old one, and nothing else would ask for them again + /// (docs/REVIEW-2026-09-07.md's R5). pub fn set_density(&mut self, density: f32) { + if density != self.density { + self.resized = true; + } self.density = density; } diff --git a/iris/src/default/mod.rs b/iris/src/default/mod.rs index f781bc9..91947c3 100644 --- a/iris/src/default/mod.rs +++ b/iris/src/default/mod.rs @@ -362,6 +362,22 @@ impl AppState for DefaultApp { render.resize((size.width, size.height)); ui_state.renderer.resize(size) } + // Dragging the window to a display with a different scale. + // Both copies again, the pair `new` sets at startup -- read + // through `content_scale` rather than from the event, so + // `IRIS_SCALE` still pins the density it was given (the + // `--phone` window must not follow the monitor). winit sends + // the matching `Resized` separately. Before 2026-09-07 this + // event was unhandled, so every `dp` and every rasterised + // glyph stayed at the density the window opened on + // (docs/REVIEW-2026-09-07.md's R5) -- invisible on this + // machine, where every display is 1.0. + WindowEvent::ScaleFactorChanged { .. } => { + let scale = content_scale(ui_state.window.as_ref()); + rsc.ui.text.density = scale; + render.set_density(scale); + ui_state.window.request_redraw(); + } WindowEvent::KeyboardInput { event, .. } => { if let Some(sel) = ui_state.focus && event.state.is_pressed() diff --git a/iris/src/sense.rs b/iris/src/sense.rs index 6d31255..5846d6d 100644 --- a/iris/src/sense.rs +++ b/iris/src/sense.rs @@ -1024,6 +1024,10 @@ const ASSUME_POINTER_MOVE_STOPPED_MS: f32 = 40.0; /// `VelocityTracker1D`'s `minSampleSize` for `Strategy.Lsq2` -- a /// quadratic needs three points, and fewer answers `0`. const MIN_SAMPLE_SIZE: usize = 3; +/// Compose's `if (norm < 0.000001f)` in `polyFitLeastSquares`: below this, +/// the vectors are linearly dependent and there is no solution, so the fit +/// returns nothing rather than dividing by it. +const DEGENERATE_NORM: f32 = 0.000001; /// The degree Compose fits (`polyFitLeastSquares(.., degree = 2)`), and /// the number of coefficients that produces. const FIT_DEGREE: usize = 2; @@ -1202,11 +1206,16 @@ impl VelocityTracker { } // The 2nd coefficient is the fitted quadratic's derivative at // x = 0, and x = 0 is the newest sample's own timestamp. ms -> s. - let velocity = poly_fit_least_squares(&ages[..count], &positions[..count])[1] * 1000.0; - // `calculateVelocity(maximumVelocity)`'s first branch. A fit - // through near-degenerate points divides by a near-zero diagonal - // of `r`; answering `NaN` would trip `List::fling`'s finiteness - // assert in a debug build and coast forever in a release one. + // `None` is Compose's "linearly dependent, no solution" -- see + // `poly_fit_least_squares`. + let Some(fit) = poly_fit_least_squares(&ages[..count], &positions[..count]) else { + return 0.0; + }; + let velocity = fit[1] * 1000.0; + // `calculateVelocity(maximumVelocity)`'s first branch, kept as the + // outer guard even though the degenerate case is now detected + // rather than clamped: a fit can still overflow on inputs nothing + // here has produced, and `List::fling` asserts finiteness. if velocity.is_finite() { velocity } else { 0.0 } } } @@ -1226,7 +1235,8 @@ impl VelocityTracker { // callers between them already answer 0 below `MIN_SAMPLE_SIZE` and check // the result with `is_finite`, so a release build has a defined outcome // rather than a silently wrong one. -fn poly_fit_least_squares(x: &[f32], y: &[f32]) -> [f32; FIT_COEFFICIENTS] { +/// `None` where Compose returns no solution: see `DEGENERATE_NORM`. +fn poly_fit_least_squares(x: &[f32], y: &[f32]) -> Option<[f32; FIT_COEFFICIENTS]> { debug_assert_eq!(x.len(), y.len()); debug_assert!( (FIT_COEFFICIENTS..=HISTORY_SIZE).contains(&x.len()), @@ -1258,7 +1268,19 @@ fn poly_fit_least_squares(x: &[f32], y: &[f32]) -> [f32; FIT_COEFFICIENTS] { w[h] -= dot * z[h]; } } - let inverse_norm = 1.0 / dot(&q[j][..m], &q[j][..m]).sqrt().max(1e-6); + // Compose's own bail-out, not a clamp. `polyFitLeastSquares` + // treats a norm this small as "the vectors are linearly dependent, + // so there is no solution" and returns nothing; clamping instead + // -- which this did until 2026-09-07 + // (docs/REVIEW-2026-09-07.md's R7) -- produces a `q` row of zeros, + // a zero on `r`'s diagonal and a 0/0 that only the caller's + // `is_finite` check happened to catch. Working by accident, and + // not what the source it is transcribed from does. + let norm = dot(&q[j][..m], &q[j][..m]).sqrt(); + if norm < DEGENERATE_NORM { + return None; + } + let inverse_norm = 1.0 / norm; for v in &mut q[j][..m] { *v *= inverse_norm; } @@ -1280,7 +1302,7 @@ fn poly_fit_least_squares(x: &[f32], y: &[f32]) -> [f32; FIT_COEFFICIENTS] { } coefficients[i] = c / r[i][i]; } - coefficients + Some(coefficients) } fn dot(a: &[f32], b: &[f32]) -> f32 { @@ -1668,6 +1690,25 @@ mod velocity_tracker_tests { ); } + /// docs/REVIEW-2026-09-07.md's R7. Three samples at one instant -- + /// which the input clock produced on its own before 2ec0fee -- leave + /// the second basis vector all zeros, and Compose calls that "linearly + /// dependent, no solution" and returns nothing. Clamping the norm to + /// 1e-6 instead reached the solve with a zero on `r`'s diagonal and + /// answered `[NaN, NaN, NaN]`, which only the caller's `is_finite` + /// check kept off the fling path. + #[test] + fn a_fit_through_linearly_dependent_points_has_no_solution() { + assert_eq!( + poly_fit_least_squares(&[0.0, 0.0, 0.0], &[0.0, 40.0, 90.0]), + None, + ); + // The other half: an ordinary set still fits. + let fit = poly_fit_least_squares(&[-8.0, -4.0, 0.0], &[1000.0, 1040.0, 1086.0]) + .expect("three distinct points describe a quadratic"); + assert!(fit.iter().all(|c| c.is_finite())); + } + #[test] fn the_sample_list_is_reported_relative_to_the_first() { // What the release log prints at debug level, and what a phone