Bug 99382

Summary: Fold setCellLogicalWidths logic into RenderTableSection layout
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: TablesAssignee: Julien Chaffraix <jchaffraix>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, eric, hyatt, inferno, robert, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Proposed change v1. none

Description Julien Chaffraix 2012-10-15 16:17:50 PDT
RenderTable's and RenderTableSection's setCellLogicalWidths propagate the table layout's logical widths to the cells. This is done as a pre-step to doing RenderTableSection::layout as it may dirty the sections.

Our implementation is artificial and adds an extra unneeded cells' walking: if any column's logical width change, we have to relayout all our sections anyway. As we lay our sections / rows, we can mark the cells as we go.

Following the previous paradigm as several advantages:
* avoids calling setNeedsLayout(true, MarkContainingBlockChain) which is a programming error.
* removes the clunky cell's repainting code.
* avoids a cells' walking.
* removes the artificial split between this "phase" and the regular layout.
Comment 1 Julien Chaffraix 2012-10-15 16:45:37 PDT
Created attachment 168809 [details]
Proposed change v1.
Comment 2 Eric Seidel (no email) 2012-10-15 16:53:50 PDT
Comment on attachment 168809 [details]
Proposed change v1.

The logic looks fine.  I wonder what (if any) perf benefit this has.
Comment 3 Julien Chaffraix 2012-10-15 18:28:30 PDT
(In reply to comment #2)
> (From update of attachment 168809 [details])
> The logic looks fine.  I wonder what (if any) perf benefit this has.

Following our discussion, I tried http://www.robohornet.org/#e=resizecol:

Before:

500x10	510.43ms ± 0.69%
500x50	884.67s ± 1.48%

After:

500x10	509.21ms ± 0.75%
500x50	878.89ms ± 1.40%

It's really a wash, which confirmed some of my rough testing on http://dglazkov.github.com/performance-tests/redraw.html.
Comment 4 Eric Seidel (no email) 2012-10-15 18:31:34 PDT
Thanks.
Comment 5 WebKit Review Bot 2012-10-16 09:42:05 PDT
Comment on attachment 168809 [details]
Proposed change v1.

Clearing flags on attachment: 168809

Committed r131465: <http://trac.webkit.org/changeset/131465>
Comment 6 WebKit Review Bot 2012-10-16 09:42:08 PDT
All reviewed patches have been landed.  Closing bug.