With the latest patch from bug 869611 (just landed on inbound) we're performing vastly better on bug 852588 (Santa's Workshop IE testdrive demo). The one thing than seems to be holding us back from equal performance with IE/Chrome is the continuous invalidation of the gently rotating (and translating down) snowflakes. I've tried to reduce the issue. Normally we don't invalidate transforming elements, and the active layer just composites them. Great. But if the transform involves a rotation, and if another element in the document invalidates, it seems to trigger an invalidation of the transforming element for some reason. This is the case even when the invalidating element is very far from the transforming element. In this testcase, turn on paint flashing, let the jiggling square stop invalidating, then mouse over the smaller square below it to apply a :hover rule to the smaller square that will change its fill color. This will invalidate the smaller square as expected, but there also seems to be a very strong correlation with the transforming square then suddenly starting to invalidate for no apparent reason.

The issue here is that the FrameLayerBuilder::ContainerParameters for the layer created for the nsDisplayTransform usually have mXScale and mYScale set to 4. For some reason though, when the small, untransformed rect changes color, the ContainerParameters for the transformed rect changes to 3 temporarily. That causes the FuzzyEqual comparison in ContainerState::CreateOrRecycleThebesLayer to return false, resulting in InvalidateEntireThebesLayer being called.

The relationship of the invalidation of the small untransformed rect to the invalidation of the larger transforming rect is purely that of changing the outcome of a race condition. The code that's going wrong is in ChooseScaleAndSetTransform here: http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=1d7c615b4b62&mark=2707-2708#2701 What's happening is that |oldScale == scale| evaluates to false as long as we're not dropping any frames, since each frame we toggle to the "other" transform matrix. However, when the small rect is invalidated, we frequently seem to skip a frame (or an odd number of frames) and end up with exactly the same transform, causing |oldScale == scale| to evaluate to true. As explained in the previous comment, that results in different mXScale and mYScale values, which in turn causes the entire layer to be invalidated. Note that for |oldScale == scale| to evaluate to true the old and new matrices do not need to be the same (as evidenced by the continuously invalidating snowflakes in the Santa's Workshop demo). This is a regression introduced by the patch for bug 706690.

Rather than try to detect panning by comparing the result of ScaleFactors() on the old and new matrices, I was thinking that this could be solved by more specifically checking whether all the matrix components except the translation components are the same. That would stop rotations falling afoul of the panning detection block. Unfortunately, it turns out that another very common cause of |oldScale == scale| evaluating to true is getting two consecutive paints between transform changes. The result is that my initial idea fails, since when that happens the non-translation components of the old and new matrices will obviously compare equal, and the code will take the "we're panning" path. One way to solve this would be to check if the matrices are the same, and in that case reuse the old mXScale/mYScale, but as far as I can tell those old scales (the actual clamped or not clamped values) are not saved or available in ChooseScaleAndSetTransform. Perhaps we could cache them on aLayer (add mXScale/mYScale members)? That should be easy and doable before the uplift next week. Another approach would perhaps be to have the frame pass on a "don't clamp me" flag that could be set by the fennec code in some way. That sounds like it might require a fair bit of digging to figure out what that should look like.

roc, any preference on how to solve this?

So you're saying that the snowflakes are just rotating, but that rotation causes the computed scale factors to change? That seems wrong.

Yeah, the above is a bit confused since I was thinking that normally the old and new scale factors should be different, and it's broken when they're the same. Actually, of course, they should always be the same (at least in the rotating snowflake case), and the problem is that they are sometimes different. The reason that the old and new scale factors are different for different angles of rotation is because there is a slight (unchanging) scale in addition to the rotation. That scale is enough to cause rounding issues in gfxMatrix::ScaleFactors that cause it to return different scale factors for different angles of rotation. For example, say that we have transform="scale(1.1) rotate(<angle>, <cx>, <cy>)". For different values of <angle>, gfxMatrix::ScaleFactors().width will be: 0 deg: 1.10000001493379 3 deg: 1.10000002829671 6 deg: 1.10000007650886 9 deg: 1.10000006170794 12 deg: 1.10000006885501 15 deg: 1.10000007862061 18 deg: 1.09999997885643 21 deg: 1.09999998058213 24 deg: 1.10000001458119 27 deg: 1.10000001696274 30 deg: 1.10000001937242 33 deg: 1.0999999874684 36 deg: 1.10000005289285 39 deg: 1.10000004386417 42 deg: 1.10000004017989 45 deg: 1.10000004386417 48 deg: 1.10000005289285 51 deg: 1.0999999874684 54 deg: 1.10000001937242 57 deg: 1.10000001696274 60 deg: 1.10000001458119 63 deg: 1.09999998058213 66 deg: 1.09999997885643 69 deg: 1.10000007862061 72 deg: 1.10000006885501 75 deg: 1.10000006170794 78 deg: 1.10000007650886 81 deg: 1.10000002829671 84 deg: 1.10000001493379 87 deg: 1.10000002384186 90 deg: 1.10000001493379 Now consider the case where script is updating the rotation on an element 30 times per second, but other invalidations in the document are causing us to paint at 60fps. We will enter ChooseScaleAndSetTransform 60 times per second. Every second paint we will find that the old and new transforms are the same, giving us the same scale factors, and causing us to take the "clamp" path. Every other paint we will find that the old and new transforms are different, ScaleFactors will return a different gfxSize due to rounding error (as shown by the table above), and we will _not_ take the "clamp" path. As a result the rotating element will invalidate 30 times per second. Of course this is a nice tidy example where the timing is neatly aligned to consistently reproduce the unwanted invalidation. In in-the-wild examples the timing and values of the changing transforms will usually result in very erratic and unpredictable unwanted invalidations with no apparent rhyme or reason.

Here's a testcase showing that 30fps invalidation of the transforming rect whenever the color of the other rect is animating.

(FWIW, for anyone looking, this doesn't currently make much difference to the Santa's Workshop demo specifically. That's only because bug 871172 means that the snowflakes and bodies of the elves don't actually render because the gradient references don't resolve. In a modified version of Santa's Workshop where the gradient references do resolve it makes a very significant difference.)

Comment on attachment 748439 [details] [diff] [review] patch to avoid bad rounding [Approval Request Comment] Bug caused by (feature/regressing bug #): unknown User impact if declined: bad perf on certain transform animations - b2g wants this Testing completed (on m-c, etc.): landed on m-i before uplift Risk to taking this patch (and alternatives if risky): low String or IDL/UUID changes made by this patch: none

Comment on attachment 748439 [details] [diff] [review] patch to avoid bad rounding If this is only useful for B2G, there's no point in taking on Firefox 22/23.

Comment on attachment 748439 [details] [diff] [review] patch to avoid bad rounding It's useful on all platforms. I only mentioned B2G because my manager told me that B2G people have been complaining a lot about SVG performance recently, and I heard that 1.1 _may_ end up being based on v23. At any rate, this is a significant win everywhere, and we only just branched.

Comment on attachment 748439 [details] [diff] [review] patch to avoid bad rounding OK - low risk fix in support of cross-platform svg perf. Approving for Aurora 23. We won't take on Beta 22 though without the regressing bug listed.

Reproduced the issue on Nightly 2013-05-09 using the testcases. Verified fixed FF 23b6 Win 7 x64.

Verified fixed FF 24b4 Win 7 x64.