Fix #134 and include a test for it.#175
Merged
jackpot51 merged 2 commits intopop-os:mainfrom Aug 28, 2023
Merged
Conversation
Try to ensure that using "the width computed during an unconstrained layout" as the width constraint during a relayout produces the same layout. This is useful for certain UI layout algorithms. See pop-os#134 * Instead of computing the LayoutLine width from the positioned and aligned glyphs, we pass through width computed during line wrapping (unless justified alignment is used, in this case we use the old approach because the use case for measuring the width isn't really applicable to justified text since that will just expand to the provided width). For the produced width to later give the same wrapping results when passed in as the `line_width` it needs to use the same exact float arithmatic that was used to compute the width that is compared against `line_width` when making line wrapping choices. Passing this width through as the LayoutLine width is the most covenient option without making more major changse to the API. Nevertheless, I am imagining that if we get a dedicated measurement method (i.e. that doesn't do the final positioning and alignment of glyphs and which caches `Vec<VisualLine>`), then this width can just be exposed there instead of preservering it in LayoutLine. * Incidentally, this fixes pop-os#169. * Switch substraction from `fit_x` to checking whether potential addition to the current line width would exceed the `line_width`. This avoids the float error being dependent on the provided `line_width` value. * When eliminating trailing space from the line width, we avoid backtracking with subtraction (which would not give the same exact value due to float error) and instead save the previous width and use that. * If the previous word did not exceed the line_width, we now include a single blank word even if it would cross the width limit since its width won't be counted. This is necessary to get the same wrapping behavior when re-using the measured width (which doesn't count a single trailing blank word). Note, this whitespace logic may be reworked anyway if <pop-os#155> is addressed. * Change tests to use `opt-level=1` to keep test runtime down. * Add `fonts` folder for fonts used in tests. * Fix an issue where a non-breaking whitespace was assumed to be the start of a section of spaces which included characters that weren't even whitespace. * Add some TODOs about incongruencies between `is_whitespace`, justification, and line breaks.
jackpot51
approved these changes
Aug 28, 2023
StewartCanva
pushed a commit
to StewartCanva/cosmic-text
that referenced
this pull request
Jun 26, 2025
Fix pop-os#134 and include a test for it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Try to ensure that using "the width computed during an unconstrained layout" as the width constraint during a relayout produces the same layout. Note, here I aim to also make this work for cases where the initial width limit isn't completely unconstrained. This is useful for certain UI layout algorithms.
Fixes #134
line_widthit needs to use the same exact float arithmetic that was used to compute the width that is compared againstline_widthwhen making line wrapping choices. Passing this width through as the LayoutLine width is the most convenient option without making more major changes to the API. Nevertheless, I am imagining that if we get a dedicated measurement method (i.e. that doesn't do the final positioning and alignment of glyphs and which cachesVec<VisualLine>), then this width can just be exposed there instead of preserving it in LayoutLine.edit: one thing that I was going to mention here is that this means the width in LayoutLine might not as precisely match the positioned glyphs where their positions have been perturbed by different floating point errors.
fit_xto checking whether potential addition to the current line width would exceed theline_width. This avoids the float error being dependent on the providedline_widthvalue.Wrap::Wordinserts newline for trailing space (after word) #155 is addressed.opt-level=1to keep test runtime down.fontsfolder for fonts used in tests.is_whitespace, justification, and line breaks.