Bug 75939 - [chromium] Make Skia canvas opaque for root layer tiles
Summary: [chromium] Make Skia canvas opaque for root layer tiles
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: Alexandre Elias
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-09 22:59 PST by Alexandre Elias
Modified: 2012-01-11 16:23 PST (History)
7 users (show)

See Also:


Attachments
Patch (7.42 KB, patch)
2012-01-09 23:06 PST, Alexandre Elias
no flags Details | Formatted Diff | Diff
Patch (8.20 KB, patch)
2012-01-10 20:38 PST, Alexandre Elias
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-02 (130.05 KB, application/zip)
2012-01-10 22:09 PST, WebKit Review Bot
no flags Details
Patch (8.23 KB, patch)
2012-01-10 22:57 PST, Alexandre Elias
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandre Elias 2012-01-09 22:59:52 PST
[chromium] Make Skia canvas opaque for root layer tiles
Comment 1 Alexandre Elias 2012-01-09 23:06:40 PST
Created attachment 121798 [details]
Patch
Comment 2 Adrienne Walker 2012-01-10 09:33:05 PST
Comment on attachment 121798 [details]
Patch

Don't we already have an opaque flag on layers that is true for the non composited content host? Can we just pass "opaque" through to the texture updater rather than "isNonCompositedContent"?

I was hoping to remove that flag on layers some day.
Comment 3 Dana Jansens 2012-01-10 09:54:59 PST
+1 to using the opaque flag as is. It should be set on the content layer in the NCCH constructor. (WebKit/chromium/src/NonCompositedContentHost.cpp).
Comment 4 Alexandre Elias 2012-01-10 20:38:57 PST
Created attachment 121968 [details]
Patch
Comment 5 Alexandre Elias 2012-01-10 20:40:30 PST
As per offline discussion, switched to using the layer's opaque flag instead, and recreate the Skia canvas if opacity changes.
Comment 6 WebKit Review Bot 2012-01-10 22:08:59 PST
Comment on attachment 121968 [details]
Patch

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

New failing tests:
compositing/absolute-position-changed-in-composited-layer.html
animations/combo-transform-rotate+scale.html
animations/3d/transform-origin-vs-functions.html
animations/animation-matrix-negative-scale-unmatrix.html
animations/3d/transform-perspective.html
animations/3d/matrix-transform-type-animation.html
animations/3d/state-at-end-event-transform.html
animations/animation-end-event-destroy-renderer.html
animations/animation-iteration-event-destroy-renderer.html
animations/animation-start-event-destroy-renderer.html
animations/big-rotation.html
animations/body-removal-crash.html
animations/combo-transform-translate+scale.html
animations/additive-transform-animations.html
animations/animation-hit-test-transform.html
animations/3d/replace-filling-transform.html
animations/3d/change-transform-in-end-event.html
animations/animation-on-inline-crash.html
animations/animation-direction-normal.html
compositing/absolute-position-changed-with-composited-parent-layer.html
Comment 7 WebKit Review Bot 2012-01-10 22:09:02 PST
Created attachment 121976 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 8 Alexandre Elias 2012-01-10 22:57:59 PST
Created attachment 121980 [details]
Patch
Comment 9 James Robinson 2012-01-11 12:31:41 PST
Comment on attachment 121980 [details]
Patch

R=me
Comment 10 WebKit Review Bot 2012-01-11 13:52:55 PST
Comment on attachment 121980 [details]
Patch

Clearing flags on attachment: 121980

Committed r104742: <http://trac.webkit.org/changeset/104742>
Comment 11 WebKit Review Bot 2012-01-11 13:53:03 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Dana Jansens 2012-01-11 14:45:17 PST
Hey aelias, would you be opposed to renaming the function on the LayerTextureUpdater to fooLayerOpaque() rather than fooOpaque()? Since it describes the layer and not the texture updater itself.
Comment 13 Alexandre Elias 2012-01-11 14:50:55 PST
Well, the patch has already been landed.  I think it can be clearer to keep consistent naming for a value getting passed down like this, in any case.
Comment 14 James Robinson 2012-01-11 14:52:25 PST
What would it mean for an updater to be opaque? I don't think this is unclear.
Comment 15 Dana Jansens 2012-01-11 14:55:21 PST
Ok. I'm sticking an opaque Rect in the TextureUpdater too, and thought maybe the term "opaque" is starting to show up in a lot of places with slightly different meaning but the same name.
Comment 16 James Robinson 2012-01-11 16:23:30 PST
The opaque bit on an updater means that every pixel that updater will ever see is opaque. opaqueRect means that all pixels within some subrect will be opaque, IIUC. It's possible that opaqueRect is overriden by setOpaque() or that setOpaque() just sets the opaqueRect, but I think they are talking about the same thing.