Bug 136563 - [CSS Blending] The composited layers isolated by the page group should blend with the default white background color.
Summary: [CSS Blending] The composited layers isolated by the page group should blend ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 95614
  Show dependency treegraph
 
Reported: 2014-09-04 18:41 PDT by Simon Fraser (smfr)
Modified: 2014-09-23 00:47 PDT (History)
12 users (show)

See Also:


Attachments
Testcase (675 bytes, text/html)
2014-09-04 18:41 PDT, Simon Fraser (smfr)
no flags Details
Screenshot (old on left, new on right) (28.90 KB, image/png)
2014-09-04 18:42 PDT, Simon Fraser (smfr)
no flags Details
Patch (5.08 KB, patch)
2014-09-11 01:29 PDT, Ion Rosca
no flags Details | Formatted Diff | Diff
Patch (8.79 KB, patch)
2014-09-16 06:13 PDT, Ion Rosca
no flags Details | Formatted Diff | Diff
Another testcase (1.36 KB, text/html)
2014-09-19 12:06 PDT, Simon Fraser (smfr)
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2014-09-04 18:41:42 PDT
Created attachment 237667 [details]
Testcase

r173293 changes backing store allocation in such a way that some mix-blend-mode tests changed behavior. However, I'm not sure if the new or old behavior is correct.
Comment 1 Simon Fraser (smfr) 2014-09-04 18:42:04 PDT
Created attachment 237668 [details]
Screenshot (old on left, new on right)
Comment 2 Simon Fraser (smfr) 2014-09-04 18:44:18 PDT
With layer borders on you can see some weird behavior in the pre-r173293 case where we force a layer which is the ancestor of the mix-blend-mode layer to get backing store, but this might be a document-sized layer, so why doesn't blending just blend with the tiles?
Comment 3 Simon Fraser (smfr) 2014-09-04 18:51:12 PDT
I guess the stacking context ancestor of the blending element needs backing store?
Comment 4 Ion Rosca 2014-09-11 01:29:55 PDT
Created attachment 237942 [details]
Patch
Comment 5 Ion Rosca 2014-09-11 03:50:10 PDT
(In reply to comment #3)
> I guess the stacking context ancestor of the blending element needs backing store?

For regular elements the initial backdrop is rgba(0, 0, 0, 0), so the stacking context can be a layer without backing. However, the root element needs a backing store, because it should have an initial backdrop with white opaque content.
I uploaded a patch that should fix the problem.
Comment 6 Darin Adler 2014-09-14 12:40:06 PDT
Simon, I didn’t want to r+ without you weighing in. Can you review?
Comment 7 Simon Fraser (smfr) 2014-09-15 12:15:41 PDT
Does this work if the body background is not white?
Comment 8 Ion Rosca 2014-09-16 06:13:50 PDT
Created attachment 238178 [details]
Patch
Comment 9 Ion Rosca 2014-09-16 06:27:42 PDT
The root renderer will get its own backing store when body has a background-color, so blending should work in this case, even without this fix. I added a couple of tests proving that blending a composited child with body works in both cases: (1) when body has a background color and (2) when body doesn't have a background-color, but the child should blend with the default white.
Comment 10 Simon Fraser (smfr) 2014-09-19 12:06:52 PDT
Created attachment 238382 [details]
Another testcase

I still don't understand the expected behavior here. In this testcase, explicitly setting a background color on the parent element changes behavior. I don't see how that can be avoided given how we interact with Core Animation, but it seems contrary to spec'd behavior.
Comment 11 Ion Rosca 2014-09-22 08:46:56 PDT
Which spec are you referring to?
The right behavior is the old one from the second attachment: https://bugs.webkit.org/attachment.cgi?id=237668 - an element that is not isolated, should blend with the page's white background.

This white background is not always painted within the root layer, which is the actual stacking context for those non-isolated elements. If we force the root renderer to get a backing store, it will become responsible for painting the background for the entire canvas, including the base white color.
Comment 12 Simon Fraser (smfr) 2014-09-22 11:26:40 PDT
Are you saying that the test case shows correct behavior (with the patch applied)? Seems surprising, given that a non-compositing implementation would, I think, show different behavior.
Comment 13 Ion Rosca 2014-09-22 13:11:17 PDT
(In reply to comment #12)
> Are you saying that the test case shows correct behavior (with the patch applied)? Seems surprising, given that a non-compositing implementation would, I think, show different behavior.

I've just tried it with the software path and it gives me the same result. Why are you suspecting them to be different?
Comment 14 Simon Fraser (smfr) 2014-09-22 13:34:22 PDT
Do all three have the same appearance?
Comment 15 Ion Rosca 2014-09-22 15:19:41 PDT
I'm sorry, I thought you were referring to the original test case.

In the "Another testcase", you have three parent elements, each of them isolating its children.
1) Parent with transparent background (no backing store) - the backdrop for blending will be transparent.
2) Parent with backing store - the backdrop for blending is transparent, except for the area with text, which we don't overlap.
3) Parent with white background - blending occurs with the white background-color.

In this test case all blending boxes have regular isolating divs, therefore they cannot have access to the content outside their parent, including the base background white color.

This patch only fixes the case when the isolating layer is the root layer. This test case should not be affected by the patch.

Let's assume all the boxes were children of body, then the three shapes would have the same appearance: the first two should blend with the base background white, and the third with the white background-color.
Comment 16 Simon Fraser (smfr) 2014-09-22 17:50:25 PDT
Fair enough. Seems super confusing for authors.
Comment 17 WebKit Commit Bot 2014-09-23 00:47:32 PDT
Comment on attachment 238178 [details]
Patch

Clearing flags on attachment: 238178

Committed r173867: <http://trac.webkit.org/changeset/173867>
Comment 18 WebKit Commit Bot 2014-09-23 00:47:38 PDT
All reviewed patches have been landed.  Closing bug.