537623 - mask/clip-path/filter attribute on <use> element uses wrong coordinate space

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Marek Raida]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100103 Minefield/3.7a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100103 Minefield/3.7a1pre

Masking is supposed to work with the same effect regardless on which element type it is applied. Sadly, it is not true for use element.
Instead of mask keeping its fixed coordinates (userSpaceOnUse), it is applied relatively to the position of the use element.
SMIL animation is not to blame there, it is applicable also on static elements.

Reproducible: Always

Steps to Reproduce:
1. open page http://svg.kvalitne.cz/mix/mask.svg
2. watch how differently is the same mask applied to the third (green) element
Actual Results:  
mask on green element is applied relatively to its position, what is wrong

Expected Results:  
it should behave like on direct rect element, so like this http://svg.kvalitne.cz/mix/mask2.svg

Webkit and ASV renders it correctly Opera (Presto) suffers the same bug as well, so it probably won't be clear case or easy one to fix.

Comment 1

[Marek Raida]

Attachment: Wrong one (visually)

Comment 2

[Marek Raida]

Attachment: Good one (visually)

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to comment #1)
> Created an attachment (id=421511) [details]
> Wrong one (visually)

Looks like this attachment has a typo (extra space between the "/" and ">" at the end of a tag), making it trigger an XML error page instead of loading.

Comment 4

[Marek Raida]

Comment on attachment 421511 [details]
Wrong one (visually)

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">

  <defs>
      <mask id="mask_b1" maskUnits="userSpaceOnUse">  <!-- objectBoundingBox vs  userSpaceOnUse -->
          <circle cx="200" cy="200" r="200" fill="white" stroke="none"/>
      </mask>
      <rect id="abc" x="0" y="0" width="800" height="500" fill="green" stroke="none"/>
  </defs>

  <rect x="0" y="0" width="800" height="300" fill="magenta" stroke="none" mask="url(#mask_b1)">
    <animate attributeName="x" dur="4s" values="0; 400" begin="0s" repeatCount="indefinite"/>
  </rect>
  
  <image x="100" y="200" width="410" height="160" xlink:href="bluesquidj.png" mask="url(#mask_b1)">
    <animate attributeName="x" dur="4s" values="0; 400" begin="0s" repeatCount="indefinite"/>
  </image>
  
  <use xlink:href="#abc" x="0" y="300" mask="url(#mask_b1)">
    <animate attributeName="x" dur="4s" by="400" fill="freeze" repeatCount="indefinite"/>
  </use>

</svg>

Comment 5

[Marek Raida]

Comment on attachment 421511 [details]
Wrong one (visually)

This one is not working one, I unintentionally pressed TAB and corrupted XMLL before uploading it,sorry... Marking as obsolete and uploading new one...

Comment 6

[Marek Raida]

Attachment: original testcase

Comment 7

[Marek Raida]

Attachment: original reference case

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: testcase 2

Here's a reduced testcase, with no animation, to demonstrate more clearly what's going on.

The dotted border shows the outline of the <use> element, and the dashed border shows the outline of the mask *in the coordinate space of the <svg> element*.

When the mask is applied, though, it's behaving as though its origin is the upper-left corner of the <use> element -- and that's the bug here (if it is a bug -- perhaps <use> elements are supposed to establish their own coordinate space for situations like this? I'm not sure)

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: reference case 2a [UPDATE: not actually a reference]

Here's one reference case for that testcase, with "x" and "y" set on the target of <use> rather than on <use> itself.  This produces expected behavior. (The mask gets applied with its origin at upper-left corner of document)

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: reference case 2b [UPDATE: not actually a reference]

Here's an alternate reference case, which gets rid of the <use> entirely and instead directly uses a <rect>. This also produces expected behavior.

Comment 11

[Daniel Holbert [:dholbert]]

(In reply to comment #0)
> Webkit and ASV renders it correctly Opera (Presto) suffers the same bug

I can confirm that in Chromium browser (Webkit) on Linux, the testcases match their corresponding reference cases here -- whereas in Opera, the testcases & reference cases *don't* match. (Opera renders them the same way Firefox does.)

Version info:
* chromium-browser version 4.0.298.0 (36221) Ubuntu
* opera version 10.10 build 4742
* Firefox version: yesterday's mozilla-central nightly
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20100113 Minefield/3.7a1pre

Pretty sure this is a bug.  From looking at the description of <use> in SVG 1.1 section 5.6 [1], I don't get the impression that it's is supposed to establish a separate coordinate system, or anything like that.  In particular:
> The 'use' element has optional attributes x, y, width and height which are
> used to map the graphical contents of the referenced element onto a
> rectangular region within the current coordinate system.

So, confirming bug. (feel free to correct me if I'm misreading the spec)

[1] http://www.w3.org/TR/SVG/struct.html#UseElement

Comment 12

[Robert Longson [:longsonr]]

Attachment: patch?

Could you produce testcases for filters and clipPaths to check whether they work or not please?

Also need some testcases for objectBoundingBox with filters, clipPaths and masks with use to prove those work with/without the patch.

Comment 13

[Robert Longson [:longsonr]]

Attachment: patch?

Comment 14

[Robert Longson [:longsonr]]

I'm fairly sure the patch is wrong for objectBoundingBox and maybe for ordinary mask in use drawing too. Hence no review request.

Comment 15

[Marek Raida]

Attachment: Next testcases

Well, I tried to prepare testcase extended to clipping path and filter.
And results? I'm not sure, to be honest.
Mask behaves wrong, like always with boundingBox in place.
Clipping path behaves similarly wrong in case of userSpace on use and ignores? clipPathUnits="objectBoundingBox"
About filters - I'm not sure which one is best for the testcase, so I tried feGaussianBlur and looks ... well, I'm really not sure...

But totally depressing is cross-rendering comparison:
Gecko, Presto and Squiggle works with the same results (except of strokes)
ASV and Webkit in Safari and Webkit in Chrome differently and using InkScape produced, once more, even another rendering results... :-(

Comment 16

[Denilson Figueiredo de Sá]

Attachment: mask-rendering-bug.svg

In this testcase, we have a <mask> element with <rect> and <use> elements as children. The <use> elements are positioned incorrectly.

Then, if I group all children into a single <g> element, they are all positioned correctly.



I found this Firefox bug while reporting an Inkscape bug: https://bugs.launchpad.net/inkscape/+bug/1504122

Another broken example: https://openclipart.org/detail/229274/film-strip-film-stock

Comment 17

[Rogier Spieker]

Attachment: expected 100% positioning (works on all but Firefox)

Comment 18

[Rogier Spieker]

Attachment: unexpected 50% positioning (works on none but Firefox)

Comment 19

[Rogier Spieker]

(In reply to rogier from comment #17 and #18)
> Created attachment 8732101 [details]
> unexpected 50% positioning (works on none but Firefox)
> Created attachment 8732100 [details]
> expected 100% positioning (works on all but Firefox)

I ran into the same issue and the workaround I found may provide some insight to what is happening, the coordinate space for <use> as immediate child of <mask> appears to be twice the size it should be.

As a workaround both variants can be combined into the same SVG file, but I'd go with the <g> workaround provided by Denilson (comment #16) as this is much cleaner.

Comment 20

[u459114]

(In reply to Rogier Spieker from comment #19)
> (In reply to rogier from comment #17 and #18)
> > Created attachment 8732101 [details]
> > unexpected 50% positioning (works on none but Firefox)
> > Created attachment 8732100 [details]
> > expected 100% positioning (works on all but Firefox)
The 1st patch in bug 1325865 can mask Firefox works fine like other browsers

Comment 21

[u459114]

(In reply to Rogier Spieker from comment #19)
> (In reply to rogier from comment #17 and #18)
> > Created attachment 8732101 [details]
> > unexpected 50% positioning (works on none but Firefox)
> > Created attachment 8732100 [details]
> > expected 100% positioning (works on all but Firefox)
The 1st patch in bug 1325865 can make Firefox have the same rendering result just like other browsers

Comment 22

[u459114]

There are two type of bug here.
The first type is relative to coordinate space of <use>:
  original testcase
  testcase 2 
  Next testcases 
We do not have solution for this type yet. And I am going to figure it out.

The second type will be fixed by the patch in 1325865:
  mask-rendering-bug.svg 
  expected 100% positioning (works on all but Firefox)
  unexpected 50% positioning (works on none but Firefox)
It's relative to mask only transform computing.

Comment 23

[u459114]

Attachment: Bug 537623 - Part 1. Remove extra translation for <use> element in nsSVGUtils.

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

Comment 24

[u459114]

Attachment: Bug 537623 - Part 2. reftests for filter/clip-path/mask painting in an use element.

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

Comment 25

[u459114]

Comment on attachment 8827146 [details]
Bug 537623 - Part 1. Remove extra translation for <use> element in nsSVGUtils.

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

Comment 26

[u459114]

Comment on attachment 8827147 [details]
Bug 537623 - Part 2. reftests for filter/clip-path/mask painting in an use element.

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

Comment 27

[u459114]

Attachment: Bug 537623 - Part 3. Fix test cases of filter-userspace-offset.svg.

There are three test cases which are actually wrong, for example:
* offsetContainer=use&mask=userSpace-at100
  This test case is actually wrong one. All the content of <use> element should
  be masked out by the selected mask. We should use <mask id="userSpace-atZero">
  instead.

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

Comment 28

[u459114]

Comment on attachment 8827146 [details]
Bug 537623 - Part 1. Remove extra translation for <use> element in nsSVGUtils.

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

Comment 29

[u459114]

Comment on attachment 8827147 [details]
Bug 537623 - Part 2. reftests for filter/clip-path/mask painting in an use element.

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

Comment 30

[u459114]

Comment on attachment 8827146 [details]
Bug 537623 - Part 1. Remove extra translation for <use> element in nsSVGUtils.

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

Comment 31

[u459114]

Comment on attachment 8827147 [details]
Bug 537623 - Part 2. reftests for filter/clip-path/mask painting in an use element.

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

Comment 32

[u459114]

Comment on attachment 8827370 [details]
Bug 537623 - Part 3. Fix test cases of filter-userspace-offset.svg.

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

Comment 33

[Robert Longson [:longsonr]]

Comment on attachment 8827147 [details]
Bug 537623 - Part 2. reftests for filter/clip-path/mask painting in an use element.

https://reviewboard.mozilla.org/r/104924/#review106032

::: layout/reftests/svg/reftest.list:473
(Diff revision 4)
>  
>  fuzzy(71,817) == filter-on-continuation-box-01.html filter-on-continuation-box-ref.html
>  == mask-contains-inner-svg-01.svg pass.svg
>  == mask-contains-inner-svg-02.svg pass.svg
> +
> +== mask-use-element-01.svg pass.svg

can you raise a followup to but reftest.list back into alphabetical order. Most of it is but recent things are not.

Comment 34

[Robert Longson [:longsonr]]

Comment on attachment 8827146 [details]
Bug 537623 - Part 1. Remove extra translation for <use> element in nsSVGUtils.

https://reviewboard.mozilla.org/r/104922/#review106036

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e32814fc5ab6
Part 1. Remove extra translation for <use> element in nsSVGUtils.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f69bd7779bc9
Part 2. reftests for filter/clip-path/mask painting in an use element.
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcb3c9a6426e
Part 3. Fix test cases of filter-userspace-offset.svg.

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5db57e49db29
Part 1. Remove extra translation for <use> element in nsSVGUtils. r=longsonr+218550
https://hg.mozilla.org/integration/mozilla-inbound/rev/71db159b7fd9
Part 2. reftests for filter/clip-path/mask painting in an use element. r=longsonr+218550
https://hg.mozilla.org/integration/mozilla-inbound/rev/da2986c59fa3
Part 3. Fix test cases of filter-userspace-offset.svg. r=longsonr+218550

Comment 37

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3acd11541ea1
Backed out changeset fcb3c9a6426e for developer's request
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7647a1c41fe
Backed out changeset f69bd7779bc9 
https://hg.mozilla.org/integration/mozilla-inbound/rev/e85cf51eeaa7
Backed out changeset e32814fc5ab6

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5db57e49db29
https://hg.mozilla.org/mozilla-central/rev/71db159b7fd9
https://hg.mozilla.org/mozilla-central/rev/da2986c59fa3

Comment 39

[Robert Longson [:longsonr]]

The Marek and Daniel Holbert testcases don't seem to display as expected with a trunk build. They actually seem better to me without this bug's patches.

Comment 40

[u459114]

Attachment: screenshot

I verified the rendering result on mac/linux and windows again.

Browsing Marek(Next testcases) and dholbert's test case(testcase 2), the rendering result of FF is consistent with safari/chrome and edge after applying patches here.

Attach rendering result of each browser on windows.

Could you update screenshot so that I can see what happens on your build?

Comment 41

[Robert Longson [:longsonr]]

Ahh, so they are. What confused me is that the testcases are not consistent with the reference cases.

Comment 42

[Daniel Holbert [:dholbert]]

(In reply to Robert Longson from comment #41)
> Ahh, so they are. What confused me is that the testcases are not consistent
> with the reference cases.

Yeah, sorry about that.
* As noted in comment 2, I wasn't entirely sure at the time whether my expectations were valid or not.
* As noted in comment 11, Chrome/WebKit *used* to render the testcase & reference case as identical [though now they do not].
* We apparently didn't used to (7 years ago), and then changed at some point to render them the same, and now (as of this bug's patches landing) we're back to not-the-same again (which seems to be correct).
* The spec text that I quoted about "optional attributes x, y" in comment 11 was not very clear [which is why I was confused].  But there's later spec text that I think makes it clear how those attributes are supposed to take effect -- "An additional transformation translate(x,y) is appended [...]". That makes it clearer that the rect should be masked as if it were at the top left corner, and then afterwards it gets translated.

Comment 43

[Daniel Holbert [:dholbert]]

Attachment: reference case for testcase 2 (2017 version)