SVG patterns aren't repeated without this.

Comment on attachment 823335 [details] [diff] [review] patch to add IsSingular and operator*= methods to Matrix Review of attachment 823335 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Matrix.h @@ +123,5 @@ > } > > + Matrix& operator*=(const Matrix &aMatrix) > + { > + Matrix resultMatrix = *this * aMatrix; See IRC convo.

pattern->SetExtend(gfxPattern::EXTEND_REPEAT); is now called twice.

(In reply to Robert Longson from comment #5) > is now called twice. Yeah, I forgot to remove that from when I was trying to work out what was going on with patterns not repeating any more. BTW, feel free to steal these reviews if you're looking at the patches anyway.

Comment on attachment 823376 [details] [diff] [review] patch to switch away from gfxASurface > if (NS_FAILED(GetTargetGeometry(&callerBBox, > viewBox, > patternContentUnits, patternUnits, > aSource, >- aContextMatrix, >+ ThebesMatrix(aContextMatrix), > aOverrideBounds))) Have a temporary for ThebesMatrix(aContextMatrix) as it's used below too. > > // Construct the CTM that we will provide to our children when we > // render them into the tile. > gfxMatrix ctm = ConstructCTM(viewBox, patternContentUnits, patternUnits, >- callerBBox, aContextMatrix, aSource); >+ callerBBox, ThebesMatrix(aContextMatrix), >+ aSource); i.e. here. > > // Get the bounding box of the pattern. This will be used to determine > // the size of the surface, and will also be used to define the bounding > // box for the pattern tile. >- gfxRect bbox = GetPatternRect(patternUnits, callerBBox, aContextMatrix, aSource); >+ gfxRect bbox = GetPatternRect(patternUnits, callerBBox, >+ ThebesMatrix(aContextMatrix), aSource); and here. > // revert the vector effect transform so that the pattern appears unchanged > if (aFillOrStroke == &nsStyleSVG::mStroke) { >- patternTransform.Multiply(nsSVGUtils::GetStrokeTransform(aSource).Invert()); >+ patternTransform *= ToMatrix(nsSVGUtils::GetStrokeTransform(aSource).Invert()); Did you want to add the If !Identity code here that you added last time you semi-converted non-scaling-stroke? >- nsRefPtr<gfxPattern> pattern = new gfxPattern(surface); >+ nsRefPtr<gfxPattern> pattern = new gfxPattern(surface, Matrix()); Presumably you should pass ThebesMatrix(pMatrix) here and omit the SetMatrix call below. > { >+ typedef mozilla::gfx::Matrix Matrix; >+ typedef mozilla::gfx::SourceSurface SourceSurface; >+ I thought we generally don't do this in headers but write the stuff out completely there and then put these in the cpp.

Comment on attachment 823369 [details] [diff] [review] patch to move MaxExpansion() from nsSVGUtils to nsSVGPatternFrame, and convert it to Moz2D >diff --git a/layout/svg/nsSVGPatternFrame.cpp b/layout/svg/nsSVGPatternFrame.cpp > //---------------------------------------------------------------------- >-// Helper classes >+// Helpers [...] >+/* Calculate the maximum expansion of a matrix */ >+static float >+MaxExpansion(const Matrix &aMatrix) >+{ This cpp file has a pre-existing "Helper functions" area (where e.g. GetPatternMatrix lives). This new function belongs there, not up here in the "Helper classes" section. >diff --git a/layout/svg/nsSVGPatternFrame.cpp b/layout/svg/nsSVGPatternFrame.cpp >@@ -167,17 +187,17 @@ GetPatternMatrix(uint16_t aPatternUnits, >- float scale = 1.0f / nsSVGUtils::MaxExpansion(callerCTM); >+ float scale = 1.0f / MaxExpansion(ToMatrix(callerCTM)); [...] >@@ -194,17 +214,17 @@ GetTargetGeometry(gfxRect *aBBox, >- float scale = nsSVGUtils::MaxExpansion(aContextMatrix); >+ float scale = MaxExpansion(ToMatrix(aContextMatrix)); [etc] All four of the MaxExpansion(ToMatrix(...)) clients in your patch take a gfxMatrix argument, and call ToMatrix() on it. As it happens, I believe they're each only called once, by the same function -- PaintPattern -- which could be responsible for doing a single ToMatrix() call, rather than doing it four times in each of these helper-functions. Please do that (and change these function-signatures) to avoid needless duplicate matrix-conversion. r=me with that.

Landed parts 1-3: https://hg.mozilla.org/integration/mozilla-inbound/rev/194ba1b8f191 https://hg.mozilla.org/integration/mozilla-inbound/rev/6123376c20f1 https://hg.mozilla.org/integration/mozilla-inbound/rev/05d9c6a32d8b Part 4 is going to need to wait until filters use a Moz2D backed temporary draw target, since without that these two test fail: layout/reftests/svg/filters/filter-patterned-rect-01.svg layout/reftests/svg/filters/filter-patterned-rect-02.svg

I span part 4 out into bug 932198.

https://hg.mozilla.org/mozilla-central/rev/194ba1b8f191 https://hg.mozilla.org/mozilla-central/rev/6123376c20f1 https://hg.mozilla.org/mozilla-central/rev/05d9c6a32d8b