Summary: | [css-grid] update the content-sized grid width before laying out a grid item with block constraints and aspect-ratio | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | zsun | ||||||||||||||||
Component: | CSS | Assignee: | zsun | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | changseok, clopez, esprehn+autocc, ews-watchlist, fred.wang, glenn, jfernandez, kondapallykalyan, pdr, rego, svillar, webkit-bug-importer, youennf | ||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Attachments: |
|
Description
zsun
2021-10-15 02:53:38 PDT
Created attachment 441359 [details]
Patch
Comment on attachment 441359 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441359&action=review > Source/WebCore/rendering/RenderGrid.cpp:974 > + child->layoutIfNeeded(); We shouldn't layout the item twice. If you need to recompute the preferred width then just do it before the child->layoutIfNeeded() call above and do the layout just once. Recompute the preferred with before laying out a grid item with block constraints Created attachment 441373 [details]
Patch
Comment on attachment 441373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441373&action=review > Source/WebCore/rendering/RenderGrid.cpp:972 > + updateLogicalWidth(); The part I don't understand is that you mention that you need to recompute the preferred width of the item but you call updateLogicalWidth() in the grid. This means that we're recomputing the width of the grid in the middle of the the layout of the items. That's problematic, note that some items might be laid out with the old grid width while some others will use the updated one. Comment on attachment 441373 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441373&action=review > Source/WebCore/ChangeLog:14 > + IOS platform. However, we noticed that they also fail for GTK at iOS* > Source/WebCore/rendering/RenderGrid.cpp:970 > + // We need to recompute the preferred width for gird items with block constraints. grid* Created attachment 443394 [details]
Patch
Created attachment 443397 [details]
Patch
Comment on attachment 443397 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443397&action=review > Source/WebCore/rendering/RenderGrid.cpp:282 > + updateLogicalWidth(); I don't understand why this solves the issue. I thought we needed to repeat the track sizing algorithm; don't we ? Additionally, wouldn't we need to call setPreferredLogicalWidthsDirty for the grid container so that we run the track sizing algorithm again to adjust its intrinsic size ? Created attachment 443532 [details]
Patch
Created attachment 443546 [details]
Patch
Comment on attachment 443546 [details]
Patch
The change looks good, but I think we should add the new test that we discussed today; the one to verify that we update the tracks after applying the aspect ratio.
Created attachment 443579 [details]
Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess Comment on attachment 443579 [details]
Patch
r=me
Please, ensure the new test is exported to the WPT repository.
Committed r285497 (244021@main): <https://commits.webkit.org/244021@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 443579 [details]. |