Code review discussion copied here:
On 2017/01/05 18:03:57, Stephen Chennney wrote:
> On 2017/01/05 17:54:14, chrishtr wrote:
> > On 2017/01/05 at 17:11:01, schenney wrote:
> > > On 2017/01/05 02:16:43, trchen wrote:
> > > > On 2017/01/05 01:51:36, chrishtr wrote:
> > > > > On 2017/01/04 at 23:49:14, trchen wrote:
> > > > > > On 2017/01/04 23:04:26, chrishtr wrote:
> > > > > > > lgtm
> > > > > > 
> > > > > > FYI: This implementation won't handle clip escaping correctly. For
> > example,
> > > > > http://output.jsbin.com/poxiwon/
> > > > > > That said, it is super difficult, if not impossible, to implement in
> > SPv1. I
> > > > > feel this CL will fix more things than it will break, so still lgtm.
> > > > > 
> > > > > This is a good point. I don't think it's safe to start clipping those,
> > it'll
> > > > > regress real content. I think we should add a compositing reason for
> such
> > > > > children. CompositingReasonOutOfFlowClipping does this for children of
a
> > > > > composited scroller already. We can also promote descendants of a
> > > > border-radius
> > > > > non-stacking PaintLayer...
> > > > 
> > > > I don't see how it solves the problem though. In the example, both the
> > opacity
> > > > element and the animating element are composited already. (For opacity
> with
> > > > composited descendant, and animation, respectively.)
> > > > 
> > > > The problem is that layer->setMaskLayer() is a group mask (i.e. applies
to
> > the
> > > > subtree as a whole) instead of a local mask (i.e. applies to the current
> > layer
> > > > only). A solution is to pull the layer contents from m_graphicsLayer to
a
> > > > separate layer, and apply the mask to that layer. (m_backgroundLayer and
> > > > m_foregroundLayer sounds like the right place.)
> > > 
> > > What's really odd about this test case and the landed patch is that the
> > animated element is masked at the corners of the containing div, but not at
> the
> > straight sides. I don't understand that at all. I guess I still have more to
> > learn.
> > 
> > Why would it be masked at all? Those children should be siblings in the
> graphics
> > layer tree.
> 
> I'll look into it. I just don't see how a mask layer that masks out the
corners
> fails to also mask out the straight bits. The only explanation I can come up
> with is some nine-piece effect.

The case in comment 0 is because the descendant is a stacking child, due
to the opacity. Therefore it's a descendant in the graphics layer tree, and
as trchen mentioned this means that the masking for the ancestor affects it.

Another case to consider is a non-stacking child which has no compositing trigger:

<div style="width:300px; height:200px; border:1px dashed black; border-radius:50px; overflow:hidden;">
  <div style="position:absolute;  left: 250px; width:100px; height:100px; background:green; "></div>
</div>

I think the child div in the above example will be clipped by the mask.

#1: The nine-piece effect is due to GL texture clamping I believe. We have a render surface that's bigger than the mask texture, and the mask sample points outside of the texture extent just get get clamped to the texture edge.

IMO the right thing to do is to use GL_CLAMP_TO_BORDER for mask textures with transparent border color.

As I'm currently working on mask tiling, I came across this test case. With my patch, both rects are clipped by the mask. Is it supposed to be so?

Yes, both should be clipped because they are both children of the overflow-hidden border-radius div.

To be clear, absolute-positioned children do escape ancestor border-radius clip if the clipping ancestor is not in the containing block chain. Just that if there is an ancestor stacking context that needs to be clipped, it is very difficult to implement clip escaping in this case.

So no, only descendants that are positioned by the clipping element shall be clipped by the border-radius by CSS spec. But yes, from the perspective of cc, both layer needs to be masked by the border-radius because that's what blink told it to do.

Just to be clear on this, both the example in comments #0 and #3 should have the blue rect clipped and the green one unclipped, because the absolute position escapes the overflow clip.

Comment #2 is related to how we build the layer tree, not how it should be rendered, right? I can't find anything that talks about stacking contexts affecting the behavior of absolute elements with overflow. Or does the stacking context change the containing block, in which case the absolute should also move when opacity is added.

And the texture issue in comment #4 is unrelated but a bug? It shows up because the ancestor rectangular clip is not applied to the absolute positioned element but the mask is?

> Just to be clear on this, both the example in comments #0 and #3 should have the blue rect clipped and the green one unclipped, because the absolute position escapes the overflow clip.

Correct.

> Comment #2 is related to how we build the layer tree, not how it should be rendered, right? I can't find anything that talks about stacking contexts affecting the behavior of absolute elements with overflow. Or does the stacking context change the containing block, in which case the absolute should also move when opacity is added.

Creation of stacking context does not imply containing block for all descendants nor the opposite. Thus the two notorious CSS corner cases: "D paints into A but not positioned by A" and "D is positioned by A but does not paint into A". Most of --enable-prefer-compositing-to-lcd-text bugs are related to one of these corner cases.

There are some spec efforts to eliminate these corner cases, but I'm afraid we're too late. It will break too many websites if opacity implies containing block. Also all vendors verbally agreed filter should imply containing block. Mozilla took the train, but others just wave hands and watched. Poor Mozilla. :(

Another approach is to apply overflow clip at stacking context level. That will break even more websites.

> And the texture issue in comment #4 is unrelated but a bug? It shows up because the ancestor rectangular clip is not applied to the absolute positioned element but the mask is?

Yes, it is a GL renderer bug that caused mask layer effectively extends its edge pixel to infinite. sunxd@'s tiled mask mode will automatically fix that. I also have a WIP that will fix single texture masks.

@trchen, please verify that this bug is fixed by BlinkGenPropertyTrees.

 Issue 827870  has been merged into this issue.

Re #10: Yes --enable-blink-gen-property-trees fixed it.

I'm leaving the team thus re-assigning bugs.