[css-grid] Applying 'justify-content' content distribution is in the wrong place in the overall grid sizing algo #2557
Comments
|
I knew this would end up in a issue here. I discussed the case with @tabatkins today, and he was pretty sure Chromium was right.
Anyway let's see what other people think about this (CC @fantasai & @atanassov). BTW, Firefox and Edge behave the same for this case, and different from Chromium and WebKit. |
|
IIRC we considered the effect of alignment on same-axis sizing, e.g. it would add some slack to a spanning element which was spanning only max-content sized tracks. However, we didn't consider the effect of alignment on sizing in the other axis, and it does seem to me that doing so would be an improvement in most cases. (If orthogonal cases don't get quite the same benefits that's probably OK, as long as we don't break them.) |
|
If we do this as part of the track sizing algorithm, this will conflict with what was discussed at #2409. For example you have 2 rows of 100px each, and then you've a orthogonal item spanning both rows. |
|
@mrego I'm not following. We use 200px height to calculate the intrinsic width of the orthogonal item, sure. And then we start the track sizing algorithm, sizing the columns based on that information. Whether we perform alignment of the tracks after that set of tracks is sized or at the end of the algorithm, the final size of the orthogonal item will still be 300px or 500px. If we apply alignment within the grid algorithm, it seems to me we get a better result regardless, no? The proposal, if I understand correctly (just so we're all clear) is to change from:
to:
|
The issue is that baseline alignment in the row-axis (justify-self) may depend on the final size of the row, as explained in #2409. |
|
@javifernandez I'm not understanding why this is a problem. :( |
|
This issue is about the place to run the Content Alignment logic; with the new baseline alignment integrated in the track sizing algorithm, the place where we finally run the Content Alignment logic will have an impact in the result of the Baseline Alignment, generating different results. |
|
Hmm, indeed. Case in point: <style>
.grid {
display: grid;
grid: 100px 100px auto / auto;
align-content: space-between;
height: 300px;
}
.ortho {
grid-row: 1 / span 2;
font: 50px Ahem;
writing-mode: vertical-rl;
background: purple;
}
.content {
grid-row: 3;
}
</style>
<div class=grid>
<div class=ortho>É É ÉÉ</div>
<div class=content>foo</div>
</div>Today, the spec says that the orthogonal item can tell how large its containing block is (200px high, as it spans only rows with fixed max track sizing functions). The actual height of the block might not be 200px, as it depends on how much space is left over in the grid after the content-sized track is sized. If we move the distribution up in the algorithm, then we have to weaken the condition we're using - the amount of gap, and thus the size of the grid area, is content-sized if any track in the entire grid is content-sized. Per the spec, this means we have to assume the grid area is infinite, and we'll get the original bad layout from #2409. (On that note, we should probably make it clearer that gaps are considered in the calculation of the grid-area size, even if we leave distribution at the end.) |
|
No, we don't change the rules for estimating the height of the rows/gaps in step 1. It still starts out assuming the fixed heights and no content distribution gaps -- but at the end of step 2, we take the actually-calculated row sizes (and alignment) into account when performing the resizing check in step 3. |
|
@fantasai points out that I'm wrong - I'm talking about Step 1 in my previous comment, but row-distribution would happen in a new Step 2.5. So for the purpose of Step 1, the gap is still 0, and the item would still lay out into a fixed size of 400px. The distribution would then likely change the min-content contribution of the item, when we hit step 3, but that will just make us cycle once, and hopefully with a closer-to-correct min/max-content contribution. |
|
@javifernandez Given the above, do you agree that it's okay to move content distribution up into new steps 1.5 and 2.5 (and eliminate step 4)? In the common case you'll get tighter, more acccurate grid sizes; in some cases (ortho flows, column-wrap flexboxes, etc) you might trigger a step-3 cycle more often than the spec currently does, but it should give a better result, too. |
@tabatkins are you talking about the original 6 steps fantasai described ?
Wouldn't be the step 5 the one that should be removed ? I don't follow why we should remove step 4, neither in the 6 original steps or in the new ones (reduced to 5 steps) In any case, and with some caution until I had the time to implement a prototype for this change, I think those new steps 1.5 and 2.5 will produce better results and I'm for it. I'd like to know the @MatsPalmgren 's opinion about this, since he was the one creating the issue; besides, I'm curious about how and why he implemented it in the first place, because this is the Firefox's current behavior, right ? |
|
I was talking about the actual spec, where Step 4 is the "align tracks" step. (Well, it's "size the grid container and align tracks", but obviously we'll keep "size the grid container" around.) |
I'd be fine with that change, at least give it a try. I think I'll have some time in the coming days to implement some prototypes, just in case we want to wait to see if there are problems with the proposed changes. In any case, I'd like know what @MatsPalmgren thinks before committing the changes. |
@tabatkins Moving content-distribution just after step 1 wouldn't be enough to address the issues that @mrego described in #2557 (comment) |
|
It would. Step 1 would still proceed as currently stated; in @mrego's example, it would still lay out into 200px. Then distribution would happen in the new step 2 (step 1.5, between current step 1 and 2). The benefit, then, is that if step 3 causes us to cycle back and re-do steps 1 and 2, you do so with the column gaps already taken into account as affecting the width. (Row gaps, in step 2.5, wouldn't have an immediate effect, as no layout is done between step 2 and step 3.) Contrast this with the current spec, which does the same thing but doesn't let the step-3 cycle take column gaps into account; the column gaps don't appear until after sizing is totally done. |
|
The new changes indeed solve cases like the one @mrego described, thanks to the Step 3, as you mentioned. However, in case of chrome and webkit implementation it only works like that when the grid container has definite inline size. Perhaps this is an implementation detail, but we only run step 1 of the algorithm in order to determine the preferred width. Once set, we use this preferred width during the layout, so we can't then change it if Step 3 gives a different result for the columns. |
|
Doing the distribution step immediately after the track sizes are known always leads to better results as far as I can tell. You need both the position and size of the columns to calculate the correct CB inline-size for column-spanning items, and the CB inline-size is required to determine any content-based block-axis contributions for the items. I don't think the proposed change makes any difference to how row sizes are estimated for orthogonal items, since that happens during (or before) step 1. (I tend to think that the row estimation is suboptimal/wrong in both Chrome and Firefox currently though and that we can improve the heuristic there, but that seems like a separate issue.)
That's basically what Gecko does too. The proposed change doesn't affect that in any way though, since distribution of space only happens when the grid container has a definite size in that axis (so it doesn't occur during intrinsic sizing). |
|
Frankly speaking, I did not dive into the implementation details about Grid Sizing Algo. Standing at the point of usage, my preference would be Firefox does correctly which at least meets my expectation and is similar to the existing layout such as Flex (here's a simple demo). |
|
Ok, I think we have an agreement on the proposed changes, so that these new steps 1.5 and 2.5 will produce better results. I also agree that it shouldn't interfere with row estimation in case of orthogonal items, which will be addressed in the extra cycle, as tab mentioned.
Well, I was talking about a case where an orthogonal item spans several rows in a grid with a definite height. For example: While in Firefox the example above is not rendered correctly because it doesn't perform a estimation of the row size, in Chrome it renders correctly, but column tracks are not computed correctly (at least not as good as we intend with the new steps we added now). The reason is that, as you said, row estimation happens during step 1, hence, it doesn't take Content Alignment into account. Once the preferred width is set, we can't change it, even if the actual rows's size is bigger during layout. Anyway, I don't think this is a blocker to land the changes in the spec to add the new steps in the sizing algorithm. |
|
Right, that's what I meant with the current row estimation being suboptimal since it leads to surprising results for simple cases. I filed issue #2697 to improve it. |
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced
|
The Working Group just discussed
The full IRC log of that discussion<dael> Topic: Applying 'justify-content' content distribution is in the wrong place in the overall grid sizing algo<dael> github: https://github.com//issues/2557 <fantasai> https://github.com//issues/2557#issuecomment-385571268 <dael> fantasai: Summary is here ^ <dael> fantasai: When we size the grid we do all the sizing account for items etc. and at end align the track or justify them. Problem is that sometimes this increases size of gap so spanning item has more space. <dael> fantasai: Mats said we should integrate alignment into sizing algo more close so that you have alignment effects on sizing accounted for when doing sizing <dael> fantasai: Seems like everyone agrees this is a good idea so bringing to WG for approval. <dael> fantasai: Questions? <dael> astearns: Comment you linked to has...does it have both 1.5 and 2.5 that are in further discussion? <dael> fantasai: 1.5 is second sentence in 1. <dael> astearns: And 2.5 is second sentence in 2? <dael> fantasai: Yes and so on down. <dael> astearns: Other comments to add? <dael> astearns: Objections to accepting https://github.com//issues/2557#issuecomment-385571268 <dael> RESOLVED: Accept proposed edits in https://github.com//issues/2557#issuecomment-385571268 to amend grid algo <fantasai> https://github.com//issues/2697 <dael> fantasai: Follow-up we have a heuristic to get rough sizes and we need to also change the heuristic <dael> fantasai: Technically different issue |
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced
|
OK, I edited in a first pass at incorporating this change in 277817b and c9e9f7f @MatsPalmgren @javifernandez @mrego Would you mind looking this over to make sure it adequately captures what we've discussed? |
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. Finally, since the ContentAlignmentData is now used as persistent memory, we had to remove the STACK_ALLOCATED flag; hence, I think using DISALLOW_COPY_AND_ASSIGN and let the functions update the structure's internal field is a more efficient approach to commit this change. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. Finally, since the ContentAlignmentData is now used as persistent memory, we had to remove the STACK_ALLOCATED flag; hence, I think using DISALLOW_COPY_AND_ASSIGN and let the functions update the structure's internal field is a more efficient approach to commit this change. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced Reviewed-on: https://chromium-review.googlesource.com/1067918 Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com> Cr-Commit-Position: refs/heads/master@{#566412}
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. Finally, since the ContentAlignmentData is now used as persistent memory, we had to remove the STACK_ALLOCATED flag; hence, I think using DISALLOW_COPY_AND_ASSIGN and let the functions update the structure's internal field is a more efficient approach to commit this change. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced Reviewed-on: https://chromium-review.googlesource.com/1067918 Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com> Cr-Commit-Position: refs/heads/master@{#566412}
The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. Finally, since the ContentAlignmentData is now used as persistent memory, we had to remove the STACK_ALLOCATED flag; hence, I think using DISALLOW_COPY_AND_ASSIGN and let the functions update the structure's internal field is a more efficient approach to commit this change. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced Reviewed-on: https://chromium-review.googlesource.com/1067918 Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com> Cr-Commit-Position: refs/heads/master@{#566412}
The changes look good to me. |
…the track sizing algorithm, a=testonly Automatic update from web-platform-tests[css-grid] Content Alignment as part of the track sizing algorithm The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. Finally, since the ContentAlignmentData is now used as persistent memory, we had to remove the STACK_ALLOCATED flag; hence, I think using DISALLOW_COPY_AND_ASSIGN and let the functions update the structure's internal field is a more efficient approach to commit this change. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced Reviewed-on: https://chromium-review.googlesource.com/1067918 Commit-Queue: Javier Fernandez <jfernandez@igalia.com> Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com> Cr-Commit-Position: refs/heads/master@{#566412} -- wpt-commits: 73ed379a1437407749c730239019bd61528b060b wpt-pr: 11115
|
One question about the final text in the spec, we still need the last step (now number 5):
If content alignment was already done as part of the previous steps, why we still need it? |
|
@mrego The previous steps calculate the effective column/row gaps taking content alignment into account, but they don't actually apply alignment e.g. centering to the tracks. (Of course if in your implementation you've worked this into your algorithm earlier, you don't need to do it at this point, but it's generally true that CSS implementations don't need to follow the steps in the spec so long as they return the expected output.) |
…space, sometimes a lot https://bugs.webkit.org/show_bug.cgi?id=191308 Reviewed by Dean Jackson. LayoutTests/imported/w3c: Imported WPT to cover the behavior changes added in this patch. * resources/import-expectations.json: * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001-expected.txt: Added. * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html: Added. * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002-expected.txt: Added. * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html: Added. * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003-expected.txt: Added. * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html: Added. * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004-expected.txt: Added. * web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html: Added. * web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log: Source/WebCore: The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. [1] w3c/csswg-drafts#2557 Tests: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-001.html imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-002.html imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-003.html imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-content-distribution-must-account-for-track-sizing-004.html imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-cols-filled-shrinkwrap-001.html imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-cols-spanned-shrinkwrap-001.html imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-rows-filled-shrinkwrap-001.html imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-percent-rows-spanned-shrinkwrap-001.html * rendering/GridTrackSizingAlgorithm.cpp: (WebCore::GridTrackSizingAlgorithm::gridAreaBreadthForChild const): * rendering/RenderGrid.cpp: (WebCore::RenderGrid::repeatTracksSizingIfNeeded): (WebCore::RenderGrid::layoutBlock): (WebCore::RenderGrid::gridItemOffset const): (WebCore::RenderGrid::trackSizesForComputedStyle const): (WebCore::RenderGrid::populateGridPositionsForDirection): (WebCore::RenderGrid::gridAreaBreadthForOutOfFlowChild): (WebCore::contentDistributionOffset): (WebCore::RenderGrid::computeContentPositionAndDistributionOffset): (WebCore::RenderGrid::nonCollapsedTracks const): * rendering/RenderGrid.h: (WebCore::ContentAlignmentData::isValid): (WebCore::ContentAlignmentData::defaultOffsets): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@237884 268f45cc-cd09-0410-ab3c-d52691b4dbfc
…the track sizing algorithm, a=testonly Automatic update from web-platform-tests[css-grid] Content Alignment as part of the track sizing algorithm The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. Finally, since the ContentAlignmentData is now used as persistent memory, we had to remove the STACK_ALLOCATED flag; hence, I think using DISALLOW_COPY_AND_ASSIGN and let the functions update the structure's internal field is a more efficient approach to commit this change. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced Reviewed-on: https://chromium-review.googlesource.com/1067918 Commit-Queue: Javier Fernandez <jfernandezigalia.com> Reviewed-by: Manuel Rego Casasnovas <regoigalia.com> Cr-Commit-Position: refs/heads/master{#566412} -- wpt-commits: 73ed379a1437407749c730239019bd61528b060b wpt-pr: 11115 UltraBlame original commit: 96df24278a6cbca9a03b0750b55c6fc834ad4742
…the track sizing algorithm, a=testonly Automatic update from web-platform-tests[css-grid] Content Alignment as part of the track sizing algorithm The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. Finally, since the ContentAlignmentData is now used as persistent memory, we had to remove the STACK_ALLOCATED flag; hence, I think using DISALLOW_COPY_AND_ASSIGN and let the functions update the structure's internal field is a more efficient approach to commit this change. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced Reviewed-on: https://chromium-review.googlesource.com/1067918 Commit-Queue: Javier Fernandez <jfernandezigalia.com> Reviewed-by: Manuel Rego Casasnovas <regoigalia.com> Cr-Commit-Position: refs/heads/master{#566412} -- wpt-commits: 73ed379a1437407749c730239019bd61528b060b wpt-pr: 11115 UltraBlame original commit: 96df24278a6cbca9a03b0750b55c6fc834ad4742
…the track sizing algorithm, a=testonly Automatic update from web-platform-tests[css-grid] Content Alignment as part of the track sizing algorithm The CSS WG resolved [1] that Content Alignment should account to the track sizing algorithm. The sizing algorithm has been modified so that two new steps (1.5 and 2.5) were added to compute the Content Alignment offsets after resolving the columns' and rows' sizes respectively. This change decouples the Content Alignment logic from the tracks position, so that we can use it as part of the track sizing algorithm. I also had to store the whole ContentAlignmentData structure in two class attributes. We need both, position and distribution offsets, to be used in different parts of the layout logic. Finally, since the ContentAlignmentData is now used as persistent memory, we had to remove the STACK_ALLOCATED flag; hence, I think using DISALLOW_COPY_AND_ASSIGN and let the functions update the structure's internal field is a more efficient approach to commit this change. [1] w3c/csswg-drafts#2557 Added WPT * grid-content-distribution-must-account-for-track-sizing-001.html * grid-content-distribution-must-account-for-track-sizing-002.html Bug: 844744 Change-Id: I90019a96b148d3713467e35fde9482d0d39b1ced Reviewed-on: https://chromium-review.googlesource.com/1067918 Commit-Queue: Javier Fernandez <jfernandezigalia.com> Reviewed-by: Manuel Rego Casasnovas <regoigalia.com> Cr-Commit-Position: refs/heads/master{#566412} -- wpt-commits: 73ed379a1437407749c730239019bd61528b060b wpt-pr: 11115 UltraBlame original commit: 96df24278a6cbca9a03b0750b55c6fc834ad4742
The Grid spec currently says that "the tracks are aligned within the grid container according to the
align-contentandjustify-contentproperties." in step 4, after the sizes of both columns and rows have been determined. This leads to the wrong layout because intrinsic row sizes are then determined using the wrong Containing Block inline-size for spanning items. Here's a testcase demonstrating the wrong layout in Chrome (the row size is too large for the text). The layout in Firefox is IMHO the correct one - it sizes the row based on the height of the text when the CB inline-size is 500px, which is what thegrid-column: span 2grid area is sized to afterjustify-contentis applied.The correct place to apply
justify-contentin the overall algo is between step 1 and 2, i.e. first you determine the column sizes, then you applyjustify-content. This allows you to calculate the correct CB inline-size (and padding/margin percentage basis) for the items. Now you can calculate the correct row sizes, and after that applyalign-content.The text was updated successfully, but these errors were encountered: