Bug 20769

Summary: SVG refresh/redraw problems on scrolled or relative positioned canvas
Product: WebKit Reporter: Anthony Glenning <aglenning>
Component: SVGAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: dino, hyatt, oliver, pam, rwlbuis, zimmermann
Priority: P2 Keywords: GoogleBug, HasReduction, InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on: 23881, 25224, 25254, 25268    
Bug Blocks:    
Description Flags
Document illustrating reproducible test case
further reduction none

Description Anthony Glenning 2008-09-10 13:08:16 PDT
SVG redraw is broken if the canvas is not positioned at the origin. More specifically, I suspect that the calculations to determine the region to refresh do not account for when the SVG canvas is offset. The SVG canvas can be offset either by using scrollbars or by putting it inside a relatively positioned DIV.

In the attached test file, at normal "zoom" you can drag the red square around and everything looks fine. As the square is moved, the canvas is redrawn to remove the old square and draw the square in the new position.

If you select "Zoom Out", moving the SVG canvas to a relative offset, and drag the square, strange things happen. Usually it appears that the drag has no effect. In reality the square is moved, it is just that the screen is not correctly repainted. You can check this by getting a canvas refresh to happen by again changing the zoom level and seeing the square in the updated position. In addition, the square has a 'move' cursor set on it. When you drag and drop the square, you'll see the move cursor appear where you dropped the square and not where the square is drawn.

Similarly, if you select "Zoom In", while the scroll bars are at (0,0) the drag works. But if you scroll, the calculated repaint region is apparently off. As a really good example, try scrolling just a little bit, and you can see the repainted region is off by the amount of the scroll.
Comment 1 Anthony Glenning 2008-09-10 13:09:18 PDT
Created attachment 23325 [details]
Document illustrating reproducible test case
Comment 2 David Kilzer (:ddkilzer) 2008-09-10 13:35:28 PDT
Comment 3 Eric Seidel (no email) 2009-02-04 16:58:52 PST
Yeah, I think we have several dupes of this.  The SVG renderers just don't know about scrolling.  We need to fix clippedOverflowRectForRepaint to not suck.

Hyatt plans to be in this code soon. If not, I'll dive in, cause this really should be fixed.
Comment 4 Eric Seidel (no email) 2009-03-27 15:31:35 PDT
Google has now launched "Sketchy" (the drawing feature for docs), which hits this bug hard.  I've been meaning to fix it for a while, but now it actually effects a shipping product. :(
Comment 5 Eric Seidel (no email) 2009-04-08 05:42:50 PDT
Created attachment 29329 [details]
further reduction
Comment 6 Eric Seidel (no email) 2009-04-08 07:02:14 PDT
I've looked at this some over the last two days (in between crash fixing for chromium 2.0) and it looks like the real fix here is to just kill absoluteTransform().

absoluteTransform() is wrong (or at least it's in conflict with how the rest of the rendering tree works) and doesn't always account for the full transform.

SVG renders will need different "local" transform methods instead.  (Which don't need to be virtual, and can just be helpers implemented on each renderer).  All SVG renderers will need one which handles the parentToLocalTransform() (and probably should take passed in tx, ty values?)  and containers will need a separate method to handle localToChildrenTransform() (for things like the viewport transform).

parentToLocalTransform will be applied to the context before all paint calls, including filter painting, clipping, and outline/focus-rect painting.

localToChildrenTransform() will be applied to the context before calling to children to have them paint.

clippedOverflowRectForRepaint will need to be re-written to use overflowRect() and computeRectForRepaint() like the HTML renders use.  I'll look at this again tomorrow.
Comment 7 Eric Seidel (no email) 2009-04-14 14:26:35 PDT
I wrote up a patch to fix this last week.  I'm fixing some related bugs the patch uncovered, and I'll be posting it shortly.
Comment 8 Eric Seidel (no email) 2009-04-15 17:44:03 PDT
I wrote up a fix for this last week, but it depends on about 20 other local git commits I have in my tree.  I'll be landing those over the next couple days, and then this bug (and bug 14015) will finally be fixed!  (And the SVG rendering tree looks a lot saner as a result)
Comment 9 Nikolas Zimmermann 2009-04-16 15:14:29 PDT
(In reply to comment #8)
> I wrote up a fix for this last week, but it depends on about 20 other local git
> commits I have in my tree.  I'll be landing those over the next couple days,
> and then this bug (and bug 14015) will finally be fixed!  (And the SVG
> rendering tree looks a lot saner as a result)

Excellent Eric! I wish I would have some time to help... very promising work!
I can at least help reviewing, if you cc me on the upcoming patches, hopefully we can finally find a sane way to integrate within the standard rendering tree :-)
Comment 10 Eric Seidel (no email) 2009-04-17 10:05:51 PDT
Ha!  Never mind my patch to bug 25268 fixes this!  YAY!
Comment 11 Eric Seidel (no email) 2009-04-17 10:29:46 PDT
I guess I'll just dupe this to bug 25268.  There are still text scroll issues, covered by bug 16188, but I'll get to those later today. :)

*** This bug has been marked as a duplicate of 25268 ***