Only overflow the cache for one required frame (flutter#7048)

Previously codec.cc needed all required frames to already be decoded
before it could decode any of their dependent frames. To accomplish this
it would always cache required frames, regardless of cache limit.

However both GIF and WEBP (the only currently supported animated image
formats) only allow the image to depend on one decoded frame at a time.
This means that there's no reason to cache all the required frames since
it's only valid for the image formats to require one previously decoded
frame at a time. (For example, frame 10 and frame 11 in a hypothetical
animated image could all depend on frame 9. But no subsequent frame
after frame 9 could depend on frames 0-8.)

Frames are always added to the cache as long as they're under the limit,
and never removed. Required frames are always stored as a separate
member on `MultiFrameCodec`.

Warning: this logic will break if we decide to support more animated
formats in the future.

Fixes flutter/flutter#24835.

lib/ui/painting/codec.cc

@@ -371,20 +371,16 @@ MultiFrameCodec::MultiFrameCodec(std::unique_ptr<SkCodec> codec,
   compressedSizeBytes_ = codec_->getInfo().computeMinByteSize();
   frameBitmaps_.clear();
   decodedCacheSize_ = 0;
-  // Initialize the frame cache, marking frames that are required for other
-  // dependent frames to render.
+  nextFrameIndex_ = 0;
+  // Go through our frame information and mark which frames are required in
+  // order to decode subsequent ones.
+  requiredFrames_.clear();
   for (size_t frameIndex = 0; frameIndex < frameInfos_.size(); frameIndex++) {
-    const auto& frameInfo = frameInfos_[frameIndex];
-    if (frameInfo.fRequiredFrame != SkCodec::kNoFrame) {
-      frameBitmaps_[frameInfo.fRequiredFrame] =
-          std::make_unique<DecodedFrame>(/*required=*/true);
-    }
-    if (frameBitmaps_.count(frameIndex) < 1) {
-      frameBitmaps_[frameIndex] =
-          std::make_unique<DecodedFrame>(/*required=*/false);
+    const int requiredFrame = frameInfos_[frameIndex].fRequiredFrame;
+    if (requiredFrame != SkCodec::kNoFrame) {
+      requiredFrames_[requiredFrame] = true;
     }
   }
-  nextFrameIndex_ = 0;
 }
 
 MultiFrameCodec::~MultiFrameCodec() {}
@@ -400,28 +396,29 @@ int MultiFrameCodec::repetitionCount() {
 sk_sp<SkImage> MultiFrameCodec::GetNextFrameImage(
     fml::WeakPtr<GrContext> resourceContext) {
   // Populate this bitmap from the cache if it exists
-  DecodedFrame& cacheEntry = *frameBitmaps_[nextFrameIndex_];
-  SkBitmap bitmap =
-      cacheEntry.bitmap_ != nullptr ? *cacheEntry.bitmap_ : SkBitmap();
-  if (!bitmap.getPixels()) {  // We haven't decoded this frame yet
+  SkBitmap bitmap = frameBitmaps_[nextFrameIndex_] != nullptr
+                        ? *frameBitmaps_[nextFrameIndex_]
+                        : SkBitmap();
+  const bool frameAlreadyCached = bitmap.getPixels();
+  if (!frameAlreadyCached) {
     const SkImageInfo info = codec_->getInfo().makeColorType(kN32_SkColorType);
     bitmap.allocPixels(info);
 
     SkCodec::Options options;
     options.fFrameIndex = nextFrameIndex_;
-    const int requiredFrame = frameInfos_[nextFrameIndex_].fRequiredFrame;
-    if (requiredFrame != SkCodec::kNoFrame) {
-      const SkBitmap* requiredBitmap =
-          frameBitmaps_[requiredFrame]->bitmap_.get();
-      if (requiredBitmap == nullptr) {
+    const int requiredFrameIndex = frameInfos_[nextFrameIndex_].fRequiredFrame;
+    if (requiredFrameIndex != SkCodec::kNoFrame) {
+      if (lastRequiredFrame_ == nullptr ||
+          lastRequiredFrameIndex_ != requiredFrameIndex) {
         FML_LOG(ERROR) << "Frame " << nextFrameIndex_ << " depends on frame "
-                       << requiredFrame << " which has not been cached.";
+                       << requiredFrameIndex << " which has not been cached.";
         return NULL;
       }
 
-      if (requiredBitmap->getPixels() &&
-          copy_to(&bitmap, requiredBitmap->colorType(), *requiredBitmap)) {
-        options.fPriorFrame = requiredFrame;
+      if (lastRequiredFrame_->getPixels() &&
+          copy_to(&bitmap, lastRequiredFrame_->colorType(),
+                  *lastRequiredFrame_)) {
+        options.fPriorFrame = requiredFrameIndex;
       }
     }
 
@@ -431,17 +428,23 @@ sk_sp<SkImage> MultiFrameCodec::GetNextFrameImage(
       return NULL;
     }
 
-    // Cache the bitmap if this is a required frame or if we're still under our
-    // ratio cap.
     const size_t cachedFrameSize = bitmap.computeByteSize();
-    if (cacheEntry.required_ ||
-        ((decodedCacheSize_ + cachedFrameSize) / compressedSizeBytes_) <=
-            decodedCacheRatioCap_) {
-      cacheEntry.bitmap_ = std::make_unique<SkBitmap>(bitmap);
+    const bool shouldCache = ((decodedCacheSize_ + cachedFrameSize) /
+                              compressedSizeBytes_) <= decodedCacheRatioCap_;
+    if (shouldCache) {
+      frameBitmaps_[nextFrameIndex_] = std::make_shared<SkBitmap>(bitmap);
       decodedCacheSize_ += cachedFrameSize;
     }
   }
 
+  // Hold onto this if we need it to decode future frames.
+  if (requiredFrames_[nextFrameIndex_]) {
+    lastRequiredFrame_ = frameAlreadyCached
+                             ? frameBitmaps_[nextFrameIndex_]
+                             : std::make_shared<SkBitmap>(bitmap);
+    lastRequiredFrameIndex_ = nextFrameIndex_;
+  }
+
   if (resourceContext) {
     SkPixmap pixmap(bitmap.info(), bitmap.pixelRef()->pixels(),
                     bitmap.pixelRef()->rowBytes());
@@ -509,11 +512,6 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) {
   return Dart_Null();
 }
 
-MultiFrameCodec::DecodedFrame::DecodedFrame(bool required)
-    : required_(required) {}
-
-MultiFrameCodec::DecodedFrame::~DecodedFrame() = default;
-
 SingleFrameCodec::SingleFrameCodec(fml::RefPtr<FrameInfo> frame)
     : frame_(std::move(frame)) {}
 

lib/ui/painting/codec.h

@@ -65,21 +65,16 @@ class MultiFrameCodec : public Codec {
   size_t decodedCacheSize_;
 
   std::vector<SkCodec::FrameInfo> frameInfos_;
-  // A struct linking the bitmap of a frame to whether it's required to render
-  // other dependent frames.
-  struct DecodedFrame {
-    std::unique_ptr<SkBitmap> bitmap_ = nullptr;
-    const bool required_;
-
-    DecodedFrame(bool required);
-    ~DecodedFrame();
-  };
+  std::map<int, bool> requiredFrames_;
 
   // A cache of previously loaded bitmaps, indexed by the frame they belong to.
-  // Always holds at least the frames marked as required for reuse by
-  // [SkCodec::getFrameInfo()]. Will cache other non-essential frames until
-  // [decodedCacheSize_] : [compressedSize_] exceeds [decodedCacheRatioCap_].
-  std::map<int, std::unique_ptr<DecodedFrame>> frameBitmaps_;
+  // Caches all frames until [decodedCacheSize_] : [compressedSize_] exceeds
+  // [decodedCacheRatioCap_].
+  std::map<int, std::shared_ptr<SkBitmap>> frameBitmaps_;
+  // The last decoded frame that's required to decode any subsequent frames.
+  std::shared_ptr<SkBitmap> lastRequiredFrame_;
+  // The index of the last decoded required frame.
+  int lastRequiredFrameIndex_ = -1;
 
   FML_FRIEND_MAKE_REF_COUNTED(MultiFrameCodec);
   FML_FRIEND_REF_COUNTED_THREAD_SAFE(MultiFrameCodec);