Bug 88895 – [chromium] Don't set visible layer rect in CCLayerTreeHost paint iteration

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adrienne Walker]

[chromium] Don't set visible layer rect in CCLayerTreeHost paint iteration

[Adrienne Walker]

Created attachment 147120 [details]
Patch

[Dana Jansens]

Comment on attachment 147120 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:840
>          } else if (it.representsContributingRenderSurface()) {

Can the root layer have a mask? (it won't be contributing)

[Adrienne Walker]

Created attachment 147126 [details]
Handle root mask

[Dana Jansens]

Comment on attachment 147126 [details]
Handle root mask

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

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:839
> +        if (it.representsTargetRenderSurface()) {
> +            LayerType* maskLayer = it->maskLayer();
> +            if (maskLayer)
> +                maskLayer->setVisibleLayerRect(IntRect(IntPoint(), it->contentBounds()));
> +            LayerType* replicaMaskLayer = it->replicaLayer() ? it->replicaLayer()->maskLayer() : 0;
> +            if (replicaMaskLayer)
> +                replicaMaskLayer->setVisibleLayerRect(IntRect(IntPoint(), it->contentBounds()));

Cool, that's better. What do you think of a continue at the end of this block? The If/else situation is a bit confusing, and that might clear it up, letting us drop the else and un-nest that block below. 

Either way, the change looks correct to me.

[Adrienne Walker]

Created attachment 147132 [details]
Conditional cleanup

[Dana Jansens]

Comment on attachment 147132 [details]
Conditional cleanup

This LGTM, thanks for the changes. The logic is changed a bit, and we won't have a visibleLayerRect set on layers that own a surface but don't drawContents themselves, but that is probably just fine.

[James Robinson]

Comment on attachment 147132 [details]
Conditional cleanup

Nice!

[WebKit Review Bot]

Comment on attachment 147132 [details]
Conditional cleanup

Clearing flags on attachment: 147132

Committed r120129: <http://trac.webkit.org/changeset/120129>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.