Closed Bug 1275451 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: u459114, Assigned: u459114)

References

Details

Attachments

(2 files)

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
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 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/
Attachment #8756188 - Flags: review?(mstange)
Attachment #8756189 - Flags: review?(mstange)
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 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?
Attachment #8756188 - Flags: review?(mstange) → review+
Comment on attachment 8756189 [details]
MozReview Request: Bug 1275451: Part 2. SVG mask composition reftests

https://reviewboard.mozilla.org/r/55000/#review51894
Attachment #8756189 - Flags: review?(mstange) → review+
(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
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 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/
Attachment #8756189 - Attachment description: MozReview Request: Bug 1275451: Part 2. SVG mask composition reftest → MozReview Request: Bug 1275451: Part 2. SVG mask composition reftests
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/
https://hg.mozilla.org/mozilla-central/rev/30d2b746df17
https://hg.mozilla.org/mozilla-central/rev/581a44c822aa
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.