I've actually figured out an easy way to fix the XXX comment that we added in bug 458031 and still keep suitable performance for HTML canvas use by avoiding these optimizations where they are not needed. It also doesn't break the current API. Whether it passes review is another story but at the moment this is the only suitable approach that has come to mind.

+ const gfxRect* aDirtyRect = nsnull); Can you not use a default parameter here? Just modify the other caller to pass nsnull. Make mDirtyRect be empty to indicate there's no dirty rect is very confusing. You should probably just have a separate flag to indicate if mDirtyRect should be used. Otherwise looks good to me, but you should get your next review from vlad.

Sure. The only reason I did the default parameter was because I thought there were more calls in canvas code than there were. This patch does not change canvas's behaviour at all, only box-shadow/text-shadow's. Tests pass.

Comment on attachment 351499 [details] [diff] [review] Patch 2 I don't like the variable name "requiredShadowArea" -- that should be "requiredBlurArea", no? Looks fine otherwise.

Yeah, that's right.

Comment on attachment 351658 [details] [diff] [review] Patch 3 [Checkin: Comment 5] http://hg.mozilla.org/mozilla-central/rev/c333209e9650

Can this be also checked into the 1.9.1 branch? It would be nice to start using box-shadow without this performance penalty.

Comment on attachment 351658 [details] [diff] [review] Patch 3 [Checkin: Comment 5] This patch has baked for a while on trunk with no problems. Sure, let's get this on 1.9.1 considering we'd like to encourage people to try out more CSS3.

Comment on attachment 351658 [details] [diff] [review] Patch 3 [Checkin: Comment 5] a191=beltzner

For the following lines in gfxBlur: if (rect.IsEmpty()) return nsnull; if (aDirtyRect) { ... rect = requiredBlurArea.Intersect(rect); Could it be that with a aDirtyRect specified, the rect can be empty after the Insect with aDirtyRect? If so, after that Intersect, one should/could also return nsnull, as there is nothing to draw?

That check happens at a higher level.

Comment on attachment 351658 [details] [diff] [review] Patch 3 [Checkin: Comment 5] { patching file gfx/thebes/src/gfxBlur.cpp Hunk #2 FAILED at 221 1 out of 2 hunks FAILED }

Pushed 3136ba367137 to 1.9.1