Joe Drew (not getting mail) Reporter
	Description • 13 years ago

roc changed the Direct2D backend to keep only a single snapshot, which is cached, and that makes fine sense to me. This patch changes the Cairo backend to do the same.

One question: SourceSurfaceCairo::GetDataSurface() currently just lets a data surface reference its context. I've thought about this a bit, and it seems correct, but I want to make sure that you think the same.

	Jeff Muizelaar [:jrmuizel]
	Comment 1 • 13 years ago

Comment on attachment 592593 [details] [diff] [review]
Only create a single snapshot in the Cairo backend

Review of attachment 592593 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/DrawTargetCairo.cpp
@@ +732,3 @@
>  {
> +  if (mSnapshot) {
> +    mSnapshot->DrawTargetWillChange();

Maybe add the special case for one reference

	Matt Woodrow (:mattwoodrow)
	Comment 2 • 13 years ago

We should probably do this for the skia backend too.

	Joe Drew (not getting mail) Reporter
	Comment 3 • 13 years ago

George Wright, I choose you!

	Joe Drew (not getting mail) Reporter
	Comment 4 • 12 years ago

Nick/Anthony, can you drive this to completion?

	Nick Cameron [:nrc] Assignee
	Comment 5 • 12 years ago

Joe: what needs doing? Any more than run it through Try, fix the bugs, and land it? (I guess we should address jrmuizel's comment too). Do you want one of us to look at the Skia version too, or can we leave that with gw280?

	Joe Drew (not getting mail) Reporter
	Comment 6 • 12 years ago

(In reply to Nick Cameron [:nrc] from comment #5)
> Joe: what needs doing? Any more than run it through Try, fix the bugs, and
> land it? (I guess we should address jrmuizel's comment too).

I think that's all that's necessary, yep.

> Do you want one
> of us to look at the Skia version too, or can we leave that with gw280?

gw280!

	Nick Cameron [:nrc] Assignee
	Comment 7 • 12 years ago

Rebased Joe's patch, carrying r=jrmuizel (still need to address his comment above)

	Nick Cameron [:nrc] Assignee
	Comment 8 • 12 years ago

Try push: https://tbpl.mozilla.org/?tree=Try&rev=79d25c2b1723

It seems to have broken all the image/encoders tests, which use a canvas for their implementation (WinXP, R). I'm ignoring the mac gradient fails, I think they are just because Azure/Cairo has never been tested on Mac before. I don't know why the image encoder tests would only fail on WinXP.

	Nick Cameron [:nrc] Assignee
	Comment 9 • 12 years ago

Addressed jrmuizel's comment and rebased again, carrying r=jrmuizel

	Nick Cameron [:nrc] Assignee
	Comment 10 • 12 years ago

The Win7 (sorry, not XP) test fails I noted above turn out to be there without this patch, so I'll fix them elsewhere. This patch is fine.

Try push: https://tbpl.mozilla.org/?tree=Try&rev=473353e9c37f

	Nick Cameron [:nrc] Assignee
	Comment 11 • 12 years ago

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=751b35e798d5

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/751b35e798d5