Restore the ability to use a different alignment width than breaking width

[DJMcNab]

This is a minimal escape hatch which allows this functionality, which was removed in #421.

I do agree that this is hard to use correctly, which is why I have it explicitly documented as an escape hatch. For my use case, I am not expecting to have a huge difference from the breaking width, so the risk is minimal.

[nicoburns]

It was removed because:

• Setting the alignment width on a line-by-line basis seems cumbersome
• It had previously been preposed that we remove this functionality in Make container_width a required alignment parameter #283, and as far as I was aware it only existed because I needed it (which I no longer do).

What is it that you need this for? And why doesn't AlignmentOption::align_when_overflowing make sense without this? A line can still overflow it's breaking width.

I was thinking that in future we could perhaps even remove the align method entirely and go back to alignment being automatically applied by the layout method.

[nicoburns]

The other thing to be aware of is that in the case where the breaking width is infinite (we treat both f32::INFINITY and f32::MAX as infinite), the inline_max_coord for every line will be recomputed and overidden in the Drop impl of BreakLines.

(this is because in that case the automatically computed alignment width falls back to "the width of the longest line", because aligning to an infinite width doesn't make sense / produces bad results)

[nicoburns]

I'm happy for this to land as a short-term hack, although it would be good if it could be documented as only working when layout is run with a definite/finite breaking width (otherwise the alignment width set here will end up being overridden).

It sounds like (in discussion of this issue during office hours) this is being used to workaround incorrect/unexpected layout being computed with the automatically computed alignment width (which "should" be correct).

I would be interested to see test cases that are not laying out as suspected. I suspect #619 (and specifically changes to how whitespace is "hung" at the end of lines) may help with this. Or else just some straight-up bug fixes to layout and/or content width computation.

[DJMcNab]

The other thing to be aware of is that in the case where the breaking width is infinite (we treat both f32::INFINITY and f32::MAX as infinite), the inline_max_coord for every line will be recomputed and overidden in the Drop impl of BreakLines.

Reviewing this again, I don't think this is actually the case; we only overwrite infinite inline_max_coord values on a per-line basis. As such, I don't think any documentation change is needed here.

It sounds like (in discussion of this issue during office hours) this is being used to workaround incorrect/unexpected layout being computed with the automatically computed alignment width (which "should" be correct).

As I understand it, the issue is a few cases where Parley's alignment is more eager to break than our reference, which is Chromium on Linux. I believe that the motivating use case was cases where the canonical width is calculated by a browser, and so the actual bound is extremely tight, so even an epsilon difference in rounding against Chromium would lead to different results. To be honest, I haven't wrapped my head around what these cases look like; the amount of shift we apply to hack around this issue is higher than I'd expect to be necessary were that the actual cause (but is lower than would be necessary if the cause is whitespace).

I do think that this will be a temporary workaround, but it's not something I'm likely to dig into before RustWeek.

I'm going to mark this as doc(hidden) for now, as you proposed during office hours, to make landing uncontroversial.

Thanks for the thorough review.

[nicoburns]

Reviewing this again, I don't think this is actually the case; we only overwrite infinite inline_max_coord values on a per-line basis. As such, I don't think any documentation change is needed here.

We overwrite it for all lines (in the case that layout_max_width and line_max_width are both infinite) here: https://github.com/linebender/parley/blob/main/parley/src/layout/line_break.rs#L1149

[DJMcNab]

Hmm, I still don't follow. Wouldn't the check here:

parley/parley/src/layout/line_break.rs

Lines 1153 to 1155 in d868052

	if line.metrics.inline_max_coord >= f32::MAX {
	line.metrics.inline_max_coord = layout_width;
	}

make that moot? That is, the individual line inline_max_coord is only overwritten if it's infinite?

[nicoburns]

Hmm, I still don't follow. Wouldn't the check here:

parley/parley/src/layout/line_break.rs

Lines 1153 to 1155 in d868052

	if line.metrics.inline_max_coord >= f32::MAX {
	line.metrics.inline_max_coord = layout_width;
	}

make that moot? That is, the individual line inline_max_coord is only overwritten if it's infinite?

Ah, that's a good point. Yeah, so I guess you can piggy-back on the existing mechanism to check whether this is needed.

[xStrom]

What is it that you need this for? And why doesn't AlignmentOption::align_when_overflowing make sense without this? A line can still overflow it's breaking width.

The ability to set alignment width was also used by Masonry and that no longer works. It does seem like the alignment of overflowing text can be resolved with align_when_overflowing. However, Masonry also does intelligent caching and I'm not sure that there's a readily available alternative for that.

For a specific example let's take window size reduction with right-aligned text.

1. Window size 100, max_advance 100, max_line_length 80, align_width 100 => x0 = 20
2. Window size 90, max_advance 90, max_line_length 80, align_width 90 => x0 = 10

With the old approach, Masonry checks its cache and sees that the cached layout for max_advance 100 would also be the result for max_advance 90 and re-uses it. It only has to re-do alignment with a new align_width.

With the new approach there is no align_width, so Masonry can't re-use the cached layout and has to do a new layout with line breaking again. Much slower.

I guess one approach here might be to evolve the Parley layout logic itself, so that it itself can skip re-doing breaking if the result would be the same. I'll look a bit into it.

[nicoburns]

Just wanted to say that as a general principle, I am open to reintroducing separate layout and alignment. The main reason I removed it is because it was (IIRC) introduced at my request, and at the time other people seemed to think that it was not great from an API perspective.

(I suspect we can probably come up with a solution where have both a simple API that do both, and lower-level APIs where they are handled separately)