Bug 1210553 - Remove the alternate flags arguments from SurfaceCache's Lookup functions. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Mon, 05 Oct 2015 17:06:34 -0700
changeset 266199 53e486b9137bb483be0adf6d757b32c40b16920f
parent 266198 a1736287f4048664a5f21af411a47b180cc7811e
child 266200 0de9cc9cb4059a867c921a7263f4f63e579351df
push id15532
push usercbook@mozilla.com
push dateTue, 06 Oct 2015 10:23:06 +0000
treeherderfx-team@7603d2860165 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1210553
milestone44.0a1
Bug 1210553 - Remove the alternate flags arguments from SurfaceCache's Lookup functions. r=dholbert
image/RasterImage.cpp
image/SurfaceCache.cpp
image/SurfaceCache.h
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -283,50 +283,47 @@ RasterImage::LookupFrameInternal(uint32_
   }
 
   if (mAnim && aFrameNum > 0) {
     MOZ_ASSERT(ToSurfaceFlags(aFlags) == DefaultSurfaceFlags(),
                "Can't composite frames with non-default surface flags");
     return mAnim->GetCompositedFrame(aFrameNum);
   }
 
-  Maybe<SurfaceFlags> alternateFlags;
-  if (IsOpaque()) {
-    // If we're opaque, we can always substitute a frame that was decoded with a
-    // different decode flag for premultiplied alpha, because that can only
-    // matter for frames with transparency.
-    alternateFlags.emplace(ToSurfaceFlags(aFlags) ^
-                             SurfaceFlags::NO_PREMULTIPLY_ALPHA);
-  }
+  SurfaceFlags surfaceFlags = ToSurfaceFlags(aFlags);
 
-  // We don't want any substitution for sync decodes (except the premultiplied
-  // alpha optimization above), so we use SurfaceCache::Lookup in this case.
+  // We don't want any substitution for sync decodes, so we use
+  // SurfaceCache::Lookup in this case.
   if (aFlags & FLAG_SYNC_DECODE) {
     return SurfaceCache::Lookup(ImageKey(this),
                                 RasterSurfaceKey(aSize,
-                                                 ToSurfaceFlags(aFlags),
-                                                 aFrameNum),
-                                alternateFlags);
+                                                 surfaceFlags,
+                                                 aFrameNum));
   }
 
   // We'll return the best match we can find to the requested frame.
   return SurfaceCache::LookupBestMatch(ImageKey(this),
                                        RasterSurfaceKey(aSize,
-                                                        ToSurfaceFlags(aFlags),
-                                                        aFrameNum),
-                                       alternateFlags);
+                                                        surfaceFlags,
+                                                        aFrameNum));
 }
 
 DrawableFrameRef
 RasterImage::LookupFrame(uint32_t aFrameNum,
                          const IntSize& aSize,
                          uint32_t aFlags)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
+  // If we're opaque, we don't need to care about premultiplied alpha, because
+  // that can only matter for frames with transparency.
+  if (IsOpaque()) {
+    aFlags &= ~FLAG_DECODE_NO_PREMULTIPLY_ALPHA;
+  }
+
   IntSize requestedSize = CanDownscaleDuringDecode(aSize, aFlags)
                         ? aSize : mSize;
 
   LookupResult result = LookupFrameInternal(aFrameNum, requestedSize, aFlags);
 
   if (!result && !mHasSize) {
     // We can't request a decode without knowing our intrinsic size. Give up.
     return DrawableFrameRef();
@@ -1280,26 +1277,33 @@ RasterImage::Decode(const IntSize& aSize
   }
   if (mTransient) {
     decoderFlags |= DecoderFlags::IMAGE_IS_TRANSIENT;
   }
   if (mHasBeenDecoded) {
     decoderFlags |= DecoderFlags::IS_REDECODE;
   }
 
+  SurfaceFlags surfaceFlags = ToSurfaceFlags(aFlags);
+  if (IsOpaque()) {
+    // If there's no transparency, it doesn't matter whether we premultiply
+    // alpha or not.
+    surfaceFlags &= ~SurfaceFlags::NO_PREMULTIPLY_ALPHA;
+  }
+
   // Create a decoder.
   nsRefPtr<Decoder> decoder;
   if (mAnim) {
     decoder = DecoderFactory::CreateAnimationDecoder(mDecoderType, this,
                                                      mSourceBuffer, decoderFlags,
-                                                     ToSurfaceFlags(aFlags));
+                                                     surfaceFlags);
   } else {
     decoder = DecoderFactory::CreateDecoder(mDecoderType, this, mSourceBuffer,
                                             targetSize, decoderFlags,
-                                            ToSurfaceFlags(aFlags),
+                                            surfaceFlags,
                                             mRequestedSampleSize);
   }
 
   // Make sure DecoderFactory was able to create a decoder successfully.
   if (!decoder) {
     return NS_ERROR_FAILURE;
   }
 
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -271,28 +271,27 @@ public:
   already_AddRefed<CachedSurface> Lookup(const SurfaceKey& aSurfaceKey)
   {
     nsRefPtr<CachedSurface> surface;
     mSurfaces.Get(aSurfaceKey, getter_AddRefs(surface));
     return surface.forget();
   }
 
   Pair<already_AddRefed<CachedSurface>, MatchType>
-  LookupBestMatch(const SurfaceKey&      aSurfaceKey,
-                  const Maybe<SurfaceFlags>& aAlternateFlags)
+  LookupBestMatch(const SurfaceKey& aSurfaceKey)
   {
     // Try for an exact match first.
     nsRefPtr<CachedSurface> exactMatch;
     mSurfaces.Get(aSurfaceKey, getter_AddRefs(exactMatch));
     if (exactMatch && exactMatch->IsDecoded()) {
       return MakePair(exactMatch.forget(), MatchType::EXACT);
     }
 
     // There's no perfect match, so find the best match we can.
-    MatchContext matchContext(aSurfaceKey, aAlternateFlags);
+    MatchContext matchContext(aSurfaceKey);
     ForEach(TryToImproveMatch, &matchContext);
 
     MatchType matchType;
     if (matchContext.mBestMatch) {
       if (!exactMatch) {
         // No exact match, but we found a substitute.
         matchType = MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
       } else if (exactMatch != matchContext.mBestMatch) {
@@ -323,24 +322,21 @@ public:
   }
 
   void SetLocked(bool aLocked) { mLocked = aLocked; }
   bool IsLocked() const { return mLocked; }
 
 private:
   struct MatchContext
   {
-    MatchContext(const SurfaceKey& aIdealKey,
-                 const Maybe<SurfaceFlags>& aAlternateFlags)
+    explicit MatchContext(const SurfaceKey& aIdealKey)
       : mIdealKey(aIdealKey)
-      , mAlternateFlags(aAlternateFlags)
     { }
 
     const SurfaceKey& mIdealKey;
-    const Maybe<SurfaceFlags> mAlternateFlags;
     nsRefPtr<CachedSurface> mBestMatch;
   };
 
   static PLDHashOperator TryToImproveMatch(const SurfaceKey& aSurfaceKey,
                                            CachedSurface*    aSurface,
                                            void*             aContext)
   {
     auto context = static_cast<MatchContext*>(aContext);
@@ -352,20 +348,18 @@ private:
     }
 
     // Matching the animation time and SVG context is required.
     if (aSurfaceKey.AnimationTime() != idealKey.AnimationTime() ||
         aSurfaceKey.SVGContext() != idealKey.SVGContext()) {
       return PL_DHASH_NEXT;
     }
 
-    // Matching the flags is required, but we can match the alternate flags as
-    // well if some were provided.
-    if (aSurfaceKey.Flags() != idealKey.Flags() &&
-        Some(aSurfaceKey.Flags()) != context->mAlternateFlags) {
+    // Matching the flags is required.
+    if (aSurfaceKey.Flags() != idealKey.Flags()) {
       return PL_DHASH_NEXT;
     }
 
     // Anything is better than nothing! (Within the constraints we just
     // checked, of course.)
     if (!context->mBestMatch) {
       context->mBestMatch = aSurface;
       return PL_DHASH_NEXT;
@@ -631,18 +625,17 @@ public:
     }
 
     MOZ_ASSERT(surface->GetSurfaceKey() == aSurfaceKey,
                "Lookup() not returning an exact match?");
     return LookupResult(Move(ref), MatchType::EXACT);
   }
 
   LookupResult LookupBestMatch(const ImageKey         aImageKey,
-                               const SurfaceKey&      aSurfaceKey,
-                               const Maybe<SurfaceFlags>& aAlternateFlags)
+                               const SurfaceKey&      aSurfaceKey)
   {
     nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       // No cached surfaces for this image.
       return LookupResult(MatchType::NOT_FOUND);
     }
 
     // Repeatedly look up the best match, trying again if the resulting surface
@@ -650,39 +643,39 @@ public:
     // surface for drawing or there are no matching surfaces left.
     // XXX(seth): This is O(N^2), but N is expected to be very small. If we
     // encounter a performance problem here we can revisit this.
 
     nsRefPtr<CachedSurface> surface;
     DrawableFrameRef ref;
     MatchType matchType = MatchType::NOT_FOUND;
     while (true) {
-      Tie(surface, matchType) =
-        cache->LookupBestMatch(aSurfaceKey, aAlternateFlags);
+      Tie(surface, matchType) = cache->LookupBestMatch(aSurfaceKey);
 
       if (!surface) {
         return LookupResult(matchType);  // Lookup in the per-image cache missed.
       }
 
       ref = surface->DrawableRef();
       if (ref) {
         break;
       }
 
       // The surface was released by the operating system. Remove the cache
       // entry as well.
       Remove(surface);
     }
 
-    MOZ_ASSERT((matchType == MatchType::EXACT) ==
-      (surface->GetSurfaceKey() == aSurfaceKey ||
-         (aAlternateFlags &&
-          surface->GetSurfaceKey() ==
-            aSurfaceKey.WithNewFlags(*aAlternateFlags))),
-      "Result differs in a way other than size or alternate flags");
+    MOZ_ASSERT_IF(matchType == MatchType::EXACT,
+                  surface->GetSurfaceKey() == aSurfaceKey);
+    MOZ_ASSERT_IF(matchType == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND ||
+                  matchType == MatchType::SUBSTITUTE_BECAUSE_PENDING,
+      surface->GetSurfaceKey().SVGContext() == aSurfaceKey.SVGContext() &&
+      surface->GetSurfaceKey().AnimationTime() == aSurfaceKey.AnimationTime() &&
+      surface->GetSurfaceKey().Flags() == aSurfaceKey.Flags());
 
     if (matchType == MatchType::EXACT) {
       MarkUsed(surface, cache);
     }
 
     return LookupResult(Move(ref), matchType);
   }
 
@@ -1045,47 +1038,36 @@ SurfaceCache::Shutdown()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(sInstance, "No singleton - was Shutdown() called twice?");
   sInstance = nullptr;
 }
 
 /* static */ LookupResult
 SurfaceCache::Lookup(const ImageKey         aImageKey,
-                     const SurfaceKey&      aSurfaceKey,
-                     const Maybe<SurfaceFlags>& aAlternateFlags
-                       /* = Nothing() */)
+                     const SurfaceKey&      aSurfaceKey)
 {
   if (!sInstance) {
     return LookupResult(MatchType::NOT_FOUND);
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
-
-  LookupResult result = sInstance->Lookup(aImageKey, aSurfaceKey);
-  if (!result && aAlternateFlags) {
-    result = sInstance->Lookup(aImageKey,
-                               aSurfaceKey.WithNewFlags(*aAlternateFlags));
-  }
-
-  return result;
+  return sInstance->Lookup(aImageKey, aSurfaceKey);
 }
 
 /* static */ LookupResult
 SurfaceCache::LookupBestMatch(const ImageKey         aImageKey,
-                              const SurfaceKey&      aSurfaceKey,
-                              const Maybe<SurfaceFlags>& aAlternateFlags
-                                /* = Nothing() */)
+                              const SurfaceKey&      aSurfaceKey)
 {
   if (!sInstance) {
     return LookupResult(MatchType::NOT_FOUND);
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
-  return sInstance->LookupBestMatch(aImageKey, aSurfaceKey, aAlternateFlags);
+  return sInstance->LookupBestMatch(aImageKey, aSurfaceKey);
 }
 
 /* static */ InsertOutcome
 SurfaceCache::Insert(imgFrame*         aSurface,
                      const ImageKey    aImageKey,
                      const SurfaceKey& aSurfaceKey)
 {
   if (!sInstance) {
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -64,21 +64,16 @@ public:
     return hash;
   }
 
   IntSize Size() const { return mSize; }
   Maybe<SVGImageContext> SVGContext() const { return mSVGContext; }
   float AnimationTime() const { return mAnimationTime; }
   SurfaceFlags Flags() const { return mFlags; }
 
-  SurfaceKey WithNewFlags(SurfaceFlags aFlags) const
-  {
-    return SurfaceKey(mSize, mSVGContext, mAnimationTime, aFlags);
-  }
-
 private:
   SurfaceKey(const IntSize& aSize,
              const Maybe<SVGImageContext>& aSVGContext,
              const float aAnimationTime,
              const SurfaceFlags aFlags)
     : mSize(aSize)
     , mSVGContext(aSVGContext)
     , mAnimationTime(aAnimationTime)
@@ -176,59 +171,44 @@ struct SurfaceCache
    * cache keeps a strong reference to such surfaces internally.
    *
    * @param aImageKey       Key data identifying which image the surface belongs
    *                        to.
    *
    * @param aSurfaceKey     Key data which uniquely identifies the requested
    *                        surface.
    *
-   * @param aAlternateFlags If not Nothing(), a different set of flags than the
-   *                        ones specified in @aSurfaceKey which are also
-   *                        acceptable to the caller. This is more efficient
-   *                        than calling Lookup() twice, which requires taking a
-   *                        lock each time.
-   *
    * @return                a LookupResult, which will either contain a
    *                        DrawableFrameRef to the requested surface, or an
    *                        empty DrawableFrameRef if the surface was not found.
    */
   static LookupResult Lookup(const ImageKey    aImageKey,
-                             const SurfaceKey& aSurfaceKey,
-                             const Maybe<SurfaceFlags>& aAlternateFlags
-                               = Nothing());
+                             const SurfaceKey& aSurfaceKey);
 
   /**
    * Looks up the best matching surface in the cache and returns a drawable
    * reference to the imgFrame containing it.
    *
    * Returned surfaces may vary from the requested surface only in terms of
-   * size, unless @aAlternateFlags is specified.
+   * size.
    *
    * @param aImageKey    Key data identifying which image the surface belongs
    *                     to.
    *
    * @param aSurfaceKey  Key data which identifies the ideal surface to return.
    *
-   * @param aAlternateFlags If not Nothing(), a different set of flags than the
-   *                        ones specified in @aSurfaceKey which are also
-   *                        acceptable to the caller. This is much more
-   *                        efficient than calling LookupBestMatch() twice.
-   *
    * @return                a LookupResult, which will either contain a
    *                        DrawableFrameRef to a surface similar to the
    *                        requested surface, or an empty DrawableFrameRef if
    *                        the surface was not found. Callers can use
    *                        LookupResult::IsExactMatch() to check whether the
    *                        returned surface exactly matches @aSurfaceKey.
    */
   static LookupResult LookupBestMatch(const ImageKey    aImageKey,
-                                      const SurfaceKey& aSurfaceKey,
-                                      const Maybe<SurfaceFlags>& aAlternateFlags
-                                        = Nothing());
+                                      const SurfaceKey& aSurfaceKey);
 
   /**
    * Insert a surface into the cache. If a surface with the same ImageKey and
    * SurfaceKey is already in the cache, Insert returns FAILURE_ALREADY_PRESENT.
    * If a matching placeholder is already present, the placeholder is removed.
    *
    * Surfaces will never expire as long as they remain locked, but if they
    * become unlocked, they can expire either because the SurfaceCache runs out