Bug 1583639 Part 4 - Stop creating more columns in column-fill:auto mode if there's still block-size left in multicol container. r=dbaron
authorTing-Yu Lin <tlin@mozilla.com>
Wed, 25 Sep 2019 23:36:41 +0000
changeset 495031 5837862212545b9ca4c7f20bee817a75389d832e
parent 495030 1c897f0c159a6c9cf322eae35b2c1eb3539d8a56
child 495032 bede638ee3951464a45b31ba7055c146e0792745
push id114131
push userdluca@mozilla.com
push dateThu, 26 Sep 2019 09:47:34 +0000
treeherdermozilla-inbound@1dc1a755079a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1583639, 673770
milestone71.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1583639 Part 4 - Stop creating more columns in column-fill:auto mode if there's still block-size left in multicol container. r=dbaron This patch makes fragmentation in "column-fill:auto" mode possible. Note that we only bail out of creating more column contents when "column-fill:auto" mode is set from the styles, i.e. when mForceAuto is false. That is because when mForceAuto is true, we usually in the case where we have gave up balancing, and we really want to create overflow columns. Note: without `!aConfig.mForceAuto` check, 673770.html can generated assertions, and the frame tree contains dangling Overflow-lines and ExcessOverflowContainersList. Differential Revision: https://phabricator.services.mozilla.com/D47005
layout/generic/nsColumnSetFrame.cpp
layout/generic/nsColumnSetFrame.h
testing/web-platform/meta/css/css-multicol/multicol-breaking-003.html.ini
testing/web-platform/meta/css/css-multicol/multicol-breaking-nobackground-003.html.ini
--- a/layout/generic/nsColumnSetFrame.cpp
+++ b/layout/generic/nsColumnSetFrame.cpp
@@ -417,16 +417,17 @@ nsColumnSetFrame::ReflowConfig nsColumnS
 
   ReflowConfig config;
   config.mUsedColCount = numColumns;
   config.mColISize = colISize;
   config.mExpectedISizeLeftOver = expectedISizeLeftOver;
   config.mColGap = colGap;
   config.mColMaxBSize = colBSize;
   config.mIsBalancing = isBalancing;
+  config.mForceAuto = aForceAuto;
   config.mKnownFeasibleBSize = NS_UNCONSTRAINEDSIZE;
   config.mKnownInfeasibleBSize = 0;
   config.mComputedBSize = computedBSize;
   config.mConsumedBSize = consumedBSize;
 
   COLUMN_SET_LOG(
       "%s: this=%p, mUsedColCount=%d, mColISize=%d, "
       "mExpectedISizeLeftOver=%d, mColGap=%d, mColMaxBSize=%d, mIsBalancing=%d",
@@ -883,27 +884,75 @@ nsColumnSetFrame::ColumnBalanceData nsCo
           contentBEnd > aReflowInput.mCBReflowInput->ComputedMaxBSize())) &&
         aConfig.mIsBalancing) {
       // We overflowed vertically, but have not exceeded the number of
       // columns. We're going to go into overflow columns now, so balancing
       // no longer applies.
       colData.mHasExcessBSize = true;
     }
 
-    if (columnCount >= aConfig.mUsedColCount - 1 && aConfig.mIsBalancing) {
+    // We have reached the maximum number of columns. If we are balancing, stop
+    // this reflow and continue finding the optimal balancing block-size.
+    //
+    // Otherwise, i.e. we are not balancing, stop this reflow and let the parent
+    // of our multicol container create a next-in-flow if all of the following
+    // conditions are met.
+    //
+    // 1) We fill columns sequentially by the request of the style, not by our
+    // internal needs, i.e. aConfig.mForceAuto is false.
+    //
+    // We don't want to stop this reflow when we force fill the columns
+    // sequentially. We usually go into this mode when giving up balancing, and
+    // this is the last resort to fit all our children by creating overflow
+    // columns.
+    //
+    // 2) In a fragmented context, our multicol container still has block-size
+    // left for its next-in-flow, i.e.
+    // aReflowInput.mFlags.mColumnSetWrapperHasNoBSizeLeft is false.
+    //
+    // Note that in a continuous context, i.e. our multicol container's
+    // available block-size is unconstrained, if it has a fixed block-size
+    // mColumnSetWrapperHasNoBSizeLeft is always true because nothing stops it
+    // from applying all its block-size in the first-in-flow. Otherwise, i.e.
+    // our multicol container has an unconstrained block-size, we shouldn't be
+    // here because all our children should fit in the very first column even if
+    // mColumnSetWrapperHasNoBSizeLeft is false.
+    //
+    // According to the definition of mColumnSetWrapperHasNoBSizeLeft, if the
+    // bit is *not* set, either our multicol container has unconstrained
+    // block-size, or it has a constrained block-size and has block-size left
+    // for its next-in-flow. In either cases, the parent of our multicol
+    // container can create a next-in-flow for the container that guaranteed to
+    // have non-zero block-size for the container's children.
+    //
+    // Put simply, if either one of the above conditions is not met, we are
+    // going to create more overflow columns until all our children are fit.
+    if (columnCount >= aConfig.mUsedColCount - 1 &&
+        (aConfig.mIsBalancing ||
+         (StaticPrefs::layout_css_column_span_enabled() &&
+          !aConfig.mForceAuto &&
+          !aReflowInput.mFlags.mColumnSetWrapperHasNoBSizeLeft))) {
+      NS_ASSERTION(aConfig.mIsBalancing ||
+                       aReflowInput.AvailableBSize() != NS_UNCONSTRAINEDSIZE,
+                   "Why are we here if we have unlimited block-size to fill "
+                   "columns sequentially.");
+
       // No more columns allowed here. Stop.
       aStatus.SetNextInFlowNeedsReflow();
       kidNextInFlow->MarkSubtreeDirty();
       // Move any of our leftover columns to our overflow list. Our
       // next-in-flow will eventually pick them up.
       const nsFrameList& continuationColumns = mFrames.RemoveFramesAfter(child);
       if (continuationColumns.NotEmpty()) {
         SetOverflowFrames(continuationColumns);
       }
       child = nullptr;
+
+      COLUMN_SET_LOG("%s: We are not going to create overflow columns.",
+                     __func__);
       break;
     }
 
     if (PresContext()->HasPendingInterrupt()) {
       // Stop the loop now while |child| still points to the frame that bailed
       // out.  We could keep going here and condition a bunch of the code in
       // this loop on whether there's an interrupt, or even just keep going and
       // trying to reflow the blocks (even though we know they'll interrupt
--- a/layout/generic/nsColumnSetFrame.h
+++ b/layout/generic/nsColumnSetFrame.h
@@ -104,16 +104,20 @@ class nsColumnSetFrame final : public ns
     // The maximum bSize of any individual column during a reflow iteration.
     // This parameter is set during each iteration of the binary search for
     // the best column block-size.
     nscoord mColMaxBSize = NS_UNCONSTRAINEDSIZE;
 
     // A boolean controlling whether or not we are balancing.
     bool mIsBalancing = false;
 
+    // A boolean controlling whether or not we are forced to fill columns
+    // sequentially.
+    bool mForceAuto = false;
+
     // The last known column block-size that was 'feasible'. A column bSize is
     // feasible if all child content fits within the specified bSize.
     nscoord mKnownFeasibleBSize = NS_UNCONSTRAINEDSIZE;
 
     // The last known block-size that was 'infeasible'. A column bSize is
     // infeasible if not all child content fits within the specified bSize.
     nscoord mKnownInfeasibleBSize = 0;
 
--- a/testing/web-platform/meta/css/css-multicol/multicol-breaking-003.html.ini
+++ b/testing/web-platform/meta/css/css-multicol/multicol-breaking-003.html.ini
@@ -1,2 +1,2 @@
 [multicol-breaking-003.html]
-  expected: FAIL
+  prefs: [layout.css.column-span.enabled:true]
--- a/testing/web-platform/meta/css/css-multicol/multicol-breaking-nobackground-003.html.ini
+++ b/testing/web-platform/meta/css/css-multicol/multicol-breaking-nobackground-003.html.ini
@@ -1,2 +1,2 @@
 [multicol-breaking-nobackground-003.html]
-  expected: FAIL
+  prefs: [layout.css.column-span.enabled:true]