Skip to content

Commit

Permalink
Bug 1176124 (Part 1) - Add a MatchType enum to LookupResult to let Lo…
Browse files Browse the repository at this point in the history
…okup*() return more detailed information. r=dholbert
  • Loading branch information
sethfowler committed Jul 20, 2015
1 parent 35f2e76 commit f811aad
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 24 deletions.
3 changes: 1 addition & 2 deletions image/FrameAnimator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,7 @@ FrameAnimator::GetCompositedFrame(uint32_t aFrameNum)

// If we have a composited version of this frame, return that.
if (mLastCompositedFrameIndex == int32_t(aFrameNum)) {
return LookupResult(mCompositingFrame->DrawableRef(),
/* aIsExactMatch = */ true);
return LookupResult(mCompositingFrame->DrawableRef(), MatchType::EXACT);
}

// Otherwise return the raw frame. DoBlend is required to ensure that we only
Expand Down
43 changes: 32 additions & 11 deletions image/LookupResult.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,53 @@
namespace mozilla {
namespace image {

enum class MatchType : uint8_t
{
NOT_FOUND, // No matching surface and no placeholder.
PENDING, // Found a matching placeholder, but no surface.
EXACT, // Found a surface that matches exactly.
SUBSTITUTE_BECAUSE_NOT_FOUND, // No exact match, but found a similar one.
SUBSTITUTE_BECAUSE_PENDING // Found a similar surface and a placeholder
// for an exact match.
};

/**
* 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)
{ }
explicit LookupResult(MatchType aMatchType)
: mMatchType(aMatchType)
{
MOZ_ASSERT(mMatchType == MatchType::NOT_FOUND ||
mMatchType == MatchType::PENDING,
"Only NOT_FOUND or PENDING make sense with no surface");
}

LookupResult(LookupResult&& aOther)
: mDrawableRef(Move(aOther.mDrawableRef))
, mIsExactMatch(aOther.mIsExactMatch)
, mMatchType(aOther.mMatchType)
{ }

LookupResult(DrawableFrameRef&& aDrawableRef, bool aIsExactMatch)
LookupResult(DrawableFrameRef&& aDrawableRef, MatchType aMatchType)
: mDrawableRef(Move(aDrawableRef))
, mIsExactMatch(aIsExactMatch)
{ }
, mMatchType(aMatchType)
{
MOZ_ASSERT(!mDrawableRef || !(mMatchType == MatchType::NOT_FOUND ||
mMatchType == MatchType::PENDING),
"Only NOT_FOUND or PENDING make sense with no surface");
MOZ_ASSERT(mDrawableRef || mMatchType == MatchType::NOT_FOUND ||
mMatchType == MatchType::PENDING,
"NOT_FOUND or PENDING do not make sense with a surface");
}

LookupResult& operator=(LookupResult&& aOther)
{
MOZ_ASSERT(&aOther != this, "Self-move-assignment is not supported");
mDrawableRef = Move(aOther.mDrawableRef);
mIsExactMatch = aOther.mIsExactMatch;
mMatchType = aOther.mMatchType;
return *this;
}

Expand All @@ -53,14 +74,14 @@ class MOZ_STACK_CLASS LookupResult
/// @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; }
/// @return what kind of match this is (exact, substitute, etc.)
MatchType Type() const { return mMatchType; }

private:
LookupResult(const LookupResult&) = delete;

DrawableFrameRef mDrawableRef;
bool mIsExactMatch;
MatchType mMatchType;
};

} // namespace image
Expand Down
5 changes: 3 additions & 2 deletions image/RasterImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,9 @@ RasterImage::LookupFrame(uint32_t aFrameNum,
return DrawableFrameRef();
}

if (!result || !result.IsExactMatch()) {
// The OS threw this frame away. We need to redecode if we can.
if (!result || result.Type() == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND) {
// We don't have a copy of this frame, and there's no decoder working on
// one. Trigger decoding so it'll be available next time.
MOZ_ASSERT(!mAnim, "Animated frames should be locked");

Decode(Some(requestedSize), aFlags);
Expand Down
26 changes: 17 additions & 9 deletions image/SurfaceCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -550,27 +550,31 @@ class SurfaceCacheImpl final : public nsIMemoryReporter
{
nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
if (!cache) {
return LookupResult(); // No cached surfaces for this image.
// No cached surfaces for this image.
return LookupResult(MatchType::NOT_FOUND);
}

nsRefPtr<CachedSurface> surface = cache->Lookup(aSurfaceKey);
if (!surface) {
return LookupResult(); // Lookup in the per-image cache missed.
// Lookup in the per-image cache missed.
return LookupResult(MatchType::NOT_FOUND);
}

DrawableFrameRef ref = surface->DrawableRef();
if (!ref) {
// The surface was released by the operating system. Remove the cache
// entry as well.
Remove(surface);
return LookupResult();
return LookupResult(MatchType::NOT_FOUND);
}

if (aMarkUsed) {
MarkUsed(surface, cache);
}

return LookupResult(Move(ref), /* aIsExactMatch = */ true);
MOZ_ASSERT(surface->GetSurfaceKey() == aSurfaceKey,
"Lookup() not returning an exact match?");
return LookupResult(Move(ref), MatchType::EXACT);
}

LookupResult LookupBestMatch(const ImageKey aImageKey,
Expand All @@ -579,7 +583,8 @@ class SurfaceCacheImpl final : public nsIMemoryReporter
{
nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
if (!cache) {
return LookupResult(); // No cached surfaces for this image.
// No cached surfaces for this image.
return LookupResult(MatchType::NOT_FOUND);
}

// Repeatedly look up the best match, trying again if the resulting surface
Expand All @@ -593,7 +598,8 @@ class SurfaceCacheImpl final : public nsIMemoryReporter
while (true) {
surface = cache->LookupBestMatch(aSurfaceKey, aAlternateFlags);
if (!surface) {
return LookupResult(); // Lookup in the per-image cache missed.
// Lookup in the per-image cache missed.
return LookupResult(MatchType::NOT_FOUND);
}

ref = surface->DrawableRef();
Expand All @@ -619,7 +625,9 @@ class SurfaceCacheImpl final : public nsIMemoryReporter
MarkUsed(surface, cache);
}

return LookupResult(Move(ref), isExactMatch);
MatchType matchType = isExactMatch ? MatchType::EXACT
: MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
return LookupResult(Move(ref), matchType);
}

void RemoveSurface(const ImageKey aImageKey,
Expand Down Expand Up @@ -992,7 +1000,7 @@ SurfaceCache::Lookup(const ImageKey aImageKey,
const Maybe<uint32_t>& aAlternateFlags /* = Nothing() */)
{
if (!sInstance) {
return LookupResult();
return LookupResult(MatchType::NOT_FOUND);
}

MutexAutoLock lock(sInstance->GetMutex());
Expand All @@ -1013,7 +1021,7 @@ SurfaceCache::LookupBestMatch(const ImageKey aImageKey,
/* = Nothing() */)
{
if (!sInstance) {
return LookupResult();
return LookupResult(MatchType::NOT_FOUND);
}

MutexAutoLock lock(sInstance->GetMutex());
Expand Down

0 comments on commit f811aad

Please sign in to comment.