Bug 81354 - [chromium] Don't occlude on main-thread behind layers/surfaces with impl-thread animations
Summary: [chromium] Don't occlude on main-thread behind layers/surfaces with impl-thre...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dana Jansens
URL:
Keywords:
Depends on: 81363
Blocks: 79536
  Show dependency treegraph
 
Reported: 2012-03-16 08:14 PDT by Dana Jansens
Modified: 2012-03-18 18:07 PDT (History)
10 users (show)

See Also:


Attachments
Patch (18.78 KB, patch)
2012-03-16 10:55 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (18.59 KB, patch)
2012-03-16 11:19 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (24.13 KB, patch)
2012-03-16 12:07 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (24.08 KB, patch)
2012-03-16 15:13 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (23.96 KB, patch)
2012-03-18 16:40 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Patch (23.95 KB, patch)
2012-03-18 16:55 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.