Bug 1290681 (Part 2) - Remove the single color optimization from imgFrame. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Sat, 30 Jul 2016 13:41:57 -0700
changeset 351179 078d914cde9aa9cdfd19fc91db609fe967b9ef66
parent 351178 2597f37b8acce63ef950714efd9b7d162467518f
child 351180 37767b4aa16985b208a37daffdefbae4e812e683
push id1324
push usermtabara@mozilla.com
push dateMon, 16 Jan 2017 13:07:44 +0000
treeherdermozilla-release@a01c49833940 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1290681
milestone51.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 1290681 (Part 2) - Remove the single color optimization from imgFrame. r=edwin
image/RasterImage.cpp
image/imgFrame.cpp
image/imgFrame.h
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -533,29 +533,24 @@ RasterImage::CopyFrame(uint32_t aWhichFr
   if (!target) {
     gfxWarning() << "RasterImage::CopyFrame failed in CreateDrawTargetForData";
     return nullptr;
   }
 
   IntRect intFrameRect = frameRef->GetRect();
   Rect rect(intFrameRect.x, intFrameRect.y,
             intFrameRect.width, intFrameRect.height);
-  if (frameRef->IsSinglePixel()) {
-    target->FillRect(rect, ColorPattern(frameRef->SinglePixelColor()),
-                     DrawOptions(1.0f, CompositionOp::OP_SOURCE));
-  } else {
-    RefPtr<SourceSurface> srcSurf = frameRef->GetSurface();
-    if (!srcSurf) {
-      RecoverFromInvalidFrames(mSize, aFlags);
-      return nullptr;
-    }
+  RefPtr<SourceSurface> srcSurf = frameRef->GetSurface();
+  if (!srcSurf) {
+    RecoverFromInvalidFrames(mSize, aFlags);
+    return nullptr;
+  }
 
-    Rect srcRect(0, 0, intFrameRect.width, intFrameRect.height);
-    target->DrawSurface(srcSurf, srcRect, rect);
-  }
+  Rect srcRect(0, 0, intFrameRect.width, intFrameRect.height);
+  target->DrawSurface(srcSurf, srcRect, rect);
 
   target->Flush();
   surf->Unmap();
 
   return surf.forget();
 }
 
 //******************************************************************************
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -167,17 +167,16 @@ imgFrame::imgFrame()
   , mBlendMethod(BlendMethod::OVER)
   , mHasNoAlpha(false)
   , mAborted(false)
   , mFinished(false)
   , mOptimizable(false)
   , mPalettedImageData(nullptr)
   , mPaletteDepth(0)
   , mNonPremult(false)
-  , mSinglePixel(false)
   , mCompositingFailed(false)
 {
 }
 
 imgFrame::~imgFrame()
 {
 #ifdef DEBUG
   MonitorAutoLock lock(mMonitor);
@@ -407,62 +406,24 @@ imgFrame::Optimize()
     mFormat = SurfaceFormat::B8G8R8X8;
     mImageSurface = CreateLockedSurface(mVBuf, mFrameRect.Size(), mFormat);
   }
 
   if (!mOptimizable || gDisableOptimize) {
     return NS_OK;
   }
 
-  if (mPalettedImageData || mOptSurface || mSinglePixel) {
-    return NS_OK;
-  }
-
-  // Don't do single-color opts on non-premult data.
-  // Cairo doesn't support non-premult single-colors.
-  if (mNonPremult) {
+  if (mPalettedImageData || mOptSurface) {
     return NS_OK;
   }
 
-  /* Figure out if the entire image is a constant color */
-
-  if (gfxPrefs::ImageSingleColorOptimizationEnabled() &&
-      mImageSurface->Stride() == mFrameRect.width * 4) {
-    uint32_t* imgData = (uint32_t*) ((uint8_t*) mVBufPtr);
-    uint32_t firstPixel = * (uint32_t*) imgData;
-    uint32_t pixelCount = mFrameRect.Area() + 1;
-
-    while (--pixelCount && *imgData++ == firstPixel)
-      ;
-
-    if (pixelCount == 0) {
-      // all pixels were the same
-      if (mFormat == SurfaceFormat::B8G8R8A8 ||
-          mFormat == SurfaceFormat::B8G8R8X8) {
-        mSinglePixel = true;
-        mSinglePixelColor.a = ((firstPixel >> 24) & 0xFF) * (1.0f / 255.0f);
-        mSinglePixelColor.r = ((firstPixel >> 16) & 0xFF) * (1.0f / 255.0f);
-        mSinglePixelColor.g = ((firstPixel >>  8) & 0xFF) * (1.0f / 255.0f);
-        mSinglePixelColor.b = ((firstPixel >>  0) & 0xFF) * (1.0f / 255.0f);
-        mSinglePixelColor.r /= mSinglePixelColor.a;
-        mSinglePixelColor.g /= mSinglePixelColor.a;
-        mSinglePixelColor.b /= mSinglePixelColor.a;
-
-        // blow away the older surfaces (if they exist), to release their memory
-        mVBuf = nullptr;
-        mVBufPtr = nullptr;
-        mImageSurface = nullptr;
-        mOptSurface = nullptr;
-
-        return NS_OK;
-      }
-    }
-
-    // if it's not RGB24/ARGB32, don't optimize, but we never hit this at the
-    // moment
+  // XXX(seth): It's currently unclear if there's any reason why we can't
+  // optimize non-premult surfaces. We should look into removing this.
+  if (mNonPremult) {
+    return NS_OK;
   }
 
 #ifdef ANDROID
   SurfaceFormat optFormat = gfxPlatform::GetPlatform()
     ->Optimal2DFormatForContent(gfxContentType::COLOR);
 
   if (mFormat != SurfaceFormat::B8G8R8A8 &&
       optFormat == SurfaceFormat::R5G6B5_UINT16) {
@@ -563,45 +524,37 @@ imgFrame::SurfaceForDrawing(bool        
                             ImageRegion&       aRegion,
                             SourceSurface*     aSurface)
 {
   MOZ_ASSERT(NS_IsMainThread());
   mMonitor.AssertCurrentThreadOwns();
 
   IntSize size(int32_t(aImageRect.Width()), int32_t(aImageRect.Height()));
   if (!aDoPadding && !aDoPartialDecode) {
-    NS_ASSERTION(!mSinglePixel, "This should already have been handled");
     return SurfaceWithFormat(new gfxSurfaceDrawable(aSurface, size), mFormat);
   }
 
   gfxRect available = gfxRect(mDecoded.x, mDecoded.y, mDecoded.width,
                               mDecoded.height);
 
-  if (aDoTile || mSinglePixel) {
+  if (aDoTile) {
     // Create a temporary surface.
     // Give this surface an alpha channel because there are
     // transparent pixels in the padding or undecoded area
     RefPtr<DrawTarget> target =
       gfxPlatform::GetPlatform()->
         CreateOffscreenContentDrawTarget(size, SurfaceFormat::B8G8R8A8);
     if (!target) {
       return SurfaceWithFormat();
     }
 
-    // Fill 'available' with whatever we've got
-    if (mSinglePixel) {
-      target->FillRect(ToRect(aRegion.Intersect(available).Rect()),
-                       ColorPattern(mSinglePixelColor),
-                       DrawOptions(1.0f, CompositionOp::OP_SOURCE));
-    } else {
-      SurfacePattern pattern(aSurface,
-                             aRegion.GetExtendMode(),
-                             Matrix::Translation(mDecoded.x, mDecoded.y));
-      target->FillRect(ToRect(aRegion.Intersect(available).Rect()), pattern);
-    }
+    SurfacePattern pattern(aSurface,
+                           aRegion.GetExtendMode(),
+                           Matrix::Translation(mDecoded.x, mDecoded.y));
+    target->FillRect(ToRect(aRegion.Intersect(available).Rect()), pattern);
 
     RefPtr<SourceSurface> newsurf = target->Snapshot();
     return SurfaceWithFormat(new gfxSurfaceDrawable(newsurf, size),
                              target->GetFormat());
   }
 
   // Not tiling, and we have a surface, so we can account for
   // padding and/or a partial decode just by twiddling parameters.
@@ -633,29 +586,18 @@ bool imgFrame::Draw(gfxContext* aContext
   nsIntMargin padding(mFrameRect.y,
                       mImageSize.width - mFrameRect.XMost(),
                       mImageSize.height - mFrameRect.YMost(),
                       mFrameRect.x);
 
   bool doPadding = padding != nsIntMargin(0,0,0,0);
   bool doPartialDecode = !AreAllPixelsWritten();
 
-  if (mSinglePixel && !doPadding && !doPartialDecode) {
-    if (mSinglePixelColor.a == 0.0) {
-      return true;
-    }
-    RefPtr<DrawTarget> dt = aContext->GetDrawTarget();
-    dt->FillRect(ToRect(aRegion.Rect()),
-                 ColorPattern(mSinglePixelColor),
-                 DrawOptions(1.0f, aContext->CurrentOp()));
-    return true;
-  }
-
   RefPtr<SourceSurface> surf = GetSurfaceInternal();
-  if (!surf && !mSinglePixel) {
+  if (!surf) {
     return false;
   }
 
   gfxRect imageRect(0, 0, mImageSize.width, mImageSize.height);
   bool doTile = !imageRect.Contains(aRegion.Rect()) &&
                 !(aImageFlags & imgIContainer::FLAG_CLAMP);
 
   ImageRegion region(aRegion);
@@ -911,30 +853,16 @@ imgFrame::UnlockImageData()
 void
 imgFrame::SetOptimizable()
 {
   AssertImageDataLocked();
   MonitorAutoLock lock(mMonitor);
   mOptimizable = true;
 }
 
-Color
-imgFrame::SinglePixelColor() const
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  return mSinglePixelColor;
-}
-
-bool
-imgFrame::IsSinglePixel() const
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  return mSinglePixel;
-}
-
 already_AddRefed<SourceSurface>
 imgFrame::GetSurface()
 {
   MonitorAutoLock lock(mMonitor);
   return GetSurfaceInternal();
 }
 
 already_AddRefed<SourceSurface>
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -332,19 +332,16 @@ public:
 
   AnimationData GetAnimationData() const;
 
   bool GetCompositingFailed() const;
   void SetCompositingFailed(bool val);
 
   void SetOptimizable();
 
-  Color SinglePixelColor() const;
-  bool IsSinglePixel() const;
-
   already_AddRefed<SourceSurface> GetSurface();
 
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf, size_t& aHeapSizeOut,
                               size_t& aNonHeapSizeOut) const;
 
 private: // methods
 
   ~imgFrame();
@@ -440,20 +437,16 @@ private: // data
 
   bool mNonPremult;
 
 
   //////////////////////////////////////////////////////////////////////////////
   // Main-thread-only mutable data.
   //////////////////////////////////////////////////////////////////////////////
 
-  // Note that the data stored in gfx::Color is *non-alpha-premultiplied*.
-  Color        mSinglePixelColor;
-
-  bool mSinglePixel;
   bool mCompositingFailed;
 };
 
 /**
  * A reference to an imgFrame that holds the imgFrame's surface in memory,
  * allowing drawing. If you have a DrawableFrameRef |ref| and |if (ref)| returns
  * true, then calls to Draw() and GetSurface() are guaranteed to succeed.
  */