[Webkit-unassigned] [Bug 55257] Support creating compositing layers for scrollable and overflowing objects

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=55257





--- Comment #23 from Daniel Sievers <sievers at google.com>  2011-03-04 14:27:23 PST ---
Simon, thanks for your feedback and apologies for missing to CC you on this bug, my bad!

(In reply to comment #21)
> (From update of attachment 84779 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84779&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:3816
> > +        || renderer()->isRenderIFrame()
> > +        || isComposited();
> 
> You can't do this; the test becomes circular. See RenderLayerCompositor::canBeComposited().
>

But isComposited() != canBeComposited(). The problem I had is that the more generic elements that I want to composite (like overflowing DIVs) are currently not self-painting. Also I didn't want to make all kinds of elements self-painting, if they will not be composited. Since I really only know until after layout, I did the inverse logic here of saying that if we eventually made the layer composited (because it is scrollable and does overflow), we should now be returning true from isSelfPaintingLayer().

In other words, I added isComposited() to the logic here to make sure we return true for 'self-painting' after we have decided to give this layers its own graphics layer in requiresCompositingForScrollableOverflow() (without duplicating the logic).


> Non-self-painting layers are used for overflow, so your changes here will break rendering in some cases.
> 

Wouldn't RenderLayers that have a GraphicsLayer (i.e. are composited) always be self-painting? I'm probably missing something, can you elaborate on what would break?

On the other hand, so what would happen if I created a composited layer that ends up returning 'false' from isSelfPaintingLayer()? This seems to be used quite a bit including in hit testing logic.



> > Source/WebCore/rendering/RenderLayerCompositor.cpp:1230
> > -    return m_hasAcceleratedCompositing && layer->isSelfPaintingLayer();
> > +    if (!m_hasAcceleratedCompositing)
> > +        return false;
> > +
> > +    bool canBeComposited = layer->isSelfPaintingLayer();
> > +
> > +    if ((m_compositingTriggers & ChromeClient::ScrollableOverflowTrigger)
> > +        && layer->renderer()->isRenderBlock() && !layer->renderer()->isTextControl()) {
> > +        // Only do a loose check now, as this function will be queried before layout.
> > +        // Whether there is actual overflow will be determined after layout
> > +        // by requiresCompositingForScrollableOverflow().
> > +        canBeComposited |= toRenderBox(layer->renderer())->scrollsOverflow();
> > +    }
> > +
> > +    return canBeComposited;
> 
> This seems wrong.

This change is basically related to the one above. If I don't want to make all sorts of render block elements self-painting, I'd have to somehow avoid to bail out from needsToBeComposited()->canBeComposited() early, because the layer is not self-painting.

I thought this change would be safe, because it only says whether a layer can theoretically be composited, and it seems like all RenderBlocks would be able to do that, and also because it doesn't mean that we will actually create a compositing layer - the latter would only happen if requiresCompositingLayer() or some other explicit logic would dictate the need for that.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.