Bug 75638 - Unnecessary and incorrect invalidation about composited fixed-position layers
Summary: Unnecessary and incorrect invalidation about composited fixed-position layers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Xianzhu Wang
URL:
Keywords:
Depends on:
Blocks: 72078
  Show dependency treegraph
 
Reported: 2012-01-05 11:35 PST by Xianzhu Wang
Modified: 2012-02-09 12:04 PST (History)
4 users (show)

See Also:


Attachments
patch (1.55 KB, patch)
2012-01-05 18:34 PST, Xianzhu Wang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xianzhu Wang 2012-01-05 11:35:36 PST
Observed when a page containing composited fixed-position layers scrolls, the root layer is invalidated for the rects of the fixed-position layers. In addition, the coordinates of the invalidation rects are not properly offset to the actual scrolled position. This causes unnecessary repaint when scrolling near the top of the page, and incorrect (but no effect) invalidation when scrolling below.
Comment 1 Xianzhu Wang 2012-01-05 17:16:20 PST
This bug is not Chromium-specific.
Comment 2 Xianzhu Wang 2012-01-05 18:34:52 PST
Created attachment 121380 [details]
patch
Comment 3 James Robinson 2012-01-06 13:12:16 PST
Example test page:

<!DOCTYPE html>
<div style="top:30px; width:50px;height:50px;position:fixed;background-color:lightblue;-webkit-transform:translateZ(0)"></div>
<div style="height:2000px"></div>

data:url: data:text/html;charset=utf-8,%3C!DOCTYPE%20html%3E%0A%3Cdiv%20style%3D%22top%3A30px%3B%20width%3A50px%3Bheight%3A50px%3Bposition%3Afixed%3Bbackground-color%3Alightblue%3B-webkit-transform%3AtranslateZ(0)%22%3E%3C%2Fdiv%3E%0A%3Cdiv%20style%3D%22height%3A2000px%22%3E%3C%2Fdiv%3E

when I scroll this page up and down in a WebKit nightly with the Debug "Show Composited Borders" flag on I see the paint count on the root layer go up on every scroll.  This patch will fix that, right?

Any ideas about how to programatically test this, Simon?
Comment 4 James Robinson 2012-01-06 13:28:05 PST
Xianzhu - can you construct a repaint test (see LayoutTests/fast/repaint/fixed-scroll-simple.html as an example) that has a fixed position composited element? The expectations might look a little weird today in the chromium harness, but we can fix that later on.
Comment 5 Xianzhu Wang 2012-01-10 15:20:47 PST
(In reply to comment #4)
> Xianzhu - can you construct a repaint test (see LayoutTests/fast/repaint/fixed-scroll-simple.html as an example) that has a fixed position composited element? The expectations might look a little weird today in the chromium harness, but we can fix that later on.

fast/repaint/fixed-scroll-simple.html seems to ensure the view is correctly updated on scrolling. My change should not break it. However, when scrolling, the whole view should be updated, no matter if the layers are repainted or not. I'm still wondering how to check if some area of a layer is repainted with the existing LayoutTestController API.
Comment 6 James Robinson 2012-01-10 17:38:39 PST
(In reply to comment #5)
> (In reply to comment #4)
> > Xianzhu - can you construct a repaint test (see LayoutTests/fast/repaint/fixed-scroll-simple.html as an example) that has a fixed position composited element? The expectations might look a little weird today in the chromium harness, but we can fix that later on.
> 
> fast/repaint/fixed-scroll-simple.html seems to ensure the view is correctly updated on scrolling. My change should not break it. However, when scrolling, the whole view should be updated, no matter if the layers are repainted or not. I'm still wondering how to check if some area of a layer is repainted with the existing LayoutTestController API.

That sounds like a known artifact of the chromium DumpRenderTree implementation - can you try in mac DRT?
Comment 7 Xianzhu Wang 2012-02-08 16:32:20 PST
(In reply to comment #6) 
> That sounds like a known artifact of the chromium DumpRenderTree implementation - can you try in mac DRT?

Sorry for no progress for a long time.

I'm now confused with the expected pixel result of fixed-scroll-simple.html. On mac it's all covered with dark gray while on chromium it's not covered at all. To me the Chromium's seems more correct because the whole visible area of the window  should be repainted. Still have no idea about how to test updates of layers.
Comment 8 Xianzhu Wang 2012-02-09 10:07:18 PST
James, I think there is some fundamental issues about repaint tests, especially about scrolling. Do you agree to submit the patch first and add layout test later after the repaint test issue resolved?
Comment 9 James Robinson 2012-02-09 11:02:54 PST
Yes, I think it's fine to land this bugfix.

I think the problems you're seeing with scrolling probably stem from using a non-windows chromium DRT. Repaint tests behavior oddly when scrolling in chromium platforms other than windows because we only support blit+backfill in DRT on windows.
Comment 10 Xianzhu Wang 2012-02-09 11:07:59 PST
(In reply to comment #9)
> Yes, I think it's fine to land this bugfix.
> 
> I think the problems you're seeing with scrolling probably stem from using a non-windows chromium DRT. Repaint tests behavior oddly when scrolling in chromium platforms other than windows because we only support blit+backfill in DRT on windows.

I also don't understand the expected result of fixed-scroll-simple.html on Mac (which is all masked with dark gray). I've asked on webkit-dev, and I'd like to work on the method proposed in the thread.
Comment 11 WebKit Review Bot 2012-02-09 12:04:17 PST
Comment on attachment 121380 [details]
patch

Clearing flags on attachment: 121380

Committed r107270: <http://trac.webkit.org/changeset/107270>
Comment 12 WebKit Review Bot 2012-02-09 12:04:22 PST
All reviewed patches have been landed.  Closing bug.