From 8e3ea14787af68d2f69ad1ca1d089f29850dd485 Mon Sep 17 00:00:00 2001 From: fzyzcjy <5236035+fzyzcjy@users.noreply.github.com> Date: Tue, 15 Nov 2022 09:39:57 +0800 Subject: [PATCH] Incorrect rendering of `SnapshotWidget` (#114400) * add tests * try to fix * fix compile error * refactor code * rename * doc * refactor test * simple rename --- .../src/material/page_transitions_theme.dart | 4 +- .../lib/src/widgets/snapshot_widget.dart | 27 +++++++++---- .../test/widgets/snapshot_widget_test.dart | 40 ++++++++++++++++++- 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/packages/flutter/lib/src/material/page_transitions_theme.dart b/packages/flutter/lib/src/material/page_transitions_theme.dart index c2ff2098acbe..b3ea77b21be9 100644 --- a/packages/flutter/lib/src/material/page_transitions_theme.dart +++ b/packages/flutter/lib/src/material/page_transitions_theme.dart @@ -907,7 +907,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter { } @override - void paintSnapshot(PaintingContext context, Offset offset, Size size, ui.Image image, double pixelRatio) { + void paintSnapshot(PaintingContext context, Offset offset, Size size, ui.Image image, Size sourceSize, double pixelRatio) { _drawScrim(context, offset, size); _drawImageScaledAndCentered(context, image, scale.value, fade.value, pixelRatio); } @@ -957,7 +957,7 @@ class _ZoomExitTransitionPainter extends SnapshotPainter { final LayerHandle _transformHandler = LayerHandle(); @override - void paintSnapshot(PaintingContext context, Offset offset, Size size, ui.Image image, double pixelRatio) { + void paintSnapshot(PaintingContext context, Offset offset, Size size, ui.Image image, Size sourceSize, double pixelRatio) { _drawImageScaledAndCentered(context, image, scale.value, fade.value, pixelRatio); } diff --git a/packages/flutter/lib/src/widgets/snapshot_widget.dart b/packages/flutter/lib/src/widgets/snapshot_widget.dart index 43307d7eb12c..d7d6a33b0c4a 100644 --- a/packages/flutter/lib/src/widgets/snapshot_widget.dart +++ b/packages/flutter/lib/src/widgets/snapshot_widget.dart @@ -231,6 +231,7 @@ class _RenderSnapshotWidget extends RenderProxyBox { } ui.Image? _childRaster; + Size? _childRasterSize; // Set to true if the snapshot mode was not forced and a platform view // was encountered while attempting to snapshot the child. bool _disableSnapshotAttempt = false; @@ -249,6 +250,7 @@ class _RenderSnapshotWidget extends RenderProxyBox { painter.removeListener(markNeedsPaint); _childRaster?.dispose(); _childRaster = null; + _childRasterSize = null; super.detach(); } @@ -258,6 +260,7 @@ class _RenderSnapshotWidget extends RenderProxyBox { painter.removeListener(markNeedsPaint); _childRaster?.dispose(); _childRaster = null; + _childRasterSize = null; super.dispose(); } @@ -265,6 +268,7 @@ class _RenderSnapshotWidget extends RenderProxyBox { _disableSnapshotAttempt = false; _childRaster?.dispose(); _childRaster = null; + _childRasterSize = null; markNeedsPaint(); } @@ -296,19 +300,24 @@ class _RenderSnapshotWidget extends RenderProxyBox { if (size.isEmpty) { _childRaster?.dispose(); _childRaster = null; + _childRasterSize = null; return; } if (!controller.allowSnapshotting || _disableSnapshotAttempt) { _childRaster?.dispose(); _childRaster = null; + _childRasterSize = null; painter.paint(context, offset, size, super.paint); return; } - _childRaster ??= _paintAndDetachToImage(); + if (_childRaster == null) { + _childRaster = _paintAndDetachToImage(); + _childRasterSize = size * devicePixelRatio; + } if (_childRaster == null) { painter.paint(context, offset, size, super.paint); } else { - painter.paintSnapshot(context, offset, size, _childRaster!, devicePixelRatio); + painter.paintSnapshot(context, offset, size, _childRaster!, _childRasterSize!, devicePixelRatio); } } } @@ -356,11 +365,13 @@ abstract class SnapshotPainter extends ChangeNotifier { /// [SnapshotPainter] paints the snapshot. This must account for the fact that the image /// width and height will be given in physical pixels, while the image must be painted with /// device independent pixels. That is, the width and height of the image is the widget and - /// height of the provided `size`, multiplied by the `pixelRatio`: + /// height of the provided `size`, multiplied by the `pixelRatio`. In addition, the actual + /// size of the scene captured by the `image` is not `image.width` or `image.height`, but + /// indeed `sourceSize`, because the former is a rounded inaccurate integer: /// /// ```dart - /// void paint(PaintingContext context, Offset offset, Size size, ui.Image image, double pixelRatio) { - /// final Rect src = Rect.fromLTWH(0, 0, image.width.toDouble(), image.height.toDouble()); + /// void paint(PaintingContext context, Offset offset, Size size, ui.Image image, Size sourceSize, double pixelRatio) { + /// final Rect src = Rect.fromLTWH(0, 0, sourceSize.width, sourceSize.height); /// final Rect dst = Rect.fromLTWH(offset.dx, offset.dy, size.width, size.height); /// final Paint paint = Paint() /// ..filterQuality = FilterQuality.low; @@ -368,7 +379,7 @@ abstract class SnapshotPainter extends ChangeNotifier { /// } /// ``` /// {@end-tool} - void paintSnapshot(PaintingContext context, Offset offset, Size size, ui.Image image, double pixelRatio); + void paintSnapshot(PaintingContext context, Offset offset, Size size, ui.Image image, Size sourceSize, double pixelRatio); /// Paint the child via [painter], applying any effects that would have been painted /// in [SnapshotPainter.paintSnapshot]. @@ -427,8 +438,8 @@ class _DefaultSnapshotPainter implements SnapshotPainter { } @override - void paintSnapshot(PaintingContext context, ui.Offset offset, ui.Size size, ui.Image image, double pixelRatio) { - final Rect src = Rect.fromLTWH(0, 0, image.width.toDouble(), image.height.toDouble()); + void paintSnapshot(PaintingContext context, ui.Offset offset, ui.Size size, ui.Image image, Size sourceSize, double pixelRatio) { + final Rect src = Rect.fromLTWH(0, 0, sourceSize.width, sourceSize.height); final Rect dst = Rect.fromLTWH(offset.dx, offset.dy, size.width, size.height); final Paint paint = Paint() ..filterQuality = FilterQuality.low; diff --git a/packages/flutter/test/widgets/snapshot_widget_test.dart b/packages/flutter/test/widgets/snapshot_widget_test.dart index b3f99e75d348..030944c0d665 100644 --- a/packages/flutter/test/widgets/snapshot_widget_test.dart +++ b/packages/flutter/test/widgets/snapshot_widget_test.dart @@ -9,8 +9,8 @@ import 'dart:ui' as ui; import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; -import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; void main() { @@ -261,6 +261,42 @@ void main() { expect(tester.takeException(), isNull); expect(tester.layers.last, isA()); }, skip: kIsWeb); // TODO(jonahwilliams): https://github.com/flutter/flutter/issues/106689 + + testWidgets('SnapshotWidget should have same result when enabled', (WidgetTester tester) async { + tester.binding.window + ..physicalSizeTestValue = const Size(10, 10) + ..devicePixelRatioTestValue = 1; + addTearDown(() => tester.binding.window + ..clearPhysicalSizeTestValue() + ..clearDevicePixelRatioTestValue()); + + const ValueKey repaintBoundaryKey = ValueKey('boundary'); + final SnapshotController controller = SnapshotController(); + await tester.pumpWidget(RepaintBoundary( + key: repaintBoundaryKey, + child: MaterialApp( + debugShowCheckedModeBanner: false, + home: Container( + color: Colors.black, + padding: const EdgeInsets.only(right: 0.6, bottom: 0.6), + child: SnapshotWidget( + controller: controller, + child: Container( + margin: const EdgeInsets.only(right: 0.4, bottom: 0.4), + color: Colors.blue, + ), + ), + ), + ), + )); + + final ui.Image imageWhenDisabled = (tester.renderObject(find.byKey(repaintBoundaryKey)) as RenderRepaintBoundary).toImageSync(); + + controller.allowSnapshotting = true; + await tester.pump(); + + await expectLater(find.byKey(repaintBoundaryKey), matchesReferenceImage(imageWhenDisabled)); + }, skip: kIsWeb); // TODO(jonahwilliams): https://github.com/flutter/flutter/issues/106689 } class TestController extends SnapshotController { @@ -321,7 +357,7 @@ class TestPainter extends SnapshotPainter { } @override - void paintSnapshot(PaintingContext context, Offset offset, Size size, ui.Image image, double pixelRatio) { + void paintSnapshot(PaintingContext context, Offset offset, Size size, ui.Image image, Size sourceSize, double pixelRatio) { count += 1; lastImage = image; }