Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix layout gap #408

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Fix layout gap #408

merged 1 commit into from
Aug 18, 2023

Conversation

joshka
Copy link
Member

@joshka joshka commented Aug 17, 2023

Fixes #367

Note: this builds on #405, so it has the commits for that here. Look at just the last commit in this PR to understand it. The relevant change is just the addition of round():

ratatui/src/layout.rs

Lines 453 to 454 in 2364d9b

let start = changes.get(&element.start).unwrap_or(&0.0).round();
let end = changes.get(&element.end).unwrap_or(&0.0).round();

Note each of the test changes is a subjectively better rendering choice. I'm not 100% sure if there are some tests that would make it easier to see this change's effect rather than those that are already changed.

I updated the colors example to use a ratio instead of a percentage as this now renders correctly without gaps.

Before (using constraint: vec![Ratio(1,8); 8] )

image

After:

image

@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Merging #408 (a1fa41d) into main (f4ed3b7) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #408      +/-   ##
==========================================
- Coverage   86.14%   86.13%   -0.02%     
==========================================
  Files          40       40              
  Lines        9256     9240      -16     
==========================================
- Hits         7974     7959      -15     
+ Misses       1282     1281       -1     
Files Changed Coverage Δ
src/layout.rs 96.28% <100.00%> (+0.05%) ⬆️

src/layout.rs Outdated Show resolved Hide resolved
src/layout.rs Outdated Show resolved Hide resolved
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.
@joshka
Copy link
Member Author

joshka commented Aug 18, 2023

Rebased on main to pickup the bug fix in #408

Removed the no longer necessary comment in tests.

I also removed the code that checked the last element filled the area as the required constraint and corrected rounding take care of that fully now. I can't think of any possible way to trigger this code to mis-allocate the last constraint to less than the full amount that wouldn't also panic earlier due. Prior to this change, this was possible when the code took the floor of the start and floor of the width. E.g.: with area of 10:

  • when the last element is (start: 9.4, width: 0.6), the old start and width were (start: 9, width: 0). With this change that becomes (start: 9, width: 1).
  • Similarly for (9.6, 0.4) old was (9, 0) and this change becomes (10, 0), with the extra space allocated to the previous element.

@joshka joshka added this pull request to the merge queue Aug 18, 2023
Merged via the queue into ratatui-org:main with commit 56455e0 Aug 18, 2023
30 checks passed
@joshka joshka deleted the layout-gap branch August 18, 2023 10:24
@joshka joshka mentioned this pull request Aug 18, 2023
@joshka joshka added this to the v0.23.0 milestone Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Layout leaves gaps
4 participants