Webrender: Image masks don't create an intermediate surface to group the masked contents

[mstange]

https://bugzilla.mozilla.org/show_bug.cgi?id=1412375

Testcase

Expected results vs actual results:

It might be worth having a different API for grouping masks vs non-grouping clips. For example, a grouping mask could be a property of a stacking context instead of a property of a clip node.

[mstange]

For example, a grouping mask could be a property of a stacking context

It could even be a filter, just like opacity.

[glennw]

This is a timely bug report - as I'm working on optimizing the clip stack bugs / issues in https://github.com/servo/webrender/issues/1817 so I'll try to cover this at the same time, since they are very related.

[Gankra]

@glennw in #2443 you said

R1 failure [2] - I haven't confirmed what is happening in this one, yet. Looking visually, it appears that the image mask is being applied to both the primitive and the resulting stacking context picture. If that is the case, the fix here would be to update Gecko once this lands to supply the image-mask clip chain only to the primitive and/or the stacking context picture, rather than both. As a side-effect of this proposed change, #1957 gets fixed. It may also be related to #2388.

Is this still accurate? Should fixing this bug "just" be a matter of updating gecko now?

[Gankra]

Also we don't currently seem to emit a StackingContext in response to a mask; we just PushClip. Are you suggesting we should push an otherwise no-op stacking context and only apply the clip to that, and it will Just Work?

[Gankra]

Note to future travelers: tried implementing the above, and it doesn't work. Stacking contexts don't seem to respect image clips, even if you apply a hack to ensure they get an intermediate surface as discussed in #2551 (like an opacity(99) filter)

[Gankra]

Edit: made an error on "should work", see next comment.

yaml that "should" work (I think):

---
root:
  items:
    - type: rect
      bounds: [25, 25, 100, 100]
      color: red
    - type: clip
      bounds: [0, 0, 200, 200]
      id: 0
      image-mask:
          image: "transparent-white.png"
          rect: [0, 0, 200, 200]
          repeat: false
    - type: stacking-context
      bounds: [0, 0, 200, 200]
      clip-and-scroll: 0
      items:
      - type: rect
        bounds: [0, 0, 100, 100]
        color: blue
      - type: rect
        bounds: [50, 50, 100, 100]
        color: green

yaml we're emitting today, but produces an undesirable result (the overlap between the blue and green squares shouldn't have any blue):

---
root:
  items:
    - type: rect
      bounds: [25, 25, 100, 100]
      color: red
    - type: clip
      bounds: [0, 0, 200, 200]
      image-mask:
        image: "transparent-white.png"
        rect: [0, 0, 200, 200]
        repeat: false
      items:
      - type: rect
        bounds: [0, 0, 100, 100]
        color: blue
      - type: rect
        bounds: [50, 50, 100, 100]
        color: green

transparent-white.png:

[Gankra]

Oops made an error with my clipping.

Test That Should Work According To Glenn (opacity(0.999) included to get a Picture in the backend):

---
root:
  items:
    - type: rect
      bounds: [25, 25, 100, 100]
      color: red
    - type: clip
      bounds: [0, 0, 200, 200]
      id: 1
      image-mask:
          image: "transparent-white.png"
          rect: [0, 0, 200, 200]
          repeat: false
    - type: stacking-context
      bounds: [0, 0, 200, 200]
      clip-and-scroll: 1
      filters:
        - "opacity(0.999)"
      items:
      - type: rect
        bounds: [0, 0, 100, 100]
        color: blue
      - type: rect
        bounds: [50, 50, 100, 100]
        color: green

Expected:

---
root:
  items:
    - type: rect
      bounds: [25, 25, 100, 100]
      color: red

    - type: stacking-context
      bounds: [0, 0, 200, 200]
      filters:
        - "opacity(0.5)"
      items:
      - type: rect
        bounds: [0, 0, 100, 100]
        color: blue
      - type: rect
        bounds: [50, 50, 100, 100]
        color: green

This is actually different from what I'm seeing in gecko (where the mask seems to be ignored completely) but still wrong (no better than clipping the contents individually).

[Gankra]

(glenn notes on irc that the difference I'm seeing could be attributed to the fact that in wrench, children default to inheriting their parent's clips, so actually my test case should probably be clearing the clips on the elements)

edit: the result would be consistent with the stacking context ignoring the mask but the children applying it; in gecko I just saw the stacking context ignoring it.

[Gankra]

Whoops, i discussed this a bunch with @glennw a week ago but forgot to write it down. Glenn do you remember what needs to be done here? istr getting blocked on someone else's fixes..?

[glennw]

@mrobinson has landed initial support for specifying a clip-chain on stacking contexts in https://github.com/servo/webrender/pull/2600.

I think the next step is for people on Gecko to do some experiments with the API to see if it covers what is needed. It's not super clear to me what work is involved on Gecko to make use of this or if we need some extra work in WR (such as de-duplicating clip chains).

There's definitely some performance impact of the current implementation, but we can improve that once we work out the correctness / API issues.

Does that sound about right @mrobinson ?

[mrobinson]

@glennw @Gankro That does sound right to me. Currently, it's a little more expensive to apply the (optional) cilp to the stacking context, but it should be possible now.

[Gankra]

Yep, using that new feature seems to work perfectly!