We should convert SVG geometry bounds calculations from gfxContext to Moz2D.

This doesn't block bug 933019 or bug 987190 anymore since I unblocked them by fixing bug 1034399.

(Path caching is a future patch/bug.)

s/poist list/points list/

Comment on attachment 8494508 [details] [diff] [review] patch Review of attachment 8494508 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/svg/content/src/SVGLineElement.cpp @@ +127,5 @@ > GetAnimatedLengthValues(&x1, &y1, &x2, &y2, nullptr); > > pathBuilder->MoveTo(Point(x1, y1)); > pathBuilder->LineTo(Point(x2, y2)); > + pathBuilder->Close(); This doesn't seem right.. closing this would mean the line gets stroked once in each direction. Was this needed for some reason?

(In reply to Bas Schouten (:bas.schouten) from comment #6) > This doesn't seem right.. closing this would mean the line gets stroked once > in each direction. Was this needed for some reason? Yeah, sorry, that was me doing experiments for zero length paths and forgetting to remove that.

Comment on attachment 8494556 [details] [diff] [review] Add some guards to protect against the different behavior of Moz2D backends for empty Path objects I suppose this is better for an SVG peer, since Bas doesn't know this code so well.

What happens if you have a line with zero length or an arc with 0 radius or other degenerate cases? Are all backends OK with those? Can we get tests for such things?

Comment on attachment 8494556 [details] [diff] [review] Add some guards to protect against the different behavior of Moz2D backends for empty Path objects Cancelling review on the guard part while I think some things through some more.

Comment on attachment 8497255 [details] [diff] [review] convert the bounds calculation code for SVG geometry to use Moz2D Path >- bbox.UnionEdges(nsSVGUtils::PathExtentsToMaxStrokeExtents(pathExtents, >- this, >- ThebesMatrix(aToBBoxUserspace))); > } > Is this nsSVGUtils::PathExtentsToMaxStrokeExtents signature dead code after this patch?

Comment on attachment 8497255 [details] [diff] [review] convert the bounds calculation code for SVG geometry to use Moz2D Path >- bbox.UnionEdges(nsSVGUtils::PathExtentsToMaxStrokeExtents(pathExtents, >- this, >- ThebesMatrix(aToBBoxUserspace))); r=longsonr if you check whether nsSVGUtils::PathExtentsToMaxStrokeExtents with this signature is now dead and remove that signature if it is.

Comment on attachment 8494556 [details] [diff] [review] Add some guards to protect against the different behavior of Moz2D backends for empty Path objects I ended up fixing the issues this patch was intended to address over in bug 1075399.

(In reply to Robert Longson from comment #13) > check whether nsSVGUtils::PathExtentsToMaxStrokeExtents > with this signature is now dead and remove that signature if it is. It is, and I'll remove it.

This code is also dead.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f7b5e239d1e https://hg.mozilla.org/integration/mozilla-inbound/rev/7c26c6d5b2fb I ended up leaving the code using PathExtentsToMaxStrokeExtents for now with the Path::GetStrokedBounds() code ifdef'ed out. The comment in the code explains why: // This disabled code is how we would calculate the stroke bounds using // Moz2D Path::GetStrokedBounds(). Unfortunately at the time of writing it // there are two problems that prevent us from using it. // // First, it seems that some of the Moz2D backends are really dumb. Not // only do some GetStrokeOptions() implementations sometimes significantly // overestimate the stroke bounds, but if an argument is passed for the // aTransform parameter then they just return bounds-of-transformed-bounds. // These two things combined can lead the bounds to be unacceptably // oversized, leading to massive over-invalidation. // // Second, the way we account for non-scaling-stroke by transforming the // path using the transform to the outer-<svg> element is not compatible // with the way that nsSVGPathGeometryFrame::Reflow() inserts a scale into // aToBBoxUserspace and then scales the bounds that we return. We're at least at the point where we aren't using the gfxContext any more. I'm going to do some Try pushes shortly to see if we can now get rid of the scale nsSVGPathGeometryFrame::Reflow inserts into aToBBoxUserspace. I'm also investigating the GetStrokeOptions() issues, and changing PathExtentsToMaxStrokeExtents so that it doesn't use mStrokeMiterlimit for shapes that can't be affected by stroke-miterlimit so that we get better bounds for them.

https://hg.mozilla.org/mozilla-central/rev/2f7b5e239d1e https://hg.mozilla.org/mozilla-central/rev/7c26c6d5b2fb