diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index 97cce68..05c24ab 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -101,8 +101,16 @@ const MAX_DISPLAY_DIM: u32 = 2048; /// TRACES: FR-UI-1 | FR-UI-2 | M-16 /// Width at which the expanded layout appears (FR-UI-1). /// -/// Logical pixels, not a device check — a narrow desktop window gets the -/// compact layout exactly as a tablet in portrait would. +/// Logical pixels, not a device check: a desktop window dragged under this gets +/// the compact layout exactly as a small screen would, and that is FR-UI-1's +/// rule rather than a convenience. +/// +/// **The tablet in portrait is not that case**, and it is worth saying so here +/// because this comment used to claim it was. The tablet's panel is about 960 +/// logical pixels across in portrait, which clears 820 — so portrait is +/// expanded, and turning the device does not change the class at all. What it +/// changes is which axis the develop column is on, which is `column_below` and +/// D-N7, and nothing to do with this number. const EXPANDED_MIN_WIDTH: f32 = 820.0; /// TRACES: FR-UI-1 @@ -127,6 +135,27 @@ const PANEL_MAX_FRACTION: f32 = 0.45; /// it stay accurate over. const PANEL_MIN_WIDTH: f32 = 280.0; +/// TRACES: FR-UI-1 +/// The window aspect — height over width — at which the develop column moves +/// from beside the photograph to under it (D-N7). +/// +/// Above 1.0 rather than at it, because a window barely taller than it is wide +/// has nothing to gain. D-N7's arithmetic only comes out for the dock once the +/// photograph left beside a 360px column has become a strip, and 1.25 is where +/// that starts: the tablet in portrait is about 960 × 1500, which is 1.56. +const DOCK_ENTER_ASPECT: f32 = 1.25; + +/// And the aspect at which it moves back beside the photograph. +/// +/// Below `DOCK_ENTER_ASPECT` rather than equal to it, and that gap is the +/// whole point. This is read on every resize event, so a single threshold +/// means a window dragged along its own diagonal crosses it several times a +/// second and the column jumps from one axis to the other and back while the +/// pointer is still down. The dead band between 1.15 and 1.25 is wide enough +/// that no plausible drag re-crosses it and narrow enough that the flag still +/// answers to the window's shape rather than to its history. +const DOCK_LEAVE_ASPECT: f32 = 1.15; + /// How long after the last change a draft frame is replaced by a sharp one. /// /// Above the interval between events in a drag, so an ordinary gesture never @@ -3418,20 +3447,33 @@ pub fn run(paths: Vec) -> Result<()> { // Shared with the resize callback below, which is where every reading // after the first one arrives. See `window_metrics_level`. let reported_geometry = std::rc::Rc::new(std::cell::Cell::new(None::<(bool, u32)>)); + // Where `column-below` stood a moment ago, which is the other half of the + // hysteresis in `column_below`. Not in `PanelChoices`: that records where + // the *user* disagreed with a layout class, and this is neither a class nor + // anything the user has said — it is the last answer, kept so the next one + // can decline to contradict it over a pixel of drag. + let dock = std::rc::Rc::new(std::cell::Cell::new(None::)); { let weak = window.as_weak(); let panels = panels.clone(); let reported_geometry = reported_geometry.clone(); - window.on_window_resized(move |width| { + let dock = dock.clone(); + window.on_window_resized(move |width, height| { let Some(window) = weak.upgrade() else { return }; - apply_layout_class(&window, width, &panels); + apply_layout_class(&window, width, height, &panels, &dock); log_window_metrics(&window, window_metrics_level(&window, &reported_geometry)); }); } { let size = window.window().size(); let scale = window.window().scale_factor().max(0.01); - apply_layout_class(&window, size.width as f32 / scale, &panels); + apply_layout_class( + &window, + size.width as f32 / scale, + size.height as f32 / scale, + &panels, + &dock, + ); // N6: the reading beside the one the layout class is derived from, // which on a desktop is taken before the window has been mapped and so // reads zero. `window_metrics_level` is what decides whether that is @@ -3702,11 +3744,55 @@ fn log_window_metrics(window: &AppWindow, level: log::Level) { ); } -fn apply_layout_class(window: &AppWindow, width: f32, panels: &PanelChoices) { +/// TRACES: FR-UI-1 +/// Which side of the photograph the develop column belongs on, given the shape +/// of the window and where the answer stood before (D-N7). +/// +/// **Not a layout class.** The class is decided by width and says how much room +/// there is; this is decided by aspect and says which way round the room is. A +/// 960-wide portrait window is expanded and wants the dock; a 1500-wide +/// landscape one is expanded and does not — so the two cannot be folded +/// together, and `PanelChoices` does not remember this one. Closing the column +/// in landscape closes the dock in portrait, because it is the same column. +/// +/// `previously` is `None` on the first call, before any resize has been +/// reported. There is nothing to be hysteretic about yet, so the entering +/// threshold decides alone: a window that opens inside the dead band opens with +/// the column beside the photograph, which is what every window that is not +/// clearly tall gets. +fn column_below(width: f32, height: f32, previously: Option) -> bool { + // A window with no width has no aspect. It happens between a minimise and + // the resize that follows it, and answering `false` here would swing the + // column back beside the photograph for the one frame in between. + if width <= 0.0 { + return previously.unwrap_or(false); + } + let aspect = height / width; + match previously { + Some(true) => aspect > DOCK_LEAVE_ASPECT, + _ => aspect >= DOCK_ENTER_ASPECT, + } +} + +fn apply_layout_class( + window: &AppWindow, + width: f32, + height: f32, + panels: &PanelChoices, + dock: &std::cell::Cell>, +) { let expanded = width >= EXPANDED_MIN_WIDTH; window.set_expanded(expanded); window.set_layout_class(if expanded { "expanded" } else { "compact" }.into()); + // FR-UI-1: which axis the column is on, set from here for the same reason + // the class is — the develop view's frame reads it, so deriving it from + // `root.width` inside that frame would be the binding loop the note on + // `expanded` in `app.slint` describes. + let below = column_below(width, height, dock.get()); + dock.set(Some(below)); + window.set_column_below(below); + // FR-UI-1: the ceiling on the develop column, computed here for the same // reason the class is — a width that both derives from and feeds the // layout is a binding loop in Slint. @@ -4087,4 +4173,56 @@ mod tests { PointsUpdate::Incompatible ); } + + #[test] + fn a_window_taller_than_it_is_wide_docks_the_column_below() { + // The tablet in portrait at either scale factor D-N7 considers, and a + // desktop window dragged into the same shape. Aspect, not width: the + // first of these is expanded by width and still wants the dock. + assert!(column_below(960.0, 1500.0, None)); + assert!(column_below(1097.0, 1714.0, None)); + assert!(column_below(700.0, 1100.0, Some(false))); + } + + #[test] + fn a_window_wider_than_it_is_tall_keeps_the_column_beside() { + // Landscape, square, and the tablet's other orientation. The last is + // the one that matters: rotating back has to bring the column back. + assert!(!column_below(1500.0, 900.0, Some(false))); + assert!(!column_below(1000.0, 1000.0, Some(true))); + assert!(!column_below(1500.0, 960.0, Some(true))); + } + + #[test] + fn a_window_resized_across_square_does_not_flap() { + // The dead band, from both sides. Between 1.15 and 1.25 the answer is + // whatever it already was, which is the whole reason there are two + // thresholds rather than one — a diagonal drag crosses this range for + // as long as the pointer is down. + let width = 1000.0; + for aspect in [1.16, 1.20, 1.24] { + let height = width * aspect; + assert!( + column_below(width, height, Some(true)), + "{aspect} from below" + ); + assert!( + !column_below(width, height, Some(false)), + "{aspect} from beside" + ); + } + } + + #[test] + fn the_first_measurement_has_no_state_to_be_hysteretic_about() { + // Before any resize has been reported there is no previous answer, so + // the entering threshold decides alone and the dead band reads as + // "beside" — the arrangement every window that is not clearly tall + // gets. And a window with no width at all keeps whatever it had, so a + // minimise does not swing the column across for one frame. + assert!(!column_below(1000.0, 1200.0, None)); + assert!(column_below(1000.0, 1250.0, None)); + assert!(!column_below(0.0, 1200.0, None)); + assert!(column_below(0.0, 400.0, Some(true))); + } } diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index eeeac91..37b28f4 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -1107,6 +1107,23 @@ export component AppWindow inherits Window { /// smaller fault than one briefly clipped. in property panel-max-width: 520px; + /// TRACES: FR-UI-1 + /// Whether the develop column sits under the photograph rather than beside + /// it (D-N7). + /// + /// **Not a layout class.** The class is decided by the window's width and + /// says how much room there is; this is decided by its aspect and says + /// which way round that room is, so a 960-wide portrait window is + /// `expanded` *and* docked while a 1500-wide landscape one is `expanded` + /// and not. Nothing here reads it but the develop view's frame, and + /// nothing remembers it: closing the column in landscape closes the dock + /// in portrait, because it is the same column. + /// + /// From Rust for the reason `panel-max-width` above is — the frame reads + /// it to lay itself out, so deriving it from `root.width` in the frame + /// would be a property that both feeds and follows the layout. + in property column-below: false; + // --- local adjustments (FR-DEV-3) --------------------------------------- /// The false-coloured region map, drawn over the photograph. @@ -1316,7 +1333,7 @@ in property panel-visible: true; callback back-requested() -> bool; callback canvas-resized(int, int); - callback window-resized(length); + callback window-resized(length, length); /// The width the interface actually has to lay out in. /// @@ -1326,8 +1343,24 @@ in property panel-visible: true; property shell-width: root.width - root.safe-area-insets.left - root.safe-area-insets.right; - // One-way: report width outward, never read layout back into it. - changed shell-width => { root.window-resized(self.shell-width); } + /// The height it has, for the same reason and with the same caveat. + /// + /// Reported alongside the width because the develop view's frame is + /// decided by the *aspect* of what the interface actually gets, not of the + /// window (D-N7) — and on Android those differ by the height of the status + /// and navigation bars, which is exactly the axis being measured. + property shell-height: + root.height - root.safe-area-insets.top - root.safe-area-insets.bottom; + + // One-way: report the size outward, never read layout back into it. Two + // handlers because Slint has no single "geometry changed" hook — the same + // shape as `canvas-resized`'s px-w/px-h below. + changed shell-width => { + root.window-resized(self.shell-width, self.shell-height); + } + changed shell-height => { + root.window-resized(self.shell-width, self.shell-height); + } // Everything the interface draws lives inside this, for two reasons that // happen to want the same element. @@ -1351,8 +1384,7 @@ in property panel-visible: true; x: root.safe-area-insets.left; y: root.safe-area-insets.top; width: root.shell-width; - height: root.height - root.safe-area-insets.top - - root.safe-area-insets.bottom; + height: root.shell-height; // Deliberately does *not* focus itself. //