Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement margin-trim for flexbox. #8166

Conversation

sammygill
Copy link
Contributor

@sammygill sammygill commented Jan 3, 2023

0efe67f

Implement margin-trim for flexbox.
https://bugs.webkit.org/show_bug.cgi?id=249208
rdar://103285847

Reviewed by Alan Baradlay.

Adds support for the margin-trim property on flexbox containers.
Depending on the values that are set for this property, it will trim
the corresponding edges of the flex items that are adjacent to those
edges of the container.

In order to achieve this, a new data structure, m_marginTrimItems, was
added to RenderFlexibleBox. This structure simply holds the items that
are trimmed according to their location in the flexbox.

The first step of this algorithm is to trim the edges of the first and
last child of the flexbox. This is done so that
RenderFlexibleBox::computeIntrinsicLogicalWidths does not incorrectly
take these margins into consideration when computing the width of the
flexbox under a min/max content constraint.

In order to figure out which items are actually being trimmed, we cannot
perform any trimming until the flex lines are created. As the lines
get created in FlexLayoutAlgorithm::computeNextFlexLine, we can trim the
main axis margins of the first and last items. This will require not
only updating the object's MarginBox but also the main axis sizing
information of the flex item itself. In order to do this, we need to
make FlexItem's mainAxisMargin mutable.

After the line has been constructed, we can trim the cross axis margins
of the items if the line is either the first line or last line of the
flexbox.

* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-end-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-end-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-start-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-start-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-block-multiline-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-block-multiline.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-grow-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-grow.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-inline-multiline-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-inline-multiline.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-orthogonal-item-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-orthogonal-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-shrink-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-shrink.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-end-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-end-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-start-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-start-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-block-multiline-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-block-multiline.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-grow-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-grow.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-inline-multiline-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-inline-multiline.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-orthogonal-item-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-orthogonal-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-shrink-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-shrink.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-trim-all-margins-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-trim-all-margins.html: Added.
* LayoutTests/platform/mac/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-template-expected.txt: Added.
* Source/WebCore/rendering/FlexibleBoxAlgorithm.cpp:
(WebCore::FlexLayoutAlgorithm::FlexLayoutAlgorithm):
(WebCore::FlexLayoutAlgorithm::canFitItemWithTrimmedMarginEnd const):
(WebCore::FlexLayoutAlgorithm::removeMarginEndFromFlexSizes const):
(WebCore::FlexLayoutAlgorithm::computeNextFlexLine):
* Source/WebCore/rendering/FlexibleBoxAlgorithm.h:
(WebCore::FlexLayoutAlgorithm::isMultiline const):
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::marginIntrinsicLogicalWidthForChild const):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::computeLogicalWidthInFragment const):
(WebCore::RenderBox::fillAvailableMeasure const):
(WebCore::RenderBox::computeOrTrimInlineMargin const):
(WebCore::RenderBox::computeInlineDirectionMargins const):
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutBlock):
(WebCore::RenderFlexibleBox::initializeMarginTrimState):
(WebCore::RenderFlexibleBox::shouldTrimChildMargin const):
(WebCore::RenderFlexibleBox::shouldTrimMainAxisMarginStart const):
(WebCore::RenderFlexibleBox::shouldTrimMainAxisMarginEnd const):
(WebCore::RenderFlexibleBox::shouldTrimCrossAxisMarginStart const):
(WebCore::RenderFlexibleBox::shouldTrimCrossAxisMarginEnd const):
(WebCore::RenderFlexibleBox::trimMainAxisMarginStart):
(WebCore::RenderFlexibleBox::trimMainAxisMarginEnd):
(WebCore::RenderFlexibleBox::trimCrossAxisMarginStart):
(WebCore::RenderFlexibleBox::trimCrossAxisMarginEnd):
(WebCore::RenderFlexibleBox::layoutFlexItems):
* Source/WebCore/rendering/RenderFlexibleBox.h:

Canonical link: https://commits.webkit.org/258563@main

1a9552c

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  πŸ§ͺ win
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ›  gtk   πŸ›  wincairo
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2   πŸ§ͺ api-mac   πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios   πŸ§ͺ mac-wk1   πŸ§ͺ api-gtk
  πŸ›  tv βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge   πŸ›  watch-sim

@sammygill sammygill self-assigned this Jan 3, 2023
@sammygill sammygill added the Layout and Rendering For bugs with layout and rendering of Web pages. label Jan 3, 2023
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trim-for-flex-containers branch from 82aa6fa to 4e012cf Compare January 5, 2023 00:44
@nt1m nt1m changed the title Implement margin-trim for Implement margin-trim for flexbox Jan 5, 2023
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trim-for-flex-containers branch from 4e012cf to 42fc1e4 Compare January 5, 2023 17:32
@sammygill sammygill changed the title Implement margin-trim for flexbox Implement margin-trim for flexbox. Jan 5, 2023
@stwrt stwrt mentioned this pull request Jan 5, 2023
2 tasks
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 5, 2023
Copy link
Contributor

@alanbaradlay alanbaradlay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rs=me
it would be great if someone with more extensive flex + margin knowledge could also take a look at it.

@stwrt stwrt self-requested a review January 6, 2023 17:07
@sammygill sammygill removed the merging-blocked Applied to prevent a change from being merged label Jan 6, 2023
@sammygill sammygill force-pushed the eng/margin-trim-Implement-margin-trim-for-flex-containers branch from 42fc1e4 to 1a9552c Compare January 6, 2023 20:14
@sammygill sammygill added the merge-queue Applied to send a pull request to merge-queue label Jan 6, 2023
https://bugs.webkit.org/show_bug.cgi?id=249208
rdar://103285847

Reviewed by Alan Baradlay.

Adds support for the margin-trim property on flexbox containers.
Depending on the values that are set for this property, it will trim
the corresponding edges of the flex items that are adjacent to those
edges of the container.

In order to achieve this, a new data structure, m_marginTrimItems, was
added to RenderFlexibleBox. This structure simply holds the items that
are trimmed according to their location in the flexbox.

The first step of this algorithm is to trim the edges of the first and
last child of the flexbox. This is done so that
RenderFlexibleBox::computeIntrinsicLogicalWidths does not incorrectly
take these margins into consideration when computing the width of the
flexbox under a min/max content constraint.

In order to figure out which items are actually being trimmed, we cannot
perform any trimming until the flex lines are created. As the lines
get created in FlexLayoutAlgorithm::computeNextFlexLine, we can trim the
main axis margins of the first and last items. This will require not
only updating the object's MarginBox but also the main axis sizing
information of the flex item itself. In order to do this, we need to
make FlexItem's mainAxisMargin mutable.

After the line has been constructed, we can trim the cross axis margins
of the items if the line is either the first line or last line of the
flexbox.

* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-end-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-end-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-start-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-start-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-block-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-block-multiline-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-block-multiline.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-grow-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-grow.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-inline-multiline-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-inline-multiline.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-orthogonal-item-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-orthogonal-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-shrink-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-column-shrink.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-end-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-end-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-start-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-start-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-trimmed-only-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-inline-trimmed-only.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-block-multiline-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-block-multiline.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-grow-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-grow.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-inline-multiline-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-inline-multiline.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-orthogonal-item-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-orthogonal-item.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-shrink-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-row-shrink.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-trim-all-margins-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-trim-all-margins.html: Added.
* LayoutTests/platform/mac/imported/w3c/web-platform-tests/css/css-box/margin-trim/flex-template-expected.txt: Added.
* Source/WebCore/rendering/FlexibleBoxAlgorithm.cpp:
(WebCore::FlexLayoutAlgorithm::FlexLayoutAlgorithm):
(WebCore::FlexLayoutAlgorithm::canFitItemWithTrimmedMarginEnd const):
(WebCore::FlexLayoutAlgorithm::removeMarginEndFromFlexSizes const):
(WebCore::FlexLayoutAlgorithm::computeNextFlexLine):
* Source/WebCore/rendering/FlexibleBoxAlgorithm.h:
(WebCore::FlexLayoutAlgorithm::isMultiline const):
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::marginIntrinsicLogicalWidthForChild const):
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::computeLogicalWidthInFragment const):
(WebCore::RenderBox::fillAvailableMeasure const):
(WebCore::RenderBox::computeOrTrimInlineMargin const):
(WebCore::RenderBox::computeInlineDirectionMargins const):
* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::layoutBlock):
(WebCore::RenderFlexibleBox::initializeMarginTrimState):
(WebCore::RenderFlexibleBox::shouldTrimChildMargin const):
(WebCore::RenderFlexibleBox::shouldTrimMainAxisMarginStart const):
(WebCore::RenderFlexibleBox::shouldTrimMainAxisMarginEnd const):
(WebCore::RenderFlexibleBox::shouldTrimCrossAxisMarginStart const):
(WebCore::RenderFlexibleBox::shouldTrimCrossAxisMarginEnd const):
(WebCore::RenderFlexibleBox::trimMainAxisMarginStart):
(WebCore::RenderFlexibleBox::trimMainAxisMarginEnd):
(WebCore::RenderFlexibleBox::trimCrossAxisMarginStart):
(WebCore::RenderFlexibleBox::trimCrossAxisMarginEnd):
(WebCore::RenderFlexibleBox::layoutFlexItems):
* Source/WebCore/rendering/RenderFlexibleBox.h:

Canonical link: https://commits.webkit.org/258563@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/margin-trim-Implement-margin-trim-for-flex-containers branch from 1a9552c to 0efe67f Compare January 6, 2023 22:42
@webkit-commit-queue
Copy link
Collaborator

Committed 258563@main (0efe67f): https://commits.webkit.org/258563@main

Reviewed commits have been landed. Closing PR #8166 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 0efe67f into WebKit:main Jan 6, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 6, 2023
@sammygill sammygill deleted the eng/margin-trim-Implement-margin-trim-for-flex-containers branch April 19, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
5 participants