search

w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.51k stars 669 forks source link

[css-grid-3] Masonry - Intrinsic sizing of tracks & masonry-grid doesn't produce to good/desirable results. #8206

Closed bfgeek closed 8 months ago

bfgeek commented 1 year ago

This is a somewhat meta issue surrounding the current definition of how intrinsic track sizing works, and the intrinsic sizing of the masonry grid.

As currently defined (https://drafts.csswg.org/css-grid-3/#track-sizing) the tracks that have min-content, max-content, fit-content, and auto don't work as they do in regular grid. For most of the tracks the content based tracks will resolve to zero, and the auto will result to 1fr?

On top of this it means that an intrinsically sized masonry grid won't match developer expectations, e.g. width: max-content or similar (being a flex-item, grid-item, floating, etc, etc). We currently have a similar issue for multi-line flexbox which we are trying to fix, but this is somewhat worse due to being the default.

There are multiple avenues to solving the above, however one would be make masonry its own display type with a reduced subset of allowable definitions for tracks. Most masonry examples I've seen choose to size all the tracks the same size. An potential path forward would to allow repeats of "intrinsic" tracks where everything gets sizing as-if it was in one track, then decide how many tracks to insert. On top of this you could additionally allow non-intrinsic tracks in combination with the repeater. E.g. masonry-tracks: 100px repeat(auto-fill, minmax(min-content, 100px));

Edit - a simple example of intrinsic sizing: https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=11094

fantasai commented 1 year ago

Thinking about it, I think you're right, that these keywords should be defined over all the masonry items (including those ending up in other tracks). Using just the first row gets you reasonable results if all your items are the same size, but is prone to broken layouts if not. Better to have a little too much space in some columns than not enough.

fantasai commented 1 year ago

First cut of trying to spec this...

  1. Delete Grid Item Placement section.
  2. Replace Grid Axis Track Sizing section as below.
  3. Adjust Masonry Layout Algorithm as below.

Grid Axis Track Sizing

Track sizing works the same as in CSS Grid, except that when considering which items contribute to intrinsic sizes:

In the case of spanning items with no explicit placement, they are assumed to be placed at every possible start position, and contribute accordingly.

Example 1: Suppose there are two columns in the grid axis and that

In this case, items A, B, C, and D all contribute to sizing the first column, and A, B, and C (but not D) contribute to the second column.

Example 2: Suppose there are 5 columns in the grid axis, with the middle having a fixed size of 100px and the other two being auto-sized. An item that spans 2 tracks and has an intrinsic contribution of 220px is essentially copied and assumed to exist:

Note: This algorithm ensures that each track is at least big enough to accommodate every item that is ultimately placed in it, and does not create dependency cycles between placement and track sizing. However, depending on the variation in sizes, tracks could be larger than necessary: an exact fit is only guaranteed if all items are explicitly placed in the grid axis or all items are the same size (or matching multiples of that size, in the case of spanning items).

Masonry Layout Algorithm

Items are placed in order-modified document order, but items with a definite placement are placed before items with an indefinite position (as in regular grid layout).

For each of the tracks in the grid axis, keep a running position initialized to zero. First for each item with a definite placement in the grid axis, then for each item with an indefinite placement:

  1. If the item has an definite placement in the grid axis, use that placement. Otherwise, resolve its grid axis placement using these substeps:
    1. Starting at the first grid axis line in the implicit grid.
    2. Find the largest running position of the grid axis tracks that the item would span if it were placed at this line, and call this position max_pos.
    3. Increment the line number and repeat step 2 until the item would no longer fit inside the grid.
    4. Pick the line that resulted in the smallest max_pos as the item’s definite placement in the grid axis.
  2. Place the item in its grid axis tracks at the maximum of the running positions of the tracks it spans.
  3. Calculate the size of the item’s containing block and then layout the item. Then calculate its resulting margin box in the masonry axis. Set the running position of the spanned grid axis tracks to max_pos + margin-box-end + grid-gap.
bfgeek commented 1 year ago

I don't think its that straightforward. There are lots of dependencies on placement hidden within the grid track sizing algorithm - for example (after a very quick look):

The contribution size for a grid-item isn't the same value for "all" tracks, it needs to be calculated per placement - I'm a little concerned about the performance of this approach.

ethanjv commented 1 year ago

My two cents: I feel that the concept of track sizing function in grid is very tied to placement happening before sizing.

To me, it feels meaningless to have a max-content track that ends up being sized much larger than any item placed within that track. The comparison at the beginning of the spec with multi-column layout feels much more appropiate to masonry rather than grid when thinking of @fantasai latter proposal, and it pretty much ties to @bfgeek alternate proposal, where repeat(auto-fill, auto) will do something very similar: consider the intrinsic size of all items to determine an appropiate size for a track that will automatically repeat to fill the available space.

Providing more context on @bfgeek's last point, the final position of a grid item is relevant in case it spans something like minmax(auto, 100px). In such scenario, if we ignore the max track sizing function (because we don't know where the item will end up being placed) we won't constraint its automatic minimum size and accommodating its unconstrained min-content size could overflow the maximum of the track(s) in which we end up placing it.

I would argue that "The full power of grid layout is available in the grid axis" can be tricky to achieve with such approach if two completely different behaviors are expected depending on whether masonry is in the equation or not.

PS. I'm also concerned that considering all non-explicitly placed grid items in every posible start position will likely result in bad performance, that's a guaranteed quadratic algorithm.

fantasai commented 1 year ago

that's a guaranteed quadratic algorithm

It's not quadratic. It's NxM where N is the number of items and M is the number of columns. And reduces to N (same as Grid, or IanK's proposal) if the columns have the same sizing function.

css-meeting-bot commented 1 year ago

The CSS Working Group just discussed [css-grid-3] Masonry - Intrinsic sizing of tracks & masonry-grid doesn't produce to good/desirable results., and agreed to the following:

The full IRC log of that discussion <emeyer> iank_: Spec as it was initially introduced has some awkward placement + intrinsic size outcomes
<emeyer> …In grid layout, you have to put everything into areas and then size areas based on that placement
<emeyer> …Masonry layout is kind of backwards, in that you place your item into the highest available track
<emeyer> …The original spec got around this, but by placing everything into the first column after it had placed an item in each track, this had a lot of bad side effects around intrinsic sizing
<emeyer> …You can have intrinsically sized track and something goes into the second column, the column might not be wide enough
<emeyer> TabAtkins: The upshot is part of our larger argument that masonry shouldn’t be defined on top of grid because of the inside-out ness
<astearns> ack emilio
<astearns> ack fantasai
<emeyer> …We think masonry should be its own display type that can have its own layout approach and maybe some new features that aren’t possible as long as it’s based on grid
<emeyer> fantasai: I’d like to sidestep display types and focus on sizing
<emeyer> …We a gree with Ian that it’s not ideal and speccing something out that handles intrinsic sizing on its own terms makes sense
<emeyer> …We’d like to spec that out in the editor’s draft
<emeyer> …Asking for a resolution to remove the “first row is special” thing from the spec
<iank_> q+
<emeyer> …And add l anguage to keep intrinsic sizing from being dependent on placement
<emeyer> …If we embed into grid, this is a bit more complicated
<astearns> ack iank_
<emeyer> astearns: This reworking is necessary whether we change display types?
<emeyer> fantasai: Yes
<emeyer> iank_: I don’t think it’s that simple because there are a lot of touchpoints with the grid specification
<emeyer> …Regarding placement in the non-masonry axis regarding how the element contributes
<emeyer> …I’m a little concerned that implementations-wise, there’s lots and lots of edge cases that rely on grid placement
<astearns> ack fantasai
<emeyer> fantasai: I recognize that this is complicated, but I’d like to at least try
<emeyer> …We can definitely come back to where we are if our proposal isn’t workable
<emeyer> iank_: We could work out the issues in a PR
<emeyer> fantasai: True, but the Draft is a draft and we all agree the current draft isn’t what we want
<emeyer> iank_: I think the Microsoft folks had a look and came to similar conclusions
<emeyer> …I want to avoid falling into quadratic behavior with variable track sizes
<emeyer> …When I originally wrote this up, I thought it could be mitigated, but now I’m pretty strongly in the camp we’ll need a new display type
<emeyer> astearns: I propose we task Elika and Ian to work on a PR and come back to the group
<emeyer> fantasai: Can we please land changes in the ED and iterate until we either have a solution or decide we can’t?
<fantasai> It's an "editor's draft" for a reason.
<fantasai> This is exactly what editor's drafts are for
<emeyer> jensimmons: It sounds like there’s a lot of disagreement between Appole and Google on this
<emeyer> astearns: I was thinkg of it more as “let’s collaborate” so we know whatever gets into the draft won’t have immediate objections
<emeyer> jensimmons: I think there’s good questions about implementability and speed, and Ian’s skepticism is perhaps correct
<emeyer> …We want to push further down the road
<TabAtkins> It wasn't minus, but I said "yes, please work in the ED"
<emeyer> astearns: So we propose Elika fix up the Editor’s Draft and bring it back
<emeyer> iank_: Okay with me
<TabAtkins> s/minus/minuted/
<emeyer> fantasai: If we’re going to stick with the grid model, we should go this direction
<emeyer> astearns: I want to avoid resolving on a direction since we don’t know what the direction will be
<emeyer> fantasai: I think we need to take Ian’s approach contributions of all auto sized items in every auto sized track
<emeyer> …Updating the draft to say that we want to push in that direction seems to make sense to me
<emeyer> iank_: Yes, the direction, like, when I proposed this direction I didn’t see all the complications I now see
<emeyer> …If we want to go down the grid path, I’m not sure this path is workable
<emeyer> …I’m fine with editing the ED with a large scary note that details what’s being worked out
<emeyer> jensimmons: There’s an inconsistency between how this is being handled and how anchor positioning is being handled
<emeyer> …We have real objections about try blocks and they’re not being heard, but here similar objections are being taken seriously
<TabAtkins> We've already said that we'd like fantasai to continue iterating on this in the Grid 3 ED. Unsure what the complaint is.
<fantasai> proposal: Remove first-row-is-special handling of intrinsic track sizing in Masonry spec, replace with all auto-placed items contribute to all intrinsically sized tracks
<emeyer> astearns: Elika, can you state the proposed resolution?
<fantasai> Also: add a scary warning
<fantasai> :)
<emeyer> RESOLVED: Remove first-row-is-special handling of intrinsic track sizing in Masonry spec, replace with all auto-placed items contribute to all intrinsically sized tracks
<TabAtkins> I'm here ://///
<TabAtkins> Dang zoom
bfgeek commented 1 year ago

It's not quadratic. It's NxM where N is the number of items and M is the number of columns. And reduces to N (same as Grid, or IanK's proposal) if the columns have the same sizing function.

If tracks have different sizing functions then it goes quadratic however.

(edit: and I think potentially when subgrid is involved - gets super complex).

ethanjv commented 1 year ago

that's a guaranteed quadratic algorithm

It's not quadratic. It's NxM where N is the number of items and M is the number of columns. And reduces to N (same as Grid, or IanK's proposal) if the columns have the same sizing function.

Well, computing the intrinsic contribution of grid items would be O(N) in such case, but if we completely rely on the grid track sizing algorithm as it is right now, then equally distributing such contribution to all the possible tracks it could be placed, which are all of them, makes it O(NxM) (...unless we change how the contribution of a grid item is accommodated for masonry).

fantasai commented 1 year ago

Merged in edits for https://github.com/w3c/csswg-drafts/issues/8206#issuecomment-1710169743 per https://github.com/w3c/csswg-drafts/issues/8206#issuecomment-1721366240

ethanjv commented 1 year ago

Btw we might want to consider that, in the current draft of masonry, having definite tracks mixed with intrinsic tracks might result in some items overflowing the spanning tracks in the grid axis. E.g., if we have a grid-template-columns: 50px auto definition, if we have two auto placed items, the first one with a min-content size of 100px and the other with 50px, the first will be placed in the first column and overflow it, then the second would be placed in the auto track.

Maybe we want to prioritize placing a grid item in a track that won't overflow?

fantasai commented 8 months ago

@ethanjv We currently don't prioritize item placement based on the resolved size of the tracks... if that's something you thin we should add, maybe open a new issue? :)