diff --git a/gfx/layers/d3d9/CanvasLayerD3D9.cpp b/gfx/layers/d3d9/CanvasLayerD3D9.cpp index 522b59c3cdf7..b410abec5da2 100644 --- a/gfx/layers/d3d9/CanvasLayerD3D9.cpp +++ b/gfx/layers/d3d9/CanvasLayerD3D9.cpp @@ -32,6 +32,7 @@ CanvasLayerD3D9::Initialize(const Data& aData) if (aData.mDrawTarget) { mDrawTarget = aData.mDrawTarget; + mSurface = gfxPlatform::GetPlatform()->GetThebesSurfaceForDrawTarget(mDrawTarget); mNeedsYFlip = false; mDataIsPremultiplied = true; } else if (aData.mSurface) { @@ -139,18 +140,11 @@ CanvasLayerD3D9::UpdateSurface() D3DLOCKED_RECT lockedRect = textureLock.GetLockRect(); nsRefPtr sourceSurface; - nsRefPtr tempSurface; - if (mDrawTarget) { - tempSurface = gfxPlatform::GetPlatform()->GetThebesSurfaceForDrawTarget(mDrawTarget); - } - else { - tempSurface = mSurface; - } - if (tempSurface->GetType() == gfxASurface::SurfaceTypeWin32) { - sourceSurface = tempSurface->GetAsImageSurface(); - } else if (tempSurface->GetType() == gfxASurface::SurfaceTypeImage) { - sourceSurface = static_cast(tempSurface.get()); + if (mSurface->GetType() == gfxASurface::SurfaceTypeWin32) { + sourceSurface = mSurface->GetAsImageSurface(); + } else if (mSurface->GetType() == gfxASurface::SurfaceTypeImage) { + sourceSurface = static_cast(mSurface.get()); if (sourceSurface->Format() != gfxASurface::ImageFormatARGB32 && sourceSurface->Format() != gfxASurface::ImageFormatRGB24) { @@ -161,7 +155,7 @@ CanvasLayerD3D9::UpdateSurface() gfxASurface::ImageFormatARGB32); nsRefPtr ctx = new gfxContext(sourceSurface); ctx->SetOperator(gfxContext::OPERATOR_SOURCE); - ctx->SetSource(tempSurface); + ctx->SetSource(mSurface); ctx->Paint(); } diff --git a/gfx/layers/opengl/CanvasLayerOGL.cpp b/gfx/layers/opengl/CanvasLayerOGL.cpp index a6b3150b4b32..14a3f87d18b4 100644 --- a/gfx/layers/opengl/CanvasLayerOGL.cpp +++ b/gfx/layers/opengl/CanvasLayerOGL.cpp @@ -75,6 +75,7 @@ CanvasLayerOGL::Initialize(const Data& aData) if (aData.mDrawTarget) { mDrawTarget = aData.mDrawTarget; + mCanvasSurface = gfxPlatform::GetPlatform()->GetThebesSurfaceForDrawTarget(mDrawTarget); mNeedsYFlip = false; } else if (aData.mSurface) { mCanvasSurface = aData.mSurface; @@ -162,11 +163,7 @@ CanvasLayerOGL::UpdateSurface() } else { nsRefPtr updatedAreaSurface; - if (mDrawTarget) { - // TODO: This is suboptimal - We should have direct handling for the surface types instead of - // going via a gfxASurface. - updatedAreaSurface = gfxPlatform::GetPlatform()->GetThebesSurfaceForDrawTarget(mDrawTarget); - } else if (mCanvasSurface) { + if (mCanvasSurface) { updatedAreaSurface = mCanvasSurface; } else if (mCanvasGLContext) { gfxIntSize size(mBounds.width, mBounds.height); @@ -226,13 +223,8 @@ CanvasLayerOGL::RenderLayer(int aPreviousDestination, drawRect.IntersectRect(drawRect, GetEffectiveVisibleRegion().GetBounds()); - nsRefPtr surf = mCanvasSurface; - if (mDrawTarget) { - surf = gfxPlatform::GetPlatform()->GetThebesSurfaceForDrawTarget(mDrawTarget); - } - mLayerProgram = - gl()->UploadSurfaceToTexture(surf, + gl()->UploadSurfaceToTexture(mCanvasSurface, nsIntRect(0, 0, drawRect.width, drawRect.height), mTexture, true, diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index 526e1ee241f6..a9039a283f1b 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -615,57 +615,53 @@ gfxPlatform::GetScaledFontForFont(DrawTarget* aTarget, gfxFont *aFont) return scaledFont; } -UserDataKey kThebesSurfaceKey; -void -DestroyThebesSurface(void *data) +cairo_user_data_key_t kDrawSourceSurface; +static void +DataSourceSurfaceDestroy(void *dataSourceSurface) { - gfxASurface *surface = static_cast(data); - surface->Release(); + static_cast(dataSourceSurface)->Release(); +} + +cairo_user_data_key_t kDrawTargetForSurface; +static void +DataDrawTargetDestroy(void *aTarget) +{ + static_cast(aTarget)->Release(); } already_AddRefed gfxPlatform::GetThebesSurfaceForDrawTarget(DrawTarget *aTarget) { - // If we have already created a thebes surface, we can just return it. - void *surface = aTarget->GetUserData(&kThebesSurfaceKey); - if (surface) { - nsRefPtr surf = static_cast(surface); - return surf.forget(); - } - - nsRefPtr surf; if (aTarget->GetType() == BACKEND_CAIRO) { cairo_surface_t* csurf = static_cast(aTarget->GetNativeSurface(NATIVE_SURFACE_CAIRO_SURFACE)); - surf = gfxASurface::Wrap(csurf); - } else { - // The semantics of this part of the function are sort of weird. If we - // don't have direct support for the backend, we snapshot the first time - // and then return the snapshotted surface for the lifetime of the draw - // target. Sometimes it seems like this works out, but it seems like it - // might result in no updates ever. - RefPtr source = aTarget->Snapshot(); - RefPtr data = source->GetDataSurface(); - - if (!data) { - return NULL; - } - - IntSize size = data->GetSize(); - gfxASurface::gfxImageFormat format = OptimalFormatForContent(ContentForFormat(data->GetFormat())); - - // We need to make a copy here because data might change its data under us - nsRefPtr imageSurf = new gfxImageSurface(gfxIntSize(size.width, size.height), format, false); - - bool resultOfCopy = imageSurf->CopyFrom(source); - NS_ASSERTION(resultOfCopy, "Failed to copy surface."); - surf = imageSurf; + return gfxASurface::Wrap(csurf); } - // add a reference to be held by the drawTarget - // careful, the reference graph is getting complicated here - surf->AddRef(); - aTarget->AddUserData(&kThebesSurfaceKey, surf.get(), DestroyThebesSurface); + // The semantics of this part of the function are sort of weird. If we + // don't have direct support for the backend, we snapshot the first time + // and then return the snapshotted surface for the lifetime of the draw + // target. Sometimes it seems like this works out, but it seems like it + // might result in no updates ever. + RefPtr source = aTarget->Snapshot(); + RefPtr data = source->GetDataSurface(); + + if (!data) { + return NULL; + } + + IntSize size = data->GetSize(); + gfxASurface::gfxImageFormat format = OptimalFormatForContent(ContentForFormat(data->GetFormat())); + + + nsRefPtr surf = + new gfxImageSurface(data->GetData(), gfxIntSize(size.width, size.height), + data->Stride(), format); + + surf->SetData(&kDrawSourceSurface, data.forget().drop(), DataSourceSurfaceDestroy); + // keep the draw target alive as long as we need its data + aTarget->AddRef(); + surf->SetData(&kDrawTargetForSurface, aTarget, DataDrawTargetDestroy); return surf.forget(); } diff --git a/gfx/thebes/gfxPlatform.h b/gfx/thebes/gfxPlatform.h index a453666a8f40..81fe98b1ca66 100644 --- a/gfx/thebes/gfxPlatform.h +++ b/gfx/thebes/gfxPlatform.h @@ -38,9 +38,6 @@ class gfxTextRun; class nsIURI; class nsIAtom; -extern mozilla::gfx::UserDataKey kThebesSurfaceKey; -void DestroyThebesSurface(void *data); - extern cairo_user_data_key_t kDrawTarget; // pref lang id's for font prefs @@ -175,9 +172,12 @@ public: CreateDrawTargetForSurface(gfxASurface *aSurface, const mozilla::gfx::IntSize& aSize); /* - * Creates a SourceSurface for a gfxASurface. This surface should -not- be - * held around by the user after the underlying gfxASurface has been - * destroyed as a copy of the data is not guaranteed. + * Creates a SourceSurface for a gfxASurface. This function does no caching, + * so the caller should cache the gfxASurface if it will be used frequently. + * The returned surface keeps a reference to aTarget, so it is OK to keep the + * surface, even if aTarget changes. + * aTarget should not keep a reference to the returned surface because that + * will cause a cycle. */ virtual mozilla::RefPtr GetSourceSurfaceForSurface(mozilla::gfx::DrawTarget *aTarget, gfxASurface *aSurface); diff --git a/gfx/thebes/gfxPlatformMac.cpp b/gfx/thebes/gfxPlatformMac.cpp index 56f19e1176e5..eb5beaf33b4b 100644 --- a/gfx/thebes/gfxPlatformMac.cpp +++ b/gfx/thebes/gfxPlatformMac.cpp @@ -380,26 +380,16 @@ already_AddRefed gfxPlatformMac::GetThebesSurfaceForDrawTarget(DrawTarget *aTarget) { if (aTarget->GetType() == BACKEND_COREGRAPHICS) { - void *surface = aTarget->GetUserData(&kThebesSurfaceKey); - if (surface) { - nsRefPtr surf = static_cast(surface); - return surf.forget(); - } else { - CGContextRef cg = static_cast(aTarget->GetNativeSurface(NATIVE_SURFACE_CGCONTEXT)); + CGContextRef cg = static_cast(aTarget->GetNativeSurface(NATIVE_SURFACE_CGCONTEXT)); - //XXX: it would be nice to have an implicit conversion from IntSize to gfxIntSize - IntSize intSize = aTarget->GetSize(); - gfxIntSize size(intSize.width, intSize.height); + //XXX: it would be nice to have an implicit conversion from IntSize to gfxIntSize + IntSize intSize = aTarget->GetSize(); + gfxIntSize size(intSize.width, intSize.height); - nsRefPtr surf = - new gfxQuartzSurface(cg, size); + nsRefPtr surf = + new gfxQuartzSurface(cg, size); - // add a reference to be held by the drawTarget - surf->AddRef(); - aTarget->AddUserData(&kThebesSurfaceKey, surf.get(), DestroyThebesSurface); - - return surf.forget(); - } + return surf.forget(); } return gfxPlatform::GetThebesSurfaceForDrawTarget(aTarget); diff --git a/gfx/thebes/gfxWindowsPlatform.cpp b/gfx/thebes/gfxWindowsPlatform.cpp index 78c222cf95de..50302b7f7ae0 100644 --- a/gfx/thebes/gfxWindowsPlatform.cpp +++ b/gfx/thebes/gfxWindowsPlatform.cpp @@ -809,40 +809,26 @@ gfxWindowsPlatform::GetThebesSurfaceForDrawTarget(DrawTarget *aTarget) { #ifdef XP_WIN if (aTarget->GetType() == BACKEND_DIRECT2D) { - void *surface = aTarget->GetUserData(&kThebesSurfaceKey); - if (surface) { - nsRefPtr surf = static_cast(surface); - return surf.forget(); - } else { - if (!GetD2DDevice()) { - // We no longer have a D2D device, can't do this. - return NULL; - } - - RefPtr texture = - static_cast(aTarget->GetNativeSurface(NATIVE_SURFACE_D3D10_TEXTURE)); - - if (!texture) { - return gfxPlatform::GetThebesSurfaceForDrawTarget(aTarget); - } - - aTarget->Flush(); - - nsRefPtr surf = - new gfxD2DSurface(texture, ContentForFormat(aTarget->GetFormat())); - - // add a reference to be held by the drawTarget - surf->AddRef(); - aTarget->AddUserData(&kThebesSurfaceKey, surf.get(), DestroyThebesSurface); - /* "It might be worth it to clear cairo surfaces associated with a drawtarget. - The strong reference means for example for D2D that cairo's scratch surface - will be kept alive (well after a user being done) and consume extra VRAM. - We can deal with this in a follow-up though." */ - - // shouldn't this hold a reference? - surf->SetData(&kDrawTarget, aTarget, NULL); - return surf.forget(); + if (!GetD2DDevice()) { + // We no longer have a D2D device, can't do this. + return NULL; } + + RefPtr texture = + static_cast(aTarget->GetNativeSurface(NATIVE_SURFACE_D3D10_TEXTURE)); + + if (!texture) { + return gfxPlatform::GetThebesSurfaceForDrawTarget(aTarget); + } + + aTarget->Flush(); + + nsRefPtr surf = + new gfxD2DSurface(texture, ContentForFormat(aTarget->GetFormat())); + + // shouldn't this hold a reference? + surf->SetData(&kDrawTarget, aTarget, NULL); + return surf.forget(); } #endif