dfrg
Make `container_width` a required alignment parameter
Some prior discussion here: https://github.com/linebender/parley/pull/278#discussion_r1959410670.
Aligning to the layout's calculated width is not necessarily a sensible default, as that width changes depending on how exactly lines were broken given the container width (max_advance) used for line breaking.
Added this to the 0.3 milestone, as we probably should resolve this decision before 0.3 to reduce churn.
We can also continue the discussion on whether container_width should be part of AlignmentOptions here. If it's non-Option (as proposed here), it can't be. If it's Option, I think it should be.
My current thinking is exactly because there's no good no-context default, we should just make container_width a required parameter. Having access to Layout::width and setting the alignment width to that in case of missing container_width is not really a good substitute, as that depends on exactly how the lines were broken. In a text box, the alignment would now jitter as users are typing.
One alternative, reusing the max_advance of line breaking for alignment probably also won't really work, as floated boxes likely impact max_advance.
One alternative, reusing the max_advance of line breaking for alignment probably also won't really work, as floated boxes likely impact
max_advance.
They will?
One alternative, reusing the
max_advanceof line breaking for alignment probably also won't really work, as floated boxes likely impactmax_advance.
My understanding was that we should be moving to using line.max_advance for alignment anyway as the separate alignment width mostly existed to account for (near) infinite line.max_advance. One of the motivations for #259 was to have a sensible cap on that. If we want to support floated boxes, we need to be able to align every line individually anyway.
One alternative, reusing the max_advance of line breaking for alignment probably also won't really work, as floated boxes likely impact
max_advance.They will?
Maybe? I'm not exactly sure yet what it will look like, but one option for line breaking in the presence of floated boxes is to line-break line-by-line, allowing the caller to adjust max_advance based on whether at this position there's now a box (or polygon!) that the text needs to wrap around.
My understanding was that we should be moving to using line.max_advance for alignment anyway as the separate alignment width mostly existed to account for (near) infinite line.max_advance.
That's a good point actually, requiring a sensible max_advance to be set by the user in all cases (which we don't currently require). How I pictured it before your point, was to align to f32::min(container_width, line.max_advance), to work around that issue.
I know it doesn't really help us now, but this would be a great use case for https://github.com/rust-lang/rust/issues/132162
My understanding was that we should be moving to using line.max_advance for alignment anyway as the separate alignment width mostly existed to account for (near) infinite line.max_advance.
That's a good point actually, requiring a sensible
max_advanceto be set by the user in all cases (which we don't currently require). How I pictured it before your point, was to align tof32::min(container_width, line.max_advance), to work around that issue.
We don't even need a sensible max_advance to be set by the user, we can always do f32::min(user_max_advance, max_content_width), and this should do what the user intended, even if they set a much larger max_advance.
@nicoburns do you by any chance remember if there were other motivations than the one I mentioned for the separate alignment width?
We don't even need a sensible max_advance to be set by the user, we can always do f32::min(user_max_advance, max_content_width), and this should do what the user intended, even if they set a much larger max_advance.
I'm not sure it would. Consider the case where the user has text that is not allowed to wrap, and should instead overflow its container if it's too wide. The max_advance is infinite, but the container has a definite width and (with default AlignmentOptions) the layout should be start-aligned to that container.
If my thinking is correct, this probably means with the current methods we can't use max_advance for alignment after all (except when line.max_advance is smaller than container_width).
Consider the case where the user has text that is not allowed to wrap, and should instead overflow its container if it's too wide. The max_advance is infinite.
This could also be represented by a finite max_advance but with a line-breaking mode that never breaks. I think https://github.com/linebender/parley/pull/259 does open up the possibility of requiring max_advance (making it non-optional). Although I'm a little reluctant to jump on that change until #259 has been proven in production.
@nicoburns do you by any chance remember if there were other motivations than the one I mentioned for the separate alignment width?
Blitz is currently doing this for alignment width (scroll up for max_advance). It's possible it could be made to work with them being the same, but I couldn't make it work when I first implemented this. This funky alignment_width was the motivation for separating out alignment into a separate step with it's own width, as it didn't seem right to pass all that info into Parley.
As of Parley 0.4, Blitz no longer needs an optional layout width or separate alignment width as it can now use the widths from compute_content_widths to pre-compute the correct width prior to both layout and alignment.