Bug 80641

Summary: REGRESSION (r93614): scrolling div does not repaint
Product: WebKit Reporter: Shawn Singh <shawnsingh>
Component: Layout and RenderingAssignee: Shawn Singh <shawnsingh>
Status: RESOLVED FIXED    
Severity: Normal CC: adele, ap, danakj, jchaffraix, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 80833    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch for landing none

Shawn Singh
Reported 2012-03-08 14:34:13 PST
I believe it is a simple fix, but Julien should be able to make the "correct final fix" far more quickly and with awareness of the performance issues involved. cc Dana just in case this is magically related to our other bugs where paint rects seem wrong, too. To reproduce the problem: go to http://www.railgun3d.com:8080/gltest5/demo.htm (requires webGL) scrolling the div does not repaint the divs contents. If you cannot reproduce: go to inspector turn off "position: absolute" for the 200x200 div that has the scrollbar turn it back on. now it should reliably reproduce. The problem is that the repaint rect associated with the 200x200 div is not re-computed, and therefore its position is way off to the side. I bisected this down to 93614. Hacked solution: forcing computeRepaintRects() in all cases for RenderLayer::updateLayerPositionsAfterScroll does indeed fix the problem.
Attachments
Patch (6.79 KB, patch)
2012-03-10 00:24 PST, Shawn Singh 		no flags
DetailsFormatted DiffDiff
Patch for landing (6.80 KB, patch)
2012-03-11 14:56 PDT, Shawn Singh 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Shawn Singh
Comment 1 2012-03-08 17:57:15 PST
To save time for Julien, he essentially told me the correct fix (and it seems to work); I'll create a reduced test case, verify the fix, and upload a patch. Thanks Julien!
Simon Fraser (smfr)
Comment 2 2012-03-08 20:42:42 PST
http://trac.webkit.org/changeset/93614
Shawn Singh
Comment 3 2012-03-09 14:05:27 PST
Update - our expected fix was not yet correct. Needs further investigation, there seem to be enough clues to continue investigating.
Shawn Singh
Comment 4 2012-03-10 00:15:40 PST
I've traced through the problem, and I have it figured out. Patch is coming in a moment. 1. At first, there is a non-composited layer which paints into an ancestor repaintContainer. because it is positioned, the repaintRect has a non-zero offset. 2. Then, the layer changes position, and becomes composited. At that time, we are not updating the repaintRect. (The code in FrameView::layout() calls updateLayerPositions() where repaintRects are already computed, before updateCompositingLayers().) 3. However, because the layer became composited, it became its own repaintContainer, and the repaintRect should have been updated. But its not, and the repaint rect is skewed by some incorrect offset, and the actual contents don't get repainted. I feel it is correct to computeRepaintRects when a newly composited layer is created. Specifically, we have to wait until the backing is created before we can computeRepaintRects correctly.
Shawn Singh
Comment 5 2012-03-10 00:24:22 PST
Created attachment 131162 [details] Patch The included layout test reproduces the problem on both webkit nightly and tip-of-tree chromium before this patch is applied.
Simon Fraser (smfr)
Comment 6 2012-03-11 11:28:45 PDT
Comment on attachment 131162 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131162&action=review > LayoutTests/compositing/repaint/newly-composited-repaint-rect.html:67 > + if (window.layoutTestController) layoutTestController.display(); Use two lines for this.
Shawn Singh
Comment 7 2012-03-11 14:56:51 PDT
Created attachment 131258 [details] Patch for landing
WebKit Review Bot
Comment 8 2012-03-11 15:17:54 PDT
Comment on attachment 131258 [details] Patch for landing Clearing flags on attachment: 131258 Committed r110401: <http://trac.webkit.org/changeset/110401>
WebKit Review Bot
Comment 9 2012-03-11 15:17:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.