Summary: | changing -webkit-order should change the paint order of flex items | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tony Chang <tony> | ||||||||
Component: | Layout and Rendering | Assignee: | Tony Chang <tony> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | dglazkov, eric, ojan, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 92042 | ||||||||||
Bug Blocks: | 62048 | ||||||||||
Attachments: |
|
Description
Tony Chang
2012-07-23 15:55:42 PDT
Created attachment 154169 [details]
Patch
In a follow up change, we should stop passing around OrderIterator as a parameter. No hurry on this. I want to get some more perf data about bug 92042 first (so far it looks fine, but a few more cycles won't hurt). Comment on attachment 154169 [details] Patch Attachment 154169 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13345019 New failing tests: http/tests/security/script-crossorigin-loads-correctly.html Created attachment 154174 [details]
Archive of layout-test-results from gce-cr-linux-05
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-05 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment on attachment 154169 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154169&action=review > Source/WebCore/rendering/RenderFlexibleBox.cpp:262 > + m_orderIterator = adoptPtr(new OrderIterator(this, orderValues)); Do we really need the member variable? All this saves is the vector copy + sort in paintChildren, which will typically be next to 0 cost. (In reply to comment #5) > (From update of attachment 154169 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=154169&action=review > > > Source/WebCore/rendering/RenderFlexibleBox.cpp:262 > > + m_orderIterator = adoptPtr(new OrderIterator(this, orderValues)); > > Do we really need the member variable? All this saves is the vector copy + sort in paintChildren, which will typically be next to 0 cost. If we don't save the vector orderValues, we would have to walk all the flexitems to figure out what the possible order: values are. We could also just save the vector, but that didn't seem significantly better than just saving the iterator. (In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 154169 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=154169&action=review > > > > > Source/WebCore/rendering/RenderFlexibleBox.cpp:262 > > > + m_orderIterator = adoptPtr(new OrderIterator(this, orderValues)); > > > > Do we really need the member variable? All this saves is the vector copy + sort in paintChildren, which will typically be next to 0 cost. > > If we don't save the vector orderValues, we would have to walk all the flexitems to figure out what the possible order: values are. We could also just save the vector, but that didn't seem significantly better than just saving the iterator. Oh right. Makes sense. Created attachment 154740 [details]
Patch for landing
Comment on attachment 154740 [details] Patch for landing Clearing flags on attachment: 154740 Committed r123842: <http://trac.webkit.org/changeset/123842> All reviewed patches have been landed. Closing bug. |