Bug 13320

Summary: rounded corners with drop shadows are really slow
Product: WebKit Reporter: Jonathan del Strother <jon>
Component: Layout and RenderingAssignee: mitz
Status: RESOLVED FIXED    
Severity: Normal CC: dev+webkit, koivisto
Priority: P2 Keywords: InRadar
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   
URL: http://bestbefore.tv/roundedShadows.html
Attachments:
Description Flags
[WIP] use a single rounded rect path
none
Use a single rounded rect path
none
Use single rounded rect path mrowe: review+

Description Jonathan del Strother 2007-04-10 09:27:58 PDT
-webkit-box-shadow and -webkit-border-radius are fine, on their own.  When combined together, they kill redraw speeds.

The page above informally demonstrates this - resizing the window is noticably slower when drop shadows & rounded corners are combined.  If necessary, I could probably come up with some sort of animating benchmark thing...
Comment 1 Antti Koivisto 2007-04-10 09:44:05 PDT
Yeah, it is pretty slow even with relatively small number of boxes. Shark shows that time is mostly spent doing fills in GraphicsContext:: fillRoundedRect() (called from RenderObject::paintBoxShadow()).
Comment 2 Dave Hyatt 2007-04-10 13:33:12 PDT
There is a bug filed in Radar about this already.  Upgrading to P1 to reflect the internal bug's status.

We'll need to change the clipping to stroke a single path instead of using the multiple ellipse approach.
Comment 3 Darin Adler 2007-04-11 01:03:34 PDT
<rdar://problem/4994191>

The internal bug isn't marked P1.
Comment 4 mitz 2007-05-19 05:29:31 PDT
(In reply to comment #2)
> We'll need to change the clipping to stroke a single path instead of using the
> multiple ellipse approach.

I'm trying to do this now.
Comment 5 mitz 2007-05-19 06:53:48 PDT
Created attachment 14625 [details]
[WIP] use a single rounded rect path

Probably breaks non-Mac platforms. Fixes an existing box-shadow+border-radius bug when the radii cannot be satisfied (the box reverts to having straight corners, but the shadow goes all crazy). Didn't touch addInnerRoundedRectClip(), although it would be nice to get rid of it.
Comment 6 mitz 2007-05-19 08:21:12 PDT
Created attachment 14626 [details]
Use a single rounded rect path
Comment 7 Darin Adler 2007-05-19 09:58:38 PDT
Comment on attachment 14626 [details]
Use a single rounded rect path

In Path::createRoundedRectangle I would expect you to use float insted of double. FloatRect, etc. are actually "float" not double, and converting back and forth can be costly.

Otherwise looks great. r=me
Comment 8 mitz 2007-05-19 10:14:11 PDT
Comment on attachment 14626 [details]
Use a single rounded rect path

(In reply to comment #7)
> (From update of attachment 14626 [details] [edit])
> In Path::createRoundedRectangle I would expect you to use float insted of
> double.

Copy&paste accident. I'm going to correct it.
Comment 9 mitz 2007-05-19 10:18:47 PDT
Created attachment 14628 [details]
Use single rounded rect path

s/double/float/
Comment 10 Mark Rowe (bdash) 2007-05-19 16:17:48 PDT
Landed in r21601.