diff --git a/lib/ui/fixtures/2_dispose_op_restore_previous.apng b/lib/ui/fixtures/2_dispose_op_restore_previous.apng new file mode 100644 index 0000000000000..54f73736abc49 Binary files /dev/null and b/lib/ui/fixtures/2_dispose_op_restore_previous.apng differ diff --git a/lib/ui/fixtures/2_dispose_op_restore_previous.apng.67.png b/lib/ui/fixtures/2_dispose_op_restore_previous.apng.67.png new file mode 100644 index 0000000000000..5410b759c04f4 Binary files /dev/null and b/lib/ui/fixtures/2_dispose_op_restore_previous.apng.67.png differ diff --git a/lib/ui/fixtures/2_dispose_op_restore_previous.apng.68.png b/lib/ui/fixtures/2_dispose_op_restore_previous.apng.68.png new file mode 100644 index 0000000000000..5f08106ccef06 Binary files /dev/null and b/lib/ui/fixtures/2_dispose_op_restore_previous.apng.68.png differ diff --git a/lib/ui/fixtures/2_dispose_op_restore_previous.apng.69.png b/lib/ui/fixtures/2_dispose_op_restore_previous.apng.69.png new file mode 100644 index 0000000000000..5335435864c99 Binary files /dev/null and b/lib/ui/fixtures/2_dispose_op_restore_previous.apng.69.png differ diff --git a/lib/ui/painting/image_generator_apng.cc b/lib/ui/painting/image_generator_apng.cc index 7f21555c968f9..1a67c6482cf4b 100644 --- a/lib/ui/painting/image_generator_apng.cc +++ b/lib/ui/painting/image_generator_apng.cc @@ -538,19 +538,19 @@ bool APNGImageGenerator::DemuxNextImageInternal() { if (!last_frame_info.has_value()) { return false; } - if (last_frame_info->disposal_method == - SkCodecAnimation::DisposalMethod::kRestorePrevious) { - FML_DLOG(INFO) - << "DisposalMethod::kRestorePrevious is not supported by the " - "MultiFrameCodec. Falling back to DisposalMethod::kRestoreBGColor " - " behavior instead."; - } if (images_.size() > first_frame_index_ && last_frame_info->disposal_method == SkCodecAnimation::DisposalMethod::kKeep) { // Mark the required frame as the previous frame in all cases. image->frame_info->required_frame = images_.size() - 1; + } else if (images_.size() > (first_frame_index_ + 1) && + last_frame_info->disposal_method == + SkCodecAnimation::DisposalMethod::kRestorePrevious) { + // Mark the required frame as the last previous frame + // It is not valid if there are 2 or above frames set |disposal_method| to + // |kRestorePrevious|. But it also works in MultiFrameCodec. + image->frame_info->required_frame = images_.size() - 2; } // Calling SkCodec::getInfo at least once prior to decoding is mandatory. diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 784355b84dd33..c66da59bf288c 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -116,9 +116,9 @@ MultiFrameCodec::State::GetNextFrameImage( std::optional prior_frame_index = std::nullopt; if (requiredFrameIndex != SkCodec::kNoFrame) { - // We currently assume that frames can only ever depend on the immediately - // previous frame, if any. This means that - // `DisposalMethod::kRestorePrevious` is not supported. + // We are here when the frame said |disposal_method| is + // `DisposalMethod::kKeep` or `DisposalMethod::kRestorePrevious` and + // |requiredFrameIndex| is set to ex-frame or ex-ex-frame. if (lastRequiredFrame_ == nullptr) { FML_DLOG(INFO) << "Frame " << nextFrameIndex_ << " depends on frame " @@ -146,9 +146,27 @@ MultiFrameCodec::State::GetNextFrameImage( return std::make_pair(nullptr, decode_error); } - // Hold onto this if we need it to decode future frames. - if (frameInfo.disposal_method == SkCodecAnimation::DisposalMethod::kKeep || - lastRequiredFrame_) { + const bool keep_current_frame = + frameInfo.disposal_method == SkCodecAnimation::DisposalMethod::kKeep; + const bool restore_previous_frame = + frameInfo.disposal_method == + SkCodecAnimation::DisposalMethod::kRestorePrevious; + const bool previous_frame_available = !!lastRequiredFrame_; + + // Store the current frame in `lastRequiredFrame_` if the frame's disposal + // method indicates we should do so. + // * When the disposal method is "Keep", the stored frame should always be + // overwritten with the new frame we just crafted. + // * When the disposal method is "RestorePrevious", the previously stored + // frame should be retained and used as the backdrop for the next frame + // again. If there isn't already a stored frame, that means we haven't + // rendered any frames yet! When this happens, we just fall back to "Keep" + // behavior and store the current frame as the backdrop of the next frame. + + if (keep_current_frame || + (previous_frame_available && !restore_previous_frame)) { + // Replace the stored frame. The `lastRequiredFrame_` will get used as the + // starting backdrop for the next frame. lastRequiredFrame_ = std::make_unique(bitmap); lastRequiredFrameIndex_ = nextFrameIndex_; } diff --git a/testing/dart/codec_test.dart b/testing/dart/codec_test.dart index ba63a592a36ba..db37bb662a047 100644 --- a/testing/dart/codec_test.dart +++ b/testing/dart/codec_test.dart @@ -177,6 +177,31 @@ void main() { expect(imageData.buffer.asUint8List(), goldenData); }); + + test('Animated apng can reuse pre-pre-frame', () async { + // https://github.com/flutter/engine/pull/42153 + + final Uint8List data = File( + path.join('flutter', 'lib', 'ui', 'fixtures', '2_dispose_op_restore_previous.apng'), + ).readAsBytesSync(); + final ui.Codec codec = await ui.instantiateImageCodec(data); + + // Capture the 67,68,69 frames of animation and then compare the pixels. + late ui.FrameInfo frameInfo; + for (int i = 0; i < 70; i++) { + frameInfo = await codec.getNextFrame(); + if (i >= 67) { + final ui.Image image = frameInfo.image; + final ByteData imageData = (await image.toByteData(format: ui.ImageByteFormat.png))!; + + final Uint8List goldenData = File( + path.join('flutter', 'lib', 'ui', 'fixtures', '2_dispose_op_restore_previous.apng.$i.png'), + ).readAsBytesSync(); + + expect(imageData.buffer.asUint8List(), goldenData); + } + } + }); } /// Returns a File handle to a file in the skia/resources directory.