Bug 75939 – [chromium] Make Skia canvas opaque for root layer tiles

• Component: New Bugs
• Status: RESOLVED FIXED

[Alexandre Elias]

[chromium] Make Skia canvas opaque for root layer tiles

[Alexandre Elias]

Created attachment 121798 [details]
Patch

[Adrienne Walker]

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.

[Dana Jansens]

+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).

[Alexandre Elias]

Created attachment 121968 [details]
Patch

[Alexandre Elias]

As per offline discussion, switched to using the layer's opaque flag instead, and recreate the Skia canvas if opacity changes.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Alexandre Elias]

Created attachment 121980 [details]
Patch

[James Robinson]

Comment on attachment 121980 [details]
Patch

R=me

[WebKit Review Bot]

Comment on attachment 121980 [details]
Patch

Clearing flags on attachment: 121980

Committed r104742: <http://trac.webkit.org/changeset/104742>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dana Jansens]

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.

[Alexandre Elias]

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.

[James Robinson]

What would it mean for an updater to be opaque? I don't think this is unclear.

[Dana Jansens]

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.

[James Robinson]

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.