1369904 - SVG getBBox & objectBoundingBox units are incorrect on Mac/Android

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Amelia Bellamy-Royds]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36

Steps to reproduce:

1. Create an SVG path where the control points of Bezier curves extend outside the bounding box (the tight rectangle that encloses all points on the path itself).
2. Apply a gradient or pattern that is scaled to use `objectBoundingBox` units to this path
3. View it in a version of Firefox that uses Skia as the underlying graphics library (Mac or Android)

or

2. Query `getBBox()` on the path element on one of the problematic operating systems.

or

Run test case: https://codepen.io/AmeliaBR/pen/dRyPJE/?editors=1111


Actual results:

The gradient is scaled too large, matching the too-large getBBox rectangle that includes the control points.


Expected results:

The bounding box should be scaled to the fill-region bounding box; it should not be affected by control points.

In the linked demo, this means that all the gradients should have the same scale and same bounding box printed to the console (x=0, y=0, width=100, height=100).

___________________________

Firefox on Windows and Linux has the correct results. IE/Edge are also correct.
Current versions of Blink and WebKit return correct results for getBBox, but use the too-large box for objectBoundingBox scaling.

Per Chromium but https://bugs.chromium.org/p/chromium/issues/detail?id=230599:

> The current implementation relies on SkPath::getBounds(), which
computes a conservative result (includes control points).

> Use SkPathOps::TightBounds() instead, for an exact answer.

I assume the same problem and solution apply for Firefox.

Comment 1

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=96759cd50dc613ace7049307e6bf0d7bd5b74e50

Comment 2

[Robert Longson [:longsonr]]

Attachment: patch with test

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8874178 [details] [diff] [review]
patch with test

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

I'll let Lee take this one.

Comment 4

[Lee Salzman [:lsalzman]]

Comment on attachment 8874178 [details] [diff] [review]
patch with test

It would be simpler to just call mPath.computeTightBounds() instead of mPath.getBounds(). This way we don't have to separately call TightBounds and don't need to import the SkPathOps.h header either.

Comment 5

[Robert Longson [:longsonr]]

Attachment: update per review comments

that's simpler, thanks for the suggestion Lee.

Comment 6

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea1a841159155dd96f4fc434f6d8b958a6913375&selectedJob=106772493

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e62702024b0
Bounding box should not include control points. r=lsalzman

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3e62702024b0