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 <noreply@anthropic.com>
This commit is contained in:
1 parent
2b20bb2c91
commit
ff1d6ea932
3 files changed
+74
-8
No files matched your search
@@ -167,7 +167,16 @@ impl UiRenderState {
|
|||||||
/// different triggers (a surface resize on every rotation or keyboard
|
/// different triggers (a surface resize on every rotation or keyboard
|
||||||
/// open; a density change only if the app follows the display to a
|
/// open; a density change only if the app follows the display to a
|
||||||
/// different screen, which Android surfaces separately).
|
/// 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) {
|
pub fn set_density(&mut self, density: f32) {
|
||||||
|
if density != self.density {
|
||||||
|
self.resized = true;
|
||||||
|
}
|
||||||
self.density = density;
|
self.density = density;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -362,6 +362,22 @@ impl<State: DefaultAppState> AppState for DefaultApp<State> {
|
|||||||
render.resize((size.width, size.height));
|
render.resize((size.width, size.height));
|
||||||
ui_state.renderer.resize(size)
|
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, .. } => {
|
WindowEvent::KeyboardInput { event, .. } => {
|
||||||
if let Some(sel) = ui_state.focus
|
if let Some(sel) = ui_state.focus
|
||||||
&& event.state.is_pressed()
|
&& event.state.is_pressed()
|
||||||
|
|||||||
+49
-8
@@ -1024,6 +1024,10 @@ const ASSUME_POINTER_MOVE_STOPPED_MS: f32 = 40.0;
|
|||||||
/// `VelocityTracker1D`'s `minSampleSize` for `Strategy.Lsq2` -- a
|
/// `VelocityTracker1D`'s `minSampleSize` for `Strategy.Lsq2` -- a
|
||||||
/// quadratic needs three points, and fewer answers `0`.
|
/// quadratic needs three points, and fewer answers `0`.
|
||||||
const MIN_SAMPLE_SIZE: usize = 3;
|
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 degree Compose fits (`polyFitLeastSquares(.., degree = 2)`), and
|
||||||
/// the number of coefficients that produces.
|
/// the number of coefficients that produces.
|
||||||
const FIT_DEGREE: usize = 2;
|
const FIT_DEGREE: usize = 2;
|
||||||
@@ -1202,11 +1206,16 @@ impl VelocityTracker {
|
|||||||
}
|
}
|
||||||
// The 2nd coefficient is the fitted quadratic's derivative at
|
// The 2nd coefficient is the fitted quadratic's derivative at
|
||||||
// x = 0, and x = 0 is the newest sample's own timestamp. ms -> s.
|
// 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;
|
// `None` is Compose's "linearly dependent, no solution" -- see
|
||||||
// `calculateVelocity(maximumVelocity)`'s first branch. A fit
|
// `poly_fit_least_squares`.
|
||||||
// through near-degenerate points divides by a near-zero diagonal
|
let Some(fit) = poly_fit_least_squares(&ages[..count], &positions[..count]) else {
|
||||||
// of `r`; answering `NaN` would trip `List::fling`'s finiteness
|
return 0.0;
|
||||||
// assert in a debug build and coast forever in a release one.
|
};
|
||||||
|
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 }
|
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
|
// 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
|
// the result with `is_finite`, so a release build has a defined outcome
|
||||||
// rather than a silently wrong one.
|
// 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_eq!(x.len(), y.len());
|
||||||
debug_assert!(
|
debug_assert!(
|
||||||
(FIT_COEFFICIENTS..=HISTORY_SIZE).contains(&x.len()),
|
(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];
|
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] {
|
for v in &mut q[j][..m] {
|
||||||
*v *= inverse_norm;
|
*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[i] = c / r[i][i];
|
||||||
}
|
}
|
||||||
coefficients
|
Some(coefficients)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn dot(a: &[f32], b: &[f32]) -> f32 {
|
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]
|
#[test]
|
||||||
fn the_sample_list_is_reported_relative_to_the_first() {
|
fn the_sample_list_is_reported_relative_to_the_first() {
|
||||||
// What the release log prints at debug level, and what a phone
|
// What the release log prints at debug level, and what a phone
|
||||||
|
|||||||
Reference in new issue
Block a user