If a render layer is composited, and is therefore a stacking context, there is no risk in putting it on the fast scrolling path. So in this case, RenderLayer::usesCompositedScrolling() should return true.
Created attachment 165179 [details] Patch
Comment on attachment 165179 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165179&action=review > Source/WebCore/ChangeLog:9 > + If the scrolling render layer is composited, then RenderLayer::usesCompositedScrolling > + will return true. Might be nice to describe what this does. :) Even if it's obvious to the compositor folks.
Comment on attachment 165179 [details] Patch I don't follow this at all. It's quite possible to have a <div> be in a compositing layer, but to not have set up the inner scrolling layers that make it scroll compositedly.
Comment on attachment 165179 [details] Patch Attachment 165179 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13950752 New failing tests: compositing/overflow/overflow-scrollbar-layers.html compositing/geometry/limit-layer-bounds-transformed-overflow.html compositing/overflow/content-gains-scrollbars.html compositing/layer-creation/overflow-scroll-overlap.html
If we do have a layer that is set up to scroll we should throw it down this path without needing the webkit-overflow-scroll property. What other internal setup is required, Simon?
It doesn't follow that being composited means that something is a stacking context.
(In reply to comment #6) > It doesn't follow that being composited means that something is a stacking context. Huh? What do we make composited that's not a stacking context?
Layers that are composited because of overlap, for example.
If the layer is a stacking context (for whatever reason) we should be able to go down this path, though.
Comment on attachment 165179 [details] Patch Attachment 165179 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13950786 New failing tests: compositing/overflow/scrollbars-with-clipped-owner.html compositing/geometry/limit-layer-bounds-transformed-overflow.html compositing/overflow/content-gains-scrollbars.html compositing/repaint/newly-composited-repaint-rect.html compositing/overflow/overflow-scrollbar-layers.html compositing/overflow/clip-content-under-overflow-controls.html WebFilterOperationsTest.saveAndRestore compositing/layer-creation/overflow-scroll-overlap.html
In light of the previous comments, I tried returning true in RenderLayer::usesCompositedScrolling if isComposited() and isStackingContext() were both true, but this caused some problems in the layout tests. The problem is that when composited scrolling is used, the scrolling contents are rendered behind transparent scroll controls rather than being clipped. Hopefully this behavior can be changed, but at present, automatically opting into composited scrolling will cause correctness issues. Respecting -webkit-overflow-scrolling:touch is also currently dangerous, because it creates a stacking context and the promotion to a composited layer can affect anti-aliasing, so we will break the z-ordering and anti-aliasing of some existing pages. Given these facts, I propose the following: - We respect -webkit-overflow-scrolling:touch *if* a) We're already a stacking context, and b) We're already composited. This would let us explicitly opt into composited scrolling in a way that prevents us from breaking the z-ordering and anti-aliasing of existing pages. In future, if the problem of rendering the scrollable content behind the overflow scroll controls is addressed, we can look into automatically opting into this path without -webkit-overflow-scrolling:touch. We might also relax the isStackingContext restriction -- it should be sufficient that promotion to a stacking context would not affect z-ordering. I will upload a patch that does this shortly. In that patch, if ENABLE(ACCELERATED_OVERFLOW_SCROLLING) is defined, then -webkit-overflow-scrolling:touch will *force* us to use composited scrolling, otherwise that property will only turn on composited scrolling if we're already a composited stacking context.
Created attachment 165271 [details] Patch Contains the changes mentioned in the previous comment.
Attachment 165271 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/rendering/RenderLayer.cpp:1597: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 165272 [details] Patch I apologize for the spam. This patch addresses the style issue.
So you are doing this for feature detection purposes?
(In reply to comment #15) > So you are doing this for feature detection purposes? It's more that I'm trying to enabled a feature rather than detect it (but perhaps I'm misunderstanding your question). Composited scrolling is great -- it saves us some very costly painting -- but designers currently have no way to take advantage of it on browsers built without ACCELERATED_OVERFLOW_SCROLLING enabled. My hope was to provide *some* recourse for designers who want to use composited scrolling on all WebKit-based browsers, but do so in a way that doesn't break existing pages. With this change, a designer can use -webkit-overflow-scrolling:touch in combination with -webkit-transform:translateZ(0px) to opt into composited scrolling. I should stress that this isn't the end of the work on this, either. My hope is that we can use composited scrolling on more and more pages, eventually without using -webkit-overflow-scrolling at all.
(In reply to comment #16) > With this change, a designer can use -webkit-overflow-scrolling:touch in combination with -webkit-transform:translateZ(0px) to opt into composited scrolling. I really don't want more translateZ(0) cargo-cultism than already exists, especially when combined with the already mysterious -webkit-overflow-scrolling.
(In reply to comment #17) > (In reply to comment #16) > > With this change, a designer can use -webkit-overflow-scrolling:touch in combination with -webkit-transform:translateZ(0px) to opt into composited scrolling. > > I really don't want more translateZ(0) cargo-cultism than already exists, especially when combined with the already mysterious -webkit-overflow-scrolling. I'm still working on a proper response, but in the meantime, I wanted to lists the tests that fail when composited scrolling is used because the scrolled contents are drawn behind/over the overflow controls. These are compositing/overflow/clip-content-under-overflow-controls.html compositing/overflow/scrollbars-with-clipped-owner.html
usesCompositedScrolling() controls too many behaviors. There are a few concepts here. For the compositor tree setup it shouldn't make a different what this value is set to, although other parts of the rendering code do care about this property. What I think we should do here is set up the RenderLayerBacking's GraphicsLayer hierarchy so that scrolling is only a matter of updating one layer's position and make sure clipping, etc, is correct. Then it's a simple matter to hook up RenderLayer::scrollTo() to simply update this position and rely on the compositor to repaint content as needed. Look at how RenderLayerCompositor's GraphicsLayer hierarchy is different to accomodate this. The m_containmentLayer/m_scrollingLayer/m_scrollingContentsLayer fields on RLB appear to be partway there, but I think we should just make it part of the default setup and not rely on things like offsetFromRenderer.
Comment on attachment 165272 [details] Patch Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.