Refactor conditions for setCompositingLayersNeedRebuild in RenderLayer::styleChanged

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Max Vujovic]

To fix bug 109098, we will need to add more conditions for calling RenderLayerCompositor::setCompositingLayersNeedRebuild in RenderLayer::styleChanged. Adding more non-trivial conditions will make this block of code even harder to follow. I suggest we refactor the conditions into functions like:

- needsCompositingLayersRebuiltForClip
- needsCompositingLayersRebuiltForOverflow
- needsCompositingLayersRebuiltForFilters (This function would be added in bug 109098.)

Currently, the block of code in RenderLayer::styleChanged looks like this:

#if USE(ACCELERATED_COMPOSITING)
    updateNeedsCompositedScrolling();
    if (compositor()->updateLayerCompositingState(this))
        compositor()->setCompositingLayersNeedRebuild();
    else if (oldStyle && (oldStyle->clip() != renderer()->style()->clip() || oldStyle->hasClip() != renderer()->style()->hasClip()))
        compositor()->setCompositingLayersNeedRebuild();
    else if (m_backing)
        m_backing->updateGraphicsLayerGeometry();
    else if (oldStyle && oldStyle->overflowX() != renderer()->style()->overflowX()) {
        if (stackingContainer()->hasCompositingDescendant())
            compositor()->setCompositingLayersNeedRebuild();
    }
#endif

[Max Vujovic]

Created attachment 193644 [details]
Patch for Review

Running this patch through the bots to make sure the logic hasn't changed.

[Alexandru Chiculita]

Comment on attachment 193644 [details]
Patch for Review

View in context: https://bugs.webkit.org/attachment.cgi?id=193644&action=review

Looks good, thanks!

> Source/WebCore/rendering/RenderLayer.cpp:5989
> +static inline bool needsCompositingLayersRebuiltForClip(const RenderStyle* oldStyle, const RenderStyle* newStyle)

I would keep both function have similar signature, so in this case, this function could also be part of the RenderLayer.

> Source/WebCore/rendering/RenderLayer.cpp:5997
> +    return !isComposited() && oldStyle && (oldStyle->overflowX() != renderer()->style()->overflowX()) && stackingContainer()->hasCompositingDescendant();

Why not pass the new style here instead of using renderer()->style()?

[Max Vujovic]

Created attachment 193656 [details]
Patch for Review

[Max Vujovic]

Comment on attachment 193644 [details]
Patch for Review

Thanks for looking, Alex.

View in context: https://bugs.webkit.org/attachment.cgi?id=193644&action=review

>> Source/WebCore/rendering/RenderLayer.cpp:5989
>> +static inline bool needsCompositingLayersRebuiltForClip(const RenderStyle* oldStyle, const RenderStyle* newStyle)
> 
> I would keep both function have similar signature, so in this case, this function could also be part of the RenderLayer.

Done. That looks better.

>> Source/WebCore/rendering/RenderLayer.cpp:5997
>> +    return !isComposited() && oldStyle && (oldStyle->overflowX() != renderer()->style()->overflowX()) && stackingContainer()->hasCompositingDescendant();
> 
> Why not pass the new style here instead of using renderer()->style()?

Done. Now the function signatures match.

[Max Vujovic]

Comment on attachment 193656 [details]
Patch for Review

Bots look good. Setting cq+.

[WebKit Review Bot]

Comment on attachment 193656 [details]
Patch for Review

Clearing flags on attachment: 193656

Committed r146213: <http://trac.webkit.org/changeset/146213>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.