Summary: | [CoordinatedGraphics] CSS fixed elements are shaking while scrolling | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||
Component: | WebCore Misc. | Assignee: | Mikhail Pozdnyakov <mikhail.pozdnyakov> | ||||||
Status: | RESOLVED DUPLICATE | ||||||||
Severity: | Normal | CC: | andersca, cmarcelo, jamesr, jesus, kenneth, luciano.wolf, luiz, noam, rafael.lobo, tonikitoo, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Mikhail Pozdnyakov
2013-04-04 04:49:36 PDT
Created attachment 196472 [details]
patch
Comment on attachment 196472 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=196472&action=review > Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:52 > + do { > + if (layer->renderer()->style()->position() == FixedPosition) > + return true; > + > + layer = layer->parent(); > + } while (layer && !layer->isComposited()); // Look between a composited layer and its nearest composited ancestor did you mean RenderLayer::enclosingCompositingLayer()? > Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:62 > + ASSERT(layer->isComposited()); If a layer is no longer fixed wouldn't it become un-composited? Comment on attachment 196472 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=196472&action=review >> Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:52 >> + } while (layer && !layer->isComposited()); // Look between a composited layer and its nearest composited ancestor > > did you mean RenderLayer::enclosingCompositingLayer()? I wouldn't say so, traversal algorithm is similar but in this case we need also check whether layer position is fixed. >> Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:62 >> + ASSERT(layer->isComposited()); > > If a layer is no longer fixed wouldn't it become un-composited? This method can only be invoked for composited layers (those having backing). I could substitute it with ASSERT(layer->backing()), would you like it? Created attachment 196631 [details]
patch
There is no need actually to traverse uncomposited parent layers for being fixed, as we have acceleratedCompositingForFixedPositionEnabled set providing constrained layers to be composited.
(In reply to comment #4) > Created an attachment (id=196631) [details] > patch > > There is no need actually to traverse uncomposited parent layers for being fixed, as we have acceleratedCompositingForFixedPositionEnabled set providing constrained layers to be composited. I mean traverse and check for being fixed. Comment on attachment 196631 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=196631&action=review > Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:50 > + // This method can only be invoked for composited layers (those having backing). (those having backing) is redundant :) > Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:56 > + toCoordinatedGraphicsLayer(backing->childForSuperlayers())->setFixedToViewport(true); why childForSuperlayers instead of graphicsLayer() ? > Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:59 > + if (backing->hasAncestorClippingLayer()) > + toCoordinatedGraphicsLayer(backing->ancestorClippingLayer())->setFixedToViewport(false); What does this do? (In reply to comment #6) > (From update of attachment 196631 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=196631&action=review > > > Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:50 > > + // This method can only be invoked for composited layers (those having backing). > > (those having backing) is redundant :) > > > Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:56 > > + toCoordinatedGraphicsLayer(backing->childForSuperlayers())->setFixedToViewport(true); > > why childForSuperlayers instead of graphicsLayer() ? FOA, this is actually what had been already in RenderLayerBacking::registerScrollingLayers() previously (before r147163) and now the similar functionality is in coordinator for chromium. As far as I understand childForSuperlayers would return a layer to apply any transform and which should actually be set as fixed on coord graphics side. It can be either m_ancestorClippingLayer (we are clipped by an ancestor which is not a stacking context) or m_contentsContainmentLayer (we need a compositing layer to render its background into a separate GraphicsLayer (e.g. for background-attachment: fixed in some scenarios) ) or m_graphicsLayer itself otherwise. > > > Source/WebCore/page/scrolling/coordinatedgraphics/ScrollingCoordinatorCoordinatedGraphics.cpp:59 > > + if (backing->hasAncestorClippingLayer()) > > + toCoordinatedGraphicsLayer(backing->ancestorClippingLayer())->setFixedToViewport(false); > > What does this do? here we actually have to reset all the layers that childForSuperlayers could return. (And apparently I forgot to reset contentsContainmentLayer) *** This bug has been marked as a duplicate of bug 114353 *** |