Bug 1161859 - Compute the size of animated image frames correctly in the SurfaceCache. r=dholbert, a=lmandel GECKO380_2015050320_RELBRANCH
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 05 May 2015 22:19:30 -0700
branchGECKO380_2015050320_RELBRANCH
changeset 260473 570b63d791b9
parent 260471 f0fbb7ca3977
child 260474 273d39c4aa20
push id795
push userryanvm@gmail.com
push date2015-05-13 21:24 +0000
treeherdermozilla-release@5fff1e20ed9c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, lmandel
bugs1161859
milestone38.0
Bug 1161859 - Compute the size of animated image frames correctly in the SurfaceCache. r=dholbert, a=lmandel
image/src/Decoder.cpp
image/src/SurfaceCache.cpp
image/src/SurfaceCache.h
image/src/imgFrame.h
--- a/image/src/Decoder.cpp
+++ b/image/src/Decoder.cpp
@@ -465,17 +465,18 @@ Decoder::InternalAddFrame(uint32_t aFram
   }
 
   if (aTargetSize.width <= 0 || aTargetSize.height <= 0 ||
       aFrameRect.width <= 0 || aFrameRect.height <= 0) {
     NS_WARNING("Trying to add frame with zero or negative size");
     return RawAccessFrameRef();
   }
 
-  if (!SurfaceCache::CanHold(aTargetSize.ToIntSize())) {
+  const uint32_t bytesPerPixel = aPaletteDepth == 0 ? 4 : 1;
+  if (!SurfaceCache::CanHold(aFrameRect.Size(), bytesPerPixel)) {
     NS_WARNING("Trying to add frame that's too large for the SurfaceCache");
     return RawAccessFrameRef();
   }
 
   nsRefPtr<imgFrame> frame = new imgFrame();
   bool nonPremult =
     aDecodeFlags & imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA;
   if (NS_FAILED(frame->InitForDecoder(aTargetSize, aFrameRect, aFormat,
--- a/image/src/SurfaceCache.cpp
+++ b/image/src/SurfaceCache.cpp
@@ -60,19 +60,20 @@ static StaticRefPtr<SurfaceCacheImpl> sI
 /**
  * Cost models the cost of storing a surface in the cache. Right now, this is
  * simply an estimate of the size of the surface in bytes, but in the future it
  * may be worth taking into account the cost of rematerializing the surface as
  * well.
  */
 typedef size_t Cost;
 
-static Cost ComputeCost(const IntSize& aSize)
+static Cost ComputeCost(const IntSize& aSize, uint32_t aBytesPerPixel)
 {
-  return aSize.width * aSize.height * 4;  // width * height * 4 bytes (32bpp)
+  MOZ_ASSERT(aBytesPerPixel == 1 || aBytesPerPixel == 4);
+  return aSize.width * aSize.height * aBytesPerPixel;
 }
 
 /**
  * Since we want to be able to make eviction decisions based on cost, we need to
  * be able to look up the CachedSurface which has a certain cost as well as the
  * cost associated with a certain CachedSurface. To make this possible, in data
  * structures we actually store a CostEntry, which contains a weak pointer to
  * its associated surface.
@@ -977,28 +978,28 @@ SurfaceCache::Insert(imgFrame*         a
                      const SurfaceKey& aSurfaceKey,
                      Lifetime          aLifetime)
 {
   if (!sInstance) {
     return InsertOutcome::FAILURE;
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
-  Cost cost = ComputeCost(aSurfaceKey.Size());
+  Cost cost = ComputeCost(aSurface->GetSize(), aSurface->GetBytesPerPixel());
   return sInstance->Insert(aSurface, cost, aImageKey, aSurfaceKey, aLifetime);
 }
 
 /* static */ bool
-SurfaceCache::CanHold(const IntSize& aSize)
+SurfaceCache::CanHold(const IntSize& aSize, uint32_t aBytesPerPixel /* = 4 */)
 {
   if (!sInstance) {
     return false;
   }
 
-  Cost cost = ComputeCost(aSize);
+  Cost cost = ComputeCost(aSize, aBytesPerPixel);
   return sInstance->CanHold(cost);
 }
 
 /* static */ bool
 SurfaceCache::CanHold(size_t aSize)
 {
   if (!sInstance) {
     return false;
--- a/image/src/SurfaceCache.h
+++ b/image/src/SurfaceCache.h
@@ -275,20 +275,23 @@ struct SurfaceCache
    * surface, but the inverse is not true: Insert() may take more information
    * into account than just image size when deciding whether to cache the
    * surface, so Insert() may still fail even if CanHold() returns true.
    *
    * Use CanHold() to avoid the need to create a temporary surface when we know
    * for sure the cache can't hold it.
    *
    * @param aSize  The dimensions of a surface in pixels.
+   * @param aBytesPerPixel  How many bytes each pixel of the surface requires.
+   *                        Defaults to 4, which is appropriate for RGBA or RGBX
+   *                        images.
    *
    * @return false if the surface cache can't hold a surface of that size.
    */
-  static bool CanHold(const IntSize& aSize);
+  static bool CanHold(const IntSize& aSize, uint32_t aBytesPerPixel = 4);
   static bool CanHold(size_t aSize);
 
   /**
    * Locks an image. Any of the image's persistent surfaces which are either
    * inserted or accessed while the image is locked will not expire.
    *
    * Locking an image does not automatically lock that image's existing
    * surfaces. A call to LockImage() guarantees that persistent surfaces which
--- a/image/src/imgFrame.h
+++ b/image/src/imgFrame.h
@@ -232,16 +232,24 @@ public:
    * aborted.
    *
    * Note that calling this on the main thread _blocks the main thread_. Be very
    * careful in your use of this method to avoid excessive main thread jank or
    * deadlock.
    */
   void WaitUntilComplete() const;
 
+  /**
+   * Returns the number of bytes per pixel this imgFrame requires.  This is a
+   * worst-case value that does not take into account the effects of format
+   * changes caused by Optimize(), since an imgFrame is not optimized throughout
+   * its lifetime.
+   */
+  uint32_t GetBytesPerPixel() const { return GetIsPaletted() ? 1 : 4; }
+
   IntSize GetImageSize() { return mImageSize; }
   nsIntRect GetRect() const;
   IntSize GetSize() const { return mSize; }
   bool NeedsPadding() const { return mOffset != nsIntPoint(0, 0); }
   void GetImageData(uint8_t **aData, uint32_t *length) const;
   uint8_t* GetImageData() const;
 
   bool GetIsPaletted() const;