Markus Stange [:mstange] Reporter
	Description • 8 years ago

In this testcase, the same frame has both an inactive transform and a mix-blend-mode. With the patches in bug 1238564 applied, The display list dump looks like this:

BlendContainer p=0x119bb28f0 f=0x119d1f9c8(Viewport(-1)) [...] ref=0x119d1f9c8 agr=0x119d1f9c8
  [...]
  CanvasBackgroundColor p=0x119bb0250 f=0x119d201a8(Canvas(html)(-1)) [...]
  MixBlendMode p=0x119bb2340 f=0x11a045e98(Block(div)(1) id:inner) [...] ref=0x119d1f9c8 agr=0x11a045e98
    nsDisplayTransform p=0x119bb2170 f=0x11a045e98(Block(div)(1) id:inner) [...] ref=0x119d1f9c8 agr=0x119d201a8
      [...]

The AGR of the MixBlendMode item is the transformed/mixblendmode frame itself, but the AGR of the nsDisplayTransform is the CanvasFrame. This confuses the code that sets scroll clips on layers. Can we avoid it?

Currently (or at least with the patches from bug 1238564 applied), this results in us setting FrameMetrics for the same scroll frame on both the MixBlendMode ContainerLayer and its child PaintedLayer that has the inactive transform.

There are other cases in which the AGR of an inner item is outside the AGR of the outer item, but in those cases we make sure to set a scroll clip on the outer item whose scrollable frame is an ancestor of the outer AGR. What makes this particular case tricky is that both the MixBlendMode and the nsDisplayTransform have the same, inner scroll clip, and the AGR of the inner item is not inside that inner scroll clip.

Bug 1238564 will add this testcase in disabled form as
layout/reftests/async-scrolling/offscreen-clipped-blendmode-3.html

	Markus Stange [:mstange] Reporter
	Comment 2 • 8 years ago

Matt, do you know how to fix this?

	Matt Woodrow (:mattwoodrow)
	Comment 3 • 8 years ago

I think you need to extend this code to check for mix-blend-mode:

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp#5129


This code is getting quite awful. 

The default AGR for any transformed frame is itself (which is what we want for all content within the transform). For the transform item itself we want to use the next AGR up the stack.

The frame in question can be considered to be two AGRs, an inner one for the transform, and an outer one for the mix-blend-mode. Thus the next AGR for this example is still the frame itself.

The main problem is that our AGR tracking only records unique AGR entries, rather than one for each reason we need an AGR, so the next one is the AGR for our parent frame. We end up with the current code, manually checking for each reason a transformed frame might also need to be an AGR outside of the transform.

I'm not entirely sure if it's worth creating multiple AGR entries for a frame to fix this right now, just an idea for the future.

(In reply to Markus Stange [:mstange] from comment #0) 
> There are other cases in which the AGR of an inner item is outside the AGR
> of the outer item, but in those cases we make sure to set a scroll clip on
> the outer item whose scrollable frame is an ancestor of the outer AGR. 

What are these? That sounds broken to me, but I might have missed something.

	Markus Stange [:mstange] Reporter
	Comment 4 • 8 years ago

(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> I think you need to extend this code to check for mix-blend-mode:
> 
> http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.
> cpp#5129

So by changing that code we can make the AGR for the transform item be the transformed frame itself. But don't we also need to make the AGR of the mix blend mode item be the parent AGR?

> (In reply to Markus Stange [:mstange] from comment #0) 
> > There are other cases in which the AGR of an inner item is outside the AGR
> > of the outer item, but in those cases we make sure to set a scroll clip on
> > the outer item whose scrollable frame is an ancestor of the outer AGR. 
> 
> What are these? That sounds broken to me, but I might have missed something.

After bug 1238564: opacity, mix blend mode, and blend container, if they contain something with position:absolute/fixed if the containing block is outside the container item.

	Matt Woodrow (:mattwoodrow)
	Comment 5 • 8 years ago

(In reply to Markus Stange [:mstange] from comment #4)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> > I think you need to extend this code to check for mix-blend-mode:
> > 
> > http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.
> > cpp#5129
> 
> So by changing that code we can make the AGR for the transform item be the
> transformed frame itself. But don't we also need to make the AGR of the mix
> blend mode item be the parent AGR?

I'm not sure if we need to, but it might make most sense that way.

I think we need to overload the AGR in the MixBlendMode constructor then.