Back out 269a48e67579 (bug 1282496) for Windows gfx crashes and assertion failures
authorPhil Ringnalda <philringnalda@gmail.com>
Mon, 27 Jun 2016 19:13:12 -0700
changeset 302819 d3c184e72f31a79f27d617b3b2401fb55c28b617
parent 302818 3f6e4108360ddd95052ad264a3fb7688ccac57d2
child 302820 5fad1dc89df8f404d1edae7484362940ab340433
push id30376
push usercbook@mozilla.com
push dateTue, 28 Jun 2016 14:09:36 +0000
treeherdermozilla-central@e45890951ce7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1282496
milestone50.0a1
backs out269a48e6757917fdad3780f1f48dc99f4b2044ca
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
Back out 269a48e67579 (bug 1282496) for Windows gfx crashes and assertion failures CLOSED TREE
image/decoders/nsGIFDecoder2.cpp
image/imgFrame.cpp
image/imgFrame.h
--- a/image/decoders/nsGIFDecoder2.cpp
+++ b/image/decoders/nsGIFDecoder2.cpp
@@ -411,17 +411,16 @@ 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,20 +885,17 @@ 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.
-      // 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,
+      memset(reinterpret_cast<uint8_t*>(mColormap) + size, 0,
              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
@@ -71,30 +71,16 @@ CreateLockedSurface(VolatileBuffer* vbuf
 
 static already_AddRefed<VolatileBuffer>
 AllocateBufferForImage(const IntSize& size, SurfaceFormat format)
 {
   int32_t stride = VolatileSurfaceStride(size, format);
   RefPtr<VolatileBuffer> buf = new VolatileBuffer();
   if (buf->Init(stride * size.height,
                 size_t(1) << gfxAlphaRecovery::GoodAlignmentLog2())) {
-    VolatileBufferPtr<uint8_t> vbufptr(buf);
-
-    if ((format == SurfaceFormat::B8G8R8X8) &&
-        (gfxPlatform::GetPlatform()->GetDefaultContentBackend() == BackendType::SKIA)) {
-      // Skia doesn't support RGBX surfaces, so ensure the alpha value is set
-      // to opaque white.
-      memset(vbufptr, 0xFF, stride * size.height);
-    } else if (buf->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 * size.height);
-    }
-
     return buf.forget();
   }
 
   return nullptr;
 }
 
 // Returns true if an image of aWidth x aHeight is allowed and legal.
 static bool
@@ -218,17 +204,21 @@ 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");
       mAborted = true;
       return NS_ERROR_OUT_OF_MEMORY;
     }
   }
@@ -274,17 +264,19 @@ 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);
 
     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
@@ -326,33 +318,16 @@ 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");
 
@@ -366,22 +341,16 @@ 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;
   }
 
@@ -863,16 +832,24 @@ 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,17 +248,16 @@ 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;