Bug 81354

Summary: [chromium] Don't occlude on main-thread behind layers/surfaces with impl-thread animations
Product: WebKit Reporter: Dana Jansens <danakj>
Component: New BugsAssignee: Dana Jansens <danakj>
Status: RESOLVED FIXED    
Severity: Normal CC: backer, cc-bugs, dglazkov, enne, jamesr, nduca, piman, reveman, vollick, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 81363    
Bug Blocks: 79536    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Dana Jansens 2012-03-16 08:14:50 PDT
[chromium] Don't occlude behind layers/surfaces with impl-animating opacity on main thread
Comment 1 Dana Jansens 2012-03-16 10:55:24 PDT
Created attachment 132318 [details]
Patch
Comment 2 Dana Jansens 2012-03-16 11:19:16 PDT
Created attachment 132323 [details]
Patch
Comment 3 Dana Jansens 2012-03-16 12:07:21 PDT
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.
Comment 4 James Robinson 2012-03-16 15:05:29 PDT
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
Comment 5 Dana Jansens 2012-03-16 15:10:28 PDT
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.
Comment 6 Dana Jansens 2012-03-16 15:13:52 PDT
Created attachment 132391 [details]
Patch
Comment 7 Adrienne Walker 2012-03-16 15:51:49 PDT
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.
Comment 8 WebKit Review Bot 2012-03-16 17:18:46 PDT
Comment on attachment 132391 [details]
Patch

Attachment 132391 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11968453
Comment 9 Dana Jansens 2012-03-18 16:40:57 PDT
Created attachment 132507 [details]
Patch
Comment 10 Dana Jansens 2012-03-18 16:42:15 PDT
(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
Comment 11 Dana Jansens 2012-03-18 16:55:12 PDT
Created attachment 132509 [details]
Patch

rebase fixes
Comment 12 WebKit Review Bot 2012-03-18 18:07:33 PDT
Comment on attachment 132509 [details]
Patch

Clearing flags on attachment: 132509

Committed r111145: <http://trac.webkit.org/changeset/111145>
Comment 13 WebKit Review Bot 2012-03-18 18:07:39 PDT
All reviewed patches have been landed.  Closing bug.