75638 2012-01-05 11:35:36 -0800 Unnecessary and incorrect invalidation about composited fixed-position layers 2012-02-09 12:04:22 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 72078 1 wangxianzhu wangxianzhu jamesr nduca simon.fraser webkit.review.bot oldest_to_newest 530477 0 wangxianzhu 2012-01-05 11:35:36 -0800 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. 530812 1 wangxianzhu 2012-01-05 17:16:20 -0800 This bug is not Chromium-specific. 530864 2 121380 wangxianzhu 2012-01-05 18:34:52 -0800 Created attachment 121380 patch 531320 3 jamesr 2012-01-06 13:12:16 -0800 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? 531336 4 jamesr 2012-01-06 13:28:05 -0800 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. 533412 5 wangxianzhu 2012-01-10 15:20:47 -0800 (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. 533526 6 jamesr 2012-01-10 17:38:39 -0800 (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? 552617 7 wangxianzhu 2012-02-08 16:32:20 -0800 (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. 553252 8 wangxianzhu 2012-02-09 10:07:18 -0800 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? 553328 9 jamesr 2012-02-09 11:02:54 -0800 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. 553336 10 wangxianzhu 2012-02-09 11:07:59 -0800 (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. 553394 11 121380 webkit.review.bot 2012-02-09 12:04:17 -0800 Comment on attachment 121380 patch Clearing flags on attachment: 121380 Committed r107270: <http://trac.webkit.org/changeset/107270> 553395 12 webkit.review.bot 2012-02-09 12:04:22 -0800 All reviewed patches have been landed. Closing bug. 121380 2012-01-05 18:34:52 -0800 2012-02-09 12:04:17 -0800 patch 75638 text/plain 1587 wangxianzhu SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNDI2MCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEyLTAxLTA1ICBYaWFuemh1 IFdhbmcgIDx3YW5neGlhbnpodUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgVW5uZWNlc3Nhcnkg YW5kIGluY29ycmVjdCBpbnZhbGlkYXRpb24gYWJvdXQgY29tcG9zaXRlZCBmaXhlZC1wb3NpdGlv biBsYXllcnMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTc1NjM4CisKKyAgICAgICAgV2hlbiBhIEZyYW1lVmlldyBzY3JvbGxzLCBjb21wb3NpdGVkIGZp eGVkLXBvc2l0aW9uIGxheWVycyBzaG91bGQKKyAgICAgICAgbm90IGNvbnRyaWJ1dGUgdG8gdGhl IGludmFsaWRhdGlvbiByZWN0IG9mIHRoZSByb290IGxheWVyLgorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gSGF2ZW4ndCBmb3Vu ZCBhIGdvb2Qgd2F5IHRvIHRlc3QgdGhpcyBwcm9ncmFtYXRpY2FsbHkuCisKKyAgICAgICAgKiBw YWdlL0ZyYW1lVmlldy5jcHA6CisgICAgICAgIChXZWJDb3JlOjpGcmFtZVZpZXc6OnNjcm9sbENv bnRlbnRzRmFzdFBhdGgpOgorCiAyMDEyLTAxLTA1ICBZb25nanVuIFpoYW5nICA8eW9uZ2p1bl96 aGFuZ0BhcHBsZS5jb20+CiAKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTc1NTkzCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5jcHAK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGFnZS9GcmFtZVZpZXcuY3BwCShyZXZpc2lv biAxMDQyMzIpCisrKyBTb3VyY2UvV2ViQ29yZS9wYWdlL0ZyYW1lVmlldy5jcHAJKHdvcmtpbmcg Y29weSkKQEAgLTE0OTksNiArMTQ5OSwxMCBAQCBib29sIEZyYW1lVmlldzo6c2Nyb2xsQ29udGVu dHNGYXN0UGF0aChjCiAgICAgICAgIFJlbmRlckJveCogcmVuZGVyQm94ID0gKml0OwogICAgICAg ICBpZiAocmVuZGVyQm94LT5zdHlsZSgpLT5wb3NpdGlvbigpICE9IEZpeGVkUG9zaXRpb24pCiAg ICAgICAgICAgICBjb250aW51ZTsKKyNpZiBVU0UoQUNDRUxFUkFURURfQ09NUE9TSVRJTkcpCisg ICAgICAgIGlmIChyZW5kZXJCb3gtPmxheWVyKCktPmlzQ29tcG9zaXRlZCgpKQorICAgICAgICAg ICAgY29udGludWU7CisjZW5kaWYKICAgICAgICAgSW50UmVjdCB1cGRhdGVSZWN0ID0gcmVuZGVy Qm94LT5sYXllcigpLT5yZXBhaW50UmVjdEluY2x1ZGluZ0Rlc2NlbmRhbnRzKCk7CiAgICAgICAg IHVwZGF0ZVJlY3QgPSBjb250ZW50c1RvUm9vdFZpZXcodXBkYXRlUmVjdCk7CiAgICAgICAgIGlm ICghaXNDb21wb3NpdGVkQ29udGVudExheWVyICYmIGNsaXBzUmVwYWludHMoKSkK