Bug 81354 – [chromium] Don't occlude on main-thread behind layers/surfaces with impl-thread animations

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

[Dana Jansens]

[chromium] Don't occlude behind layers/surfaces with impl-animating opacity on main thread

[Dana Jansens]

Created attachment 132318 [details]
Patch

[Dana Jansens]

Created attachment 132323 [details]
Patch

[Dana Jansens]

Created attachment 132340 [details]
Patch

Remove double negatives.
Include logic so that we don't use occlusion within a space that we can't transform to.
 - i.e. If layer can't be transformed to target reliably, then occlusion in target can't be applied to the layer.

[James Robinson]

Comment on attachment 132340 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:87
> +static inline bool layerOpacityKnown(const CCLayerImpl* /* layer */) { return true; }

slightly better to omit the parameter name completely instead of commenting it out

[Dana Jansens]

Comment on attachment 132340 [details]
Patch

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

>> Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:87
>> +static inline bool layerOpacityKnown(const CCLayerImpl* /* layer */) { return true; }
> 
> slightly better to omit the parameter name completely instead of commenting it out

Oh, yes definitely. I thought I saw that in the style guide but now I can't find it, so yay.

[Dana Jansens]

Created attachment 132391 [details]
Patch

[Adrienne Walker]

Comment on attachment 132391 [details]
Patch

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

Looks good.  R=me.

> Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:91
> +static inline bool layerTransformsToScreenKnown(const CCLayerImpl* layer) { return true; }

nit: unused param.

> Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:98
> +static inline bool surfaceTransformsToScreenKnown(const CCRenderSurface* surface) { return true; }

nit: unused param.

[WebKit Review Bot]

Comment on attachment 132391 [details]
Patch

Attachment 132391 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11968453

[Dana Jansens]

Created attachment 132507 [details]
Patch

[Dana Jansens]

(In reply to comment #7)
> (From update of attachment 132391 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=132391&action=review
> 
> Looks good.  R=me.

Thanks for R!

> > Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:91
> > +static inline bool layerTransformsToScreenKnown(const CCLayerImpl* layer) { return true; }
> 
> nit: unused param.

done

> > Source/WebCore/platform/graphics/chromium/cc/CCOcclusionTracker.cpp:98
> > +static inline bool surfaceTransformsToScreenKnown(const CCRenderSurface* surface) { return true; }
> 
> nit: unused param.

done

[Dana Jansens]

Created attachment 132509 [details]
Patch

rebase fixes

[WebKit Review Bot]

Comment on attachment 132509 [details]
Patch

Clearing flags on attachment: 132509

Committed r111145: <http://trac.webkit.org/changeset/111145>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.