Bug 74002 - Moving SVG elements on the page doesn't always erase element at the old position
Summary: Moving SVG elements on the page doesn't always erase element at the old position
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL: http://www.mysparebrain.com/svgbug.html
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-07 08:45 PST by Tim M
Modified: 2011-12-08 16:07 PST (History)
5 users (show)

See Also:


Attachments
Screenshot illustration of text box incorrectly redrawn (4.61 KB, image/png)
2011-12-07 08:45 PST, Tim M
no flags Details
Reduced test - red box should not be visible (777 bytes, image/svg+xml)
2011-12-08 12:16 PST, Florin Malita
no flags Details
Patch (5.04 KB, patch)
2011-12-08 12:47 PST, Florin Malita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim M 2011-12-07 08:45:48 PST
Created attachment 118215 [details]
Screenshot illustration of text box incorrectly redrawn

When SVG elements are moved on the page (ie the DOM is manipulated) then the screen redraw logic doesn't always correctly invalidate the old position, so the display is left with portions of the old display.
The test page referenced above shows this with test case number 1 - moving the element down the page leaves artefacts in Safari and Chrome (see also screenshot attached)
Bug also posted to chrome as http://code.google.com/p/chromium/issues/detail?id=98391 but advised to post it here instead.
Comment 1 Florin Malita 2011-12-08 12:16:52 PST
Created attachment 118443 [details]
Reduced test - red box should not be visible

Tracked this down to RenderSVGContainer::layout():

    // Allow RenderSVGViewportContainer to update its viewport.
    calcViewport();

    LayoutRepainter repainter(*this, checkForRepaintDuringLayout() || selfWillPaint());


The viewport is updated before grabbing the old repaint bounds - hence repainter really holds the new bounds (inferred from the updated viewport). Later on in repaint, we're only seeing these new bounds and not repainting the old area.

I have a patch to swap the operations coming up shortly.
Comment 2 Florin Malita 2011-12-08 12:47:06 PST
Created attachment 118452 [details]
Patch
Comment 3 WebKit Review Bot 2011-12-08 16:07:25 PST
Comment on attachment 118452 [details]
Patch

Clearing flags on attachment: 118452

Committed r102391: <http://trac.webkit.org/changeset/102391>
Comment 4 WebKit Review Bot 2011-12-08 16:07:29 PST
All reviewed patches have been landed.  Closing bug.