Summary: | [New Multicolumn] Column balancing is much slower than the old multi-column layout on float-multicol.html | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dave Hyatt <hyatt> | ||||
Component: | Layout and Rendering | Assignee: | Dave Hyatt <hyatt> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, esprehn+autocc, glenn, jonlee, kondapallykalyan, mstensho, webkit-bug-importer, WebkitBugTracker | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Dave Hyatt
2014-04-17 13:10:26 PDT
In principle, it shouldn't have to do a lot more relayouts than the number of columns you have (unless you have a bug). I see that your test case has floats, and there's a bug in the new implementation in this regard. I reported it in Blink [1], and I have submitted a patch [2]. [1] https://code.google.com/p/chromium/issues/detail?id=361551 [2] https://codereview.chromium.org/242203003/ (In reply to comment #2) > In principle, it shouldn't have to do a lot more relayouts than the number of columns you have (unless you have a bug). I see that your test case has floats, and there's a bug in the new implementation in this regard. I reported it in Blink [1], and I have submitted a patch [2]. > > [1] https://code.google.com/p/chromium/issues/detail?id=361551 > [2] https://codereview.chromium.org/242203003/ I'll look into merging this (see my email to you though that I consider tall-float1.html to be an invalid test case, so I'm not going to merge that). tall-float1.html doesn't pass in WebKit, since we properly handle the case where you don't create columns and don't establish a block formatting context in that case. I verified that the fix for 361551 does stop the assertion and resolve the performance issue with this test case, so: \0/ Created attachment 229815 [details]
Patch
Attachment 229815 [details] did not pass style-queue:
ERROR: Source/WebCore/rendering/RenderBlock.cpp:1475: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Total errors found: 1 in 8 files
If any of these errors are false positives, please file a bug against check-webkit-style.
I didn't mean to include tall-float1.html, so just ignore that part. Comment on attachment 229815 [details]
Patch
Looks good.
|