From f18cc406a1263d069073200d86720e4bc50e690e Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Tue, 30 Jun 2015 18:57:03 -0700 Subject: [PATCH] Bug 1139641 - Return more information from SurfaceCache::Lookup and SurfaceCache::LookupBestMatch. r=dholbert --- image/FrameAnimator.cpp | 19 +++++++----- image/FrameAnimator.h | 4 +-- image/LookupResult.h | 69 +++++++++++++++++++++++++++++++++++++++++ image/RasterImage.cpp | 48 ++++++++++++++-------------- image/RasterImage.h | 7 +++-- image/SurfaceCache.cpp | 51 +++++++++++++++++------------- image/SurfaceCache.h | 33 +++++++++++--------- image/VectorImage.cpp | 9 +++--- image/imgFrame.h | 4 +++ 9 files changed, 168 insertions(+), 76 deletions(-) create mode 100644 image/LookupResult.h diff --git a/image/FrameAnimator.cpp b/image/FrameAnimator.cpp index 1589792ba17..c4cf5fb3a60 100644 --- a/image/FrameAnimator.cpp +++ b/image/FrameAnimator.cpp @@ -8,6 +8,7 @@ #include "mozilla/MemoryReporting.h" #include "mozilla/Move.h" #include "imgIContainer.h" +#include "LookupResult.h" #include "MainThreadUtils.h" #include "RasterImage.h" @@ -265,25 +266,27 @@ FrameAnimator::GetFirstFrameRefreshArea() const return mFirstFrameRefreshArea; } -DrawableFrameRef +LookupResult FrameAnimator::GetCompositedFrame(uint32_t aFrameNum) { MOZ_ASSERT(aFrameNum != 0, "First frame is never composited"); // If we have a composited version of this frame, return that. if (mLastCompositedFrameIndex == int32_t(aFrameNum)) { - return mCompositingFrame->DrawableRef(); + return LookupResult(mCompositingFrame->DrawableRef(), + /* aIsExactMatch = */ true); } // Otherwise return the raw frame. DoBlend is required to ensure that we only // hit this case if the frame is not paletted and doesn't require compositing. - DrawableFrameRef ref = + LookupResult result = SurfaceCache::Lookup(ImageKey(mImage), RasterSurfaceKey(mSize, 0, // Default decode flags. aFrameNum)); - MOZ_ASSERT(!ref || !ref->GetIsPaletted(), "About to return a paletted frame"); - return ref; + MOZ_ASSERT(!result || !result.DrawableRef()->GetIsPaletted(), + "About to return a paletted frame"); + return result; } int32_t @@ -367,13 +370,13 @@ FrameAnimator::CollectSizeOfCompositingSurfaces( RawAccessFrameRef FrameAnimator::GetRawFrame(uint32_t aFrameNum) const { - DrawableFrameRef ref = + LookupResult result = SurfaceCache::Lookup(ImageKey(mImage), RasterSurfaceKey(mSize, 0, // Default decode flags. aFrameNum)); - return ref ? ref->RawAccessRef() - : RawAccessFrameRef(); + return result ? result.DrawableRef()->RawAccessRef() + : RawAccessFrameRef(); } //****************************************************************************** diff --git a/image/FrameAnimator.h b/image/FrameAnimator.h index 00fccf54776..a404d01c714 100644 --- a/image/FrameAnimator.h +++ b/image/FrameAnimator.h @@ -128,10 +128,10 @@ public: /** * If we have a composited frame for @aFrameNum, returns it. Otherwise, - * returns an empty DrawableFrameRef. It is an error to call this method with + * returns an empty LookupResult. It is an error to call this method with * aFrameNum == 0, because the first frame is never composited. */ - DrawableFrameRef GetCompositedFrame(uint32_t aFrameNum); + LookupResult GetCompositedFrame(uint32_t aFrameNum); /* * Returns the frame's adjusted timeout. If the animation loops and the diff --git a/image/LookupResult.h b/image/LookupResult.h new file mode 100644 index 00000000000..91fb2b4033a --- /dev/null +++ b/image/LookupResult.h @@ -0,0 +1,69 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/** + * LookupResult is the return type of SurfaceCache's Lookup*() functions. It + * combines a surface with relevant metadata tracked by SurfaceCache. + */ + +#ifndef mozilla_image_LookupResult_h +#define mozilla_image_LookupResult_h + +#include "mozilla/Attributes.h" +#include "mozilla/Move.h" +#include "imgFrame.h" + +namespace mozilla { +namespace image { + +/** + * LookupResult is the return type of SurfaceCache's Lookup*() functions. It + * combines a surface with relevant metadata tracked by SurfaceCache. + */ +class MOZ_STACK_CLASS LookupResult +{ +public: + LookupResult() + : mIsExactMatch(false) + { } + + LookupResult(LookupResult&& aOther) + : mDrawableRef(Move(aOther.mDrawableRef)) + , mIsExactMatch(aOther.mIsExactMatch) + { } + + LookupResult(DrawableFrameRef&& aDrawableRef, bool aIsExactMatch) + : mDrawableRef(Move(aDrawableRef)) + , mIsExactMatch(aIsExactMatch) + { } + + LookupResult& operator=(LookupResult&& aOther) + { + MOZ_ASSERT(&aOther != this, "Self-move-assignment is not supported"); + mDrawableRef = Move(aOther.mDrawableRef); + mIsExactMatch = aOther.mIsExactMatch; + return *this; + } + + DrawableFrameRef& DrawableRef() { return mDrawableRef; } + const DrawableFrameRef& DrawableRef() const { return mDrawableRef; } + + /// @return true if this LookupResult contains a surface. + explicit operator bool() const { return bool(mDrawableRef); } + + /// @return true if the surface is an exact match for the Lookup*() arguments. + bool IsExactMatch() const { return mIsExactMatch; } + +private: + LookupResult(const LookupResult&) = delete; + + DrawableFrameRef mDrawableRef; + bool mIsExactMatch; +}; + +} // namespace image +} // namespace mozilla + +#endif // mozilla_image_LookupResult_h diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp index 06bf5e9395d..603af3d88e0 100644 --- a/image/RasterImage.cpp +++ b/image/RasterImage.cpp @@ -20,6 +20,7 @@ #include "ImageContainer.h" #include "ImageRegion.h" #include "Layers.h" +#include "LookupResult.h" #include "nsPresContext.h" #include "SourceBuffer.h" #include "SurfaceCache.h" @@ -469,7 +470,7 @@ RasterImage::GetType(uint16_t* aType) return NS_OK; } -DrawableFrameRef +LookupResult RasterImage::LookupFrameInternal(uint32_t aFrameNum, const IntSize& aSize, uint32_t aFlags) @@ -522,14 +523,14 @@ RasterImage::LookupFrame(uint32_t aFrameNum, IntSize requestedSize = CanDownscaleDuringDecode(aSize, aFlags) ? aSize : mSize; - DrawableFrameRef ref = LookupFrameInternal(aFrameNum, requestedSize, aFlags); + LookupResult result = LookupFrameInternal(aFrameNum, requestedSize, aFlags); - if (!ref && !mHasSize) { + if (!result && !mHasSize) { // We can't request a decode without knowing our intrinsic size. Give up. return DrawableFrameRef(); } - if (!ref || ref->GetImageSize() != requestedSize) { + if (!result || !result.IsExactMatch()) { // The OS threw this frame away. We need to redecode if we can. MOZ_ASSERT(!mAnim, "Animated frames should be locked"); @@ -537,29 +538,30 @@ RasterImage::LookupFrame(uint32_t aFrameNum, // If we can sync decode, we should already have the frame. if (aFlags & FLAG_SYNC_DECODE) { - ref = LookupFrameInternal(aFrameNum, requestedSize, aFlags); + result = LookupFrameInternal(aFrameNum, requestedSize, aFlags); } } - if (!ref) { + if (!result) { // We still weren't able to get a frame. Give up. return DrawableFrameRef(); } - if (ref->GetCompositingFailed()) { + if (result.DrawableRef()->GetCompositingFailed()) { return DrawableFrameRef(); } - MOZ_ASSERT(!ref || !ref->GetIsPaletted(), "Should not have paletted frame"); + MOZ_ASSERT(!result.DrawableRef()->GetIsPaletted(), + "Should not have a paletted frame"); // Sync decoding guarantees that we got the frame, but if it's owned by an // async decoder that's currently running, the contents of the frame may not // be available yet. Make sure we get everything. - if (ref && mHasSourceData && (aFlags & FLAG_SYNC_DECODE)) { - ref->WaitUntilComplete(); + if (mHasSourceData && (aFlags & FLAG_SYNC_DECODE)) { + result.DrawableRef()->WaitUntilComplete(); } - return ref; + return Move(result.DrawableRef()); } uint32_t @@ -1835,19 +1837,19 @@ RasterImage::DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef, DrawableFrameRef frameRef; if (CanScale(aFilter, aSize, aFlags)) { - frameRef = + LookupResult result = SurfaceCache::Lookup(ImageKey(this), RasterSurfaceKey(aSize, DecodeFlags(aFlags), 0)); - if (!frameRef) { + if (!result) { // We either didn't have a matching scaled frame or the OS threw it away. // Request a new one so we'll be ready next time. For now, we'll fall back // to aFrameRef below. RequestScale(aFrameRef.get(), aFlags, aSize); } - if (frameRef && !frameRef->IsImageComplete()) { - frameRef.reset(); // We're still scaling, so we can't use this yet. + if (result && result.DrawableRef()->IsImageComplete()) { + frameRef = Move(result.DrawableRef()); // The scaled version is ready. } } @@ -2247,21 +2249,21 @@ RasterImage::OptimalImageSizeForDest(const gfxSize& aDest, uint32_t aWhichFrame, CanDownscaleDuringDecode(destSize, aFlags)) { return destSize; } else if (CanScale(aFilter, destSize, aFlags)) { - DrawableFrameRef frameRef = + LookupResult result = SurfaceCache::Lookup(ImageKey(this), RasterSurfaceKey(destSize, DecodeFlags(aFlags), 0)); - if (frameRef && frameRef->IsImageComplete()) { - return destSize; // We have an existing HQ scale for this size. + if (result && result.DrawableRef()->IsImageComplete()) { + return destSize; // We have an existing HQ scale for this size. } - if (!frameRef) { + if (!result) { // We could HQ scale to this size, but we haven't. Request a scale now. - frameRef = LookupFrame(GetRequestedFrameIndex(aWhichFrame), - mSize, aFlags); - if (frameRef) { - RequestScale(frameRef.get(), aFlags, destSize); + DrawableFrameRef ref = LookupFrame(GetRequestedFrameIndex(aWhichFrame), + mSize, aFlags); + if (ref) { + RequestScale(ref.get(), aFlags, destSize); } } } diff --git a/image/RasterImage.h b/image/RasterImage.h index b579f05df22..cc060345be7 100644 --- a/image/RasterImage.h +++ b/image/RasterImage.h @@ -23,6 +23,7 @@ #include "nsIProperties.h" #include "nsTArray.h" #include "imgFrame.h" +#include "LookupResult.h" #include "nsThreadUtils.h" #include "DecodePool.h" #include "Orientation.h" @@ -301,9 +302,9 @@ private: Pair> GetFrameInternal(uint32_t aWhichFrame, uint32_t aFlags); - DrawableFrameRef LookupFrameInternal(uint32_t aFrameNum, - const gfx::IntSize& aSize, - uint32_t aFlags); + LookupResult LookupFrameInternal(uint32_t aFrameNum, + const gfx::IntSize& aSize, + uint32_t aFlags); DrawableFrameRef LookupFrame(uint32_t aFrameNum, const nsIntSize& aSize, uint32_t aFlags); diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp index a919ad1c20a..5d0b3850343 100644 --- a/image/SurfaceCache.cpp +++ b/image/SurfaceCache.cpp @@ -25,6 +25,7 @@ #include "gfxPrefs.h" #include "imgFrame.h" #include "Image.h" +#include "LookupResult.h" #include "nsAutoPtr.h" #include "nsExpirationTracker.h" #include "nsHashKeys.h" @@ -543,17 +544,17 @@ public: "More available cost than we started with"); } - DrawableFrameRef Lookup(const ImageKey aImageKey, - const SurfaceKey& aSurfaceKey) + LookupResult Lookup(const ImageKey aImageKey, + const SurfaceKey& aSurfaceKey) { nsRefPtr cache = GetImageCache(aImageKey); if (!cache) { - return DrawableFrameRef(); // No cached surfaces for this image. + return LookupResult(); // No cached surfaces for this image. } nsRefPtr surface = cache->Lookup(aSurfaceKey); if (!surface) { - return DrawableFrameRef(); // Lookup in the per-image cache missed. + return LookupResult(); // Lookup in the per-image cache missed. } DrawableFrameRef ref = surface->DrawableRef(); @@ -561,7 +562,7 @@ public: // The surface was released by the operating system. Remove the cache // entry as well. Remove(surface); - return DrawableFrameRef(); + return LookupResult(); } if (cache->IsLocked()) { @@ -570,16 +571,16 @@ public: mExpirationTracker.MarkUsed(surface); } - return ref; + return LookupResult(Move(ref), /* aIsExactMatch = */ true); } - DrawableFrameRef LookupBestMatch(const ImageKey aImageKey, - const SurfaceKey& aSurfaceKey, - const Maybe& aAlternateFlags) + LookupResult LookupBestMatch(const ImageKey aImageKey, + const SurfaceKey& aSurfaceKey, + const Maybe& aAlternateFlags) { nsRefPtr cache = GetImageCache(aImageKey); if (!cache) { - return DrawableFrameRef(); // No cached surfaces for this image. + return LookupResult(); // No cached surfaces for this image. } // Repeatedly look up the best match, trying again if the resulting surface @@ -593,7 +594,7 @@ public: while (true) { surface = cache->LookupBestMatch(aSurfaceKey, aAlternateFlags); if (!surface) { - return DrawableFrameRef(); // Lookup in the per-image cache missed. + return LookupResult(); // Lookup in the per-image cache missed. } ref = surface->DrawableRef(); @@ -612,7 +613,15 @@ public: mExpirationTracker.MarkUsed(surface); } - return ref; + SurfaceKey key = surface->GetSurfaceKey(); + const bool isExactMatch = key.Size() == aSurfaceKey.Size(); + + MOZ_ASSERT(isExactMatch == + (key == aSurfaceKey || + (aAlternateFlags && key == aSurfaceKey.WithNewFlags(*aAlternateFlags))), + "Result differs in a way other than size or alternate flags"); + + return LookupResult(Move(ref), isExactMatch); } void RemoveSurface(const ImageKey aImageKey, @@ -970,34 +979,34 @@ SurfaceCache::Shutdown() sInstance = nullptr; } -/* static */ DrawableFrameRef +/* static */ LookupResult SurfaceCache::Lookup(const ImageKey aImageKey, const SurfaceKey& aSurfaceKey, const Maybe& aAlternateFlags /* = Nothing() */) { if (!sInstance) { - return DrawableFrameRef(); + return LookupResult(); } MutexAutoLock lock(sInstance->GetMutex()); - DrawableFrameRef ref = sInstance->Lookup(aImageKey, aSurfaceKey); - if (!ref && aAlternateFlags) { - ref = sInstance->Lookup(aImageKey, - aSurfaceKey.WithNewFlags(*aAlternateFlags)); + LookupResult result = sInstance->Lookup(aImageKey, aSurfaceKey); + if (!result && aAlternateFlags) { + result = sInstance->Lookup(aImageKey, + aSurfaceKey.WithNewFlags(*aAlternateFlags)); } - return ref; + return result; } -/* static */ DrawableFrameRef +/* static */ LookupResult SurfaceCache::LookupBestMatch(const ImageKey aImageKey, const SurfaceKey& aSurfaceKey, const Maybe& aAlternateFlags /* = Nothing() */) { if (!sInstance) { - return DrawableFrameRef(); + return LookupResult(); } MutexAutoLock lock(sInstance->GetMutex()); diff --git a/image/SurfaceCache.h b/image/SurfaceCache.h index bd600d17e71..cedf38d0837 100644 --- a/image/SurfaceCache.h +++ b/image/SurfaceCache.h @@ -24,9 +24,9 @@ namespace mozilla { namespace image { -class DrawableFrameRef; class Image; class imgFrame; +class LookupResult; struct SurfaceMemoryCounter; /* @@ -174,7 +174,7 @@ struct SurfaceCache * * If the imgFrame was found in the cache, but had stored its surface in a * volatile buffer which was discarded by the OS, then it is automatically - * removed from the cache and an empty DrawableFrameRef is returned. Note that + * removed from the cache and an empty LookupResult is returned. Note that * this will never happen to persistent surfaces associated with a locked * image; the cache keeps a strong reference to such surfaces internally. * @@ -190,14 +190,13 @@ struct SurfaceCache * than calling Lookup() twice, which requires taking a * lock each time. * - * @return a DrawableFrameRef to the imgFrame wrapping the - * requested surface, or an empty DrawableFrameRef if - * not found. + * @return a LookupResult, which will either contain a + * DrawableFrameRef to the requested surface, or an + * empty DrawableFrameRef if the surface was not found. */ - static DrawableFrameRef Lookup(const ImageKey aImageKey, - const SurfaceKey& aSurfaceKey, - const Maybe& aAlternateFlags - = Nothing()); + static LookupResult Lookup(const ImageKey aImageKey, + const SurfaceKey& aSurfaceKey, + const Maybe& aAlternateFlags = Nothing()); /** * Looks up the best matching surface in the cache and returns a drawable @@ -216,13 +215,17 @@ struct SurfaceCache * acceptable to the caller. This is much more * efficient than calling LookupBestMatch() twice. * - * @return a DrawableFrameRef to the imgFrame wrapping a surface similar to - * the requested surface, or an empty DrawableFrameRef if not found. + * @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 DrawableFrameRef LookupBestMatch(const ImageKey aImageKey, - const SurfaceKey& aSurfaceKey, - const Maybe& aAlternateFlags - = Nothing()); + static LookupResult LookupBestMatch(const ImageKey aImageKey, + const SurfaceKey& aSurfaceKey, + const Maybe& aAlternateFlags + = Nothing()); /** * Insert a surface into the cache. If a surface with the same ImageKey and diff --git a/image/VectorImage.cpp b/image/VectorImage.cpp index 469ea1b9fcc..5c60a7ce5fd 100644 --- a/image/VectorImage.cpp +++ b/image/VectorImage.cpp @@ -27,6 +27,7 @@ #include "nsSVGEffects.h" // for nsSVGRenderingObserver #include "nsWindowMemoryReporter.h" #include "ImageRegion.h" +#include "LookupResult.h" #include "Orientation.h" #include "SVGDocumentWrapper.h" #include "nsIDOMEventListener.h" @@ -829,18 +830,18 @@ VectorImage::Draw(gfxContext* aContext, return DrawResult::SUCCESS; } - DrawableFrameRef frameRef = + LookupResult result = SurfaceCache::Lookup(ImageKey(this), VectorSurfaceKey(params.size, params.svgContext, params.animationTime)); // Draw. - if (frameRef) { - RefPtr surface = frameRef->GetSurface(); + if (result) { + RefPtr surface = result.DrawableRef()->GetSurface(); if (surface) { nsRefPtr svgDrawable = - new gfxSurfaceDrawable(surface, frameRef->GetSize()); + new gfxSurfaceDrawable(surface, result.DrawableRef()->GetSize()); Show(svgDrawable, params); return DrawResult::SUCCESS; } diff --git a/image/imgFrame.h b/image/imgFrame.h index 43b212edb10..460ff837b81 100644 --- a/image/imgFrame.h +++ b/image/imgFrame.h @@ -444,6 +444,8 @@ public: } private: + DrawableFrameRef(const DrawableFrameRef& aOther) = delete; + nsRefPtr mFrame; VolatileBufferPtr mRef; }; @@ -526,6 +528,8 @@ public: } private: + RawAccessFrameRef(const RawAccessFrameRef& aOther) = delete; + nsRefPtr mFrame; };