1903546 - Wrong non-scaling-stroke rendering in a nested svg

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[rayhan.azizi9]

Attachment: bug.png

Steps to reproduce:

Render this SVG

    <svg id="svg3" width="300" height="200" style="overflow: visible">
      <svg
        width="100%" height="100%" style="overflow: visible" preserveAspectRatio="xMidYMid meet"
        viewBox="-1.6111111640930176 -1.1111111640930176 3.222222328186035 2.222222328186035"
      >
        <svg overflow="visible" x="0" y="0">
          <polygon
            style=" fill: rgb(199, 199, 199); stroke: black; stroke-width: 1px;
              stroke-linejoin: round; vector-effect: non-scaling-stroke;"
            points="-1.5,1 1.5,1 1.5,-1 -1.5,-1"
          ></polygon>
        </svg>
      </svg>
    </svg>

or the full HTML

<!doctype html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
  </head>
  <body>
    <svg id="svg3" width="300" height="200" style="overflow: visible">
      <svg
        width="100%" height="100%" style="overflow: visible" preserveAspectRatio="xMidYMid meet"
        viewBox="-1.6111111640930176 -1.1111111640930176 3.222222328186035 2.222222328186035"
      >
        <svg overflow="visible" x="0" y="0">
          <polygon
            style=" fill: rgb(199, 199, 199); stroke: black; stroke-width: 1px;
              stroke-linejoin: round; vector-effect: non-scaling-stroke;"
            points="-1.5,1 1.5,1 1.5,-1 -1.5,-1"
          ></polygon>
        </svg>
      </svg>
    </svg>
  </body>
</html>

Actual results:

Starting from version 127 forward, the calculation of polygon stroke with vector-effect: non-scaling-stroke inside the nested SVG is incorrect. The scale is wrong.
I believe because the viewBox attribute is not properly inherited to the inner svg

attached (bug.png)

Expected results:

The scale should be correct. The calculation for the width of the border should follow the viewBox attribute in the parent's parent <svg> element

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Graphics: WebRender' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Robert Longson [:longsonr]]

Attachment: reporter's testcase

Comment 3

[Robert Longson [:longsonr]]

Please explain with reference to the SVG specification why you think Firefox is incorrect.

Per https://www.w3.org/TR/SVG2/painting.html#non-scaling-stroke

The stroke outline is calculated in the following manner: first, the shape's path is transformed into the host coordinate space. Stroke outline is calculated in the host coordinate space. The resulting outline is transformed back to the user coordinate system.

Note that an <svg> element establishes a new viewport per https://www.w3.org/TR/SVG2/coords.html#EstablishingANewSVGViewport

Comment 4

[Timothy Nikkel (:tnikkel)]

Seems like the report here is at the level of svg, not graphics, even if this is not a valid bug that would be a better place. Please revert if I'm mistaken.

Comment 5

[rayhan.azizi9]

Attachment: comparison.png

Top

1. Eye of GNOME 45.3
2. Brave Browser Version 1.67.119 Chromium: 126.0.6478.114 (Official Build) (64-bit)
3. Mozilla firefox-127.0.r20240606181944

Bottom
4. GIMP 2.10.38
5. Inkscape 1.2.2 (b0a8486541, 2022-12-01)
6. Canva (as of June 21st 2024)

Comment 6

[rayhan.azizi9]

Attachment: test.svg

Comment 7

[rayhan.azizi9]

I'm not sure if this is an actual bug or not, I can't say that I understand the SVG specification. I just noticed that other software renders it differently

I've attached a screenshot, comparing how other software renders the same SVG file (comparison.png)

1. Eye of GNOME 45.3
2. Brave Browser Version 1.67.119 Chromium: 126.0.6478.114 (Official Build) (64-bit)
3. Mozilla firefox-127.0.r20240606181944
4. GIMP 2.10.38
5. Inkscape 1.2.2 (b0a8486541, 2022-12-01)
6. Canva (as of June 21st 2024)

It might be the case that Firefox is the one that actually correctly follows the specs while the other software doesn't handle this case yet

Comment 8

[Robert Longson [:longsonr]]

I think we're right per the specification i.e.

https://www.w3.org/TR/SVG/coords.html#VectorEffects

vector-effect defaults to viewport (and we only support viewport anyway, I suspect other viewers do to i.e. nobody supports screen)

Per https://www.w3.org/TR/SVG/coords.html#EstablishingANewSVGViewport an <svg> element establishes a new viewport.

And then for viewport...

viewport
Specifies immediate viewport coordinate system as the host coordinate space. When that element belongs to nested viewport coordinate system, vector effects are applied toward viewport coordinate system to which that element belongs directly. That is, that vector effect is not effective for change of CTM on ancestral viewport coordinate system.

You would seem to be asking for the default to be screen rather than viewport but it isn't.

Comment 9

[Timothy Nikkel (:tnikkel)]

Should someone file bugs on other browsers and/or land a wpt test?

Comment 10

[Robert Longson [:longsonr]]

I guess so, yes.

Comment 12

[Robert Longson [:longsonr]]

Attachment: testcase with screen

Comment 13

[Robert Longson [:longsonr]]

bug 1904891 will implement the screen option. It's likely this is what you want i.e.

vector-effect: non-scaling-stroke screen;

Comment 14

[Robert Longson [:longsonr]]

Attachment: Bug 1903546 - Support viewport and screen options for non-scaling-stroke r=emilio

Comment 15

[Phabricator Automation]

Comment on attachment 9410002 [details]
Bug 1903546 - Support viewport and screen options for non-scaling-stroke r=emilio

Revision D215138 was moved to bug 1904891. Setting attachment 9410002 [details] to obsolete.

Comment 16

[Robert Longson [:longsonr]]

bug 1904891 will implement vector-effect: non-scaling-stroke screen; and that's probably what you're looking for here.

Comment 17

[Robert Longson [:longsonr]]

It seems that https://github.com/w3c/svgwg/issues/582 supersedes the spec here and so we'll need to revert the viewport/screen support in favour of that interpretation.

Comment 18

[Robert Longson [:longsonr]]

Attachment: Bug 1903546 - fix non-scaling-stroke r=emilio

This implements https://github.com/w3c/svgwg/issues/582 and therefore mostly reverts bug 1904891

Comment 19

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/jobs?repo=try&revision=e5f5ab252cbbd447ebcf7c1b842f6f724b378b08

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a11e8418e383
fix non-scaling-stroke r=emilio

Comment 21

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/47097 for changes under testing/web-platform/tests

Comment 22

[tszentpeteri]

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

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 24

[Robert Longson [:longsonr]]

Comment on attachment 9412279 [details]
Bug 1903546 - fix non-scaling-stroke r=emilio

Beta/Release Uplift Approval Request

• User impact if declined: non-scaling-stroke will be incorrect for nested <svg> elements.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This reverts most of the changes in bug 1904891 which is currently in beta. That bug implements part of the SVG specification that has been superseded and is not compatible with Chrome or Safari. This bug restores that compatibility.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 25

[Donal Meehan [:dmeehan]]

Comment on attachment 9412279 [details]
Bug 1903546 - fix non-scaling-stroke r=emilio

Approved for 129.0b4

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/556215a082cc