In SVG pattern with patternUnits="objectBoundingBox" and preserveAspectRatio="... meet" stretches to the size of a shape

RESOLVED FIXED in Firefox 40

Status

()

Core
SVG
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: Aleksey, Assigned: Robert Longson)

Tracking

29 Branch
mozilla40
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox40 fixed)

Details

(URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140506152807

Steps to reproduce:

1. Create an inline SVG element in HTML code.
2. Create <defs> and a <pattern> inside it, with 
 - id="pattern-1" - doesn't matter;
 - patternUnits="objectBoundingBox" - to use a host shape's coordinate system for sizing the pattern;
 - height="1" width="1" - to make pattern exactly fit a host shape;
 - preserveAspectRatio="... meet" - to make the pattern's shape fit a the host shape.
3. Add a shape inside the <pattern>
4. Give the pattern the appropriate viewBox - according to the size and position of the shape inside it.

5. Create a shape to use the <pattern> as a fill, give it some size and an attribute fill="url('#pattern-1')",
for example: <rect id="host" x="0" y="0" width="100" height="200" fill="url('#pattern-1')">


Actual results:

#pattern-1 stretches to the whole #host area.


Expected results:

#pattern-1 should have fit inside #host, just as background-size:contain; in CSS works. At least, in all other tested browsers (Chrome 29, Safari 5.1 Windows, Opera 12.15, Opera 16, IE 10) it worked the expected way.

The live example is here: http://codepen.io/bonflash/pen/iCfka The red anchor is not expected to stretch.
(Reporter)

Updated

3 years ago
Comment hidden (obsolete)
(Assignee)

Updated

3 years ago
Attachment #8466793 - Attachment mime type: text/plain → image/svg+xml
(Assignee)

Comment 2

3 years ago
Created attachment 8466794 [details]
corrected testcase
Attachment #8466793 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8466794 - Attachment mime type: text/plain → image/svg+xml
(Assignee)

Comment 3

3 years ago
IE11 and Opera 12 seem to display the testcase differently to each other.
Component: Untriaged → SVG
Product: Firefox → Core
(Assignee)

Comment 4

3 years ago
Would be nice if you could create a simpler testcase e.g. no transforms on the pattern content items. Only rect shapes with simple integer coordinates etc.
(Assignee)

Comment 5

3 years ago
Here's an example of a simple testcase: http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/objectBoundingBox-and-pattern-01a.svg?raw=1
Comment hidden (obsolete)
Comment hidden (obsolete)
Created attachment 8467244 [details]
testcase 2

I think this demonstrates the preserveAspectRatio issue (the stretching in the first chunk of the first testcase).

The <pattern> has preserveAspectRatio="xMidYMid meet", but we render this as if it had preserveAspectRatio="none". (i.e. we stretch the purple circle horizontally, whereas Blink & Presto & emacs do not)
(Reporter)

Comment 9

3 years ago
 Daniel, yes, that's right. According to https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/preserveAspectRatio :

meet (the default) - Scale the graphic such that:

    aspect ratio is **preserved**
    the entire viewBox is visible within the viewport
    the viewBox is scaled up as much as possible, while still meeting the other criteria

And thank you for a testcase!

IE 10, meanwhile, indeed does it in a wrong way.
I thought I'd add a comment that the problem with `preserveAspectRatio` and patterns is general, it applies to `slice` values as well as `meet`.

See for example, this fiddle: http://fiddle.jshell.net/xhh7a/1/
Which is explained in detail on this Stack Overflow post: http://stackoverflow.com/a/22886596/3128209

The third row should have undistorted images because of the viewBox and preserveAspectRatio values.

It works as intended in Chrome and IE.
(Assignee)

Updated

2 years ago
Assignee: nobody → longsonr
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 11

2 years ago
Created attachment 8588320 [details] [diff] [review]
patch with reftest
Attachment #8588320 - Flags: review?(dholbert)
(Assignee)

Comment 12

2 years ago
Created attachment 8588322 [details] [diff] [review]
just the one bug fix this time
Attachment #8588320 - Attachment is obsolete: true
Attachment #8588320 - Flags: review?(dholbert)
Attachment #8588322 - Flags: review?(dholbert)
Comment on attachment 8588322 [details] [diff] [review]
just the one bug fix this time

Review of attachment 8588322 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/svg/viewBox-and-pattern-04.svg
@@ +1,5 @@
>  <!--
>       Any copyright is dedicated to the Public Domain.
>       http://creativecommons.org/publicdomain/zero/1.0/
>  -->
> +<svg xmlns="http://www.w3.org/2000/svg">

Nit: right now you're creating this testcase as an 'hg cp' of viewBox-and-pattern-03.svg, but the file is almost entirely different, so I don't think there's any benefit to tracking it as a copy.

Might be better to just remove it & re-add it as a new file, like so:
 hg forget layout/reftests/svg/viewBox-and-pattern-04.svg
 hg qref
 hg add layout/reftests/svg/viewBox-and-pattern-04.svg
 hg qref

::: layout/svg/nsSVGPatternFrame.cpp
@@ +642,3 @@
>    SVGSVGElement *ctx = nullptr;
>    nsIContent* targetContent = aTarget->GetContent();
> +  double scaleX, scaleY;

s/double/gfxFloat/, I think.
Attachment #8588322 - Flags: review?(dholbert) → review+
(Assignee)

Comment 14

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e197f7a92f4
(Assignee)

Comment 15

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/731579510bef
Flags: in-testsuite+
OS: Windows 7 → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/731579510bef
Status: NEW → RESOLVED
Last Resolved: 3 years ago2 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.