70067 2011-10-13 15:37:15 -0700 Add support for client triggered composited layers 2011-12-19 11:12:23 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 69796 1 vangelis vangelis enne hclam jamesr simon.fraser oldest_to_newest 483563 0 vangelis 2011-10-13 15:37:15 -0700 Modify the RenderLayerCompositor to also check with the chrome client on whether an element should get its own composited layer. This will provide client code additional flexibility in determining compositing requirements. 483637 1 110931 vangelis 2011-10-13 16:39:39 -0700 Created attachment 110931 Patch 483657 2 110931 simon.fraser 2011-10-13 16:58:25 -0700 Comment on attachment 110931 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110931&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:115 > + Frame* frame = m_renderView->frameView()->frame(); > + Page* page = frame ? frame->page() : 0; > + if (page) > + m_chromeClient = page->chrome()->client(); Is it OK to cache the chrome client? What happens when the compositor's Frame goes into the page cache? > Source/WebCore/rendering/RenderLayerCompositor.cpp:1314 > + || (m_chromeClient && m_chromeClient->requiresCompositingLayerForObject(renderer, &m_compositingDependsOnGeometry)); I'm a bit worried about the perf impact of this. To be clear, I'm fine allowing position:fixed to create compositing layers in some cases, but the specifics of when position:fixed causes composting should be hidden in client code. In iOS, RLC stores a cache of position:fixed layers, and tells ChromeClient when something goes into and out of that cache. Moving those layers around on scrolling is handled outside of WebCore, by poking things under GraphicsLayer. 483709 3 vangelis 2011-10-13 18:25:14 -0700 (In reply to comment #2) > (From update of attachment 110931 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=110931&action=review > > > Source/WebCore/rendering/RenderLayerCompositor.cpp:115 > > + Frame* frame = m_renderView->frameView()->frame(); > > + Page* page = frame ? frame->page() : 0; > > + if (page) > > + m_chromeClient = page->chrome()->client(); > > Is it OK to cache the chrome client? What happens when the compositor's Frame goes into the page cache? Thanks for flagging that. The page cache code is new to me but from an inspection it doesn't look the Frame's Page and the Pages's Chrome change when the Frame goes in and out of the cache. Is there reason to think otherwise? > > > Source/WebCore/rendering/RenderLayerCompositor.cpp:1314 > > + || (m_chromeClient && m_chromeClient->requiresCompositingLayerForObject(renderer, &m_compositingDependsOnGeometry)); > > I'm a bit worried about the perf impact of this. To be clear, I'm fine allowing position:fixed to create compositing layers in some cases, but the specifics of when position:fixed causes composting should be hidden in client code. > I started by adding a specific method on the ChromeClient just for dealing with fixed position but it occurred to me that it would be more flexible to have a generic hook like the one above. It can be used in the future to experiment with other implementation specific compositing triggers (e.g. maybe one for overflow divs) without poking again into RLC or needing to further extend ChromeClient. Regarding performance, are you worried about the addition of the virtual call into ChromeClient? > In iOS, RLC stores a cache of position:fixed layers, and tells ChromeClient when something goes into and out of that cache. Moving those layers around on scrolling is handled outside of WebCore, by poking things under GraphicsLayer. That's great to know, thanks. Are you opposed to adding layout-related information to GraphicsLayers? It seems that it could be generally useful. 484848 4 110931 dbates 2011-10-16 17:21:18 -0700 Comment on attachment 110931 Patch Attachment 110931 did not pass mac-ews (mac): Output: http://queues.webkit.org/results/10073819 484849 5 110931 dbates 2011-10-16 17:23:29 -0700 Comment on attachment 110931 Patch Attachment 110931 did not pass mac-ews (mac): Output: http://queues.webkit.org/results/10073822 485253 6 vangelis 2011-10-17 10:29:39 -0700 Simon, how would you like to proceed with this patch? Are you opposed to making virtual method calls to client code? Would you prefer to have a flag that lets the RLC know that the client should be asked whether to create a layer so that the virtual calls aren't made on platforms that don't use this functionality? 496933 7 vangelis 2011-11-06 18:00:57 -0800 Closing as client-triggered compositor triggers aren't needed anymore. 524033 8 110931 eric 2011-12-19 11:12:23 -0800 Comment on attachment 110931 Patch Cleared review? from attachment 110931 so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again). 110931 2011-10-13 16:39:39 -0700 2011-12-19 11:12:22 -0800 Patch bug-70067-20111013163938.patch text/plain 5681 vangelis SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk3NDEzKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjcgQEAKKzIwMTEtMTAtMTMgIFZhbmdlbGlz IEtva2tldmlzICA8dmFuZ2VsaXNAY2hyb21pdW0ub3JnPgorCisgICAgICAgIEFkZCBzdXBwb3J0 IGZvciBjbGllbnQtdHJpZ2dlcmVkIGNvbXBvc2l0ZWQgbGF5ZXJzLgorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NzAwNjcKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGlzIHdpbGwgYWxsb3cgaW5kaXZpZHVh bCBpbXBsZW1lbnRhdGlvbnMgdG8KKyAgICAgICAgYWRkIGNsaWVudC1zcGVjaWZpYyB0cmlnZ2Vy cyBmb3IgY29tcG9zaXRpbmcuCisKKyAgICAgICAgVGhpcyBjb2RlIGp1c3QgYWRkcyB0aGUgbmVj ZXNzYXJ5IGhvb2tzIGFuZCByZXF1aXJlZAorICAgICAgICBDaHJvbWVDbGllbnQgQVBJLiAgSXQg ZG9lc24ndCBhbHRlciBleGlzdGluZyBmdW5jdGlvbmFsaXR5LgorCisgICAgICAgIE5vIGZ1bmN0 aW9uYWxpdHkgYWRkZWQgb3IgY2hhbmdlZC4gIE5vIHRlc3RzLgorCisgICAgICAgICogcGFnZS9D aHJvbWVDbGllbnQuaDoKKyAgICAgICAgKFdlYkNvcmU6OkNocm9tZUNsaWVudDo6cmVxdWlyZXND b21wb3NpdGluZ0xheWVyRm9yT2JqZWN0KToKKyAgICAgICAgKiByZW5kZXJpbmcvUmVuZGVyTGF5 ZXJDb21wb3NpdG9yLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckxheWVyQ29tcG9zaXRv cjo6UmVuZGVyTGF5ZXJDb21wb3NpdG9yKToKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckxheWVy Q29tcG9zaXRvcjo6Y2FjaGVBY2NlbGVyYXRlZENvbXBvc2l0aW5nRmxhZ3MpOgorICAgICAgICAo V2ViQ29yZTo6UmVuZGVyTGF5ZXJDb21wb3NpdG9yOjpzY2hlZHVsZUxheWVyRmx1c2gpOgorICAg ICAgICAoV2ViQ29yZTo6UmVuZGVyTGF5ZXJDb21wb3NpdG9yOjpyZXF1aXJlc0NvbXBvc2l0aW5n TGF5ZXIpOgorICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJMYXllckNvbXBvc2l0b3IuaDoKKwog MjAxMS0xMC0xMyAgQ2hyaXMgTWFycmluICA8Y21hcnJpbkBhcHBsZS5jb20+CiAKICAgICAgICAg Rml4IExlb3BhcmQgYnVpbGQKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BhZ2UvQ2hyb21lQ2xpZW50 LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGFnZS9DaHJvbWVDbGllbnQuaAkocmV2 aXNpb24gOTc0MDEpCisrKyBTb3VyY2UvV2ViQ29yZS9wYWdlL0Nocm9tZUNsaWVudC5oCSh3b3Jr aW5nIGNvcHkpCkBAIC01OSw2ICs1OSw3IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKICAgICBjbGFz cyBOb2RlOwogICAgIGNsYXNzIFBhZ2U7CiAgICAgY2xhc3MgUG9wdXBNZW51Q2xpZW50OworICAg IGNsYXNzIFJlbmRlck9iamVjdDsKICAgICBjbGFzcyBTZWN1cml0eU9yaWdpbjsKICAgICBjbGFz cyBHcmFwaGljc0NvbnRleHQzRDsKICAgICBjbGFzcyBXaWRnZXQ7CkBAIC0yNjAsNiArMjYxLDEw IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKICAgICAgICAgLy8gUmV0dXJucyB3aGV0aGVyIG9yIG5v dCB0aGUgY2xpZW50IGNhbiByZW5kZXIgdGhlIGNvbXBvc2l0ZWQgbGF5ZXIsCiAgICAgICAgIC8v IHJlZ2FyZGxlc3Mgb2YgdGhlIHNldHRpbmdzLgogICAgICAgICB2aXJ0dWFsIGJvb2wgYWxsb3dz QWNjZWxlcmF0ZWRDb21wb3NpdGluZygpIGNvbnN0IHsgcmV0dXJuIHRydWU7IH0KKyAgICAgICAg Ly8gUmV0dXJucyB0cnVlIGlmIHRoZSBnaXZlbiBSZW5kZXJPYmplY3QgcmVxdWlyZXMgYSBjb21w b3NpdGVkIGxheWVyLgorICAgICAgICAvLyBXaWxsIHNldCBkZXBlbmRzT25HZW9tZXRyeSB0byB0 cnVlIGlmIHRoZSBkZWNpc2lvbiBjYW4gb25seSBiZSBtYWRlCisgICAgICAgIC8vIGlmIHRoZXJl J3Mgbm8gcGVuZGluZyBsYXlvdXQuCisgICAgICAgIHZpcnR1YWwgYm9vbCByZXF1aXJlc0NvbXBv c2l0aW5nTGF5ZXJGb3JPYmplY3QoUmVuZGVyT2JqZWN0KiwgYm9vbCogZGVwZW5kc09uR2VvbWV0 cnkpIGNvbnN0IHsgcmV0dXJuIGZhbHNlOyB9CiAKICAgICAgICAgZW51bSBDb21wb3NpdGluZ1Ry aWdnZXIgewogICAgICAgICAgICAgVGhyZWVEVHJhbnNmb3JtVHJpZ2dlciA9IDEgPDwgMCwKSW5k ZXg6IFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJMYXllckNvbXBvc2l0b3IuY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJMYXllckNvbXBvc2l0 b3IuY3BwCShyZXZpc2lvbiA5NzQwMSkKKysrIFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5k ZXJMYXllckNvbXBvc2l0b3IuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC05Miw2ICs5Miw3IEBAIHN0 cnVjdCBDb21wb3NpdGluZ1N0YXRlIHsKIAogUmVuZGVyTGF5ZXJDb21wb3NpdG9yOjpSZW5kZXJM YXllckNvbXBvc2l0b3IoUmVuZGVyVmlldyogcmVuZGVyVmlldykKICAgICA6IG1fcmVuZGVyVmll dyhyZW5kZXJWaWV3KQorICAgICwgbV9jaHJvbWVDbGllbnQoMCkKICAgICAsIG1fdXBkYXRlQ29t cG9zaXRpbmdMYXllcnNUaW1lcih0aGlzLCAmUmVuZGVyTGF5ZXJDb21wb3NpdG9yOjp1cGRhdGVD b21wb3NpdGluZ0xheWVyc1RpbWVyRmlyZWQpCiAgICAgLCBtX2hhc0FjY2VsZXJhdGVkQ29tcG9z aXRpbmcodHJ1ZSkKICAgICAsIG1fY29tcG9zaXRpbmdUcmlnZ2VycyhzdGF0aWNfY2FzdDxDaHJv bWVDbGllbnQ6OkNvbXBvc2l0aW5nVHJpZ2dlckZsYWdzPihDaHJvbWVDbGllbnQ6OkFsbFRyaWdn ZXJzKSkKQEAgLTEwOCw2ICsxMDksMTAgQEAgUmVuZGVyTGF5ZXJDb21wb3NpdG9yOjpSZW5kZXJM YXllckNvbXBvcwogICAgICwgbV9yb290TGF5ZXJVcGRhdGVDb3VudCgwKQogI2VuZGlmIC8vIFBS T0ZJTEVfTEFZRVJfUkVCVUlMRAogeworICAgIEZyYW1lKiBmcmFtZSA9IG1fcmVuZGVyVmlldy0+ ZnJhbWVWaWV3KCktPmZyYW1lKCk7CisgICAgUGFnZSogcGFnZSA9IGZyYW1lID8gZnJhbWUtPnBh Z2UoKSA6IDA7CisgICAgaWYgKHBhZ2UpCisgICAgICAgIG1fY2hyb21lQ2xpZW50ID0gcGFnZS0+ Y2hyb21lKCktPmNsaWVudCgpOwogfQogCiBSZW5kZXJMYXllckNvbXBvc2l0b3I6On5SZW5kZXJM YXllckNvbXBvc2l0b3IoKQpAQCAtMTQ4LDExICsxNTMsOCBAQCB2b2lkIFJlbmRlckxheWVyQ29t cG9zaXRvcjo6Y2FjaGVBY2NlbGVyCiAgICAgLy8gV2UgYWxsb3cgdGhlIGNocm9tZSB0byBvdmVy cmlkZSB0aGUgc2V0dGluZ3MsIGluIGNhc2UgdGhlIHBhZ2UgaXMgcmVuZGVyZWQKICAgICAvLyBv biBhIGNocm9tZSB0aGF0IGRvZXNuJ3QgYWxsb3cgYWNjZWxlcmF0ZWQgY29tcG9zaXRpbmcuCiAg ICAgaWYgKGhhc0FjY2VsZXJhdGVkQ29tcG9zaXRpbmcpIHsKLSAgICAgICAgRnJhbWUqIGZyYW1l ID0gbV9yZW5kZXJWaWV3LT5mcmFtZVZpZXcoKS0+ZnJhbWUoKTsKLSAgICAgICAgUGFnZSogcGFn ZSA9IGZyYW1lID8gZnJhbWUtPnBhZ2UoKSA6IDA7Ci0gICAgICAgIGlmIChwYWdlKSB7Ci0gICAg ICAgICAgICBDaHJvbWVDbGllbnQqIGNocm9tZUNsaWVudCA9IHBhZ2UtPmNocm9tZSgpLT5jbGll bnQoKTsKLSAgICAgICAgICAgIG1fY29tcG9zaXRpbmdUcmlnZ2VycyA9IGNocm9tZUNsaWVudC0+ YWxsb3dlZENvbXBvc2l0aW5nVHJpZ2dlcnMoKTsKKyAgICAgICAgaWYgKG1fY2hyb21lQ2xpZW50 KSB7CisgICAgICAgICAgICBtX2NvbXBvc2l0aW5nVHJpZ2dlcnMgPSBtX2Nocm9tZUNsaWVudC0+ YWxsb3dlZENvbXBvc2l0aW5nVHJpZ2dlcnMoKTsKICAgICAgICAgICAgIGhhc0FjY2VsZXJhdGVk Q29tcG9zaXRpbmcgPSBtX2NvbXBvc2l0aW5nVHJpZ2dlcnM7CiAgICAgICAgIH0KICAgICB9CkBA IC0xNzksMTIgKzE4MSwxMCBAQCB2b2lkIFJlbmRlckxheWVyQ29tcG9zaXRvcjo6c2V0Q29tcG9z aXRpCiAKIHZvaWQgUmVuZGVyTGF5ZXJDb21wb3NpdG9yOjpzY2hlZHVsZUxheWVyRmx1c2goKQog ewotICAgIEZyYW1lKiBmcmFtZSA9IG1fcmVuZGVyVmlldy0+ZnJhbWVWaWV3KCktPmZyYW1lKCk7 Ci0gICAgUGFnZSogcGFnZSA9IGZyYW1lID8gZnJhbWUtPnBhZ2UoKSA6IDA7Ci0gICAgaWYgKCFw YWdlKQorICAgIGlmICghbV9jaHJvbWVDbGllbnQpCiAgICAgICAgIHJldHVybjsKIAotICAgIHBh Z2UtPmNocm9tZSgpLT5jbGllbnQoKS0+c2NoZWR1bGVDb21wb3NpdGluZ0xheWVyU3luYygpOwor ICAgIG1fY2hyb21lQ2xpZW50LT5zY2hlZHVsZUNvbXBvc2l0aW5nTGF5ZXJTeW5jKCk7CiB9CiAK IHZvaWQgUmVuZGVyTGF5ZXJDb21wb3NpdG9yOjpmbHVzaFBlbmRpbmdMYXllckNoYW5nZXMoYm9v bCBpc0ZsdXNoUm9vdCkKQEAgLTEzMTAsNyArMTMxMCw4IEBAIGJvb2wgUmVuZGVyTGF5ZXJDb21w b3NpdG9yOjpyZXF1aXJlc0NvbXAKICAgICAgICAgICAgICB8fCAoY2FuUmVuZGVyM0RUcmFuc2Zv cm1zKCkgJiYgcmVuZGVyZXItPnN0eWxlKCktPmJhY2tmYWNlVmlzaWJpbGl0eSgpID09IEJhY2tm YWNlVmlzaWJpbGl0eUhpZGRlbikKICAgICAgICAgICAgICB8fCBjbGlwc0NvbXBvc2l0aW5nRGVz Y2VuZGFudHMobGF5ZXIpCiAgICAgICAgICAgICAgfHwgcmVxdWlyZXNDb21wb3NpdGluZ0ZvckFu aW1hdGlvbihyZW5kZXJlcikKLSAgICAgICAgICAgICB8fCByZXF1aXJlc0NvbXBvc2l0aW5nRm9y RnVsbFNjcmVlbihyZW5kZXJlcik7CisgICAgICAgICAgICAgfHwgcmVxdWlyZXNDb21wb3NpdGlu Z0ZvckZ1bGxTY3JlZW4ocmVuZGVyZXIpCisgICAgICAgICAgICAgfHwgKG1fY2hyb21lQ2xpZW50 ICYmIG1fY2hyb21lQ2xpZW50LT5yZXF1aXJlc0NvbXBvc2l0aW5nTGF5ZXJGb3JPYmplY3QocmVu ZGVyZXIsICZtX2NvbXBvc2l0aW5nRGVwZW5kc09uR2VvbWV0cnkpKTsKIH0KIAogYm9vbCBSZW5k ZXJMYXllckNvbXBvc2l0b3I6OmNhbkJlQ29tcG9zaXRlZChjb25zdCBSZW5kZXJMYXllciogbGF5 ZXIpIGNvbnN0CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyTGF5ZXJDb21w b3NpdG9yLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckxh eWVyQ29tcG9zaXRvci5oCShyZXZpc2lvbiA5NzQwMSkKKysrIFNvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJMYXllckNvbXBvc2l0b3IuaAkod29ya2luZyBjb3B5KQpAQCAtMjg0LDYgKzI4 NCw3IEBAIHByaXZhdGU6CiAKIHByaXZhdGU6CiAgICAgUmVuZGVyVmlldyogbV9yZW5kZXJWaWV3 OworICAgIENocm9tZUNsaWVudCogbV9jaHJvbWVDbGllbnQ7CiAgICAgT3duUHRyPEdyYXBoaWNz TGF5ZXI+IG1fcm9vdENvbnRlbnRMYXllcjsKICAgICBUaW1lcjxSZW5kZXJMYXllckNvbXBvc2l0 b3I+IG1fdXBkYXRlQ29tcG9zaXRpbmdMYXllcnNUaW1lcjsKIAo=