From 59a79319c0f7b6acc29d678b148da45e7291d963 Mon Sep 17 00:00:00 2001 From: John Donaghy Date: Thu, 30 Apr 2026 19:03:44 -0500 Subject: [PATCH] fix(quadraui): MultiSectionView snaps section bounds to integer cells (TUI) `LayoutMetrics` gains a `cell_quantum: f32` field. When > 0, the layout function snaps each section's resolved size to integer multiples of the quantum BEFORE emitting bounds. The TUI rasteriser passes 1.0 (terminal cell precision); GTK passes 0.0 (Cairo sub-pixel). Why: the TUI rasteriser already snapped paint coordinates to integer rows via bounds.y.round() as u16, but MultiSectionViewLayout::hit_test consumed the raw fractional bounds. With fractional EqualShare distributions (e.g. 4 sections in 21 cells -> 5.25 each), section 1's header paints at row 7 (because 7.25.round() == 7) while hit_test keeps the boundary at y=7.25 -- so click at row 7 lands in section 0's body. Every section after the first then drifts. This is exactly the paint/click drift bug class MultiSectionView was designed to eliminate. Per-consumer Cell-on-engine bridge fields don't help; the fix has to be structural inside the layout itself. Distribution algorithm: floor each fractional size, then award the remaining cells to sections with the largest fractional remainders (largest-remainder / Hare-Niemeyer). Sum still equals usable_main exactly. Adds two regression tests in the primitive: - cell_quantum_snaps_section_bounds_to_integers: every header_bounds and body_bounds y/height is integer-aligned. - cell_quantum_paint_and_hit_test_agree_on_every_row: for each row the paint draws a header on (rounded header_bounds.y), hit_test at that row returns Header for the same section. Either test would have caught the Session 343-346 #296 smoke wave bug. Updates the two vimcode TUI consumers that build LayoutMetrics inline (src/tui_main/mouse.rs, src/tui_main/panels.rs) to set cell_quantum 1.0. Quality gate: cargo build / clippy / fmt / 1950 lib + integration tests green; quadraui 217 tests green (215 + 2 new). Part of the Session 346 course correction (PLAN.md "Course correction"). This is step 2 of the harness-first plan: ship the structural fix first as a standalone change, then build the paint<->click round-trip harness in step 3, then re-attempt #296 with the harness gating the migration. Co-Authored-By: Claude Opus 4.7 (1M context) --- quadraui/src/gtk/multi_section_view.rs | 2 + quadraui/src/primitives/multi_section_view.rs | 165 +++++++++++++++++- quadraui/src/tui/multi_section_view.rs | 6 +- src/tui_main/mouse.rs | 1 + src/tui_main/panels.rs | 1 + 5 files changed, 173 insertions(+), 2 deletions(-) diff --git a/quadraui/src/gtk/multi_section_view.rs b/quadraui/src/gtk/multi_section_view.rs index 4a8f42b8..f3813293 100644 --- a/quadraui/src/gtk/multi_section_view.rs +++ b/quadraui/src/gtk/multi_section_view.rs @@ -43,6 +43,8 @@ pub fn metrics_for(line_height: f64, allow_resize: bool) -> LayoutMetrics { // backgrounds; the previous 4px was easy to miss. Hosts that // want a thinner scrollbar can compose `Scrollbar` directly. scrollbar_size: 8.0, + // GTK paints at sub-pixel precision via Cairo; no quantization. + cell_quantum: 0.0, } } diff --git a/quadraui/src/primitives/multi_section_view.rs b/quadraui/src/primitives/multi_section_view.rs index 0349c36a..d4deff0d 100644 --- a/quadraui/src/primitives/multi_section_view.rs +++ b/quadraui/src/primitives/multi_section_view.rs @@ -421,6 +421,12 @@ pub struct LayoutMetrics { /// Scrollbar gutter size in cross-axis units. Reserved on the /// trailing edge of each section's body when the body overflows. pub scrollbar_size: f32, + /// Snap distributed section sizes to integer multiples of this + /// value. `0.0` (default) means no snapping. TUI sets `1.0` so + /// section bounds align to terminal cells — paint (which rounds to + /// `u16` rows) and hit-test (which uses raw bounds) then agree by + /// construction. GTK leaves it `0.0` for sub-pixel layout. + pub cell_quantum: f32, } impl Default for LayoutMetrics { @@ -429,6 +435,7 @@ impl Default for LayoutMetrics { header_size: 1.0, divider_size: 0.0, scrollbar_size: 1.0, + cell_quantum: 0.0, } } } @@ -524,7 +531,7 @@ impl MultiSectionView { } // ── Resolve per-section main-axis sizes ──────────────────── - let resolved = match self.scroll_mode { + let mut resolved = match self.scroll_mode { ScrollMode::WholePanel => { // Every section sized to chrome + content height. let mut sizes = Vec::with_capacity(n); @@ -548,6 +555,54 @@ impl MultiSectionView { ), }; + // Snap resolved sizes to integer cell multiples when the host + // requests it (TUI). Without this, fractional section sizes + // (e.g. 5.5 cells) cause paint to round to integer rows while + // hit_test uses the raw fractional bounds — paint and click + // disagree about which row is in which section. Distributes the + // remainder one cell at a time so the sum still equals the + // (integer) usable_main exactly. + if metrics.cell_quantum > 0.0 && n > 0 { + let q = metrics.cell_quantum; + let target_total: f32 = resolved.iter().sum(); + let target_cells = (target_total / q).round() as i32; + let mut snapped: Vec = resolved.iter().map(|s| (s / q).floor() as i32).collect(); + let mut remainder = target_cells - snapped.iter().sum::(); + // Order the indices by the size of the dropped fractional + // part so the largest fractional shares get the spare + // cells first — matches how layout managers usually break + // ties on integer distribution. + let mut order: Vec<(usize, f32)> = resolved + .iter() + .enumerate() + .map(|(i, s)| (i, (s / q) - (s / q).floor())) + .collect(); + order.sort_by(|a, b| b.1.partial_cmp(&a.1).unwrap_or(std::cmp::Ordering::Equal)); + let mut cursor = 0; + while remainder > 0 && !order.is_empty() { + let (idx, _) = order[cursor % order.len()]; + snapped[idx] += 1; + remainder -= 1; + cursor += 1; + } + // If we overshot (remainder negative), shave from the + // smallest-fractional sections. + if remainder < 0 { + let mut order_rev = order; + order_rev.reverse(); + let mut c = 0; + while remainder < 0 && !order_rev.is_empty() { + let (idx, _) = order_rev[c % order_rev.len()]; + if snapped[idx] > 1 { + snapped[idx] -= 1; + remainder += 1; + } + c += 1; + } + } + resolved = snapped.into_iter().map(|s| (s as f32) * q).collect(); + } + // ── Walk and emit per-section layouts + dividers ─────────── let mut sections_out = Vec::with_capacity(n); let mut dividers = Vec::new(); @@ -1383,6 +1438,114 @@ mod tests { assert!(layout.panel_scrollbar.is_some()); } + #[test] + fn cell_quantum_snaps_section_bounds_to_integers() { + // Regression: with a fractional EqualShare distribution + // (4 sections in 21 cells → 5.25 each), paint snaps via + // `bounds.y.round()` while hit_test used to consume raw f32 + // bounds. Click at the integer row that paint drew the header + // on resolved to the previous section's body, breaking + // every section after the first. With `cell_quantum: 1.0` + // the layout itself snaps to whole cells so paint and click + // can never disagree. + let v = view(vec![ + empty_section("a", SectionSize::EqualShare), + empty_section("b", SectionSize::EqualShare), + empty_section("c", SectionSize::EqualShare), + empty_section("d", SectionSize::EqualShare), + ]); + let metrics = LayoutMetrics { + cell_quantum: 1.0, + ..LayoutMetrics::default() + }; + let layout = v.layout(Rect::new(0.0, 0.0, 30.0, 21.0), metrics, |_| { + SectionMeasure { + content_size: 3.0, + aux_size: 0.0, + } + }); + for s in &layout.sections { + let hb = s.header_bounds; + assert_eq!( + hb.y, + hb.y.round(), + "section {} header y {} not integer-aligned", + s.section_idx, + hb.y + ); + assert_eq!( + hb.height, + hb.height.round(), + "section {} header h {} not integer-aligned", + s.section_idx, + hb.height + ); + let bb = s.body_bounds; + assert_eq!( + bb.y, + bb.y.round(), + "section {} body y {} not integer-aligned", + s.section_idx, + bb.y + ); + assert_eq!( + bb.height, + bb.height.round(), + "section {} body h {} not integer-aligned", + s.section_idx, + bb.height + ); + } + // Sum of resolved sizes equals usable_main exactly (21 cells). + let total: f32 = layout.sections.iter().map(|s| s.resolved_size).sum(); + assert_eq!(total, 21.0); + } + + #[test] + fn cell_quantum_paint_and_hit_test_agree_on_every_row() { + // For each integer row in the panel, the row paint draws a + // header on (rounding `header_bounds.y`) must equal the row + // hit_test resolves to that section's header. With + // `cell_quantum: 1.0` this is true by construction. + let v = view(vec![ + empty_section("a", SectionSize::EqualShare), + empty_section("b", SectionSize::EqualShare), + empty_section("c", SectionSize::EqualShare), + empty_section("d", SectionSize::EqualShare), + ]); + let metrics = LayoutMetrics { + cell_quantum: 1.0, + ..LayoutMetrics::default() + }; + // 21 rows: 4 headers + 17 body cells, distributed unevenly. + let layout = v.layout(Rect::new(0.0, 2.0, 30.0, 21.0), metrics, |_| { + SectionMeasure { + content_size: 5.0, + aux_size: 0.0, + } + }); + for s in &layout.sections { + // Paint draws header at this integer row. + let painted_row = s.header_bounds.y.round(); + // Hit-test at that row's center must return Header for this section. + let hit = layout.hit_test(15.0, painted_row + 0.0); + match hit { + MultiSectionViewHit::Header { + section: hit_section, + .. + } => assert_eq!( + hit_section, s.section_idx, + "row {} paints section {} header but hit_test returns section {}", + painted_row, s.section_idx, hit_section + ), + other => panic!( + "row {} paints section {} header but hit_test returns {:?}", + painted_row, s.section_idx, other + ), + } + } + } + #[test] fn aux_input_hit_returns_input_kind() { let mut s = empty_section("sc", SectionSize::EqualShare); diff --git a/quadraui/src/tui/multi_section_view.rs b/quadraui/src/tui/multi_section_view.rs index e0ef3e1a..f4650783 100644 --- a/quadraui/src/tui/multi_section_view.rs +++ b/quadraui/src/tui/multi_section_view.rs @@ -45,11 +45,15 @@ pub fn draw_multi_section_view( // TUI metrics: 1 cell per header row, 1 cell per scrollbar gutter, // 1 cell per divider (only when allow_resize is true; otherwise we - // omit the strip entirely). + // omit the strip entirely). `cell_quantum: 1.0` snaps section sizes + // to whole cells inside `MultiSectionView::layout` so paint + // (rounded to integer rows) and hit_test (raw fractional bounds) + // agree by construction. let metrics = LayoutMetrics { header_size: 1.0, divider_size: if view.allow_resize { 1.0 } else { 0.0 }, scrollbar_size: 1.0, + cell_quantum: 1.0, }; let bounds = QRect::new( diff --git a/src/tui_main/mouse.rs b/src/tui_main/mouse.rs index 815cdcd7..432c63f7 100644 --- a/src/tui_main/mouse.rs +++ b/src/tui_main/mouse.rs @@ -2991,6 +2991,7 @@ pub(super) fn handle_mouse( header_size: 1.0, divider_size: 0.0, scrollbar_size: 1.0, + cell_quantum: 1.0, }; let layout = view.layout(body_bounds, metrics, |i| { let s = &view.sections[i]; diff --git a/src/tui_main/panels.rs b/src/tui_main/panels.rs index 4541213a..e86655f6 100644 --- a/src/tui_main/panels.rs +++ b/src/tui_main/panels.rs @@ -2916,6 +2916,7 @@ pub(super) fn render_ext_sidebar( header_size: 1.0, divider_size: 0.0, scrollbar_size: 1.0, + cell_quantum: 1.0, }; let body_qbounds = quadraui::Rect::new( body_area.x as f32,