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

Handle overflows in wrap_optimal_fit by divide-and-conquer #259

Closed
wants to merge 2 commits into from

Conversation

mgeisler
Copy link
Owner

The wrap_optimal_fit algorithm computes the penalty for a gap as gap * gap. If a fragment has a size near usize::max_value() and if the line width is small, this computation can easily overflow.

When this happened, we would previously abort or unwind. Now, we instead do the computations with checked arithmetic and detect the overflow. We then proceed to wrap the half of the fragments by themselves. If this work, we then wrap the second half. This way, we might be able to wrap everything without overflow.

Should there be a single fragment which causes the overflow by itself, this fragment is put on a line by itself.

When wrapping part of the fragments, we might of course end up with a partial last line. To fix this, we simply pop this line and re-wrap the fragments that were put onto this line. This ensures no “seams” in the wrapping.

Fixes #247.

@mgeisler
Copy link
Owner Author

I still need to add a test or two for the new behavior regarding wrapping over-sized fragments. So far I've simply tested that the existing tests still work the same.

@mgeisler
Copy link
Owner Author

mgeisler commented Dec 27, 2020

I've added two simple tests for the overflow behavior.

When wrapping part of the fragments, we might of course end up with a partial last line. To fix this, we simply pop this line and re-wrap the fragments that were put onto this line. This ensures no “seams” in the wrapping.

This is not perfect because I only back up like this in case the current slice resulted in more than two wrapped lines. This is to ensure progress in the wrapping, but it might have the bad side-effect of stitching the wrapped slices together in a bad way.

In general, the above approach implicitly assumes that over-sized fragments are rare so that they occur multiple lines apart. If this is not the case, I don't think the output will be so nice since the over-sized fragments end up forcing a line-break.

The `wrap_optimal_fit‘ algorithm computes the penalty for a gap as
`gap * gap`. If a fragment has a size near `usize::max_value()` and if
the line width is large, this computation can easily overflow.

When this happened, we would simply crash. Now, we instead do the
computations with checked arithmetic and detect the overflow. We then
proceed to wrap the first half of the fragments by themselves. If this
work, we then wrap the second half. This way, we might be able to wrap
everything without overflow.

Should there be a single fragment which causes the overflow by itself,
this fragment is put on a line by itself.

When wrapping part of the fragments, we might of course end up with a
partial last line. To fix this, we pop the last line and re-wrap the
fragments that were put onto this line. This helps remove some of the
“seams” that would otherwise occur. However, this is not perfect if
there are many over-sized fragments since they can still cause
half-empty lines to appear in the output.

Fixes #247.
@mgeisler mgeisler changed the title Handle overflows in wrap_optimal_fit by divide-and-conquer Handle overflows in wrap_optimal_fit by divide-and-conquer Apr 19, 2021
@mgeisler mgeisler force-pushed the master branch 3 times, most recently from 90541dd to e47928b Compare May 4, 2021 09:46
@mgeisler
Copy link
Owner Author

mgeisler commented Jan 9, 2022

Superseded by #421 which simply makes wrap_optimal_fit return a Result instead of trying to do to something fancy and handle overflows by recursion.

@mgeisler mgeisler closed this Jan 9, 2022
@mgeisler mgeisler deleted the wrap-optimal-fit-checked branch January 9, 2022 11:30
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.

Integer size and overflow when calculating penalty
1 participant