From 70730aef787cb543a41b5c182ff03023f02c51d9 Mon Sep 17 00:00:00 2001 From: Matt Woodrow Date: Wed, 5 Mar 2014 16:04:05 +1300 Subject: [PATCH] Bug 973264 - If we wrap gfxImageSurface data when creating a SourceSurface, hold a reference to the original surface so the data stays alive. r=Bas --HG-- extra : rebase_source : 91833db0c39af3984dc7e7a1965486916a9f47f8 --- gfx/2d/2D.h | 10 +++ gfx/thebes/gfxPlatform.cpp | 130 +++++++++++++++++++++++++++---------- 2 files changed, 107 insertions(+), 33 deletions(-) diff --git a/gfx/2d/2D.h b/gfx/2d/2D.h index 7d4c6e0d7970..8b96cbb3ff3c 100644 --- a/gfx/2d/2D.h +++ b/gfx/2d/2D.h @@ -336,6 +336,16 @@ public: * DataSourceSurface's data can be accessed directly. */ virtual TemporaryRef GetDataSurface() = 0; + + void AddUserData(UserDataKey *key, void *userData, void (*destroy)(void*)) { + mUserData.Add(key, userData, destroy); + } + void *GetUserData(UserDataKey *key) { + return mUserData.Get(key); + } + +protected: + UserData mUserData; }; class DataSourceSurface : public SourceSurface diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index e36e035c20a0..4f2b16b35817 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -632,6 +632,18 @@ void SourceBufferDestroy(void *srcSurfUD) delete static_cast(srcSurfUD); } +UserDataKey kThebesSurface; + +struct DependentSourceSurfaceUserData +{ + nsRefPtr mSurface; +}; + +void SourceSurfaceDestroyed(void *aData) +{ + delete static_cast(aData); +} + #if MOZ_TREE_CAIRO void SourceSnapshotDetached(cairo_surface_t *nullSurf) { @@ -654,6 +666,34 @@ gfxPlatform::ClearSourceSurfaceForSurface(gfxASurface *aSurface) aSurface->SetData(&kSourceSurface, nullptr, nullptr); } +static TemporaryRef +CopySurface(gfxASurface* aSurface) +{ + const nsIntSize size = aSurface->GetSize(); + gfxImageFormat format = gfxPlatform::GetPlatform()->OptimalFormatForContent(aSurface->GetContentType()); + RefPtr data = + Factory::CreateDataSourceSurface(ToIntSize(size), + ImageFormatToSurfaceFormat(format)); + if (!data) { + return nullptr; + } + + DataSourceSurface::MappedSurface map; + DebugOnly result = data->Map(DataSourceSurface::WRITE, &map); + MOZ_ASSERT(result, "Should always succeed mapping raw data surfaces!"); + + nsRefPtr image = new gfxImageSurface(map.mData, size, map.mStride, format); + RefPtr dt = gfxPlatform::GetPlatform()->CreateDrawTargetForSurface(image, ToIntSize(size)); + + RefPtr source = gfxPlatform::GetPlatform()->GetSourceSurfaceForSurface(dt, aSurface); + + dt->CopySurface(source, IntRect(0, 0, size.width, size.height), IntPoint()); + + data->Unmap(); + + return data; +} + RefPtr gfxPlatform::GetSourceSurfaceForSurface(DrawTarget *aTarget, gfxASurface *aSurface) { @@ -722,18 +762,24 @@ gfxPlatform::GetSourceSurfaceForSurface(DrawTarget *aTarget, gfxASurface *aSurfa } } + bool dependsOnData = false; if (!srcBuffer) { nsRefPtr imgSurface = aSurface->GetAsImageSurface(); - bool isWin32ImageSurf = imgSurface && - aSurface->GetType() == gfxSurfaceType::Win32; - + RefPtr copy; if (!imgSurface) { - imgSurface = new gfxImageSurface(aSurface->GetSize(), OptimalFormatForContent(aSurface->GetContentType())); - nsRefPtr ctx = new gfxContext(imgSurface); - ctx->SetSource(aSurface); - ctx->SetOperator(gfxContext::OPERATOR_SOURCE); - ctx->Paint(); + copy = CopySurface(aSurface); + + if (!copy) { + return nullptr; + } + + DataSourceSurface::MappedSurface map; + DebugOnly result = copy->Map(DataSourceSurface::WRITE, &map); + MOZ_ASSERT(result, "Should always succeed mapping raw data surfaces!"); + + imgSurface = new gfxImageSurface(map.mData, aSurface->GetSize(), map.mStride, + SurfaceFormatToImageFormat(copy->GetFormat())); } gfxImageFormat cairoFormat = imgSurface->Format(); @@ -760,38 +806,56 @@ gfxPlatform::GetSourceSurfaceForSurface(DrawTarget *aTarget, gfxASurface *aSurfa imgSurface->Stride(), format); - if (!srcBuffer) { - // We need to check if our gfxASurface will keep the underlying data - // alive. This is true if gfxASurface actually -is- an ImageSurface or - // if it is a gfxWindowsSurface which supports GetAsImageSurface. - if (imgSurface != aSurface && !isWin32ImageSurf) { - return nullptr; - } - - srcBuffer = Factory::CreateWrappingDataSourceSurface(imgSurface->Data(), - imgSurface->Stride(), - size, format); - + if (copy) { + copy->Unmap(); } + if (!srcBuffer) { + // If we had to make a copy, then just return that. Otherwise aSurface + // must have supported GetAsImageSurface, so we can just wrap that data. + if (copy) { + srcBuffer = copy; + } else { + srcBuffer = Factory::CreateWrappingDataSourceSurface(imgSurface->Data(), + imgSurface->Stride(), + size, format); + dependsOnData = true; + } + } + + if (!srcBuffer) { + return nullptr; + } + + if (!dependsOnData) { #if MOZ_TREE_CAIRO - cairo_surface_t *nullSurf = - cairo_null_surface_create(CAIRO_CONTENT_COLOR_ALPHA); - cairo_surface_set_user_data(nullSurf, - &kSourceSurface, - imgSurface, - nullptr); - cairo_surface_attach_snapshot(imgSurface->CairoSurface(), nullSurf, SourceSnapshotDetached); - cairo_surface_destroy(nullSurf); + cairo_surface_t *nullSurf = + cairo_null_surface_create(CAIRO_CONTENT_COLOR_ALPHA); + cairo_surface_set_user_data(nullSurf, + &kSourceSurface, + imgSurface, + nullptr); + cairo_surface_attach_snapshot(imgSurface->CairoSurface(), nullSurf, SourceSnapshotDetached); + cairo_surface_destroy(nullSurf); #else - cairo_surface_set_mime_data(imgSurface->CairoSurface(), "mozilla/magic", (const unsigned char*) "data", 4, SourceSnapshotDetached, imgSurface.get()); + cairo_surface_set_mime_data(imgSurface->CairoSurface(), "mozilla/magic", (const unsigned char*) "data", 4, SourceSnapshotDetached, imgSurface.get()); #endif + } } - SourceSurfaceUserData *srcSurfUD = new SourceSurfaceUserData; - srcSurfUD->mBackendType = aTarget->GetType(); - srcSurfUD->mSrcSurface = srcBuffer; - aSurface->SetData(&kSourceSurface, srcSurfUD, SourceBufferDestroy); + if (dependsOnData) { + // If we wrapped the underlying data of aSurface, then we need to add user data + // to make sure aSurface stays alive until we are done with the data. + DependentSourceSurfaceUserData *srcSurfUD = new DependentSourceSurfaceUserData; + srcSurfUD->mSurface = aSurface; + srcBuffer->AddUserData(&kThebesSurface, srcSurfUD, SourceSurfaceDestroyed); + } else { + // Otherwise add user data to aSurface so we can cache lookups in the future. + SourceSurfaceUserData *srcSurfUD = new SourceSurfaceUserData; + srcSurfUD->mBackendType = aTarget->GetType(); + srcSurfUD->mSrcSurface = srcBuffer; + aSurface->SetData(&kSourceSurface, srcSurfUD, SourceBufferDestroy); + } return srcBuffer; }