Bug 70085 – [Chromium] Disable blending when drawing opaque layers

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

[Antoine Labour]

Disable blending when drawing opaque layers

[Antoine Labour]

Created attachment 110950 [details]
Patch

[Adrienne Walker]

It's unfortunate that there's no way to test this in WebKit, since nothing in WebKit sets the opaque flag yet (except for Safari on the noncomposited content).

[James Robinson]

Once https://bugs.webkit.org/show_bug.cgi?id=70554 lands you should be able to construct a layout test for this (or see if we have layout tests already), right? This patch looks pretty correct by inspection but I'd feel a lot better going forward if I knew we had test coverage for pixel correctness, in case a future factor accidentally messes this up.

[James Robinson]

Oh - what about setting the opaque flag for the root layer?  Right now we disable blending for this layer using the 'is non-composited content host' bit:

http://google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/platform/graphics/chromium/cc/CCTiledLayerImpl.cpp&q=disable%20BLEND%20file:chromium&exact_package=chromium&l=145

The code would be much better if this used the 'is opaque' bit instead, IMO.

[Dana Jansens]

Is checking this==rootLayer() sufficient for what you are asking?

In a unified compositor, would the root layer be the desktop?  Presumably there is/would be some other flag to tell a tab contents root window. Is this something I shouldn't worry about?

[James Robinson]

By "root" here I mean the non-composited content host layer. I don't know if there's an equivalent for users of the WebLayer API, but my point is you could clean up the webkit compositing logic by reusing this bit instead of what we currently do of special-casing this layer.

[Antoine Labour]

Created attachment 112579 [details]
Patch

[Antoine Labour]

This new patch added a unit test, please take a look (it also made MockWebGraphicsContext3D a bit more useful).
I'll try to see if I can also make a layout test.

[James Robinson]

Comment on attachment 112579 [details]
Patch

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

Cool tests! R=me

> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:204
> +    BlendStateCheckLayer(int id)

nit: explicit preferred on one-arg c'tors, although since this is private it's not a big deal

[James Robinson]

I think we won't be able to make useful layout tests until https://bugs.webkit.org/show_bug.cgi?id=70634 or https://bugs.webkit.org/show_bug.cgi?id=70564 are addressed

[Antoine Labour]

Created attachment 112580 [details]
Patch

[Antoine Labour]

Same one, with explicit.

[WebKit Review Bot]

Comment on attachment 112580 [details]
Patch

Clearing flags on attachment: 112580

Committed r98567: <http://trac.webkit.org/changeset/98567>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.