1275451 - mask-source(SVG mask) should also respect mask-composite

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[u459114]

According to css-masking spec:
Make clear that the used value of the properties mask-image, mask-repeat, mask-position, mask-clip, mask-origin and mask-size must be ignored for <mask-source>

So we still have take mask-composite into account.

[1] https://drafts.fxtf.org/css-masking-1/#the-mask-origin

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The relevant paragraph from the spec is:

"The used value of the properties mask-repeat, mask-position, mask-clip, mask-origin and mask-size must have no effect if <mask-reference> references a mask element. In this case the element defines position, sizing and clipping of the mask layer image."


Note that mask-image isn't in there.  Though it's incorrectly listed in the changes section as:

"Make clear that the used value of the properties mask-image, mask-repeat, mask-position, mask-clip, mask-origin and mask-size must be ignored for <mask-source>, not the properties."

Comment 2

[u459114]

Attachment: MozReview Request: Bug 1275451: Part 1. Respect mask-composite while composing a SVG mask layer

Review commit: https://reviewboard.mozilla.org/r/54998/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54998/

Comment 3

[u459114]

Attachment: MozReview Request: Bug 1275451: Part 2. SVG mask composition reftests

Review commit: https://reviewboard.mozilla.org/r/55000/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55000/

Comment 4

[u459114]

Comment on attachment 8756188 [details]
MozReview Request: Bug 1275451: Part 1. Respect mask-composite while composing a SVG mask layer

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54998/diff/1-2/

Comment 5

[u459114]

Comment on attachment 8756189 [details]
MozReview Request: Bug 1275451: Part 2. SVG mask composition reftests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55000/diff/1-2/

Comment 6

[Markus Stange [:mstange]]

Comment on attachment 8756188 [details]
MozReview Request: Bug 1275451: Part 1. Respect mask-composite while composing a SVG mask layer

https://reviewboard.mozilla.org/r/54998/#review51890

::: layout/svg/nsSVGIntegrationUtils.cpp:485
(Diff revision 2)
>    // Multiple SVG masks interleave with image mask. Paint each layer onto maskDT
>    // one at a time.
>    for (int i = svgMaskFrames.Length() - 1; i >= 0 ; i--) {
>      nsSVGMaskFrame *maskFrame = svgMaskFrames[i];
>  
> +    CompositionOp compositionOp = (i == svgMaskFrames.Length() - 1)

So this also changes behavior for the bottom layer if the bottom layer is an image mask, right? Can we add a reftest for that? Or is there already a reftest for it that was failing before this patch?

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 8756189 [details]
MozReview Request: Bug 1275451: Part 2. SVG mask composition reftests

https://reviewboard.mozilla.org/r/55000/#review51894

Comment 8

[u459114]

(In reply to Markus Stange [:mstange] from comment #6)
> So this also changes behavior for the bottom layer if the bottom layer is an
> image mask, right? Can we add a reftest for that? Or is there already a
> reftest for it that was failing before this patch?
Inside nsCSSRendering::PaintBackgroundWithSC[1] we use this logic already. 
Maybe I should remove that check in PaintBackgroundWithSC.

[1] https://dxr.mozilla.org/mozilla-central/source/layout/base/nsCSSRendering.cpp#3091

Comment 9

[Markus Stange [:mstange]]

Ah, I see. Yeah, I think it would be a good idea to remove that check. Can be done in a separate patch or bug.

Comment 10

[u459114]

Comment on attachment 8756188 [details]
MozReview Request: Bug 1275451: Part 1. Respect mask-composite while composing a SVG mask layer

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54998/diff/2-3/

Comment 11

[u459114]

Comment on attachment 8756189 [details]
MozReview Request: Bug 1275451: Part 2. SVG mask composition reftests

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55000/diff/2-3/

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/30d2b746df17
https://hg.mozilla.org/integration/mozilla-inbound/rev/581a44c822aa

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/30d2b746df17
https://hg.mozilla.org/mozilla-central/rev/581a44c822aa

Comment 14

[u459114]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e89e3437efc4d23aaa7483547a708303af84d015
Bug 1275451: (follow-up) Part 3. Correct content of assert. r=me

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e89e3437efc4