From 8889c3ca93cd2f32e07fa8c9804d1da16c63e0a9 Mon Sep 17 00:00:00 2001 From: Csaba Date: Wed, 18 Feb 2026 11:00:16 +0100 Subject: [PATCH] fix(wm): correct layout rounding for uneven integer division The grid layout calculates row heights using integer division, which truncates the result when the area height is not evenly divisible by the number of rows in a column. Since every row received this truncated height, columns with an odd row count (e.g. 3 rows in 800px: 266*3=798) would fall short of the full area height, leaving a visible gap at the bottom compared to columns with an even row count (e.g. 2 rows: 400*2=800). The last row in each column now absorbs the remainder pixels from the integer division. The vertical flip position calculation was also updated so that the last row, which becomes the topmost window when flipped, starts at the area top edge instead of being offset by the truncation error. The same integer division truncation also affected column widths in the grid layout when the area width is not evenly divisible by the number of columns. The last column now absorbs the width remainder using the same pattern, and this correction is applied before the flipped column positions are calculated so that horizontal flips also tile correctly. The two shared helper functions columns_with_ratios and rows_with_ratios had the same issue, which propagated to every layout that delegates to them: Columns, Rows, VerticalStack stack rows, RightMainVerticalStack stack rows, HorizontalStack stack columns, and UltrawideVerticalStack tertiary rows. Both functions now correct the last element after the sizing loop so that the total tiled dimension matches the area exactly. The Scrolling layout computes a uniform column width by dividing the area width by the visible column count, which can also leave a remainder gap on the right edge. The last visible column in the current viewport now absorbs this remainder. Since the layout is recalculated on every scroll event, the correction stays accurate regardless of which column is rightmost. --- komorebi-layouts/src/arrangement.rs | 66 ++- komorebi-layouts/src/arrangement_tests.rs | 571 ++++++++++++++++++++++ 2 files changed, 632 insertions(+), 5 deletions(-) diff --git a/komorebi-layouts/src/arrangement.rs b/komorebi-layouts/src/arrangement.rs index 95565cec..ea895d21 100644 --- a/komorebi-layouts/src/arrangement.rs +++ b/komorebi-layouts/src/arrangement.rs @@ -141,6 +141,15 @@ impl Arrangement for DefaultLayout { }); } + // Last visible column absorbs any remainder from integer division + // so that visible columns tile the full area width without gaps + let width_remainder = area.right - column_width * visible_columns; + if width_remainder > 0 { + let last_visible_idx = + (first_visible as usize + visible_columns as usize - 1).min(len - 1); + layouts[last_visible_idx].right += width_remainder; + } + let adjustment = calculate_scrolling_adjustment(resize_dimensions); layouts .iter_mut() @@ -660,6 +669,16 @@ impl Arrangement for DefaultLayout { current_left += width; } + // Last column absorbs any remainder from integer division + // so that columns tile the full area width without gaps + let total_width: i32 = col_widths.iter().sum(); + let width_remainder = area.right - total_width; + if width_remainder > 0 + && let Some(last) = col_widths.last_mut() + { + *last += width_remainder; + } + // Pre-calculate flipped column positions: same widths laid out // in reverse order so that the last column sits at area.left let flipped_col_lefts = if matches!( @@ -691,8 +710,10 @@ impl Arrangement for DefaultLayout { remaining_windows / remaining_columns }; - // Rows within each column are equal height (no row_ratios support for Grid) - let win_height = area.bottom / num_rows_in_this_col; + // Rows within each column: base height from integer division, + // last row absorbs any remainder to cover the full area height + let base_height = area.bottom / num_rows_in_this_col; + let height_remainder = area.bottom - base_height * num_rows_in_this_col; let col_idx = col as usize; let win_width = col_widths[col_idx]; @@ -700,19 +721,34 @@ impl Arrangement for DefaultLayout { for row in 0..num_rows_in_this_col { if let Some((_idx, win)) = iter.next() { + let is_last_row = row == num_rows_in_this_col - 1; + let win_height = if is_last_row { + base_height + height_remainder + } else { + base_height + }; + let mut left = col_left; - let mut top = area.top + win_height * row; + let mut top = area.top + base_height * row; match layout_flip { Some(Axis::Horizontal) => { left = flipped_col_lefts[col_idx]; } Some(Axis::Vertical) => { - top = area.bottom - win_height * (row + 1) + area.top; + top = if is_last_row { + area.top + } else { + area.top + area.bottom - base_height * (row + 1) + }; } Some(Axis::HorizontalAndVertical) => { left = flipped_col_lefts[col_idx]; - top = area.bottom - win_height * (row + 1) + area.top; + top = if is_last_row { + area.top + } else { + area.top + area.bottom - base_height * (row + 1) + }; } None => {} } @@ -948,6 +984,16 @@ fn columns_with_ratios( left += right; } + // Last column absorbs any remainder from integer division + // so that columns tile the full area width without gaps + let total_width: i32 = layouts.iter().map(|r| r.right).sum(); + let remainder = area.right - total_width; + if remainder > 0 + && let Some(last) = layouts.last_mut() + { + last.right += remainder; + } + layouts } @@ -1019,6 +1065,16 @@ fn rows_with_ratios( top += bottom; } + // Last row absorbs any remainder from integer division + // so that rows tile the full area height without gaps + let total_height: i32 = layouts.iter().map(|r| r.bottom).sum(); + let remainder = area.bottom - total_height; + if remainder > 0 + && let Some(last) = layouts.last_mut() + { + last.bottom += remainder; + } + layouts } diff --git a/komorebi-layouts/src/arrangement_tests.rs b/komorebi-layouts/src/arrangement_tests.rs index 36c2b4d8..7b056bfc 100644 --- a/komorebi-layouts/src/arrangement_tests.rs +++ b/komorebi-layouts/src/arrangement_tests.rs @@ -203,6 +203,43 @@ mod columns_with_ratios_tests { assert_eq!(layouts[i].right, 200); } } + + #[test] + fn test_columns_cover_full_width_no_ratios() { + // 1000 / 3 = 333, 333*3 = 999 => 1px remainder + let area = test_area(); + let layouts = columns_with_ratios(&area, 3, None); + + let total_width: i32 = layouts.iter().map(|r| r.right).sum(); + assert_eq!( + total_width, area.right, + "columns should cover full width, got {total_width} expected {}", + area.right, + ); + + let last = layouts.last().unwrap(); + let right_edge = last.left + last.right; + assert_eq!(right_edge, area.left + area.right); + } + + #[test] + fn test_columns_cover_full_width_with_ratios() { + // ratio=0.3 with 4 columns: col0=300, remaining 700/3=233, 233*3=699 => 1px remainder + let area = test_area(); + let opts = layout_options_with_column_ratios(&[0.3]); + let layouts = columns_with_ratios(&area, 4, opts.column_ratios); + + let total_width: i32 = layouts.iter().map(|r| r.right).sum(); + assert_eq!( + total_width, area.right, + "columns should cover full width, got {total_width} expected {}", + area.right, + ); + + let last = layouts.last().unwrap(); + let right_edge = last.left + last.right; + assert_eq!(right_edge, area.left + area.right); + } } mod rows_with_ratios_tests { @@ -261,6 +298,43 @@ mod rows_with_ratios_tests { // Last row gets remaining: 600 assert_eq!(layouts[1].bottom, 600); } + + #[test] + fn test_rows_cover_full_height_no_ratios() { + // 800 / 3 = 266, 266*3 = 798 => 2px remainder + let area = test_area(); + let layouts = rows_with_ratios(&area, 3, None); + + let total_height: i32 = layouts.iter().map(|r| r.bottom).sum(); + assert_eq!( + total_height, area.bottom, + "rows should cover full height, got {total_height} expected {}", + area.bottom, + ); + + let last = layouts.last().unwrap(); + let bottom_edge = last.top + last.bottom; + assert_eq!(bottom_edge, area.top + area.bottom); + } + + #[test] + fn test_rows_cover_full_height_with_ratios() { + // ratio=0.3 with 4 rows: row0=240, remaining 560/3=186, 186*3=558 => 2px remainder + let area = test_area(); + let opts = layout_options_with_row_ratios(&[0.3]); + let layouts = rows_with_ratios(&area, 4, opts.row_ratios); + + let total_height: i32 = layouts.iter().map(|r| r.bottom).sum(); + assert_eq!( + total_height, area.bottom, + "rows should cover full height, got {total_height} expected {}", + area.bottom, + ); + + let last = layouts.last().unwrap(); + let bottom_edge = last.top + last.bottom; + assert_eq!(bottom_edge, area.top + area.bottom); + } } mod vertical_stack_layout_tests { @@ -361,6 +435,90 @@ mod horizontal_stack_layout_tests { // Primary row should be 70% height assert_eq!(layouts[0].bottom, 560); } + + #[test] + fn test_horizontal_stack_columns_cover_full_width() { + // 4 windows: primary row + 3 stack columns + // stack width = 1000, 1000/3 = 333, 333*3 = 999 => 1px gap + let area = test_area(); + let len = NonZeroUsize::new(4).unwrap(); + let layouts = + DefaultLayout::HorizontalStack.calculate(&area, len, None, None, &[], 0, None, &[]); + + // Stack windows (indices 1..4) share the bottom row + let stack = &layouts[1..]; + let last = stack.last().unwrap(); + let right_edge = last.left + last.right; + assert_eq!( + right_edge, + area.left + area.right, + "stack columns should cover full width, right edge is {right_edge} expected {}", + area.left + area.right, + ); + } +} + +mod vertical_stack_rows_cover_full_height_tests { + use super::*; + + #[test] + fn test_vertical_stack_rows_cover_full_height() { + // 4 windows: primary column + 3 stack rows + // stack height = 800, 800/3 = 266, 266*3 = 798 => 2px gap + let area = test_area(); + let len = NonZeroUsize::new(4).unwrap(); + let layouts = + DefaultLayout::VerticalStack.calculate(&area, len, None, None, &[], 0, None, &[]); + + // Stack windows (indices 1..4) share the right column + let stack = &layouts[1..]; + let last = stack.last().unwrap(); + let bottom_edge = last.top + last.bottom; + assert_eq!( + bottom_edge, + area.top + area.bottom, + "stack rows should cover full height, bottom edge is {bottom_edge} expected {}", + area.top + area.bottom, + ); + } +} + +mod scrolling_layout_tests { + use super::*; + + #[test] + fn test_scrolling_visible_columns_cover_full_width() { + // 1921 / 3 = 640, 640*3 = 1920 => 1px gap + let area = Rect { + left: 0, + top: 0, + right: 1921, + bottom: 800, + }; + let len = NonZeroUsize::new(5).unwrap(); + let opts = LayoutOptions { + scrolling: Some(crate::ScrollingLayoutOptions { + columns: 3, + center_focused_column: None, + }), + grid: None, + column_ratios: None, + row_ratios: None, + }; + let layouts = + DefaultLayout::Scrolling.calculate(&area, len, None, None, &[], 0, Some(opts), &[]); + + // First 3 windows should be visible (focused_idx=0) + let visible = &layouts[0..3]; + let last_visible = visible.last().unwrap(); + let right_edge = last_visible.left + last_visible.right; + assert_eq!( + right_edge, + area.left + area.right, + "visible columns should cover full width, right edge is {right_edge} expected {}", + area.left + area.right, + ); + } } mod ultrawide_layout_tests { @@ -861,6 +1019,245 @@ mod grid_layout_tests { ); } } + + #[test] + fn test_grid_uneven_rows_cover_full_height() { + // 7 windows => ceil(sqrt(7)) = 3 columns + // Distribution: col0=2 rows, col1=2 rows, col2=3 rows + // With area.bottom=800: + // 2-row columns: 800/2=400 each, total=800 (ok) + // 3-row column: 800/3=266 each, total=798 (2px gap!) + let area = Rect { + left: 0, + top: 0, + right: 1200, + bottom: 800, + }; + let layouts = DefaultLayout::Grid.calculate( + &area, + NonZeroUsize::new(7).unwrap(), + None, + None, + &[], + 0, + None, + &[], + ); + + assert_eq!(layouts.len(), 7); + + // Group windows by column (by their left position) + let mut columns: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + for layout in &layouts { + columns.entry(layout.left).or_default().push(layout); + } + + // Every column's windows should cover the full area height + for (&col_left, windows) in &columns { + // Sort by top position + let mut sorted: Vec<&&Rect> = windows.iter().collect(); + sorted.sort_by_key(|w| w.top); + + // First window should start at area.top + assert_eq!( + sorted[0].top, area.top, + "column at left={col_left}: first window should start at area.top" + ); + + // Last window's bottom edge should reach area.bottom + let last = sorted.last().unwrap(); + let bottom_edge = last.top + last.bottom; + assert_eq!( + bottom_edge, + area.bottom, + "column at left={col_left} ({} rows): bottom edge is {bottom_edge}, \ + expected {}. Gap of {} pixels", + windows.len(), + area.bottom, + area.bottom - bottom_edge, + ); + } + } + + #[test] + fn test_grid_uneven_rows_cover_full_height_with_vertical_flip() { + let area = Rect { + left: 0, + top: 0, + right: 1200, + bottom: 800, + }; + + for flip in [Axis::Vertical, Axis::HorizontalAndVertical] { + let layouts = DefaultLayout::Grid.calculate( + &area, + NonZeroUsize::new(7).unwrap(), + None, + Some(flip), + &[], + 0, + None, + &[], + ); + + let mut columns: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + for layout in &layouts { + columns.entry(layout.left).or_default().push(layout); + } + + for (&col_left, windows) in &columns { + let mut sorted: Vec<&&Rect> = windows.iter().collect(); + sorted.sort_by_key(|w| w.top); + + assert_eq!( + sorted[0].top, area.top, + "{flip:?}: column at left={col_left}: first window should start at area.top" + ); + + let last = sorted.last().unwrap(); + let bottom_edge = last.top + last.bottom; + assert_eq!( + bottom_edge, + area.bottom, + "{flip:?}: column at left={col_left} ({} rows): bottom edge is {bottom_edge}, \ + expected {}. Gap of {} pixels", + windows.len(), + area.bottom, + area.bottom - bottom_edge, + ); + + // Adjacent windows within the column should have no gaps + for pair in sorted.windows(2) { + let edge = pair[0].top + pair[0].bottom; + assert_eq!( + edge, pair[1].top, + "{flip:?}: column at left={col_left}: gap between rows at y={edge} and y={}", + pair[1].top, + ); + } + } + } + } + + #[test] + fn test_grid_uneven_columns_cover_full_width() { + // 5 windows => ceil(sqrt(5)) = 3 columns + // With area.right=1000: 1000/3=333 each, total=999 (1px gap!) + let area = Rect { + left: 0, + top: 0, + right: 1000, + bottom: 800, + }; + let layouts = DefaultLayout::Grid.calculate( + &area, + NonZeroUsize::new(5).unwrap(), + None, + None, + &[], + 0, + None, + &[], + ); + + assert_eq!(layouts.len(), 5); + + // Group windows by column (by their left position) + let mut columns: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + for layout in &layouts { + columns.entry(layout.left).or_default().push(layout); + } + + // First column should start at area.left + let first_left = *columns.keys().next().unwrap(); + assert_eq!( + first_left, area.left, + "first column should start at area.left" + ); + + // Last column's right edge should reach area.right + let (&last_left, last_windows) = columns.iter().last().unwrap(); + let last_right_edge = last_left + last_windows[0].right; + assert_eq!( + last_right_edge, + area.left + area.right, + "last column right edge is {last_right_edge}, expected {}. Gap of {} pixels", + area.left + area.right, + area.left + area.right - last_right_edge, + ); + + // Adjacent columns should have no gaps + let col_entries: Vec<_> = columns.iter().collect(); + for pair in col_entries.windows(2) { + let (&left_a, windows_a) = pair[0]; + let (&left_b, _) = pair[1]; + let right_edge_a = left_a + windows_a[0].right; + assert_eq!( + right_edge_a, left_b, + "gap between columns at x={right_edge_a} and x={left_b}", + ); + } + } + + #[test] + fn test_grid_uneven_columns_cover_full_width_with_horizontal_flip() { + let area = Rect { + left: 0, + top: 0, + right: 1000, + bottom: 800, + }; + + for flip in [Axis::Horizontal, Axis::HorizontalAndVertical] { + let layouts = DefaultLayout::Grid.calculate( + &area, + NonZeroUsize::new(5).unwrap(), + None, + Some(flip), + &[], + 0, + None, + &[], + ); + + let mut columns: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + for layout in &layouts { + columns.entry(layout.left).or_default().push(layout); + } + + let first_left = *columns.keys().next().unwrap(); + assert_eq!( + first_left, area.left, + "{flip:?}: first column should start at area.left" + ); + + let (&last_left, last_windows) = columns.iter().last().unwrap(); + let last_right_edge = last_left + last_windows[0].right; + assert_eq!( + last_right_edge, + area.left + area.right, + "{flip:?}: last column right edge is {last_right_edge}, expected {}. Gap of {} pixels", + area.left + area.right, + area.left + area.right - last_right_edge, + ); + + // Adjacent columns should have no gaps + let col_entries: Vec<_> = columns.iter().collect(); + for pair in col_entries.windows(2) { + let (&left_a, windows_a) = pair[0]; + let (&left_b, _) = pair[1]; + let right_edge_a = left_a + windows_a[0].right; + assert_eq!( + right_edge_a, left_b, + "{flip:?}: gap between columns at x={right_edge_a} and x={left_b}", + ); + } + } + } } mod layout_flip_tests { @@ -911,6 +1308,180 @@ mod layout_flip_tests { } } +mod flip_remainder_coverage_tests { + use super::*; + + /// Verify that layouts tile the full area with no gaps after flipping. + /// Checks that the leftmost edge == area.left, rightmost edge == area.left + area.right, + /// topmost edge == area.top, bottommost edge == area.top + area.bottom, + /// and no two windows overlap. + fn assert_full_coverage(layouts: &[Rect], area: &Rect, label: &str) { + assert!(!layouts.is_empty(), "{label}: no layouts produced"); + + let left_edge = layouts.iter().map(|r| r.left).min().unwrap(); + let top_edge = layouts.iter().map(|r| r.top).min().unwrap(); + let right_edge = layouts.iter().map(|r| r.left + r.right).max().unwrap(); + let bottom_edge = layouts.iter().map(|r| r.top + r.bottom).max().unwrap(); + + assert_eq!(left_edge, area.left, "{label}: left edge gap"); + assert_eq!(top_edge, area.top, "{label}: top edge gap"); + assert_eq!( + right_edge, + area.left + area.right, + "{label}: right edge gap of {} pixels", + area.left + area.right - right_edge, + ); + assert_eq!( + bottom_edge, + area.top + area.bottom, + "{label}: bottom edge gap of {} pixels", + area.top + area.bottom - bottom_edge, + ); + + // No overlaps + for (i, a) in layouts.iter().enumerate() { + for (j, b) in layouts.iter().enumerate() { + if i >= j { + continue; + } + let h = a.left < b.left + b.right && b.left < a.left + a.right; + let v = a.top < b.top + b.bottom && b.top < a.top + a.bottom; + assert!( + !(h && v), + "{label}: windows {i} and {j} overlap: {a:?} vs {b:?}" + ); + } + } + } + + // Area whose dimensions are not evenly divisible by 3 + fn uneven_area() -> Rect { + Rect { + left: 0, + top: 0, + right: 1000, // 1000/3 = 333 rem 1 + bottom: 800, // 800/3 = 266 rem 2 + } + } + + #[test] + fn test_columns_flipped_cover_full_area() { + let area = uneven_area(); + let len = NonZeroUsize::new(3).unwrap(); + for flip in [Axis::Horizontal, Axis::HorizontalAndVertical] { + let layouts = + DefaultLayout::Columns.calculate(&area, len, None, Some(flip), &[], 0, None, &[]); + assert_full_coverage(&layouts, &area, &format!("Columns {flip:?}")); + } + } + + #[test] + fn test_rows_flipped_cover_full_area() { + let area = uneven_area(); + let len = NonZeroUsize::new(3).unwrap(); + for flip in [Axis::Vertical, Axis::HorizontalAndVertical] { + let layouts = + DefaultLayout::Rows.calculate(&area, len, None, Some(flip), &[], 0, None, &[]); + assert_full_coverage(&layouts, &area, &format!("Rows {flip:?}")); + } + } + + #[test] + fn test_vertical_stack_flipped_cover_full_area() { + let area = uneven_area(); + // 4 windows: 1 primary + 3 stack rows (triggers remainder in rows_with_ratios) + let len = NonZeroUsize::new(4).unwrap(); + for flip in [ + Axis::Horizontal, + Axis::Vertical, + Axis::HorizontalAndVertical, + ] { + let layouts = DefaultLayout::VerticalStack.calculate( + &area, + len, + None, + Some(flip), + &[], + 0, + None, + &[], + ); + assert_full_coverage(&layouts, &area, &format!("VerticalStack {flip:?}")); + } + } + + #[test] + fn test_horizontal_stack_flipped_cover_full_area() { + let area = uneven_area(); + // 4 windows: 1 primary + 3 stack columns (triggers remainder in columns_with_ratios) + let len = NonZeroUsize::new(4).unwrap(); + for flip in [ + Axis::Horizontal, + Axis::Vertical, + Axis::HorizontalAndVertical, + ] { + let layouts = DefaultLayout::HorizontalStack.calculate( + &area, + len, + None, + Some(flip), + &[], + 0, + None, + &[], + ); + assert_full_coverage(&layouts, &area, &format!("HorizontalStack {flip:?}")); + } + } + + #[test] + fn test_right_main_vertical_stack_flipped_cover_full_area() { + let area = uneven_area(); + let len = NonZeroUsize::new(4).unwrap(); + for flip in [ + Axis::Horizontal, + Axis::Vertical, + Axis::HorizontalAndVertical, + ] { + let layouts = DefaultLayout::RightMainVerticalStack.calculate( + &area, + len, + None, + Some(flip), + &[], + 0, + None, + &[], + ); + assert_full_coverage(&layouts, &area, &format!("RightMainVerticalStack {flip:?}")); + } + } + + #[test] + fn test_ultrawide_vertical_stack_flipped_cover_full_area() { + let area = uneven_area(); + // 5 windows: primary + secondary + 3 tertiary rows (triggers remainder) + let len = NonZeroUsize::new(5).unwrap(); + for flip in [ + Axis::Horizontal, + Axis::Vertical, + Axis::HorizontalAndVertical, + ] { + let layouts = DefaultLayout::UltrawideVerticalStack.calculate( + &area, + len, + None, + Some(flip), + &[], + 0, + None, + &[], + ); + assert_full_coverage(&layouts, &area, &format!("UltrawideVerticalStack {flip:?}")); + } + } +} + mod container_padding_tests { use super::*;