This fixes the bug. The underlying problem was that we were hitting the
case where ConstructBorderRenderer sets aNeedsClip to true, but nothing
in the column-rule drawing path was honoring that.
(Choosing clone rather than slice may make a difference for dotted and
dashed column-rules, but I think the clone behavior is preferable.)
MozReview-Commit-ID: 7YYzyxYjhsV
--HG--
extra : rebase_source : da488a54642d7900c2c36a9dc14e9f694b1ae244
This is followup to bug 1361668, and is just cleanup in advance of the
patch to fix this bug.
MozReview-Commit-ID: 4HOKLA5WYNq
--HG--
extra : rebase_source : dc6ceb81cc6c19b5ab8a79d10812501bc0f525df
This is just cleanup in advance of the patch to fix this bug.
MozReview-Commit-ID: 1pzauGix51m
--HG--
extra : rebase_source : b36a4ef7da6fb6b6ef7a1a82c56aa489e10935d5
This patch was generated automatically by the "modeline.py" script, available
here: https://github.com/amccreight/moz-source-tools/blob/master/modeline.py
For every file that is modified in this patch, the changes are as follows:
(1) The patch changes the file to use the exact C++ mode lines from the
Mozilla coding style guide, available here:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Mode_Line
(2) The patch deletes any blank lines between the mode line & the MPL
boilerplate comment.
(3) If the file previously had the mode lines and MPL boilerplate in a
single contiguous C++ comment, then the patch splits them into
separate C++ comments, to match the boilerplate in the coding style.
MozReview-Commit-ID: EuRsDue63tK
--HG--
extra : rebase_source : 3356d4b80ff6213935192e87cdbc9103fec6084c
Instead of just keeping a count of how many "extra clips" (aka
out-of-band clips) we have pushed, track more complex information for
each clip. In particular, track the display item's normal clip chain, as
well as the clip id of the extra clip that was pushed. This will be
needed to override clip cache information in the next patch.
MozReview-Commit-ID: AWKDTkelhyL
--HG--
extra : rebase_source : 379e38550cf45d54862850f6c4aad0ac488f6ca9
This part is not necessary for fixing the reftests. But it would be better to just
return true if we don't want to render this item.
MozReview-Commit-ID: 6w6MCUAFPj6
--HG--
extra : rebase_source : 858ed1e406cd537eb5754f3d8f8eec1497034e70
nsReflowStatus::IsEmpty() assertions are added after DISPLAY_REFLOW in the
beginning of the Reflow().
A few Reflow() implementations have Reset() calls at the end which are left
in place by this patch (with an explanatory comment added to each). These
ending Reset()s are only needed for cases where a non-splittable frame
passes its own nsReflowStatus to a child's reflow method. Just in case the
child leaves a "not fully complete" value in the nsReflowStatus, the
non-splittable parent frame must clear out the nsReflowStatus before
returning, so that its own parent doesn't then try to split it.
MozReview-Commit-ID: 6Jj3jfMAqj4
--HG--
extra : rebase_source : e1fe6a775ad97e16a6d7cc224634ff962ccb0fbf
This ensures the Reflow() call in nsContainerFrame::ReflowChild() is using
an empty reflow status.
MozReview-Commit-ID: K2Ln2i4XkB5
--HG--
extra : rebase_source : ec74a2fece0cda7e61f112e778afe13f128dc76b
This is the primary patch in this bug, and makes the performance
improvement that fixes this bug.
The assertion count increase for layout/generic/crashtests/1015844.html
is accompanied by a layout change in the testcase as well. However, I'm
not planning to fix it in this sequence; fundamentally columnsets with
specified heights inside a paginated context (like another columnset) do
not work in any reasonable way, and changing the number of times we
reflow them can change the layout. At least, assuming I didn't lose
something in the process of simplifying the testcase.
ISSUES:
- may make block inside XUL worse in performance by marking dirty more (see subdoc in Firefox UI, or text control innards?)
MozReview-Commit-ID: GdOvPynqcFP
This fixes the regression in
layout/reftests/columns/column-balancing-nested-001.html from the
primary patch in this bug, for which a simplified testcase is
https://bugzilla.mozilla.org/attachment.cgi?id=8848293 . I believe it's
simply a pre-existing bug that wasn't previously exposed. I suspect
it may be possible to write a test that shows the bug prior to the
patch. However, it's difficult, since it requires triggering height
changes in a multicol with an 'auto' height (since a non-'auto' height
would cause the multicol to have NS_FRAME_CONTAINS_RELATIVE_BSIZE set by
nsColumnSetFrame::Reflow, which would make skipIncremental false because
ShouldReflowAllKids returns true). I suspect any working testcase would
likely be rather brittle, so I haven't pursued it further (particularly
given the complexity of the minimal testcase).
MozReview-Commit-ID: Gve3XKEPSxL
This fixes (confirmed by testing locally) a regression in
layout/reftests/w3c-css/received/css-multicol-1/multicol-nested-margin-004.xht
resulting from the primary patch in this bug, which tends to make frames
dirty less often. The problem with that test is that (at least in a
simplified form), in the final reflow of the inner ColumnSet in the
first column of the outer ColumnSet, the inner ColumnSet chooses not to
reflow its first column, thus leaving that first column having a height
that is too large for the inner ColumnSet to fit in the first column of
the outer ColumnSet, causing the entire inner ColumnSet (rather than
just part of it) to be pushed to the next column.
I believe this existing incremental reflow code just doesn't make sense.
The code I'm modifying dates back primarily to:
c237520c89 (October 2004, initial columns implementation)
ee070ec95f (March 2005)
31e3540d1e (November 2006)
The first thing that doesn't make sense is the condition modified at the
end of this patch:
(!reflowNext && (skipIncremental || skipResizeBSizeShrink))
There's simply no reason that that || isn't required to be an &&, as far
as I can tell. Even if we don't need to reflow due to any of the
standard incremental reflow conditions, we can need to reflow because
the block size is shrinking and the column no longer fits.
Note that things were already OK when we required reflow due to
NS_SUBTREE_DIRTY(this), because of the way shrinkingBSizeOnly was
initialized using !NS_SUBTREE_DIRTY(this), thus excluding such cases
from the optimization.
The rest of the patch falls out of turning the || into an && in an
efficient way (i.e., without the extra !NS_SUBTREE_DIRTY(this) test, and
avoiding doing extra tests that we know we're not going to need by
coalescing all the incremental reflow tests into a single variable).
I tested that this patch passes try on its own (on 64-bit Linux debug):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a279023fb7e8f3349d5ecbfb95807d6b097cdbcb
MozReview-Commit-ID: BD3ofmWN5Wl
This is the primary patch in this bug, and makes the performance
improvement that fixes this bug.
The assertion count increase for layout/generic/crashtests/1015844.html
is accompanied by a layout change in the testcase as well. However, I'm
not planning to fix it in this sequence; fundamentally columnsets with
specified heights inside a paginated context (like another columnset) do
not work in any reasonable way, and changing the number of times we
reflow them can change the layout. At least, assuming I didn't lose
something in the process of simplifying the testcase.
ISSUES:
- may make block inside XUL worse in performance by marking dirty more (see subdoc in Firefox UI, or text control innards?)
MozReview-Commit-ID: GdOvPynqcFP
--HG--
extra : transplant_source : %9F%01%88%EC%0F%DC%D5%BB%BE%85G%E7%E5%B0%E0%CEl%A0o%1A
This fixes the regression in
layout/reftests/columns/column-balancing-nested-001.html from the
primary patch in this bug, for which a simplified testcase is
https://bugzilla.mozilla.org/attachment.cgi?id=8848293 . I believe it's
simply a pre-existing bug that wasn't previously exposed. I suspect
it may be possible to write a test that shows the bug prior to the
patch. However, it's difficult, since it requires triggering height
changes in a multicol with an 'auto' height (since a non-'auto' height
would cause the multicol to have NS_FRAME_CONTAINS_RELATIVE_BSIZE set by
nsColumnSetFrame::Reflow, which would make skipIncremental false because
ShouldReflowAllKids returns true). I suspect any working testcase would
likely be rather brittle, so I haven't pursued it further (particularly
given the complexity of the minimal testcase).
MozReview-Commit-ID: Gve3XKEPSxL
--HG--
extra : transplant_source : %3FU%81Ti%B5%29RfAI%03%89%16%E2%BA%D4%BC%B9%2B
This fixes (confirmed by testing locally) a regression in
layout/reftests/w3c-css/received/css-multicol-1/multicol-nested-margin-004.xht
resulting from the primary patch in this bug, which tends to make frames
dirty less often. The problem with that test is that (at least in a
simplified form), in the final reflow of the inner ColumnSet in the
first column of the outer ColumnSet, the inner ColumnSet chooses not to
reflow its first column, thus leaving that first column having a height
that is too large for the inner ColumnSet to fit in the first column of
the outer ColumnSet, causing the entire inner ColumnSet (rather than
just part of it) to be pushed to the next column.
I believe this existing incremental reflow code just doesn't make sense.
The code I'm modifying dates back primarily to:
c237520c89 (October 2004, initial columns implementation)
ee070ec95f (March 2005)
31e3540d1e (November 2006)
The first thing that doesn't make sense is the condition modified at the
end of this patch:
(!reflowNext && (skipIncremental || skipResizeBSizeShrink))
There's simply no reason that that || isn't required to be an &&, as far
as I can tell. Even if we don't need to reflow due to any of the
standard incremental reflow conditions, we can need to reflow because
the block size is shrinking and the column no longer fits.
Note that things were already OK when we required reflow due to
NS_SUBTREE_DIRTY(this), because of the way shrinkingBSizeOnly was
initialized using !NS_SUBTREE_DIRTY(this), thus excluding such cases
from the optimization.
The rest of the patch falls out of turning the || into an && in an
efficient way (i.e., without the extra !NS_SUBTREE_DIRTY(this) test, and
avoiding doing extra tests that we know we're not going to need by
coalescing all the incremental reflow tests into a single variable).
I tested that this patch passes try on its own (on 64-bit Linux debug):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a279023fb7e8f3349d5ecbfb95807d6b097cdbcb
MozReview-Commit-ID: BD3ofmWN5Wl
--HG--
extra : transplant_source : %0F%A3%18%24%CF3%9A%5E%B9%7E%EBB%A12%84%3B%12L%A7c
Mats Palmgren
Emilio Cobos Álvarez
Ryan Hunt
Cosmin Sabou
Matt Woodrow
Dorel Luca
Daniel Holbert
L. David Baron
Miko Mynttinen
Samathy Barratt
Ciure Andrei
Ethan Lin
Kartikaya Gupta
Nicolas Silva
Ting-Yu Lin
Sebastian Hengst
Kevin Chen
Sylvestre Ledru
Cameron McCormack