125263 2013-12-04 15:43:59 -0800 Bad repaints on Twitter when the tile cache has a margin 2013-12-05 09:54:48 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 bdakin bdakin bdakin simon.fraser thorton oldest_to_newest 956674 0 bdakin 2013-12-04 15:43:59 -0800 With the new backgroundShouldExtendBeyondPage() Setting, there are painting artifacts on Twitter and other webpages that grow their contents size. <rdar://problem/15576884> 956676 1 218468 bdakin 2013-12-04 15:46:32 -0800 Created attachment 218468 Patch 956693 2 218468 thorton 2013-12-04 16:22:24 -0800 Comment on attachment 218468 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218468&action=review > Source/WebCore/platform/graphics/ca/mac/TileController.mm:755 > + // If there are magin tiles and the bounds have grown taller or wider, then the tiles that used to margin > Source/WebCore/platform/graphics/ca/mac/TileController.mm:761 > + IntRect boundsWithoutMargin = IntRect(IntPoint(), expandedIntSize(m_tileCacheLayer->bounds().size())); > + IntRect oldBoundsWithoutMargin = IntRect(IntPoint(), m_boundsAtLastRevalidate.size()); > + oldBoundsWithoutMargin.contract(IntSize(leftMarginWidth() + rightMarginWidth(), topMarginHeight() + bottomMarginHeight())); it might be nice if these things were factored out, especially the things that are done elsewhere > Source/WebCore/platform/graphics/ca/mac/TileController.mm:763 > + if (bounds.height() > m_boundsAtLastRevalidate.height()) { smfr mentioned when things get shorter/skinnier, and you said that it didn't seem to be an issue, but I wonder why? what is invalidating the layer in that case? why doesn't it have stale content? would be nice to have a clear understanding. 956936 3 bdakin 2013-12-05 09:50:42 -0800 (In reply to comment #2) > (From update of attachment 218468 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218468&action=review > > > Source/WebCore/platform/graphics/ca/mac/TileController.mm:755 > > + // If there are magin tiles and the bounds have grown taller or wider, then the tiles that used to > > margin > Hehe > > Source/WebCore/platform/graphics/ca/mac/TileController.mm:761 > > + IntRect boundsWithoutMargin = IntRect(IntPoint(), expandedIntSize(m_tileCacheLayer->bounds().size())); > > + IntRect oldBoundsWithoutMargin = IntRect(IntPoint(), m_boundsAtLastRevalidate.size()); > > + oldBoundsWithoutMargin.contract(IntSize(leftMarginWidth() + rightMarginWidth(), topMarginHeight() + bottomMarginHeight())); > > it might be nice if these things were factored out, especially the things that are done elsewhere > Will do. > > Source/WebCore/platform/graphics/ca/mac/TileController.mm:763 > > + if (bounds.height() > m_boundsAtLastRevalidate.height()) { > > smfr mentioned when things get shorter/skinnier, and you said that it didn't seem to be an issue, but I wonder why? what is invalidating the layer in that case? why doesn't it have stale content? would be nice to have a clear understanding. I did some debugging here and Tim and I talked about it in person. Basically it seems like the cached repaint rects in the GraphicsLayerCA class always seem to have a repaint rect equivalent to the "old" size in changing-content-size test cases. So when the content goes from small to large, there is a small repaint rect that doesn't cover everything. Normal content ends up being fine due to other normal-content invalidations. When the page goes from large to small there is a large repaint rect that is big enough to cover the margin. 956939 4 bdakin 2013-12-05 09:54:48 -0800 Thanks Tim! http://trac.webkit.org/changeset/160179 218468 2013-12-04 15:46:32 -0800 2013-12-04 16:22:24 -0800 Patch for-review.txt text/plain 2907 bdakin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2MDEzNykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEzLTEyLTA0ICBCZXRoIERh a2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBCYWQgcmVwYWludHMgb24gdHdpdHRl ciB3aGVuIHRoZSB0aWxlIGNhY2hlIGhhcyBhIG1hcmdpbgorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI1MjYzCisgICAgICAgIC1hbmQgY29ycmVzcG9u ZGluZy0KKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzE1NTc2ODg0PgorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFdoZW4gdGlsZXMgdGhhdCB1c2VkIHRv IGJlIG1hcmdpbiB0aWxlcyBiZWNvbWUgcmVhbC1jb250ZW50IHRpbGVzLCB0aGV5IG5lZWQgdG8g YmUgCisgICAgICAgIGludmFsaWRhdGVkLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3Mv Y2EvbWFjL1RpbGVDb250cm9sbGVyLm1tOgorICAgICAgICAoV2ViQ29yZTo6VGlsZUNvbnRyb2xs ZXI6OnJldmFsaWRhdGVUaWxlcyk6CisKIDIwMTMtMTItMDQgIFpvbHRhbiBIb3J2YXRoICA8em9s dGFuQHdlYmtpdC5vcmc+CiAKICAgICAgICAgTW92ZSBUcmFpbGluZ09iamVjdHMgY2xhc3MgaW50 byBpdHMgb3duIGgvY3BwCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9j YS9tYWMvVGlsZUNvbnRyb2xsZXIubW0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vZ3JhcGhpY3MvY2EvbWFjL1RpbGVDb250cm9sbGVyLm1tCShyZXZpc2lvbiAxNjAxMDEp CisrKyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jYS9tYWMvVGlsZUNvbnRyb2xs ZXIubW0JKHdvcmtpbmcgY29weSkKQEAgLTc1Miw2ICs3NTIsMjggQEAgdm9pZCBUaWxlQ29udHJv bGxlcjo6cmV2YWxpZGF0ZVRpbGVzKFRpbAogICAgIH0KIAogICAgIGlmIChtX2JvdW5kc0F0TGFz dFJldmFsaWRhdGUgIT0gYm91bmRzKSB7CisgICAgICAgIC8vIElmIHRoZXJlIGFyZSBtYWdpbiB0 aWxlcyBhbmQgdGhlIGJvdW5kcyBoYXZlIGdyb3duIHRhbGxlciBvciB3aWRlciwgdGhlbiB0aGUg dGlsZXMgdGhhdCB1c2VkIHRvCisgICAgICAgIC8vIGJlIGJvdHRvbSBvciByaWdodCBtYXJnaW4g dGlsZXMgbmVlZCB0byBiZSBpbnZhbGlkYXRlZC4KKyAgICAgICAgaWYgKGhhc01hcmdpbnMoKSkg eworICAgICAgICAgICAgaWYgKGJvdW5kcy53aWR0aCgpID4gbV9ib3VuZHNBdExhc3RSZXZhbGlk YXRlLndpZHRoKCkgfHwgYm91bmRzLmhlaWdodCgpID4gbV9ib3VuZHNBdExhc3RSZXZhbGlkYXRl LmhlaWdodCgpKSB7CisgICAgICAgICAgICAgICAgSW50UmVjdCBib3VuZHNXaXRob3V0TWFyZ2lu ID0gSW50UmVjdChJbnRQb2ludCgpLCBleHBhbmRlZEludFNpemUobV90aWxlQ2FjaGVMYXllci0+ Ym91bmRzKCkuc2l6ZSgpKSk7CisgICAgICAgICAgICAgICAgSW50UmVjdCBvbGRCb3VuZHNXaXRo b3V0TWFyZ2luID0gSW50UmVjdChJbnRQb2ludCgpLCBtX2JvdW5kc0F0TGFzdFJldmFsaWRhdGUu c2l6ZSgpKTsKKyAgICAgICAgICAgICAgICBvbGRCb3VuZHNXaXRob3V0TWFyZ2luLmNvbnRyYWN0 KEludFNpemUobGVmdE1hcmdpbldpZHRoKCkgKyByaWdodE1hcmdpbldpZHRoKCksIHRvcE1hcmdp bkhlaWdodCgpICsgYm90dG9tTWFyZ2luSGVpZ2h0KCkpKTsKKworICAgICAgICAgICAgICAgIGlm IChib3VuZHMuaGVpZ2h0KCkgPiBtX2JvdW5kc0F0TGFzdFJldmFsaWRhdGUuaGVpZ2h0KCkpIHsK KyAgICAgICAgICAgICAgICAgICAgSW50UmVjdCBmb3JtZXJCb3R0b21NYXJnaW5SZWN0ID0gSW50 UmVjdChvbGRCb3VuZHNXaXRob3V0TWFyZ2luLngoKSwgb2xkQm91bmRzV2l0aG91dE1hcmdpbi5o ZWlnaHQoKSwKKyAgICAgICAgICAgICAgICAgICAgICAgIG9sZEJvdW5kc1dpdGhvdXRNYXJnaW4u d2lkdGgoKSwgYm91bmRzV2l0aG91dE1hcmdpbi5oZWlnaHQoKSAtIG9sZEJvdW5kc1dpdGhvdXRN YXJnaW4uaGVpZ2h0KCkpOworICAgICAgICAgICAgICAgICAgICBzZXROZWVkc0Rpc3BsYXlJblJl Y3QoZm9ybWVyQm90dG9tTWFyZ2luUmVjdCk7CisgICAgICAgICAgICAgICAgfQorCisgICAgICAg ICAgICAgICAgaWYgKGJvdW5kcy53aWR0aCgpID4gbV9ib3VuZHNBdExhc3RSZXZhbGlkYXRlLndp ZHRoKCkpIHsKKyAgICAgICAgICAgICAgICAgICAgSW50UmVjdCBmb3JtZXJSaWdodE1hcmdpblJl Y3QgPSBJbnRSZWN0KG9sZEJvdW5kc1dpdGhvdXRNYXJnaW4ud2lkdGgoKSwgb2xkQm91bmRzV2l0 aG91dE1hcmdpbi55KCksCisgICAgICAgICAgICAgICAgICAgICAgICBib3VuZHNXaXRob3V0TWFy Z2luLndpZHRoKCkgLSBvbGRCb3VuZHNXaXRob3V0TWFyZ2luLndpZHRoKCksIG9sZEJvdW5kc1dp dGhvdXRNYXJnaW4uaGVpZ2h0KCkpOworICAgICAgICAgICAgICAgICAgICBzZXROZWVkc0Rpc3Bs YXlJblJlY3QoZm9ybWVyUmlnaHRNYXJnaW5SZWN0KTsKKyAgICAgICAgICAgICAgICB9CisgICAg ICAgICAgICB9CisgICAgICAgIH0KKwogICAgICAgICBGbG9hdFJlY3Qgc2NhbGVkQm91bmRzKGJv dW5kcyk7CiAgICAgICAgIHNjYWxlZEJvdW5kcy5zY2FsZShtX3NjYWxlKTsKICAgICAgICAgSW50 UmVjdCBib3VuZHNJblRpbGVDb29yZHMoZW5jbG9zaW5nSW50UmVjdChzY2FsZWRCb3VuZHMpKTsK