We have a variety of svg opacity regressions which showed up on Oct 3. http://graphs.mozilla.org/graph.html#tests=%5B%5B225,131,31%5D,%5B225,131,33%5D,%5B225,131,35%5D%5D&sel=1412268851425,1412873651425&displayrange=7&datatype=running it appears the linux regressions are fixed (with push http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=75c93e9a7c97), so we are stuck with win8 remaining. I did some retriggers: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=mozilla-inbound%20talos%20svgr&startdate=2014-10-03&enddate=2014-10-04 here is the push that appears to cause the problem: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=01bbf35c82bd&tochange=7c26c6d5b2fb

This was caused by bug 934183, part 1: http://hg.mozilla.org/integration/mozilla-inbound/rev/2f7b5e239d1e The svg opacity tests are very rect heavy (in fact entirely based on rects) and gfxContext has optimizations for rect specifically. Previously we used to call gfxContext::Rectangle, which stores a Rect, and then call gfxContext::GetUserPathExtent, which just does a simple transformation of the stored Rect. Now we create a Moz2D Path object and do the calculations via that. I just landed a fix for bug 1074161 where I introduced a mechanism to avoid creating Moz2D Path objects for painting. We should leverage this new GetSimplePath() mechanism in nsSVGPathGeometryFrame::GetBBoxContribution to fix this regression.

Actually let's do this in a way that is more bug 1034958 friendly, since I want to fix that soon too.

Everything inside the |if (!gotSimpleBounds) {| block is just increasing the indentation (well, except for the use of the factored out |getFill| and |getStroke| variables.

In SVGRectElement::GetGeometryBounds the Inflate should be by aStrokeWidth/2. Fixed that locally.

Comment on attachment 8509501 [details] [diff] [review] patch I did catch the stroke width issue too but then so did you :-) > >+inline bool HasNonScalingStroke(nsIFrame* aFrame) { >+ return aFrame->StyleSVGReset()->mVectorEffect == >+ NS_STYLE_VECTOR_EFFECT_NON_SCALING_STROKE; >+} >+ Better in StyleSVGReset as a const member function. I guess I can live with it as is though. nsSVGPathGeometryFrame::GetBBoxContribution could really do with being split up into smaller pieces e.g. have separate getSimpleBounds and getComplexBounds functions, call the first and if it returns false call the second. That would make the logic easier to follow. This could be a follow up though.

Comment on attachment 8509501 [details] [diff] [review] patch https://hg.mozilla.org/integration/mozilla-inbound/rev/f51420708a03 I moved the HasNonScalingStroke() function to StyleSVGReset as you suggested. I'm looking at splitting and cleaning up GetBBoxContribution too, although I want to move much of this to content so that we can get bounds without a frame tree.

any chance this could land on Aurora?

We got some decent wins on the 'svg opacity' tests from this: http://graphs.mozilla.org/graph.html#tests=[[225,131,31],[225,131,25],[225,131,35],[225,131,37],[225,63,21],[225,63,24]]&sel=1414244738397.378,1414388462799.2917,60,513.3333333333334&displayrange=7&datatype=running Roughly: 7% on Win 8 10% on Win 7 4% on Win XP Not much change on the Mac or Linux runs.

(In reply to Joel Maher (:jmaher) from comment #8) > any chance this could land on Aurora? It wouldn't apply cleanly, but it could probably be safely landed with some hand merging. That said, while the wins are nice, the 2.4% regression on a few synthetic tests is probably not something to worry about as long as we know a fix is coming. I don't think we need to rush this to Aurora unless there's some B2G milestone or something I don't know about that would otherwise miss getting these improvements.

agreed, lets leave this alone.e the wins are nice, the 2.4% regression on a few synthetic tests is probably not something to worry about as long as we know a fix is coming. I don't think we need to rush this to Aurora unless there's some B2G milestone or something I don't know about that would otherwise miss getting these improvements.

Comment on attachment 8509501 [details] [diff] [review] patch Requesting approval so that we can land bug 1099197 on Aurora. This should be low risk.

Comment on attachment 8509501 [details] [diff] [review] patch approving for aurora because of the need in bug 1099197