[css-grid] Consider setting base sizes to growth limits when sizing under max-content constraint

[Loirooriol]

Consider https://jsfiddle.net/ectdsqz5/

.grid {
  display: inline-grid;
  grid-template-columns: max-content minmax(0, 100px);
  background: magenta;
}
.item {
  grid-column: 1 / 3;
  width: 100px;
  height: 100px;
  background: cyan;
}

<div class="grid">
  <div class="item"></div>
</div>

The 2nd column has a growth limit of 100px. When the grid is sized under a max-content constraint, https://drafts.csswg.org/css-grid/#algo-grow-tracks will increase the column to these 100px.

However, when distributing the width of the item across spanned tracks' base sizes, the formula only takes into account the base size, i.e. 0px:

extra-space = max(0, size-contribution - ∑track-sizes)

So the 1st column ends up receiving the whole 100px contribution of the item, even if the 2nd column will end up being wide enough to contain these!

Instead of getting a 100px+100px=200px wide grid, wouldn't 0px+100px=100px make more sense? Once this size is known and layout is performed again for real, then the 2nd column won't be forced to increase, so using the base size as the spec currently says can make more sense, then the final result would be 100px+0px=100px.

More precisely, I'm proposing that each time the growth limit of a track is updated (and is finite), the base size is smaller, and the grid container is being sized under a max-content constraint, the base size should be increased to match the growth limit. This would be at the end of https://drafts.csswg.org/css-grid/#algo-init, https://drafts.csswg.org/css-grid/#algo-single-span-items, and https://drafts.csswg.org/css-grid/#algo-spanning-items (for each span amount).

Another advantage is that I think this will allow a nice optimization. Theoretically, if the grid container is sized shrink-to-fit, you may need to perform layout under a max-content constraint and under a min-content constraint, but this is double work. Instead, Chromium does it only once, and then estimates the min-content constraint size using the base sizes of the tracks, and the max-content constraint size using the growth limits. This seems to work well for non-spanning items, but fails for spanning ones (see https://crbug.com/930857, and Edge has the same problem). My proposal above should make base sizes effectively useless when under a max-content constraint, so it seems it would allow tweaking the optimization to behave properly in the spanning case while still avoiding two layouts (before the real one).

A downside is that, since the size distribution will behave differently once the size of the grid container is known, it can be possible that the tracks will end up being occupying more space than the container due to #2356. For example, by adding two max-content columns to the example above, and a grid item spanning columns 2-4 with a min/max-content contribution of 50px. When under a max-content constraint, the columns will sum 0px+100px+0px+0px=100px. But when laying out for real, the contribution of the 1st item will go to the 1st column, and the contribution of the 2nd item to columns 3-4, so the result will be 100px+0px+25px+25px=150px.

But of course, this kind of things can already happen now (just less likely), e.g. https://jsfiddle.net/jh3q5k1u/

[fantasai]

Seems somewhat related to #3648 ?

[css-meeting-bot]

The CSS Working Group just discussed Grid (base sizes and growth length).

The full IRC log of that discussion

<fremy> Topic: Grid (base sizes and growth length)
<astearns> github: https://github.com//issues/3646
<fremy> oriol: there are two issues in there
<fremy> oriol: one is that when sizing a grid under max-content constraint
<fremy> oriol: when computing the space for item with intrinsic size
<fremy> oriol: we only consider the base size of tracks
<fremy> oriol: which can be smaller than the size of the track in the end
<fremy> oriol: so, maybe we should consider instead of the contribution of the item minus the track size
<fremy> oriol: consider the contribution of the item minus the growth limit
<fremy> oriol: (explains the example in the issue)
<fremy> oriol: when the growth limit of a track increases, we could increase to base size up to that growth limit
<fremy> oriol: this means that when you are using fit-content
<fremy> oriol: you need to do first min-content then max-content
<fremy> oriol: this is a lot of work
<fremy> oriol: but chromium isn't doing this all the time
<fremy> oriol: they perform a single layout track, and sets a base size of the tracks which is the min-content, and the growth-limit is the max-content
<fremy> oriol: which is ok if there are no spanning items, but if there are spanning items the result is wrong
<fremy> oriol: however, with my proposal, we wouldn't need to keep track of different base sizes for these two cases, and therefore we can always do only one single layout for fit-content
<fremy> oriol: (one single *extra* layout pass)
<fremy> astearns: thanks oriol
<fremy> astearns: some people in the room are reviewing this in a bit more details
<fremy> astearns: anyone has a response?
<fremy> astearns: or should we continue this in the issue?
<fremy> rego: one question, are we sure that what the change is what authors would expect?
<fremy> rego: if it is, I think the change oriol proposed should get done, but maybe it isn't what authors expect, I'm not sure
<fremy> astearns: jensimmons_ ? do you have an opinion?
<fremy> jensimmons_: not yet
<fremy> Rossen: the issue is fairly new
<fremy> Rossen: maybe we need to take more time
<tantek> TBH I can't visualize this. Any chance of projecting an example for things like this? E.g. before/after change?
<bkardell_> tantek, yes please
<fremy> Rossen: maybe we can use whiteboards at lunch
<florian> I am a little confused. Does that relate to https://github.com//issues/2356 ?
<fremy> fantasai: I'm in favor of that
<fremy> fantasai: but I would also desire to publish a new grid update
<fremy> fantasai: so I'm classifying changes between big changes and minor fixes
<fremy> fantasai: and I would want to first get to the changes that are just fixes
<fremy> fantasai: and continue to work on bigger changes on a more relaxed timeline
<fremy> rachelandrew: is that just a perf issue?
<fremy> rego: well, I mean, not entirely
<fremy> rego: but it would enable firefox to match both the spec and chrome, because in Chrome we don't follow the spec because it's both faster and easier not to follow it
<fantasai> s/but I would also desire/Wrt the labels, I want/
<fremy> rego: with the proposed change, we could both update to this new text
<tantek> (wants what Jen Simmons said minuted, but can't remember exactly to type it himself)
<jensimmons_> I said: Let’s figure this out. It should make sense for Authors. And we want to squash any interop problems…. yes.

[MatsPalmgren]

rego: but it would enable firefox to match both the spec and chrome, because in Chrome we don't follow the spec because it's both faster and easier not to follow it

Wow, really?!?! That's a pretty lousy attitude coming from a CSSWG member who ought to acknowledge that it's the W3C specs that are the standard that we all should implement. I guess the rest of us should just reverse-engineer Chrome now instead of reading the specs, huh?

[Loirooriol]

Seems somewhat related to #3648?

Not exactly the same. This one is about the calculation of the extra space

extra-space = max(0, size-contribution - ∑track-sizes)

I'm saying that, since tracks end up growing to reach their growth limit when sizing under a max-content constraint, in the formula above the "track sizes" should be the growth limits (or equivalently, the base sizes should be increased to be the growth limit whenever the later increases, not just at the end).

#3648 is about, once you have calculated the extra-space, how you distribute it among spanned columns.

[astearns]

@MatsPalmgren in context, we were (and are currently) discussing whether the change is closer to what authors want, and @mrego was noting that making the change might also allow Firefox to simplify their code. If we do not make the change to the spec I fully expect chrome to fix their implementation.

[mrego]

rego: but it would enable firefox to match both the spec and chrome, because in Chrome we don't follow the spec because it's both faster and easier not to follow it

Wow, really?!?! That's a pretty lousy attitude coming from a CSSWG member who ought to acknowledge that it's the W3C specs that are the standard that we all should implement. I guess the rest of us should just reverse-engineer Chrome now instead of reading the specs, huh?

Sorry this was wrong minuted, so let me clarify what I said.
I said that Chromium and WebKit are not following the spec, we're only doing 1 pass to compute intrinsic size (and not two 1 under min-content and another under max-content); and that Firefox is doing two passes and following the spec.
I also explained that we're not following the spec because we haven't found many cases broken due to this and it's also better to avoid doing the two layouts (we know it's wrong but this part of the implementation comes from old times and we never changed it).
Then I also said that if we go with the change suggested here, Firefox won't need to do two passes and that was all.

I never wanted to meant that specs should follow what Chromium does at all.

Sorry for any misunderstanding.

[fantasai]

@tabatkins and I reviewed this issue again. We think that we shouldn't make spec changes that would result in more chances of overflow when laying out content: it's better to have a little too much space in some cases, than not enough in others.

If there's a way to tighten up these situations without increasing the potential for overflow, that would be worth considering.

[Loirooriol]

Yeah I'm also leaning towards closing no change. It's not clear that the benefits surpass the downsides, and nowadays the compat risk of changing the algorithm is much bigger.