Bug 1282496 - memset unoptimized images with RGBX surface format to 0xFF. r=seth
authorMason Chang <mchang@mozilla.com>
Thu, 30 Jun 2016 18:59:29 -0700
changeset 345436 13cd8e7c973a86d612e04819118d2ca1234059ef
parent 345435 14928a6b38f3d99912df076d13626b28fbaadac4
child 345437 803e3e88b5c0f8f60b20635b8a706a168e47dade
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs1282496
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1282496 - memset unoptimized images with RGBX surface format to 0xFF. r=seth
image/decoders/nsGIFDecoder2.cpp
image/imgFrame.cpp
image/imgFrame.h
--- a/image/decoders/nsGIFDecoder2.cpp
+++ b/image/decoders/nsGIFDecoder2.cpp
@@ -412,16 +412,17 @@ ConvertColormap(uint32_t* aColormap, uin
 {
   // Apply CMS transformation if enabled and available
   if (gfxPlatform::GetCMSMode() == eCMSMode_All) {
     qcms_transform* transform = gfxPlatform::GetCMSRGBTransform();
     if (transform) {
       qcms_transform_data(transform, aColormap, aColormap, aColors);
     }
   }
+
   // Convert from the GIF's RGB format to the Cairo format.
   // Work from end to begin, because of the in-place expansion
   uint8_t* from = ((uint8_t*)aColormap) + 3 * aColors;
   uint32_t* to = aColormap + aColors;
 
   // Convert color entries to Cairo format
 
   // set up for loops below
@@ -886,17 +887,20 @@ nsGIFDecoder2::ReadImageDescriptor(const
           static_cast<uint32_t*>(moz_xmalloc(mColormapSize));
       }
       mColormap = mGIFStruct.local_colormap;
     }
 
     const size_t size = 3 << depth;
     if (mColormapSize > size) {
       // Clear the part of the colormap which will be unused with this palette.
-      memset(reinterpret_cast<uint8_t*>(mColormap) + size, 0,
+      // If a GIF references an invalid palette entry, ensure the entry is opaque white.
+      // This is needed for Skia as if it isn't, RGBX surfaces will cause blending issues
+      // with Skia.
+      memset(reinterpret_cast<uint8_t*>(mColormap) + size, 0xFF,
              mColormapSize - size);
     }
 
     MOZ_ASSERT(mColorTablePos == 0);
 
     // We read the local color table in unbuffered mode since it can be quite
     // large and it'd be preferable to avoid unnecessary copies.
     return Transition::ToUnbuffered(State::FINISHED_LOCAL_COLOR_TABLE,
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -50,17 +50,17 @@ static already_AddRefed<DataSourceSurfac
 CreateLockedSurface(VolatileBuffer* vbuf,
                     const IntSize& size,
                     SurfaceFormat format)
 {
   VolatileBufferPtr<unsigned char>* vbufptr =
     new VolatileBufferPtr<unsigned char>(vbuf);
   MOZ_ASSERT(!vbufptr->WasBufferPurged(), "Expected image data!");
 
-  int32_t stride = VolatileSurfaceStride(size, format);
+  const int32_t stride = VolatileSurfaceStride(size, format);
 
   // The VolatileBufferPtr is held by this DataSourceSurface.
   RefPtr<DataSourceSurface> surf =
     Factory::CreateWrappingDataSourceSurface(*vbufptr, stride, size, format,
                                              &VolatileBufferRelease,
                                              static_cast<void*>(vbufptr));
   if (!surf) {
     delete vbufptr;
@@ -78,16 +78,42 @@ AllocateBufferForImage(const IntSize& si
   if (buf->Init(stride * size.height,
                 size_t(1) << gfxAlphaRecovery::GoodAlignmentLog2())) {
     return buf.forget();
   }
 
   return nullptr;
 }
 
+static bool
+ClearSurface(VolatileBuffer* aVBuf, const IntSize& aSize, SurfaceFormat aFormat)
+{
+  VolatileBufferPtr<unsigned char> vbufptr(aVBuf);
+  if (vbufptr.WasBufferPurged()) {
+    NS_WARNING("VolatileBuffer was purged");
+    return false;
+  }
+
+  int32_t stride = VolatileSurfaceStride(aSize, aFormat);
+  if (aFormat == SurfaceFormat::B8G8R8X8) {
+    // Skia doesn't support RGBX surfaces, so ensure the alpha value is set
+    // to opaque white. While it would be nice to only do this for Skia,
+    // imgFrame can run off main thread and past shutdown where
+    // we might not have gfxPlatform, so just memset everytime instead.
+    memset(vbufptr, 0xFF, stride * aSize.height);
+  } else if (aVBuf->OnHeap()) {
+    // We only need to memset it if the buffer was allocated on the heap.
+    // Otherwise, it's allocated via mmap and refers to a zeroed page and will
+    // be COW once it's written to.
+    memset(vbufptr, 0, stride * aSize.height);
+  }
+
+  return true;
+}
+
 // Returns true if an image of aWidth x aHeight is allowed and legal.
 static bool
 AllowedImageSize(int32_t aWidth, int32_t aHeight)
 {
   // reject over-wide or over-tall images
   const int32_t k64KLimit = 0x0000FFFF;
   if (MOZ_UNLIKELY(aWidth > k64KLimit || aHeight > k64KLimit )) {
     NS_WARNING("image too big");
@@ -205,25 +231,27 @@ imgFrame::InitForDecoder(const nsIntSize
   } else {
     MOZ_ASSERT(!mImageSurface, "Called imgFrame::InitForDecoder() twice?");
 
     mVBuf = AllocateBufferForImage(mFrameRect.Size(), mFormat);
     if (!mVBuf) {
       mAborted = true;
       return NS_ERROR_OUT_OF_MEMORY;
     }
-    if (mVBuf->OnHeap()) {
-      int32_t stride = VolatileSurfaceStride(mFrameRect.Size(), mFormat);
-      VolatileBufferPtr<uint8_t> ptr(mVBuf);
-      memset(ptr, 0, stride * mFrameRect.height);
-    }
+
     mImageSurface = CreateLockedSurface(mVBuf, mFrameRect.Size(), mFormat);
 
     if (!mImageSurface) {
-      NS_WARNING("Failed to create VolatileDataSourceSurface");
+      NS_WARNING("Failed to create ImageSurface");
+      mAborted = true;
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
+
+    if (!ClearSurface(mVBuf, mFrameRect.Size(), mFormat)) {
+      NS_WARNING("Could not clear allocated buffer");
       mAborted = true;
       return NS_ERROR_OUT_OF_MEMORY;
     }
   }
 
   return NS_OK;
 }
 
@@ -265,20 +293,30 @@ imgFrame::InitWithDrawable(gfxDrawable* 
     }
 
     int32_t stride = VolatileSurfaceStride(mFrameRect.Size(), mFormat);
     VolatileBufferPtr<uint8_t> ptr(mVBuf);
     if (!ptr) {
       mAborted = true;
       return NS_ERROR_OUT_OF_MEMORY;
     }
-    if (mVBuf->OnHeap()) {
-      memset(ptr, 0, stride * mFrameRect.height);
+
+    mImageSurface = CreateLockedSurface(mVBuf, mFrameRect.Size(), mFormat);
+
+    if (!mImageSurface) {
+      NS_WARNING("Failed to create ImageSurface");
+      mAborted = true;
+      return NS_ERROR_OUT_OF_MEMORY;
     }
-    mImageSurface = CreateLockedSurface(mVBuf, mFrameRect.Size(), mFormat);
+
+    if (!ClearSurface(mVBuf, mFrameRect.Size(), mFormat)) {
+      NS_WARNING("Could not clear allocated buffer");
+      mAborted = true;
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
 
     target = gfxPlatform::GetPlatform()->
       CreateDrawTargetForData(ptr, mFrameRect.Size(), stride, mFormat);
   } else {
     // We can't use data surfaces for content, so we'll create an offscreen
     // surface instead.  This means if someone later calls RawAccessRef(), we
     // may have to do an expensive readback, but we warned callers about that in
     // the documentation for this method.
@@ -319,16 +357,33 @@ imgFrame::InitWithDrawable(gfxDrawable* 
 #ifdef DEBUG
   MonitorAutoLock lock(mMonitor);
   MOZ_ASSERT(AreAllPixelsWritten());
 #endif
 
   return NS_OK;
 }
 
+bool
+imgFrame::CanOptimizeOpaqueImage()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(!ShutdownTracker::ShutdownHasStarted());
+  mMonitor.AssertCurrentThreadOwns();
+
+  // If we're using a surface format with alpha but the image has no alpha,
+  // change the format. This doesn't change the underlying data at all, but
+  // allows DrawTargets to avoid blending when drawing known opaque images.
+  // This optimization is free and safe, so we always do it when we can except
+  // if we have a Skia backend. Skia doesn't support RGBX so ensure we don't
+  // optimize to a RGBX surface.
+  return mHasNoAlpha && mFormat == SurfaceFormat::B8G8R8A8 && mImageSurface &&
+         (gfxPlatform::GetPlatform()->GetDefaultContentBackend() != BackendType::SKIA);
+}
+
 nsresult
 imgFrame::Optimize()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mMonitor.AssertCurrentThreadOwns();
   MOZ_ASSERT(mLockCount == 1,
              "Should only optimize when holding the lock exclusively");
 
@@ -342,16 +397,22 @@ imgFrame::Optimize()
     hasCheckedOptimize = true;
   }
 
   // Don't optimize during shutdown because gfxPlatform may not be available.
   if (ShutdownTracker::ShutdownHasStarted()) {
     return NS_OK;
   }
 
+  // This optimization is basically free, so we perform it even if optimization is disabled.
+  if (CanOptimizeOpaqueImage()) {
+    mFormat = SurfaceFormat::B8G8R8X8;
+    mImageSurface = CreateLockedSurface(mVBuf, mFrameRect.Size(), mFormat);
+  }
+
   if (!mOptimizable || gDisableOptimize) {
     return NS_OK;
   }
 
   if (mPalettedImageData || mOptSurface || mSinglePixel) {
     return NS_OK;
   }
 
@@ -833,24 +894,16 @@ imgFrame::UnlockImageData()
   if (mLockCount == 1 && !mPalettedImageData) {
     // We can't safely optimize off-main-thread, so create a runnable to do it.
     if (!NS_IsMainThread()) {
       nsCOMPtr<nsIRunnable> runnable = new UnlockImageDataRunnable(this);
       NS_DispatchToMainThread(runnable);
       return NS_OK;
     }
 
-    // If we're using a surface format with alpha but the image has no alpha,
-    // change the format. This doesn't change the underlying data at all, but
-    // allows DrawTargets to avoid blending when drawing known opaque images.
-    if (mHasNoAlpha && mFormat == SurfaceFormat::B8G8R8A8 && mImageSurface) {
-      mFormat = SurfaceFormat::B8G8R8X8;
-      mImageSurface = CreateLockedSurface(mVBuf, mFrameRect.Size(), mFormat);
-    }
-
     // Convert the data surface to a GPU surface or a single color if possible.
     // This will also release mImageSurface if possible.
     Optimize();
 
     // Allow the OS to release our data surface.
     mVBufPtr = nullptr;
   }
 
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -248,16 +248,17 @@ public:
                               size_t& aNonHeapSizeOut) const;
 
 private: // methods
 
   ~imgFrame();
 
   nsresult LockImageData();
   nsresult UnlockImageData();
+  bool     CanOptimizeOpaqueImage();
   nsresult Optimize();
 
   void AssertImageDataLocked() const;
 
   bool AreAllPixelsWritten() const;
   nsresult ImageUpdatedInternal(const nsIntRect& aUpdateRect);
   void GetImageDataInternal(uint8_t** aData, uint32_t* length) const;
   uint32_t GetImageBytesPerRow() const;