Bug 124273

Summary: [CSS Regions] Redundant computation of flowThreadPortionOverflowRect
Product: WebKit Reporter: Manuel Rego Casasnovas <rego>
Component: CSSAssignee: Manuel Rego Casasnovas <rego>
Status: RESOLVED INVALID    
Severity: Normal CC: buildbot, commit-queue, eflews.bot, esprehn+autocc, glenn, gyuyoung.kim, jfernandez, kondapallykalyan, mihnea, rniwa, WebkitBugTracker
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 118665, 119230    
Bug Blocks: 118668    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
none
Patch none

Manuel Rego Casasnovas
Reported 2013-11-13 03:22:52 PST
During the selection in documents with CSS Regions RenderSelectionInfo::repaint() is called several times. Which ends up calling RenderRegion::repaintFlowThreadContent(): void RenderRegion::repaintFlowThreadContent(const LayoutRect& repaintRect, bool immediate) const { repaintFlowThreadContentRectangle(repaintRect, immediate, flowThreadPortionRect(), flowThreadPortionOverflowRect(), contentBoxRect().location()); } flowThreadPortionRect() is already stored in a data member of RenderRengion. However flowThreadPortionOverflowRect() is computed again every time calling overflowRectForFlowThreadPortion().
Attachments
Patch (4.08 KB, patch)
2013-11-13 03:33 PST, Manuel Rego Casasnovas
no flags
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (2.26 MB, application/zip)
2013-11-13 06:06 PST, Build Bot
no flags
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (2.26 MB, application/zip)
2013-11-13 07:04 PST, Build Bot
no flags
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (2.21 MB, application/zip)
2013-11-13 13:28 PST, Build Bot
no flags
Patch (4.18 KB, patch)
2013-11-14 08:12 PST, Manuel Rego Casasnovas
no flags
Manuel Rego Casasnovas
Comment 1 2013-11-13 03:33:29 PST
EFL EWS Bot
Comment 2 2013-11-13 03:47:38 PST
Manuel Rego Casasnovas
Comment 3 2013-11-13 03:54:34 PST
(In reply to comment #1) > Created an attachment (id=216789) [details] > Patch Just for reference in my machine these are the results of running the performance test Layout/RegionsSelection.html (uploaded to bug #119230) with and without this patch: * Without this patch: 5204.433 ms * With this patch: 4696.187 ms Also you can do a quick test with the following HTML example and check the time required to do the selection in the JavaScript console: http://people.igalia.com/mrego/css/perf/test-regions.html Again the results in my machine are: * Without this patch: 186.957 ms * With this patch: 163.778 ms
Build Bot
Comment 4 2013-11-13 06:06:09 PST
Comment on attachment 216789 [details] Patch Attachment 216789 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/22539954 New failing tests: fast/multicol/newmulticol/layers-split-across-columns.html fast/multicol/transform-inside-opacity.html fast/multicol/newmulticol/float-paginate.html fast/multicol/newmulticol/float-multicol.html fast/multicol/newmulticol/cell-shrinkback.html fast/multicol/newmulticol/hide-box-vertical-rl.html fast/multicol/newmulticol/layers-in-multicol.html fast/multicol/mixed-positioning-stacking-order.html fast/multicol/newmulticol/clipping.html fast/multicol/newmulticol/fixed-height-fill-auto.html fast/multicol/newmulticol/float-avoidance.html fast/multicol/newmulticol/single-line.html fast/multicol/positioned-outside-of-columns.html fast/multicol/newmulticol/float-paginate-complex.html fast/multicol/mixed-opacity-test.html fast/multicol/newmulticol/float-paginate-empty-lines.html fast/multicol/mixed-opacity-fixed-test.html fast/multicol/newmulticol/direct-child-column-span-all.html fast/multicol/newmulticol/hide-box-vertical-lr.html fast/multicol/newmulticol/columns-shorthand-parsing.html fast/regions/region-styling/region-style-in-columns.html fast/multicol/newmulticol/positioned-split.html fast/multicol/newmulticol/column-rules-fixed-height.html fast/multicol/newmulticol/positioned-with-constrained-height.html
Build Bot
Comment 5 2013-11-13 06:06:11 PST
Created attachment 216794 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 6 2013-11-13 07:04:43 PST
Comment on attachment 216789 [details] Patch Attachment 216789 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/22640020 New failing tests: fast/multicol/newmulticol/layers-split-across-columns.html fast/multicol/transform-inside-opacity.html fast/multicol/newmulticol/float-paginate.html fast/multicol/newmulticol/float-multicol.html fast/multicol/newmulticol/cell-shrinkback.html fast/multicol/newmulticol/hide-box-vertical-rl.html fast/multicol/newmulticol/layers-in-multicol.html fast/multicol/mixed-positioning-stacking-order.html fast/multicol/newmulticol/clipping.html fast/multicol/newmulticol/fixed-height-fill-auto.html fast/multicol/newmulticol/float-avoidance.html fast/multicol/newmulticol/single-line.html fast/multicol/positioned-outside-of-columns.html fast/multicol/newmulticol/float-paginate-complex.html fast/multicol/mixed-opacity-test.html fast/multicol/newmulticol/float-paginate-empty-lines.html fast/multicol/mixed-opacity-fixed-test.html fast/multicol/newmulticol/direct-child-column-span-all.html fast/multicol/newmulticol/hide-box-vertical-lr.html fast/multicol/newmulticol/columns-shorthand-parsing.html fast/regions/region-styling/region-style-in-columns.html fast/multicol/newmulticol/positioned-split.html fast/multicol/newmulticol/column-rules-fixed-height.html fast/multicol/newmulticol/positioned-with-constrained-height.html
Build Bot
Comment 7 2013-11-13 07:04:45 PST
Created attachment 216797 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 8 2013-11-13 13:28:29 PST
Comment on attachment 216789 [details] Patch Attachment 216789 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/22779999 New failing tests: fast/multicol/newmulticol/layers-split-across-columns.html fast/multicol/transform-inside-opacity.html fast/multicol/newmulticol/float-multicol.html fast/multicol/newmulticol/float-paginate.html fast/multicol/newmulticol/cell-shrinkback.html fast/multicol/newmulticol/hide-box-vertical-rl.html fast/multicol/newmulticol/layers-in-multicol.html fast/multicol/mixed-positioning-stacking-order.html fast/multicol/newmulticol/clipping.html fast/multicol/newmulticol/fixed-height-fill-auto.html fast/multicol/newmulticol/float-avoidance.html fast/multicol/newmulticol/single-line.html fast/multicol/positioned-outside-of-columns.html fast/multicol/newmulticol/float-paginate-complex.html fast/multicol/mixed-opacity-test.html fast/multicol/newmulticol/float-paginate-empty-lines.html fast/multicol/newmulticol/direct-child-column-span-all.html fast/multicol/newmulticol/hide-box-vertical-lr.html fast/multicol/newmulticol/columns-shorthand-parsing.html fast/regions/region-styling/region-style-in-columns.html fast/multicol/newmulticol/positioned-split.html fast/multicol/newmulticol/column-rules-fixed-height.html fast/multicol/newmulticol/positioned-with-constrained-height.html
Build Bot
Comment 9 2013-11-13 13:28:31 PST
Created attachment 216848 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Manuel Rego Casasnovas
Comment 10 2013-11-14 08:12:01 PST
Created attachment 216937 [details] Patch Code related to this patch is going to change in bug #118665, so this patch would need to be updated once it lands.
Manuel Rego Casasnovas
Comment 11 2013-11-21 14:10:54 PST
After patch for bug #118665 has landed (r159609), RenderRegion::flowThreadPortionOverflowRect() is not used anymore in RenderRegion::repaintFlowThreadContent() so this patch is invalid.
Note You need to log in before you can comment on or make changes to this bug.