Bug 131375

Created attachment 228858 [details] Test-case

It looks like this only is a problem when -webkit-clip-path is used on the floating box. If the clip path is removed from the example, then the issue doesn't occur.

I landed a fix for this in Blink: http://src.chromium.org/viewvc/blink?view=rev&rev=172619

Created attachment 230204 [details] Patch

Comment on attachment 230204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230204&action=review > Source/WebCore/rendering/RenderBlock.cpp:2971 > + if (!clipPath->pathForReferenceRect(borderBoxRect()).contains(locationInContainer.point() - localOffset, clipPath->windRule())) Same question as in Blink. Hit-testing occurs dozen of times just by hovering over the original element size. Can you try to a create (manual) performance test? I am thinking about a script that creates hundred of div boxes with fixed size. Each box with a circle clip shape and a radius of 50%. Add :hover pseudo selectors that change the background color. Then hover with the mouse really fast and compare it with the same document where the div boxes are not clipped. It would be interesting if it makes sense to investigate in more heuristics before asking the graphic lib if the point is in the path. Especially for circle, inset, ellipse it would be very easy to add heuristics. For polygon we could at least check the bounding box.

Hi Dirk, I made the performance tests. The test displays 30 circles (radius 100px) clipped by clip path. It exposes mouse hover/unhover from the 0,0 point until the center of the circle. DESCRIPTION: Testing clip-path using 30 shapes. RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Time= 1417.15 ms median= 1439.0 ms, stdev= 171.044246353 ms, min= 1038.0 ms, max= 1700.0 ms RESULT Layout: Shapes: ShapeOutsideClipPathSelection: JSHeap= 598262.2 bytes median= 604022.0 bytes, stdev= 11827.0697603 bytes, min= 575207.0 bytes, max= 604038.0 bytes RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Malloc= 1014103.6 bytes median= 1010400.0 bytes, stdev= 8320.6471307 bytes, min= 1009616.0 bytes, max= 1036768.0 bytes Finished: 35.947498 s DESCRIPTION: Testing clip-path + shapes with circle() using 30 shapes. RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Time= 1510.9 ms median= 1546.5 ms, stdev= 130.736335705 ms, min= 1128.0 ms, max= 1594.0 ms RESULT Layout: Shapes: ShapeOutsideClipPathSelection: JSHeap= 596199.2 bytes median= 601959.0 bytes, stdev= 11827.0697603 bytes, min= 573144.0 bytes, max= 601975.0 bytes RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Malloc= 1017173.6 bytes median= 1015184.0 bytes, stdev= 7281.3986395 bytes, min= 1010456.0 bytes, max= 1033488.0 bytes Finished: 37.944173 s No-Shapes: 1439.0ms stdev~8% Shapes: 1546.5ms stdev ~11.8% Shapes case is ~7.4% worse. Although, the standard deviations are pretty high, I've got the similar results after multiple runs. If you want to see these tests in the trunk, I can add them in a separate bug. Also, if you think it's worth to introduce optimizations, we can open the bug for it, but I see those task as possible follow up work after landing this. What do you think?

(In reply to comment #6) > Hi Dirk, > > I made the performance tests. > > The test displays 30 circles (radius 100px) clipped by clip path. It exposes mouse hover/unhover from the 0,0 point until the center of the circle. > > DESCRIPTION: Testing clip-path using 30 shapes. > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Time= 1417.15 ms > median= 1439.0 ms, stdev= 171.044246353 ms, min= 1038.0 ms, max= 1700.0 ms > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: JSHeap= 598262.2 bytes > median= 604022.0 bytes, stdev= 11827.0697603 bytes, min= 575207.0 bytes, max= 604038.0 bytes > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Malloc= 1014103.6 bytes > median= 1010400.0 bytes, stdev= 8320.6471307 bytes, min= 1009616.0 bytes, max= 1036768.0 bytes > Finished: 35.947498 s > > DESCRIPTION: Testing clip-path + shapes with circle() using 30 shapes. > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Time= 1510.9 ms > median= 1546.5 ms, stdev= 130.736335705 ms, min= 1128.0 ms, max= 1594.0 ms > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: JSHeap= 596199.2 bytes > median= 601959.0 bytes, stdev= 11827.0697603 bytes, min= 573144.0 bytes, max= 601975.0 bytes > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Malloc= 1017173.6 bytes > median= 1015184.0 bytes, stdev= 7281.3986395 bytes, min= 1010456.0 bytes, max= 1033488.0 bytes > Finished: 37.944173 s > > No-Shapes: 1439.0ms stdev~8% > Shapes: 1546.5ms stdev ~11.8% > > Shapes case is ~7.4% worse. > > Although, the standard deviations are pretty high, I've got the similar results after multiple runs. If you want to see these tests in the trunk, I can add them in a separate bug. Also, if you think it's worth to introduce optimizations, we can open the bug for it, but I see those task as possible follow up work after landing this. What do you think? Thanks for the tests. I am not sure if you understood my request though. Just want to have performance tests for hit testing: 1) with clip-path 2) without clip-path. It makes sense that shapes will make it slower. My concern was just about clip-path hit testing performance. The reason, if it is bad, we might want to compute the bounding box of the element after clipping and do hit testing on the bounding box first before asking isPointInPath().

Comment on attachment 230204 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230204&action=review > Source/WebCore/rendering/RenderBlock.cpp:2970 > + // FIXME: handle marginBox etc. Even if borderBox is probably the most use reference box, could you add the other boxes as well? the logic to compute these boxes should be in RenderLayer probably.

(In reply to comment #7) > (In reply to comment #6) > > Hi Dirk, > > > > I made the performance tests. > > > > The test displays 30 circles (radius 100px) clipped by clip path. It exposes mouse hover/unhover from the 0,0 point until the center of the circle. > > > > DESCRIPTION: Testing clip-path using 30 shapes. > > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Time= 1417.15 ms > > median= 1439.0 ms, stdev= 171.044246353 ms, min= 1038.0 ms, max= 1700.0 ms > > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: JSHeap= 598262.2 bytes > > median= 604022.0 bytes, stdev= 11827.0697603 bytes, min= 575207.0 bytes, max= 604038.0 bytes > > RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Malloc= 1014103.6 bytes > > median= 1010400.0 bytes, stdev= 8320.6471307 bytes, min= 1009616.0 bytes, max= 1036768.0 bytes > > Finished: 35.947498 s DESCRIPTION: Testing no-clip-path 30 boxes. RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Time= 1343.85 ms median= 1356.5 ms, stdev= 83.9801198279 ms, min= 997.0 ms, max= 1429.0 ms RESULT Layout: Shapes: ShapeOutsideClipPathSelection: JSHeap= 598390.2 bytes median= 604150.0 bytes, stdev= 11827.0697603 bytes, min= 575335.0 bytes, max= 604166.0 bytes RESULT Layout: Shapes: ShapeOutsideClipPathSelection: Malloc= 1017564.8 bytes median= 1015432.0 bytes, stdev= 7189.77538336 bytes, min= 1010640.0 bytes, max= 1037768.0 bytes Finished: 33.877189 s > Thanks for the tests. I am not sure if you understood my request though. > > Just want to have performance tests for hit testing: > 1) with clip-path > 2) without clip-path. clip-path: 1439ms ~6% worse no-clip-path: 1356ms I've run the the test for that case as well, results are above.

(In reply to comment #8) > (From update of attachment 230204 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=230204&action=review > > > Source/WebCore/rendering/RenderBlock.cpp:2970 > > + // FIXME: handle marginBox etc. > > Even if borderBox is probably the most use reference box, could you add the other boxes as well? the logic to compute these boxes should be in RenderLayer probably. I'll start to look into this.

(In reply to comment #9) > clip-path: 1439ms ~6% worse > no-clip-path: 1356ms > > I've run the the test for that case as well, results are above. Ok, that sounds reasonable. Then just the other references boxes and the patch is good to go :).

Created attachment 231042 [details] Patch - Added support and testing for the CSS box models

Created attachment 231043 [details] Same as previous, just renamed selectors in the test case

Comment on attachment 231043 [details] Same as previous, just renamed selectors in the test case Clearing flags on attachment: 231043 Committed r168463: <http://trac.webkit.org/changeset/168463>

All reviewed patches have been landed. Closing bug.