[CSS Blending] Add compositing reason for isolation in RenderLayerCompositor.

• Component: CSS
• Status: RESOLVED FIXED

[Ion Rosca]

There are 2 reasons for a layer to be composited involving blend modes:
1) When a layer has blend mode and has composited descendants, it should be composited as well in order to apply blending to the whole group at once. CompositingReasonBlendingWithCompositedDescendants is the existing reason for this case.
2) When a layer has to isolate composited blending descendants (it is the enclosing stacking context for those blending descendants), that layer should be composited as well. This will restrict blending layers to use the backdrop information only from the current stacking context. This blending reason should be added.

I will add in a follow up patch the inspector side of this issue.

[Ion Rosca]

Created attachment 228473 [details]
adding compositing reason for isolation

[Ion Rosca]

Created attachment 228475 [details]
nit

[Dirk Schulze]

Is there no way to test it? Does DRT expose these information?

[Mihai Tica]

(In reply to comment #3)
> Is there no way to test it? Does DRT expose these information?

You should be able to dump the layer hierarchy into a text file.
https://bugs.webkit.org/attachment.cgi?id=161391&action=prettypatch is an example

[Ion Rosca]

(In reply to comment #3)
> Is there no way to test it? Does DRT expose these information?

No, DRT does not expose this information. In WebCore reasons are used only for logging. This is the patch where other reasons were added: https://bugs.webkit.org/show_bug.cgi?id=110505

I'm going to add another patch with the implementation in WebInspector and I will use the WebInspector protocol to test this flag (like in this bug: https://bugs.webkit.org/show_bug.cgi?id=111703)

[Dirk Schulze]

(In reply to comment #5)
> (In reply to comment #3)
> > Is there no way to test it? Does DRT expose these information?
> 
> No, DRT does not expose this information. In WebCore reasons are used only for logging. This is the patch where other reasons were added: https://bugs.webkit.org/show_bug.cgi?id=110505
> 
> I'm going to add another patch with the implementation in WebInspector and I will use the WebInspector protocol to test this flag (like in this bug: https://bugs.webkit.org/show_bug.cgi?id=111703)

How much bigger is the inspector patch? Maybe you can combine them or the reviewer  can review them together.

[Ion Rosca]

It's not big. I was thinking that there might be different people to review them. I can combine them.

[Ion Rosca]

Created attachment 228532 [details]
adding inspector code & test

[Dean Jackson]

Comment on attachment 228532 [details]
adding inspector code & test

This looks good to me, but I'd like to see an Inspector person review it too.

[Joseph Pecoraro]

Comment on attachment 228532 [details]
adding inspector code & test

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

r=me

> Source/WebInspectorUI/UserInterface/Views/LayerTreeSidebarPanel.js:436
> +        if (compositingReasons.isolatesCompositedBlendingDescendants)
> +            addReason(WebInspector.UIString("Element isolates composited descendants having CSS blending applied"));

Very interesting! The patch looks good, As a user facing feature, I'd want to get a little more information on what isolation is so that maybe we can improve the UI String. Right now it sounds fine.

Is "-webkit-isolation" with children having blending the only way to get this state? Is just -webkit-isolation enough?

Note some of the other reasons include the CSS property, so it might be useful here to include "-webkit-isolation".

> LayoutTests/inspector-protocol/layers/layers-blending-compositing-reasons.html:114
> +            if (messageObject.hasOwnProperty("error")) {

Nit: This can be shorted to:

    InspectorTest.checkForError(messageObject);

Which will print a fail message, complete the test, and short circuit by throwing an exception.

[Ion Rosca]

Comment on attachment 228532 [details]
adding inspector code & test

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

>> Source/WebInspectorUI/UserInterface/Views/LayerTreeSidebarPanel.js:436
>> +            addReason(WebInspector.UIString("Element isolates composited descendants having CSS blending applied"));
> 
> Very interesting! The patch looks good, As a user facing feature, I'd want to get a little more information on what isolation is so that maybe we can improve the UI String. Right now it sounds fine.
> 
> Is "-webkit-isolation" with children having blending the only way to get this state? Is just -webkit-isolation enough?
> 
> Note some of the other reasons include the CSS property, so it might be useful here to include "-webkit-isolation".

> I'd want to get a little more information on what isolation
We say that a layer isolates, or create an isolated group as defined here: http://dev.w3.org/fxtf/compositing-1/#isolatedgroups, if descendant layers are composited over a fully transparent black backdrop, so that the layers with CSS blending will be able to access only the elements from inside the isolated group.

> Is "-webkit-isolation" with children having blending the only way to get this state? Is just -webkit-isolation enough?
A layer isolates if it is a stacking context and has blending descendants: http://dev.w3.org/fxtf/compositing-1/#csscompositingrules_CSS . -webkit-isolation just turns the element into a stacking context: http://dev.w3.org/fxtf/compositing-1/#isolation, so a positioned layer with z-index would have the same effect.

I think we should not say "-webkit-isolation", maybe "Element is a stacking context and has composited descendants with CSS blending applied"?.

[Joseph Pecoraro]

(In reply to comment #11)
> (From update of attachment 228532 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228532&action=review
> 
> >> Source/WebInspectorUI/UserInterface/Views/LayerTreeSidebarPanel.js:436
> >> +            addReason(WebInspector.UIString("Element isolates composited descendants having CSS blending applied"));
> > 
> > Very interesting! The patch looks good, As a user facing feature, I'd want to get a little more information on what isolation is so that maybe we can improve the UI String. Right now it sounds fine.
> > 
> > Is "-webkit-isolation" with children having blending the only way to get this state? Is just -webkit-isolation enough?
> > 
> > Note some of the other reasons include the CSS property, so it might be useful here to include "-webkit-isolation".
> 
> > I'd want to get a little more information on what isolation
> We say that a layer isolates, or create an isolated group as defined here: http://dev.w3.org/fxtf/compositing-1/#isolatedgroups, if descendant layers are composited over a fully transparent black backdrop, so that the layers with CSS blending will be able to access only the elements from inside the isolated group.
> 
> > Is "-webkit-isolation" with children having blending the only way to get this state? Is just -webkit-isolation enough?
> A layer isolates if it is a stacking context and has blending descendants: http://dev.w3.org/fxtf/compositing-1/#csscompositingrules_CSS . -webkit-isolation just turns the element into a stacking context: http://dev.w3.org/fxtf/compositing-1/#isolation, so a positioned layer with z-index would have the same effect.

Excellent, thanks for the links.

> I think we should not say "-webkit-isolation", maybe "Element is a stacking context and has composited descendants with CSS blending applied"?.

I'm fine with both "Element isolates" or "Element is a stacking context". The 2nd will be more familiar to web developers, and if that really is all isolation is in this context, than it might be the more user friendly string. It seems that "isolates" is a more technical term. For example, I had no idea what it meant =).

Thanks

[Ion Rosca]

Created attachment 228589 [details]
fixing user facing string & test error check

[Joseph Pecoraro]

Comment on attachment 228589 [details]
fixing user facing string & test error check

r=me

[WebKit Commit Bot]

Comment on attachment 228589 [details]
fixing user facing string & test error check

Clearing flags on attachment: 228589

Committed r166800: <http://trac.webkit.org/changeset/166800>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.