Skip to content

Commit

Permalink
[flex] Mitigate performance drop during dynamic flex change.
Browse files Browse the repository at this point in the history
I haven't got a good reproducible test yet, but the issue experienced
in 1479477 is related to a dynamic size change.

There are two layouts which can occur during flexbox column wrap
intrinsic sizing.
1) During base size calculation. For this we simply return the
   border/padding if the child is layout clean.
2) During cross-axis flex-line calculation (primarily for determining
   the baseline, and the cross-axis size). Use the max-content
   contribution for the cross-axis size, and ignore baseline alignment.

(cherry picked from commit b458db7)

Bug: 1272533, 1479477
Change-Id: Ic926616e88b7fb90eb5937897b00db62badae045
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4854071
Reviewed-by: Alison Maher <almaher@microsoft.com>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1194378}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4857171
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Ian Kilpatrick <ikilpatrick@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5993@{#186}
Cr-Branched-From: 5113507-refs/heads/main@{#1192594}
  • Loading branch information
bfgeek authored and Chromium LUCI CQ committed Sep 12, 2023
1 parent 646e636 commit 9772489
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -546,8 +546,12 @@ void FlexLine::ComputeLineItemsPosition(LayoutUnit main_axis_start_offset,

LayoutUnit child_cross_axis_margin_box_extent;
const auto alignment = flex_item.Alignment();
if (alignment == ItemPosition::kBaseline ||
alignment == ItemPosition::kLastBaseline) {
// TODO(crbug.com/1272533): We may not have a layout-result during min/max
// calculations. This is incorrect, and should be re-enabled once we have
// more cache slots.
if (flex_item.layout_result_ &&
(alignment == ItemPosition::kBaseline ||
alignment == ItemPosition::kLastBaseline)) {
LayoutUnit ascent = flex_item.MarginBoxAscent(
alignment == ItemPosition::kLastBaseline, is_wrap_reverse);
LayoutUnit descent =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -832,9 +832,12 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems(
if (!layout_result) {
NGConstraintSpace child_space =
BuildSpaceForIntrinsicBlockSize(child, max_content_contribution);
absl::optional<NGDisableSideEffectsScope> disable_side_effects;
// TODO(crbug.com/1272533): This shouldn't return border/padding for
// children which are layout-clean. More cache slots are needed to
// handle the performance degradation to allow a layout within the
// min/max sizing pass.
if (phase != Phase::kLayout && !Node().GetLayoutBox()->NeedsLayout()) {
disable_side_effects.emplace();
return border_padding_in_child_writing_mode.BlockSum();
}
layout_result = child.Layout(child_space, /* break_token */ nullptr);
DCHECK(layout_result);
Expand Down Expand Up @@ -1318,16 +1321,11 @@ void NGFlexLayoutAlgorithm::PlaceFlexItems(
flex_item.cross_axis_size_ = ComputeInlineSizeForFragment(
child_space, flex_item.ng_input_node_, border + padding);
}
} else if (is_computing_multiline_column_intrinsic_size) {
flex_item.cross_axis_size_ = *flex_item.max_content_contribution_;
} else {
DCHECK((child_space.CacheSlot() == NGCacheSlot::kLayout) ||
!flex_item.layout_result_)
<< "If we already have a 'measure' result from "
"ConstructAndAppendFlexItems, we don't want to evict it.";
absl::optional<NGDisableSideEffectsScope> disable_side_effects;
if (is_computing_multiline_column_intrinsic_size &&
!flex_item.ng_input_node_.GetLayoutBox()->NeedsLayout()) {
disable_side_effects.emplace();
}
!flex_item.layout_result_);
flex_item.layout_result_ = flex_item.ng_input_node_.Layout(
child_space, nullptr /*break token*/);
// TODO(layout-dev): Handle abortions caused by block fragmentation.
Expand Down

0 comments on commit 9772489

Please sign in to comment.