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, a=sledru
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 05 Mar 2014 16:04:05 +1300
changeset 191497 0717e2e53e18ce6b87ee6c5881fa5c5c150b114b
parent 191496 fb1a149156a9624a0cace0c57eca1d1f6324ca72
child 191498 a7a88d8d692c85162a03c67679892da0acebae17
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersBas, sledru
bugs973264
milestone30.0a2
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, a=sledru
gfx/2d/2D.h
gfx/thebes/gfxPlatform.cpp
--- a/gfx/2d/2D.h
+++ b/gfx/2d/2D.h
@@ -331,16 +331,26 @@ public:
    */
   virtual bool IsValid() const { return true; }
 
   /*
    * This function will get a DataSourceSurface for this surface, a
    * DataSourceSurface's data can be accessed directly.
    */
   virtual TemporaryRef<DataSourceSurface> 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
 {
 public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(DataSourceSurface)
   DataSourceSurface()
     : mIsMapped(false)
--- a/gfx/thebes/gfxPlatform.cpp
+++ b/gfx/thebes/gfxPlatform.cpp
@@ -615,16 +615,28 @@ struct SourceSurfaceUserData
   BackendType mBackendType;
 };
 
 void SourceBufferDestroy(void *srcSurfUD)
 {
   delete static_cast<SourceSurfaceUserData*>(srcSurfUD);
 }
 
+UserDataKey kThebesSurface;
+
+struct DependentSourceSurfaceUserData
+{
+  nsRefPtr<gfxASurface> mSurface;
+};
+
+void SourceSurfaceDestroyed(void *aData)
+{
+  delete static_cast<DependentSourceSurfaceUserData*>(aData);
+}
+
 #if MOZ_TREE_CAIRO
 void SourceSnapshotDetached(cairo_surface_t *nullSurf)
 {
   gfxImageSurface* origSurf =
     static_cast<gfxImageSurface*>(cairo_surface_get_user_data(nullSurf, &kSourceSurface));
 
   origSurf->SetData(&kSourceSurface, nullptr, nullptr);
 }
@@ -637,16 +649,44 @@ void SourceSnapshotDetached(void *nullSu
 #endif
 
 void
 gfxPlatform::ClearSourceSurfaceForSurface(gfxASurface *aSurface)
 {
   aSurface->SetData(&kSourceSurface, nullptr, nullptr);
 }
 
+static TemporaryRef<DataSourceSurface>
+CopySurface(gfxASurface* aSurface)
+{
+  const nsIntSize size = aSurface->GetSize();
+  gfxImageFormat format = gfxPlatform::GetPlatform()->OptimalFormatForContent(aSurface->GetContentType());
+  RefPtr<DataSourceSurface> data =
+    Factory::CreateDataSourceSurface(ToIntSize(size),
+                                     ImageFormatToSurfaceFormat(format));
+  if (!data) {
+    return nullptr;
+  }
+
+  DataSourceSurface::MappedSurface map;
+  DebugOnly<bool> result = data->Map(DataSourceSurface::WRITE, &map);
+  MOZ_ASSERT(result, "Should always succeed mapping raw data surfaces!");
+
+  nsRefPtr<gfxImageSurface> image = new gfxImageSurface(map.mData, size, map.mStride, format);
+  nsRefPtr<gfxContext> ctx = new gfxContext(image);
+
+  ctx->SetSource(aSurface);
+  ctx->SetOperator(gfxContext::OPERATOR_SOURCE);
+  ctx->Paint();
+
+  data->Unmap();
+
+  return data;
+}
+
 RefPtr<SourceSurface>
 gfxPlatform::GetSourceSurfaceForSurface(DrawTarget *aTarget, gfxASurface *aSurface)
 {
   if (!aSurface->CairoSurface() || aSurface->CairoStatus()) {
     return nullptr;
   }
 
   if (!aTarget) {
@@ -705,28 +745,34 @@ gfxPlatform::GetSourceSurfaceForSurface(
 
     if (srcBuffer) {
       // It's cheap enough to make a new one so we won't keep it around and
       // keeping it creates a cycle.
       return srcBuffer;
     }
   }
 
+  bool dependsOnData = false;
   if (!srcBuffer) {
     nsRefPtr<gfxImageSurface> imgSurface = aSurface->GetAsImageSurface();
 
-    bool isWin32ImageSurf = imgSurface &&
-                            aSurface->GetType() == gfxSurfaceType::Win32;
-
+    RefPtr<DataSourceSurface> copy;
     if (!imgSurface) {
-      imgSurface = new gfxImageSurface(aSurface->GetSize(), OptimalFormatForContent(aSurface->GetContentType()));
-      nsRefPtr<gfxContext> 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<bool> 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();
     switch(cairoFormat) {
       case gfxImageFormat::ARGB32:
         format = SurfaceFormat::B8G8R8A8;
         break;
       case gfxImageFormat::RGB24:
@@ -743,48 +789,66 @@ gfxPlatform::GetSourceSurfaceForSurface(
     }
 
     IntSize size = IntSize(imgSurface->GetSize().width, imgSurface->GetSize().height);
     srcBuffer = aTarget->CreateSourceSurfaceFromData(imgSurface->Data(),
                                                      size,
                                                      imgSurface->Stride(),
                                                      format);
 
+    if (copy) {
+      copy->Unmap();
+    }
+
     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;
+      // 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;
       }
+    }
 
-      srcBuffer = Factory::CreateWrappingDataSourceSurface(imgSurface->Data(),
-                                                           imgSurface->Stride(),
-                                                           size, format);
-
+    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;
 }
 
 TemporaryRef<ScaledFont>
 gfxPlatform::GetScaledFontForFont(DrawTarget* aTarget, gfxFont *aFont)
 {
   NativeFont nativeFont;