Closed Bug 537623 Opened 10 years ago Closed 3 years ago

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

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: marek.raida, Assigned: u459114)

References

()

Details

Attachments

(14 files, 4 obsolete files)

1005 bytes, image/svg+xml
Details
1.02 KB, image/svg+xml
Details
817 bytes, image/svg+xml
Details
817 bytes, image/svg+xml
Details
777 bytes, image/svg+xml
Details
2.16 KB, image/svg+xml
Details
1.45 KB, image/svg+xml
Details
1.72 KB, image/svg+xml
Details
1.82 KB, image/svg+xml
Details
59 bytes, text/x-review-board-request
longsonr
: review+
Details
59 bytes, text/x-review-board-request
longsonr
: review+
Details
59 bytes, text/x-review-board-request
longsonr
: review+
Details
554.49 KB, image/png
Details
811 bytes, image/svg+xml
Details
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.
Attached image Wrong one (visually) (obsolete) —
Attached image Good one (visually) (obsolete) —
(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 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 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...
Attachment #421511 - Attachment is obsolete: true
Attached image original testcase
Attachment #421512 - Attachment is obsolete: true
Attached image original reference case
Attached image 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)
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)
Here's an alternate reference case, which gets rid of the <use> entirely and instead directly uses a <rect>. This also produces expected behavior.
Attachment #421592 - Attachment description: Wrong one (visually) → original testcase
Attachment #421593 - Attachment description: Good one (visually) → original reference case
(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
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All
Version: unspecified → Trunk
Summary: Masking behaves differently on use element than on image, rect etc. → mask attribute on <use> element uses wrong coordinate space
Attached patch patch? (obsolete) — Splinter Review
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.
Attached patch patch? (obsolete) — Splinter Review
Attachment #421612 - Attachment is obsolete: true
Attached image 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... :-(
Attached image 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
(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.
(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
Assignee: nobody → cku
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.
Summary: mask attribute on <use> element uses wrong coordinate space → mask/clip-path/filter attribute on <use> element uses wrong coordinate space
Attachment #421613 - Attachment is obsolete: true
Attachment #8827146 - Flags: review?(longsonr)
Attachment #8827147 - Flags: review?(longsonr)
Attachment #8827370 - Flags: review?(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.
Attachment #8827147 - Flags: review?(longsonr) → review+
Attachment #8827370 - Flags: review?(longsonr) → review+
Comment on attachment 8827146 [details]
Bug 537623 - Part 1. Remove extra translation for <use> element in nsSVGUtils.

https://reviewboard.mozilla.org/r/104922/#review106036
Attachment #8827146 - Flags: review?(longsonr) → review+
Pushed by cku@mozilla.com:
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.
Pushed by cku@mozilla.com:
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
Backout by ihsiao@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/5db57e49db29
https://hg.mozilla.org/mozilla-central/rev/71db159b7fd9
https://hg.mozilla.org/mozilla-central/rev/da2986c59fa3
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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.
Flags: needinfo?(cku)
Attached image 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?
Flags: needinfo?(cku)
Ahh, so they are. What confused me is that the testcases are not consistent with the reference cases.
(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.
Attachment #421598 - Attachment description: reference case 2a → reference case 2a [UPDATE: not actually a reference]
Attachment #421599 - Attachment description: reference case 2b → reference case 2b [UPDATE: not actually a reference]
Depends on: 1361639
You need to log in before you can comment on or make changes to this bug.