Closed
Bug 462307
Opened 17 years ago
Closed 17 years ago
Speed up columns case hit by HTML5 spec
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
People
(Reporter: roc, Assigned: roc)
Details
Attachments
(2 files, 1 obsolete file)
4.59 KB,
text/html
|
Details | |
3.05 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
I noticed that the HTML5 spec uses -moz-column-width. That's applied to a really big table (the table of all HTML entities). Unfortunately we don't break tables across column boundaries, so it doesn't do anything useful. Even more unfortunately, it slows down our layout *a lot* because when we reflow the column set, we do a binary search for the correct balanced column height; in my testing it reflows the table 15 times to find that height (and each reflow is quite expensive since the table is huge and we're adjusting the available height, so we can't optimize it very well).
I'm attaching a patch. The idea is to observe that when a column doesn't fit in its available height, that means there's no break opportunity inside the content of that column. So, no height less than that column's height is a feasible height for the balanced column set. Using that fact, we can reflow a column set containing a single non-breakable element (like a table) in just three iterations:
-- First, we reflow with unconstrained height to find the single-column height of the table (which gives us a feasible height)
-- Then we reflow with some reduced height to try to balance the columns. It fails to fit, and returns the same desired height as the first iteration; we learn that no smaller height than the desired height will work
-- So we reflow again with that desired height, and we're done.
Attachment #345455 -
Flags: superreview?(dbaron)
Attachment #345455 -
Flags: review?(dbaron)
Assignee | ||
Comment 1•17 years ago
|
||
Load the testcase, wait for the load to finish, then zoom. The relayout does 11 iterations of the column balance algorithm, for me on trunk. With the patch, it's only 3.
Comment on attachment 345455 [details] [diff] [review]
fix
>+ // If the first column's available height was constrained, and
>+ // it didn't fit, its current height must be its minimum height
>+ // and therefore no smaller height can be feasible.
Doesn't this statement apply to any column? Shouldn't we also apply this fix to:
<div style="column stuff">
<p>caption</p>
<table>really big table</table>
</div>
?
Assignee | ||
Comment 3•17 years ago
|
||
Yes. In fact, I had a patch to do that, but I seem to have attached the old patch (and lost the new one) :-(.
Assignee | ||
Comment 4•17 years ago
|
||
Correct patch
Attachment #345455 -
Attachment is obsolete: true
Attachment #346183 -
Flags: superreview?(dbaron)
Attachment #346183 -
Flags: review?(dbaron)
Attachment #345455 -
Flags: superreview?(dbaron)
Attachment #345455 -
Flags: review?(dbaron)
Attachment #346183 -
Flags: superreview?(dbaron)
Attachment #346183 -
Flags: superreview+
Attachment #346183 -
Flags: review?(dbaron)
Attachment #346183 -
Flags: review+
Comment on attachment 346183 [details] [diff] [review]
fix v2
r+sr=dbaron
Assignee | ||
Comment 6•17 years ago
|
||
Changeset 469a48d37280
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•