1092125 - Make nsSVGPathGeometryElement::GetGeometryBounds handle non-scaling-stroke

Status: RESOLVED FIXED

(Core :: SVG, defect)

Comment 1

[Jonathan Watt [:jwatt]]

In bug 1077355 I added some fuzz to rect3b (a non-scaling-stroke case) in test_bounds.html. Fixing this bug (for SVGRectElement at least) will make that unnecessary at which time the fuzz should be removed.

Comment 2

[Tom Klein]

Attachment: Part 1.diff

This adds non-scaling stroke support to GetGeometryBounds for all except line (and path).

Note that I added a comment to the old SVGRectElement::GetGeometryBounds to the effect that the bounds being returned may not be tight depending on stroke-linejoin and stroke-miterlimit.  The same issue exists in the new non-scaling-stroke case.  It should be possible to handle those cases without too much effort in future work if desired (would need to transform 8 corners instead of 4), but for now I think these bounds are still better than what nsSVGUtils::PathExtentsToMaxStrokeExtents would give.

Comment 3

[Tom Klein]

Attachment: Part 2.diff

This adds support to line.

A couple notes:
1) I think it should have been a PreMultiply in nsSVGUtils::PathExtentsToMaxStrokeExtents?

2) I had to rewrite the line stroke corners-computation code - the old code did its job, but the line "corners" it computed usually weren't the actual corners of the stroked line, they were just points that gave the same bounds as the actual corners.  Now that I'm transforming those "corners" into a different space before taking bounds, they need to be the real corners.  See the next comment for some code that tests those new equations.

Comment 4

[Tom Klein]

Attachment: lineTesting.html

This runs the new and old SVGLineElement::GetGeometryBounds corner-computation equations in javascript and displays the resulting corners (joined by non-lime lines with the start cap in orange and the end cap in purple) over the lime stroked line the corners are for.  (The zero-length line doesn't render on cairo (bug 1187770)).

Comment 5

[Tom Klein]

Attachment: nonScalingAndHitTesting.pdf

I don't know if this will be news to anyone else, but:

a) this new code still doesn't lead to getBoundingClientRect returning tight bounds for non-rectilinearly transformed shapes because of bug 958160.  (The tests that have been added in the patches above are exceptions.)

b) with the way things currently work, if I understand correctly, it's actually not possible to store a bounds in mRect that will transform correctly to give tight bounds in outer svg space.  The attached pdf illustrates why not.  I had thought/hoped one could artificially make mRect smaller so that when it was transformed to outer svg, the bounds of the image of mRect would be tight on the rendered shape.  But in order to do that in general you would have to use an mRect that doesn't contain the image of the rendered (in outer svg space, for the sake of discussion in this bug) shape transformed back to user space, and that would break SVG_HIT_TEST_CHECK_MRECT.  (There are probably other reasons why mRect can't behave that way, but breaking hit testing was what I ran into.)

(Note that display lists also use transforms of bounds (I think) instead of using bounds computed in the transformed-to space, so we're over invalidating there on shapes like these.)

Comment 6

[Tom Klein]

Note bug 958160, comment 9 which illustrates a case where the current line corner computing code does already lead to too small bounds (contrary to what I implied in comment 3).

Comment 7

[Robert Longson [:longsonr]]

b) true, see bug 958160 comment 6. You've lost information with each transform and you can't get it back.

Comment 8

[Robert Longson [:longsonr]]

Also our style is else on the same line as }

Comment 9

[Tom Klein]

Attachment: Part1v2.diff

Fixed style issues - thanks Robert.  No other changes from previous version.

Comment 10

[Tom Klein]

Attachment: Part2v2.diff

Fixed style issues, no other changes from previous version.

Comment 11

[Jonathan Watt [:jwatt]]

Comment on attachment 8640651 [details] [diff] [review]
Part1v2.diff

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

::: dom/svg/SVGContentUtils.cpp
@@ +467,5 @@
> +void
> +SVGContentUtils::RectilinearGetStrokeBounds(const Rect& aSourceSpaceBounds,
> +                                            const Matrix& aToBoundsSpace,
> +                                            const Matrix& aToNonScalingStrokeSpace,
> +                                            float aLineWidth,

For consistency with other names just call this aStrokeWidth.

@@ +483,5 @@
> +  // adjust aBounds to account for what a full stroke on the bounds in
> +  // non-scaling-stroke space looks like in aBounds space
> +  Float dx = 0.0f;
> +  Float dy = 0.0f;
> +  if (FuzzyEqual(nonScalingToBounds._12, 0)) { // diagonals are non-zero

I don't think your use of the terms "diagonal" and "off-diagonal" here are correct. I guess "diagonal" may be used as shorthand for the term "main diagonal", but "off diagonal" as I understand it refers to elements that are off the main diagonal, and you're only referring to two of those elements here. The term "anti diagonal" would be closer but since conceptually SVG matrices are 3x3 it would still refer to the wrong thing. ("-1 anti diagonal" might be correct I think, but is probably not going to be understood by many people reading this code.)

Maybe it would be helpful to put these comments inside the |if| block so that you can expand on them a bit. Maybe "_11 and _22 are the non-zero scaling components (0 or 180 degree rotation)" and "_21 and _12 are the non-zero scaling components (90 or 270 degree rotation)"?

Note also that you should probably check _21 too, since in principle _12, _11 and _12 could all be zero while _21 is not.

::: dom/svg/SVGContentUtils.h
@@ +184,5 @@
> +
> +  /**
> +   * Transforms the stroke bounds computed in
> +   * |aToNonScalingStrokeSpace| onto the |aSourceSpaceBounds| as transformed to
> +   * |aToBoundsSpace|, and returns them in |aBounds|.

This sentence seems very confusing to me. How about just renaming aSourceSpaceBounds to aRect and changing the comment to "Gets the tight bounds-space stroke bounds of the non-scaling-stroked rect aRect"?

::: dom/svg/SVGRectElement.cpp
@@ +145,5 @@
> +      if (aToNonScalingStrokeSpace->IsRectilinear()) {
> +        rect = aToNonScalingStrokeSpace->TransformBounds(rect);
> +        // XXX This may lead to too large bounds after the transform depending on
> +        // stroke-linejoin and stroke-miterlimit (think beveled corners and rotation
> +        // by 45)

stroke-linejoin and stroke-miterlimit don't affect rect. Also, since the transform is rectilinear, the angles between the adjacent edges remain 90 degrees after this transformation and therefore the Inflate() call will result in tight bounds. And we know there's no rotation at this point.

@@ +156,5 @@
> +      return false;
> +    }
> +    // XXX This may lead to too large bounds after the transform depending on
> +    // stroke-linejoin and stroke-miterlimit (think beveled corners and rotation
> +    // by 45)

For the same reasons, this comment is also wrong.

::: dom/svg/nsSVGPathGeometryElement.h
@@ +76,5 @@
>     * GetStrokedBounds on it.  It also helps us avoid rounding error for simple
>     * shapes and simple transforms where the Moz2D Path backends can fail to
>     * produce the clean integer bounds that content authors expect in some cases.
> +   *
> +   * If |aToNonScalingStrokeSpace| then |aBounds|, which is computed in

Please add in "is non-null" to make this easier to read.

@@ +77,5 @@
>     * shapes and simple transforms where the Moz2D Path backends can fail to
>     * produce the clean integer bounds that content authors expect in some cases.
> +   *
> +   * If |aToNonScalingStrokeSpace| then |aBounds|, which is computed in
> +   * target-of-|aToBoundsSpace| space, has the property that it's the smallest

On the same note I think replacing "target-of-|aToBoundsSpace|" with simply "bounds" would be better.

@@ +82,5 @@
> +   * (axis-aligned) rectangular bound containing the image of this shape as
> +   * stroked in non-scaling-stroke space.  (When all transforms involved are
> +   * rectilinear the bounds of the image of |aBounds| in non-scaling-stroke
> +   * space will be tight, but if there are non-rectilinear transforms involved
> +   * then that may be impossible).

After "impossible" tack on "and this method will return false".

::: dom/svg/nsSVGPolyElement.cpp
@@ +129,2 @@
>  {
> +  if (aStrokeOptions.mLineWidth > 0 || aToNonScalingStrokeSpace) {

I don't think this section should be moved up. Please put it back down below the |if (!points.Length())| check so that we will avoid ending up unnecessarily taking the complex bounds calculation paths in a case where we can return true here.

And please change the comment to "We don't handle non-scaling stroke or stroke-miterlimit etc. yet".

::: dom/svg/test/bounds-helper.svg
@@ +38,5 @@
>    <g id="g2">
>      <rect x="100" y="100" width="50" height="50" fill="pink"/>
>      <text x="200" y="200"/>
>    </g>
> +  <circle id="c1" cx="0" cy="0" r="10" transform="translate(45 130) scale(3 -2)"

I think it would be helpful to give these elements better IDs. You might as well do that in the case of your new elements. Perhaps "nonScalingStrokedCircle1" and "nonScalingStrokedEllipse1"?

::: layout/svg/nsSVGPathGeometryFrame.cpp
@@ +470,5 @@
>                     ((aFlags & nsSVGUtils::eBBoxIncludeStroke) &&
>                      nsSVGUtils::HasStroke(this));
>  
> +  SVGContentUtils::AutoStrokeOptions strokeOptions;
> +  strokeOptions.mLineWidth = 0.f;

Our use of |strokeOptions.mLineWidth = 0.f| is unnecessary given that the StrokeOptions ctor does that. Just change that line to an assert along the lines of:

  MOZ_ASSERT(strokeOptions.mLineWidth == 0.f, "This needs to be initialized "
             "for GetGeometryBounds in the !getStroke case");

@@ +482,2 @@
>    bool gotSimpleBounds = false;
> +  gfxMatrix gfxUserToOuterSVG;

Call this |userToOuterSVG| to avoid confusion about this somehow being a device space transform. (We use the 'gfx' prefix in names to denote this in other places.) Then rename your existing |userToOuterSVG| to |moz2dUserToOuterSVG|.

@@ +482,3 @@
>    bool gotSimpleBounds = false;
> +  gfxMatrix gfxUserToOuterSVG;
> +  if (nsSVGUtils::GetNonScalingStrokeTransform(this, &gfxUserToOuterSVG)) {

Make this:

  if (getStroke &&
      nsSVGUtils::GetNonScalingStrokeTransform(this, &gfxUserToOuterSVG)) {

Comment 12

[Jonathan Watt [:jwatt]]

Comment on attachment 8640652 [details] [diff] [review]
Part2v2.diff

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

::: dom/svg/SVGLineElement.cpp
@@ +135,5 @@
>  {
> +  float x1, y1, x2, y2;
> +  // transform from non-scaling-stroke space to the space in which we compute
> +  // bounds
> +  Matrix nonScalingToBounds;

I think this should live down after the |return true|.

@@ +173,5 @@
>  
> +  // Handle butt and square linecap, normal and non-scaling stroke cases
> +  // together: start with endpoints (x1, y1), (x2, y2) in the stroke space,
> +  // compute the four corners of the stroked line, transform the corners to
> +  // bounds space, and compute bounds there.

Put a blank line after this comment, since the comment doesn't just apply to the following block but rather multiple blocks.

Comment 13

[Tom Klein]

Attachment: rect bevel and miterlimit.html

(In reply to Jonathan Watt [:jwatt] from comment #11)

Thanks for the feedback!  A couple follow-ups:

> ::: dom/svg/SVGRectElement.cpp
> @@ +145,5 @@
> > +      if (aToNonScalingStrokeSpace->IsRectilinear()) {
> > +        rect = aToNonScalingStrokeSpace->TransformBounds(rect);
> > +        // XXX This may lead to too large bounds after the transform depending on
> > +        // stroke-linejoin and stroke-miterlimit (think beveled corners and rotation
> > +        // by 45)
> 
> stroke-linejoin and stroke-miterlimit don't affect rect. Also, since the
> transform is rectilinear, the angles between the adjacent edges remain 90
> degrees after this transformation and therefore the Inflate() call will
> result in tight bounds. And we know there's no rotation at this point.
> 

To be honest I tested whether stroke-linejoin and stroke-miterlimit affect rect by just seeing what firefox does, and firefox does apply both to rect (testcase attached), as do, now that I check, chrome and IE.  And the spec (11.4) seems to say both properties apply to all "shapes" and rect is a shape.  Is it more specific someplace else that I'm missing?

Also, aToNonScalingStrokeSpace is rectilinear, but aToBoundsSpace need not be, and that's part of the transform I'm referring to when I say "after the transform" - here are the lines after that XXX comment:

        rect.Inflate(aStrokeOptions.mLineWidth / 2.f);
        Matrix nonScalingToBounds =
          aToNonScalingStrokeSpace->Inverse() * aToBoundsSpace;
        *aBounds = nonScalingToBounds.TransformBounds(rect);
        return true;

nonScalingToBounds need not be rectilinear, so I think there is an issue here if bevel or miterlimit can cut the corners of the rectangle.  I should be more specific in my comment though about which transform I was referring to.  Or do you think we should just return false if bevel or miterlimit can and has cut off some of the corners?  (Of course it's possible to handle those cases precisely here, but that seems like work for another time.)

> @@ +156,5 @@
> > +      return false;
> > +    }
> > +    // XXX This may lead to too large bounds after the transform depending on
> > +    // stroke-linejoin and stroke-miterlimit (think beveled corners and rotation
> > +    // by 45)
> 
> For the same reasons, this comment is also wrong.

Similarly here, aToBoundsSpace need not be rectilinear.

> 
> ::: dom/svg/nsSVGPolyElement.cpp
> @@ +129,2 @@
> >  {
> > +  if (aStrokeOptions.mLineWidth > 0 || aToNonScalingStrokeSpace) {
> 
> I don't think this section should be moved up. Please put it back down below

Thanks for catching this!

> ::: layout/svg/nsSVGPathGeometryFrame.cpp
> @@ +470,5 @@
> >                     ((aFlags & nsSVGUtils::eBBoxIncludeStroke) &&
> >                      nsSVGUtils::HasStroke(this));
> >  
> > +  SVGContentUtils::AutoStrokeOptions strokeOptions;
> > +  strokeOptions.mLineWidth = 0.f;
> 
> Our use of |strokeOptions.mLineWidth = 0.f| is unnecessary given that the
> StrokeOptions ctor does that. Just change that line to an assert along the
> lines of:
> 
>   MOZ_ASSERT(strokeOptions.mLineWidth == 0.f, "This needs to be initialized "
>              "for GetGeometryBounds in the !getStroke case");

I think the StrokeOptions constructor actually defaults mLineWidth to 1:
 https://dxr.mozilla.org/mozilla-central/source/gfx/2d/2D.h#102
(and that's what I'm seeing in gdb).

Comment 14

[Jonathan Watt [:jwatt]]

(In reply to Tom Klein from comment #13)
> To be honest I tested whether stroke-linejoin and stroke-miterlimit affect
> rect by just seeing what firefox does, and firefox does apply both to rect
> (testcase attached), as do, now that I check, chrome and IE.  And the spec
> (11.4) seems to say both properties apply to all "shapes" and rect is a
> shape.  Is it more specific someplace else that I'm missing?

Ah I see. I somehow had in mind that you were referring to stroke-miterlimit kicking in under certain transforms, such as when under a skew or something. Can you change the comment to something like:

  // Note that, in principle, an author could cause the corners of the rect to
  // be beveled by specifying stroke-linejoin or setting stroke-miterlimit to
  // be less than sqrt(2). In that very unlikely event the bounds that we
  // calculate here will be too big if aToBoundsSpace is non-rectilinear. This
  // is likely to be so rare it's not worth handling though.

> Or do you think we should just return false if bevel or miterlimit can
> and has cut off some of the corners?  (Of course it's possible to handle
> those cases precisely here, but that seems like work for another time.)

I don't think it's worth worrying about or writing code for that case. It's too much of an edge case. The above comment would be fine I think.

> > @@ +156,5 @@
> > > +      return false;
> > > +    }
> > > +    // XXX This may lead to too large bounds after the transform depending on
> > > +    // stroke-linejoin and stroke-miterlimit (think beveled corners and rotation
> > > +    // by 45)
> > 
> > For the same reasons, this comment is also wrong.
> 
> Similarly here, aToBoundsSpace need not be rectilinear.

Just make this next comment 'The "beveled" comment above applies here too'.

> > Our use of |strokeOptions.mLineWidth = 0.f| is unnecessary given that the
> > StrokeOptions ctor does that. Just change that line to an assert along the
> > lines of:
> > 
> >   MOZ_ASSERT(strokeOptions.mLineWidth == 0.f, "This needs to be initialized "
> >              "for GetGeometryBounds in the !getStroke case");
> 
> I think the StrokeOptions constructor actually defaults mLineWidth to 1:
>  https://dxr.mozilla.org/mozilla-central/source/gfx/2d/2D.h#102
> (and that's what I'm seeing in gdb).

Good catch on the different value! Can you add a comment along the lines of "Override the default line width of 1.f - this is needed for GetGeometryBounds in the !getStroke case".

Comment 15

[Tom Klein]

Attachment: non-scaling-stroke rectangle miter limit example.html

(In reply to Jonathan Watt [:jwatt] from comment #14)
> Ah I see. I somehow had in mind that you were referring to stroke-miterlimit
> kicking in under certain transforms, such as when under a skew or something.
> Can you change the comment to something like:
> 
>   // Note that, in principle, an author could cause the corners of the rect
> to
>   // be beveled by specifying stroke-linejoin or setting stroke-miterlimit to
>   // be less than sqrt(2). In that very unlikely event the bounds that we
>   // calculate here will be too big if aToBoundsSpace is non-rectilinear.
> This
>   // is likely to be so rare it's not worth handling though.

Sounds good, I'll do that.

Note though that in the case of non-scaling-stroke on a rectangle, stroke-miterlimit _does_ kick in under certain transforms, since when we draw in that case we're first applying any transforms to the rectangle and then passing that transformed path to the drawTarget with whatever stroke options existed on the rectangle (so _all_ of the stroke options, not just stroke-width, are being applied to the _transformed_ path) - example attached.  So that's a bug?

Comment 16

[Tom Klein]

Attachment: Part1 v3.diff

(By the way, that was too many underscores in comment 15, sorry about that!)

Comment 17

[Tom Klein]

Attachment: Part2 v3.diff

Jonathan, note that I changed the id names of the line elements I added in bounds_helper.svg and test_bounds.html, as you suggested for the circle and ellipse in Part 1.
Assuming that's still an r+ from you on Part 2.

Comment 18

[Robert Longson [:longsonr]]

I think the current rendering in comment 15 is correct. It's the same as Opera 15 for instance.

Comment 19

[Jonathan Watt [:jwatt]]

(In reply to Tom Klein from comment #15)
> Note though that in the case of non-scaling-stroke on a rectangle,
> stroke-miterlimit _does_ kick in under certain transforms, since when we
> draw in that case we're first applying any transforms to the rectangle and
> then passing that transformed path to the drawTarget with whatever stroke
> options existed on the rectangle (so _all_ of the stroke options, not just
> stroke-width, are being applied to the _transformed_ path) - example
> attached.

Oh, that's a good point!

(In reply to Robert Longson from comment #18)
> I think the current rendering in comment 15 is correct. It's the same as
> Opera 15 for instance.

I think it's a bug, since my feeling would be that non-scaling stroke should only change the stroke width. :) But I also think it's an edge case of little importance and that our time is better spent on other things. Interesting case though!

Comment 20

[Jonathan Watt [:jwatt]]

Comment on attachment 8650140 [details] [diff] [review]
Part1 v3.diff

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

::: layout/svg/nsSVGPathGeometryFrame.cpp
@@ +473,5 @@
> +  SVGContentUtils::AutoStrokeOptions strokeOptions;
> +  // Override the default line width of 1.f - this is needed for
> +  // GetGeometryBounds in the !getStroke case
> +  strokeOptions.mLineWidth = 0.f;
> +  if (getStroke) {

Actually I think this would be better written as:

  } else {
    // Override the default line width of 1.f so that when we call
    // GetGeometryBounds below the result doesn't include stroke bounds.
    strokeOptions.mLineWidth = 0.f;
  }

Comment 21

[Tom Klein]

Attachment: Part1 v4.diff

Thanks Jonathan!

Comment 22

[Tom Klein]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b178ad698c7

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f22cefbef23
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a68226609c8

Comment 24

[Nigel Babu [:nigelb]]

Pretty sure this is the cause for the failures like this


805 INFO TEST-UNEXPECTED-FAIL | dom/svg/test/test_bounds.html | nonScalingStrokedLine2.getBoundingClientRect().width - got 28.400009155273438, expected 28.2842712474619 ± 0.1
806 INFO TEST-UNEXPECTED-FAIL | dom/svg/test/test_bounds.html | nonScalingStrokedLine2.getBoundingClientRect().height - got 28.400009155273438, expected 28.2842712474619 ± 0.1 

https://treeherder.mozilla.org/logviewer.html#?job_id=13598196&repo=mozilla-inbound

Going to back this out.

Comment 25

[Nigel Babu [:nigelb]]

Backed out in 
https://hg.mozilla.org/integration/mozilla-inbound/rev/63bf0b9aed0f
https://hg.mozilla.org/integration/mozilla-inbound/rev/192893adb2c3

Comment 26

[Tom Klein]

I increased the rounding allowance for nonScalingStrokedLine2 width and height from 0.1 to 0.15 - testing that now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=53d6f8cd1359

Comment 27

[Jonathan Watt [:jwatt]]

Tom, what is the ideal result? Did your changes actually make us return values that are closer to that value, in which case the test would be buggy. If not, do you know why we became less accurate?

Comment 28

[Tom Klein]

(In reply to Jonathan Watt [:jwatt] from comment #27)
> Tom, what is the ideal result? Did your changes actually make us return
> values that are closer to that value, in which case the test would be buggy.
> If not, do you know why we became less accurate?

We didn't become less accurate (nonScalingStrokedLine2 is one of the new test lines I added in this patch), we're just getting different rounding errors on different platforms.  For example, nonScalingStrokedLine2.width should be 28.2842712474619, and on my local linux build I get 28.366668701171875, which is within 0.1; also on all of the other non-win32 platforms on the inbound testing the result was within 0.1 (since those tests passed: see https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=1d96862c8a6a or my try run linked in comment 22), but on the win32 platforms (XP and Win7) we get more rounding error and the computed value is 28.400009155273438.

I'm confident the expected width and height values are correct, and I'm confident the new code is computing those values correctly, so I think it's just rounding issues.  What do you think?

Comment 29

[Tom Klein]

Did some more digging with 32-bit debug builds on Windows 8 and linux in the nonScalingStrokedLine2 case.

On windows we compute mRect = (7650, 199, 901, 202), on linux mRect = (7650, 199, 901, 201).  It looks like that one digit difference in the height is what's leading to the need for a 0.15 error bounds on 32-bit windows.

More details:
In nsSVGUtils::TransformFrameRectToOuterSVG,

on windows we get
  r.Scale(1.0 / nsPresContext::AppUnitsPerCSSPixel()) = (127.5, 3.31666..., 15.01666..., 3.3666...)
and
  aMatrix.TransformBounds(r) = (252.893..., 7.893..., 28.378..., 28.378...)

while on linux we get
  r.Scale(1.0 / nsPresContext::AppUnitsPerCSSPixel()) = (127.5, 3.31666..., 15.01666..., 3.35)
and
  aMatrix.TransformBounds(r) = (252.928..., 7.893..., 28.343..., 28.343...)

(Note that these transformed bounds values are all within 0.1 of the expected values, for both windows and linux.)
And then we call nsLayoutUtils::RoundGfxRectToAppRect with the above transformed rectangle and return the result of
ScaleRoundOut(60) on it, which
for windows gives
  scaledRect = (15173, 473, 1704, 1704)
and for linux gives
  scaledRect = (15175, 473, 1702, 1702)

Dividing by 60 gives a width of 28.4 in the windows case and 28.3666... in the linux case (the expected is 28.2842712474619).  So I still think it's down to rounding issues.

Note also that I already used 0.15 for the error bounds in the nonScalingStrokedCircle1 and nonScalingStrokedEllipse1 cases because I was getting similar rounding errors for those two when I was testing on linux.

Comment 30

[Tom Klein]

Attachment: Part2 v4.diff

The only change from the previous version is that I changed the tolerance on the nonScalingStrokedLine2 width and height from 0.1 to 0.15.
The try runs were green (comment 26 and pgo builds at https://treeherder.mozilla.org/#/jobs?repo=try&revision=41f2a7f74785).

Comment 31

[Jonathan Watt [:jwatt]]

(In reply to Tom Klein from comment #28)
> We didn't become less accurate (nonScalingStrokedLine2 is one of the new
> test lines I added in this patch), we're just getting different rounding
> errors on different platforms.  [snip]  What do you think?

Ah, sounds fine then. Thanks for the details in comment 29, that's nice to have in the record.

Comment 32

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f87f5ec61e1
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e35091f326b

Comment 33

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7f87f5ec61e1
https://hg.mozilla.org/mozilla-central/rev/8e35091f326b