1792313 - Poor SVG rendering with small viewBox

Status: RESOLVED FIXED

(Core :: SVG, defect, P3)

Description

[Michael Markl]

Attachment: test.svg

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:106.0) Gecko/20100101 Firefox/106.0

Steps to reproduce:

Open the attached test.svg file using Firefox and Chrome and Inkscape.

Actual results:

Chrome and Inkscape render the SVG as expected: The blue rectangle does not overlap with the opaque black border. On Firefox the blue rectangle overlaps with the border a lot.

Expected results:

Firefox should have rendered the SVG as expected (as in Chrome and Inkscape).

Comment 1

[Michael Markl]

Attachment: Firefox's result

Comment 2

[Michael Markl]

Attachment: Chrome's result

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 4

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=cddcdc46d3484be662cc56ceefe6b282481ca115&tochange=950227a483519fe71798772a0996c02dec2125f1

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

:nical, since you are the author of the regressor, bug 1686654, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 6

[Nicolas Silva [:nical]]

This looks like a precision issue. If I multiply the coordinates by 10 the error is much less pronounced and multiplied by 100 it is gone.
That would suggest that parts of the svg/blob code is working with nscoord where it should instead use floats.

Comment 7

[Nicolas Silva [:nical]]

I suspect that the precision loss from using nscoord happens in places like https://searchfox.org/mozilla-central/rev/b0e929cfaf3994cad2dd02afdac138083ee3fc84/layout/svg/SVGGeometryFrame.cpp#816

Comment 8

[Nicolas Silva [:nical]]

Attachment: cBug 1792313 - Avoid using nscoord for intermediate computation with SVG rectangles. r=jrmuizel,tnikkel

Under strong scales, going through the integer coordinate representation loses too much information. This patch tries to keep the same operations while sticking to floating point coordinates.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:TYLin, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e97d9856e880
cBug 1792313 - Avoid using nscoord for intermediate computation with SVG rectangles. r=tnikkel

Comment 11

[Nicolas Silva [:nical]]

Attachment: Bug 1792313 - Add fuziness to an intermittent reftest. r=#gfx-reviewers

Why the fuzziness is intermittent is beyond me, but the idea of the test is to stress preicsion issues with a scaling transform applied to an SVG rect so it is prone to result in small difference on the antialiased edges.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/efd4a567d0dc
Add fuziness to an intermittent reftest. r=gfx-reviewers,jnicol

Comment 13

[Nicolas Silva [:nical]]

Comment on attachment 9298834 [details]
Bug 1792313 - Add fuziness to an intermittent reftest. r=#gfx-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: None. It will only prevent a test from intermittently failing. No need to uplift if we don't mind the intermittent failure on beta.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• 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): It's only a test change so there is no impact to the user.
• String changes made/needed:
• Is Android affected?: Yes

Comment 14

[Robert Longson [:longsonr]]

Might have been better simply to have turned off antialiasing via shape-rendering="crispEdges" in the testcase and the reference.

Comment 15

[Norisz Fay [:noriszfay]]

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

Comment 16

[Donal Meehan [:dmeehan]]

Comment on attachment 9298834 [details]
Bug 1792313 - Add fuziness to an intermittent reftest. r=#gfx-reviewers

Approved for 107.0b3.

Comment 17

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6204e3c79832

Comment 18

[Donal Meehan [:dmeehan]]

For posterity, the push to autoland in Comment 10 landed in central during 107 nightly
Bugherder didn't change the 107 status to fixed
https://hg.mozilla.org/mozilla-central/rev/e97d9856e8806f3cd8fb6ec824b8340578192a31