From 56455e0fee57616f87ea43872fb7d5d9bb14aff5 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Fri, 18 Aug 2023 03:23:13 -0700 Subject: [PATCH] fix(layout): don't leave gaps between chunks (#408) Previously the layout used the floor of the calculated start and width as the value to use for the split Rects. This resulted in gaps between the split rects. This change modifies the layout to round to the nearest column instead of taking the floor of the start and width. This results in the start and end of each rect being rounded the same way and being strictly adjacent without gaps. Because there is a required constraint that ensures that the last end is equal to the area end, there is no longer the need to fixup the last item width when the fill (as e.g. width = x.99 now rounds to x+1 not x). The colors example has been updated to use Ratio(1, 8) instead of Percentage(13), as this now renders without gaps for all possible sizes, whereas previously it would have left odd gaps between columns. --- examples/colors.rs | 4 +- src/layout.rs | 119 +++++++++++++++++------------------------ tests/widgets_table.rs | 40 +++++++------- 3 files changed, 71 insertions(+), 92 deletions(-) diff --git a/examples/colors.rs b/examples/colors.rs index 4d4bfda4..ec0ebbe8 100644 --- a/examples/colors.rs +++ b/examples/colors.rs @@ -107,7 +107,7 @@ fn render_fg_named_colors(frame: &mut Frame, bg: Color, area: Rec .flat_map(|area| { Layout::default() .direction(Direction::Horizontal) - .constraints(vec![Constraint::Percentage(13); 8]) + .constraints(vec![Constraint::Ratio(1, 8); 8]) .split(*area) .to_vec() }) @@ -132,7 +132,7 @@ fn render_bg_named_colors(frame: &mut Frame, fg: Color, area: Rec .flat_map(|area| { Layout::default() .direction(Direction::Horizontal) - .constraints(vec![Constraint::Percentage(13); 8]) + .constraints(vec![Constraint::Ratio(1, 8); 8]) .split(*area) .to_vec() }) diff --git a/src/layout.rs b/src/layout.rs index bb491906..c0ae99d5 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -568,47 +568,28 @@ fn try_split(area: Rect, layout: &Layout) -> Result, AddConstraintErr let changes: HashMap = solver.fetch_changes().iter().copied().collect(); // convert to Rects - let mut results = elements + let results = elements .iter() .map(|element| { - let start = *changes.get(&element.start).unwrap_or(&0.0); - let end = *changes.get(&element.end).unwrap_or(&0.0); + let start = changes.get(&element.start).unwrap_or(&0.0).round() as u16; + let end = changes.get(&element.end).unwrap_or(&0.0).round() as u16; let size = end - start; match layout.direction { Direction::Horizontal => Rect { - x: start as u16, + x: start, y: inner.y, - width: size as u16, + width: size, height: inner.height, }, Direction::Vertical => Rect { x: inner.x, - y: start as u16, + y: start, width: inner.width, - height: size as u16, + height: size, }, } }) .collect::>(); - - if layout.expand_to_fill { - // Because it's not always possible to satisfy all constraints, the last item might be - // slightly smaller than the available space. E.g. when the available space is 10, and the - // constraints are [Length(5), Max(4)], the last item will be 4 wide instead of 5. Fix this - // by extending the last item a bit if necessary. (`unwrap()` is safe here, because results - // has no shared references right now). - if let Some(last) = Rc::get_mut(&mut results).unwrap().last_mut() { - match layout.direction { - Direction::Horizontal => { - last.width = inner.right() - last.x; - } - Direction::Vertical => { - last.height = inner.bottom() - last.y; - } - } - } - } - Ok(results) } @@ -930,15 +911,15 @@ mod tests { test(Rect::new(0, 0, 1, 1), &[TEN, FULL], "b"); test(Rect::new(0, 0, 1, 1), &[TEN, DOUBLE], "b"); - test(Rect::new(0, 0, 1, 1), &[HALF, ZERO], "b"); - test(Rect::new(0, 0, 1, 1), &[HALF, HALF], "b"); - test(Rect::new(0, 0, 1, 1), &[HALF, FULL], "b"); - test(Rect::new(0, 0, 1, 1), &[HALF, DOUBLE], "b"); + test(Rect::new(0, 0, 1, 1), &[HALF, ZERO], "a"); + test(Rect::new(0, 0, 1, 1), &[HALF, HALF], "a"); + test(Rect::new(0, 0, 1, 1), &[HALF, FULL], "a"); + test(Rect::new(0, 0, 1, 1), &[HALF, DOUBLE], "a"); - test(Rect::new(0, 0, 1, 1), &[NINETY, ZERO], "b"); - test(Rect::new(0, 0, 1, 1), &[NINETY, HALF], "b"); - test(Rect::new(0, 0, 1, 1), &[NINETY, FULL], "b"); - test(Rect::new(0, 0, 1, 1), &[NINETY, DOUBLE], "b"); + test(Rect::new(0, 0, 1, 1), &[NINETY, ZERO], "a"); + test(Rect::new(0, 0, 1, 1), &[NINETY, HALF], "a"); + test(Rect::new(0, 0, 1, 1), &[NINETY, FULL], "a"); + test(Rect::new(0, 0, 1, 1), &[NINETY, DOUBLE], "a"); test(Rect::new(0, 0, 1, 1), &[FULL, ZERO], "a"); test(Rect::new(0, 0, 1, 1), &[FULL, HALF], "a"); @@ -957,18 +938,17 @@ mod tests { test(Rect::new(0, 0, 2, 1), &[TEN, FULL], "bb"); test(Rect::new(0, 0, 2, 1), &[TEN, DOUBLE], "bb"); - // should probably be "ab" but this is the current behavior - test(Rect::new(0, 0, 2, 1), &[QUARTER, ZERO], "bb"); - test(Rect::new(0, 0, 2, 1), &[QUARTER, QUARTER], "bb"); - test(Rect::new(0, 0, 2, 1), &[QUARTER, HALF], "bb"); - test(Rect::new(0, 0, 2, 1), &[QUARTER, FULL], "bb"); - test(Rect::new(0, 0, 2, 1), &[QUARTER, DOUBLE], "bb"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, ZERO], "ab"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, QUARTER], "ab"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, HALF], "ab"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, FULL], "ab"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, DOUBLE], "ab"); - test(Rect::new(0, 0, 2, 1), &[THIRD, ZERO], "bb"); - test(Rect::new(0, 0, 2, 1), &[THIRD, QUARTER], "bb"); - test(Rect::new(0, 0, 2, 1), &[THIRD, HALF], "bb"); - test(Rect::new(0, 0, 2, 1), &[THIRD, FULL], "bb"); - test(Rect::new(0, 0, 2, 1), &[THIRD, DOUBLE], "bb"); + test(Rect::new(0, 0, 2, 1), &[THIRD, ZERO], "ab"); + test(Rect::new(0, 0, 2, 1), &[THIRD, QUARTER], "ab"); + test(Rect::new(0, 0, 2, 1), &[THIRD, HALF], "ab"); + test(Rect::new(0, 0, 2, 1), &[THIRD, FULL], "ab"); + test(Rect::new(0, 0, 2, 1), &[THIRD, DOUBLE], "ab"); test(Rect::new(0, 0, 2, 1), &[HALF, ZERO], "ab"); test(Rect::new(0, 0, 2, 1), &[HALF, HALF], "ab"); @@ -977,9 +957,8 @@ mod tests { test(Rect::new(0, 0, 2, 1), &[FULL, HALF], "aa"); test(Rect::new(0, 0, 2, 1), &[FULL, FULL], "aa"); - // should probably be "abb" but this is what the current algorithm produces - test(Rect::new(0, 0, 3, 1), &[THIRD, THIRD], "bbb"); - test(Rect::new(0, 0, 3, 1), &[THIRD, TWO_THIRDS], "bbb"); + test(Rect::new(0, 0, 3, 1), &[THIRD, THIRD], "abb"); + test(Rect::new(0, 0, 3, 1), &[THIRD, TWO_THIRDS], "abb"); } #[test] @@ -1027,15 +1006,15 @@ mod tests { test(Rect::new(0, 0, 1, 1), &[TEN, FULL], "b"); test(Rect::new(0, 0, 1, 1), &[TEN, DOUBLE], "b"); - test(Rect::new(0, 0, 1, 1), &[HALF, ZERO], "b"); // bug? - test(Rect::new(0, 0, 1, 1), &[HALF, HALF], "b"); // bug? - test(Rect::new(0, 0, 1, 1), &[HALF, FULL], "b"); // bug? - test(Rect::new(0, 0, 1, 1), &[HALF, DOUBLE], "b"); // bug? + test(Rect::new(0, 0, 1, 1), &[HALF, ZERO], "a"); + test(Rect::new(0, 0, 1, 1), &[HALF, HALF], "a"); + test(Rect::new(0, 0, 1, 1), &[HALF, FULL], "a"); + test(Rect::new(0, 0, 1, 1), &[HALF, DOUBLE], "a"); - test(Rect::new(0, 0, 1, 1), &[NINETY, ZERO], "b"); // bug? - test(Rect::new(0, 0, 1, 1), &[NINETY, HALF], "b"); // bug? - test(Rect::new(0, 0, 1, 1), &[NINETY, FULL], "b"); // bug? - test(Rect::new(0, 0, 1, 1), &[NINETY, DOUBLE], "b"); // bug? + test(Rect::new(0, 0, 1, 1), &[NINETY, ZERO], "a"); + test(Rect::new(0, 0, 1, 1), &[NINETY, HALF], "a"); + test(Rect::new(0, 0, 1, 1), &[NINETY, FULL], "a"); + test(Rect::new(0, 0, 1, 1), &[NINETY, DOUBLE], "a"); test(Rect::new(0, 0, 1, 1), &[FULL, ZERO], "a"); test(Rect::new(0, 0, 1, 1), &[FULL, HALF], "a"); @@ -1054,19 +1033,17 @@ mod tests { test(Rect::new(0, 0, 2, 1), &[TEN, FULL], "bb"); test(Rect::new(0, 0, 2, 1), &[TEN, DOUBLE], "bb"); - // should probably be "ab" but this is what the current algorithm produces - test(Rect::new(0, 0, 2, 1), &[QUARTER, ZERO], "bb"); - test(Rect::new(0, 0, 2, 1), &[QUARTER, QUARTER], "bb"); - test(Rect::new(0, 0, 2, 1), &[QUARTER, HALF], "bb"); - test(Rect::new(0, 0, 2, 1), &[QUARTER, FULL], "bb"); - test(Rect::new(0, 0, 2, 1), &[QUARTER, DOUBLE], "bb"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, ZERO], "ab"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, QUARTER], "ab"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, HALF], "ab"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, FULL], "ab"); + test(Rect::new(0, 0, 2, 1), &[QUARTER, DOUBLE], "ab"); - // should probably be "ab" but this is what the current algorithm produces - test(Rect::new(0, 0, 2, 1), &[THIRD, ZERO], "bb"); - test(Rect::new(0, 0, 2, 1), &[THIRD, QUARTER], "bb"); - test(Rect::new(0, 0, 2, 1), &[THIRD, HALF], "bb"); - test(Rect::new(0, 0, 2, 1), &[THIRD, FULL], "bb"); - test(Rect::new(0, 0, 2, 1), &[THIRD, DOUBLE], "bb"); + test(Rect::new(0, 0, 2, 1), &[THIRD, ZERO], "ab"); + test(Rect::new(0, 0, 2, 1), &[THIRD, QUARTER], "ab"); + test(Rect::new(0, 0, 2, 1), &[THIRD, HALF], "ab"); + test(Rect::new(0, 0, 2, 1), &[THIRD, FULL], "ab"); + test(Rect::new(0, 0, 2, 1), &[THIRD, DOUBLE], "ab"); test(Rect::new(0, 0, 2, 1), &[HALF, ZERO], "ab"); test(Rect::new(0, 0, 2, 1), &[HALF, HALF], "ab"); @@ -1117,8 +1094,8 @@ mod tests { assert_eq!( layout[..], [ - Rect::new(0, 0, 1, 0), - Rect::new(0, 0, 1, 0), + Rect::new(0, 0, 1, 1), + Rect::new(0, 1, 1, 0), Rect::new(0, 1, 1, 0) ] ); @@ -1134,7 +1111,7 @@ mod tests { layout[..], [ Rect::new(0, 0, 1, 0), - Rect::new(0, 0, 1, 0), + Rect::new(0, 0, 1, 1), Rect::new(0, 1, 1, 0) ] ); diff --git a/tests/widgets_table.rs b/tests/widgets_table.rs index 7f883610..6d34a08f 100644 --- a/tests/widgets_table.rs +++ b/tests/widgets_table.rs @@ -304,7 +304,8 @@ fn widgets_table_columns_widths_can_use_percentage_constraints() { #[test] fn widgets_table_columns_widths_can_use_mixed_constraints() { - let test_case = |widths, expected| { + #[track_caller] + fn test_case(widths: &[Constraint], expected: Buffer) { let backend = TestBackend::new(30, 10); let mut terminal = Terminal::new(backend).unwrap(); @@ -324,7 +325,7 @@ fn widgets_table_columns_widths_can_use_mixed_constraints() { }) .unwrap(); terminal.backend().assert_buffer(&expected); - }; + } // columns of zero width show nothing test_case( @@ -356,12 +357,12 @@ fn widgets_table_columns_widths_can_use_mixed_constraints() { ], Buffer::with_lines(vec![ "┌────────────────────────────┐", - "│Hea Head2 He │", + "│Hea Head2 Hea│", "│ │", - "│Row Row12 Ro │", - "│Row Row22 Ro │", - "│Row Row32 Ro │", - "│Row Row42 Ro │", + "│Row Row12 Row│", + "│Row Row22 Row│", + "│Row Row32 Row│", + "│Row Row42 Row│", "│ │", "│ │", "└────────────────────────────┘", @@ -398,12 +399,12 @@ fn widgets_table_columns_widths_can_use_mixed_constraints() { ], Buffer::with_lines(vec![ "┌────────────────────────────┐", - "│Head1 Head2 │", + "│Head1 Head2 │", "│ │", - "│Row11 Row12 │", - "│Row21 Row22 │", - "│Row31 Row32 │", - "│Row41 Row42 │", + "│Row11 Row12 │", + "│Row21 Row22 │", + "│Row31 Row32 │", + "│Row41 Row42 │", "│ │", "│ │", "└────────────────────────────┘", @@ -413,7 +414,8 @@ fn widgets_table_columns_widths_can_use_mixed_constraints() { #[test] fn widgets_table_columns_widths_can_use_ratio_constraints() { - let test_case = |widths, expected| { + #[track_caller] + fn test_case(widths: &[Constraint], expected: Buffer) { let backend = TestBackend::new(30, 10); let mut terminal = Terminal::new(backend).unwrap(); @@ -434,7 +436,7 @@ fn widgets_table_columns_widths_can_use_ratio_constraints() { }) .unwrap(); terminal.backend().assert_buffer(&expected); - }; + } // columns of zero width show nothing test_case( @@ -487,12 +489,12 @@ fn widgets_table_columns_widths_can_use_ratio_constraints() { ], Buffer::with_lines(vec![ "┌────────────────────────────┐", - "│Head1 Head2 Head3 │", + "│Head1 Head2 Head3 │", "│ │", - "│Row11 Row12 Row13 │", - "│Row21 Row22 Row23 │", - "│Row31 Row32 Row33 │", - "│Row41 Row42 Row43 │", + "│Row11 Row12 Row13 │", + "│Row21 Row22 Row23 │", + "│Row31 Row32 Row33 │", + "│Row41 Row42 Row43 │", "│ │", "│ │", "└────────────────────────────┘",