Skip to content

Commit

Permalink
fix(layout): don't leave gaps between chunks (#408)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@joshka
joshka committed Aug 18, 2023
1 parent f4ed3b7 commit 56455e0
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 92 deletions.
4 changes: 2 additions & 2 deletions examples/colors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ fn render_fg_named_colors<B: Backend>(frame: &mut Frame<B>, 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()
})
Expand All @@ -132,7 +132,7 @@ fn render_bg_named_colors<B: Backend>(frame: &mut Frame<B>, 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()
})
Expand Down
119 changes: 48 additions & 71 deletions src/layout.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,47 +568,28 @@ fn try_split(area: Rect, layout: &Layout) -> Result<Rc<[Rect]>, AddConstraintErr
let changes: HashMap<Variable, f64> = 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::<Rc<[Rect]>>();

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)
}

Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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]
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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)
]
);
Expand All @@ -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)
]
);
Expand Down
40 changes: 21 additions & 19 deletions tests/widgets_table.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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(
Expand Down Expand Up @@ -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│",
"│ │",
"│ │",
"└────────────────────────────┘",
Expand Down Expand Up @@ -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 │",
"│ │",
"│ │",
"└────────────────────────────┘",
Expand All @@ -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();

Expand All @@ -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(
Expand Down Expand Up @@ -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 │",
"│ │",
"│ │",
"└────────────────────────────┘",
Expand Down

0 comments on commit 56455e0

Please sign in to comment.