Bug 32757

Summary: Repaint bug with -webkit-shadow on svg shapes
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: SVGAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, krit, simon.fraser, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Reduction
none
Patch
bdakin: review-
New patch
none
Even newer patch oliver: review+

Description Beth Dakin 2009-12-18 16:43:04 PST
Created attachment 45210 [details]
Reduction

Dragging around the shapes in the attached test cases leaves behind traces of the shadow.

<rdar://problem/7389149>
Comment 1 Beth Dakin 2009-12-18 16:44:28 PST
I have a patch that I will attach momentarily.
Comment 2 Beth Dakin 2009-12-18 16:50:18 PST
Created attachment 45211 [details]
Patch

I'm not sure how to write a test for this, but I am open to suggestions!
Comment 3 WebKit Review Bot 2009-12-18 16:54:26 PST
style-queue ran check-webkit-style on attachment 45211 [details] without any errors.
Comment 4 Simon Fraser (smfr) 2009-12-18 16:57:31 PST
You should be able to write a repaint test that moves an SVG object in JS.
Comment 5 Beth Dakin 2009-12-18 17:04:07 PST
Comment on attachment 45211 [details]
Patch

R-minusing this for now. This solution needs to be expanded beyond the root.
Comment 6 Dirk Schulze 2009-12-19 11:49:35 PST
This does not only happen on shadows. I can reproduce it without shadows and the same example. But only for the star, where the stroke is not drawn at the edge.
We also need a general fix for repaintRectInLocalCoordinates independent of the bug for shadow.
Comment 7 Beth Dakin 2010-01-07 16:40:14 PST
Created attachment 46098 [details]
New patch

Here's a new patch that applies to more than just the root and also includes tests. Unfortunately, it does not fix the non-shadow bug that Dirk pointed out. I was able to reproduce that as well, and I spent some time working on a patch to fix both at once. Ultimately, it seems like it is just a separate bug. In the no-shadow case, I think that strokeBoundingBox() might be buggy, but I haven't worked out the details yet. I will file a new bug for that repaint issue.
Comment 8 Beth Dakin 2010-01-08 15:23:55 PST
Created attachment 46168 [details]
Even newer patch

Here's a better patch based on comments from Oliver and Dan on irc. This patch rewrites inflateForShadow to be more like RenderStyle::getBoxShadowExtent().
Comment 9 Oliver Hunt 2010-01-08 15:26:44 PST
Comment on attachment 46168 [details]
Even newer patch

r=me
Comment 10 Sam Weinig 2010-01-08 15:28:19 PST
Comment on attachment 46168 [details]
Even newer patch

> +static void getSVGShadowExtent(ShadowData* shadow, int &top, int &right, int &bottom, int &left)
> +{

The & is on the wrong side here.
Comment 11 Beth Dakin 2010-01-08 15:37:45 PST
Good eye, Sammy, good eye. I fixed that before I checked in.

http://trac.webkit.org/changeset/53017
Comment 12 Beth Dakin 2010-01-08 15:52:34 PST
I filed https://bugs.webkit.org/show_bug.cgi?id=33406 for the bug that Dirk found.