Skip to content

Commit

Permalink
Layout animated GIFs only once (#143188)
Browse files Browse the repository at this point in the history
Fixes flutter/flutter#138610.

When `RenderImage` receives a new `Image` it only needs to fire up the layout machinery when the dimensions of the image have changed compared to the previous image. If the dimensions are the same, a repaint is sufficient.
  • Loading branch information
goderbauer authored Feb 9, 2024
1 parent 5f1a3c1 commit 0aa9b5e
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 1 deletion.
3 changes: 2 additions & 1 deletion packages/flutter/lib/src/rendering/image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,11 @@ class RenderImage extends RenderBox {
value.dispose();
return;
}
final bool sizeChanged = _image?.width != value?.width || _image?.height != value?.height;
_image?.dispose();
_image = value;
markNeedsPaint();
if (_width == null || _height == null) {
if (sizeChanged && (_width == null || _height == null)) {
markNeedsLayout();
}
}
Expand Down
55 changes: 55 additions & 0 deletions packages/flutter/test/widgets/image_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2065,6 +2065,61 @@ void main() {
: isNot(throwsA(anything)),
);
});

testWidgets('Animated GIFs do not require layout for subsequent frames', (WidgetTester tester) async {
final ui.Codec codec = (await tester.runAsync(() {
return ui.instantiateImageCodec(Uint8List.fromList(kAnimatedGif));
}))!;

Future<ui.Image> nextFrame() async {
final ui.FrameInfo frameInfo = (await tester.runAsync(codec.getNextFrame))!;
return frameInfo.image;
}

final _TestImageStreamCompleter streamCompleter = _TestImageStreamCompleter();
final _TestImageProvider imageProvider = _TestImageProvider(streamCompleter: streamCompleter);
int? lastFrame;

await tester.pumpWidget(
Center(
child: Image(
image: imageProvider,
frameBuilder: (BuildContext context, Widget child, int? frame, bool wasSynchronouslyLoaded) {
lastFrame = frame;
return child;
},
),
),
);

expect(tester.getSize(find.byType(Image)), Size.zero);

streamCompleter.setData(imageInfo: ImageInfo(image: await nextFrame()));
await tester.pump();
expect(lastFrame, 0);
expect(tester.allRenderObjects.whereType<RenderImage>().single.debugNeedsLayout, isFalse);
expect(tester.allRenderObjects.whereType<RenderImage>().single.debugNeedsPaint, isFalse);
expect(tester.getSize(find.byType(Image)), const Size(1, 1));

streamCompleter.setData(imageInfo: ImageInfo(image: await nextFrame()));
// We only complete the build phase and expect that it does not mark the
// RenderImage for layout because the new frame has the same dimensions as
// the old one. We only need to repaint.
await tester.pump(null, EnginePhase.build);
expect(lastFrame, 1);
expect(tester.allRenderObjects.whereType<RenderImage>().single.debugNeedsLayout, isFalse);
expect(tester.allRenderObjects.whereType<RenderImage>().single.debugNeedsPaint, isTrue);
expect(tester.getSize(find.byType(Image)), const Size(1, 1));

streamCompleter.setData(imageInfo: ImageInfo(image: await nextFrame()));
await tester.pump();
expect(lastFrame, 2);
expect(tester.allRenderObjects.whereType<RenderImage>().single.debugNeedsLayout, isFalse);
expect(tester.allRenderObjects.whereType<RenderImage>().single.debugNeedsPaint, isFalse);
expect(tester.getSize(find.byType(Image)), const Size(1, 1));

codec.dispose();
});
}

@immutable
Expand Down

0 comments on commit 0aa9b5e

Please sign in to comment.