Unverified Commit 8e3ea147 authored by fzyzcjy's avatar fzyzcjy Committed by GitHub

Incorrect rendering of `SnapshotWidget` (#114400)

* add tests

* try to fix

* fix compile error

* refactor code

* rename

* doc

* refactor test

* simple rename
parent 2e51077d
...@@ -907,7 +907,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter { ...@@ -907,7 +907,7 @@ class _ZoomEnterTransitionPainter extends SnapshotPainter {
} }
@override @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); _drawScrim(context, offset, size);
_drawImageScaledAndCentered(context, image, scale.value, fade.value, pixelRatio); _drawImageScaledAndCentered(context, image, scale.value, fade.value, pixelRatio);
} }
...@@ -957,7 +957,7 @@ class _ZoomExitTransitionPainter extends SnapshotPainter { ...@@ -957,7 +957,7 @@ class _ZoomExitTransitionPainter extends SnapshotPainter {
final LayerHandle<TransformLayer> _transformHandler = LayerHandle<TransformLayer>(); final LayerHandle<TransformLayer> _transformHandler = LayerHandle<TransformLayer>();
@override @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); _drawImageScaledAndCentered(context, image, scale.value, fade.value, pixelRatio);
} }
......
...@@ -231,6 +231,7 @@ class _RenderSnapshotWidget extends RenderProxyBox { ...@@ -231,6 +231,7 @@ class _RenderSnapshotWidget extends RenderProxyBox {
} }
ui.Image? _childRaster; ui.Image? _childRaster;
Size? _childRasterSize;
// Set to true if the snapshot mode was not forced and a platform view // Set to true if the snapshot mode was not forced and a platform view
// was encountered while attempting to snapshot the child. // was encountered while attempting to snapshot the child.
bool _disableSnapshotAttempt = false; bool _disableSnapshotAttempt = false;
...@@ -249,6 +250,7 @@ class _RenderSnapshotWidget extends RenderProxyBox { ...@@ -249,6 +250,7 @@ class _RenderSnapshotWidget extends RenderProxyBox {
painter.removeListener(markNeedsPaint); painter.removeListener(markNeedsPaint);
_childRaster?.dispose(); _childRaster?.dispose();
_childRaster = null; _childRaster = null;
_childRasterSize = null;
super.detach(); super.detach();
} }
...@@ -258,6 +260,7 @@ class _RenderSnapshotWidget extends RenderProxyBox { ...@@ -258,6 +260,7 @@ class _RenderSnapshotWidget extends RenderProxyBox {
painter.removeListener(markNeedsPaint); painter.removeListener(markNeedsPaint);
_childRaster?.dispose(); _childRaster?.dispose();
_childRaster = null; _childRaster = null;
_childRasterSize = null;
super.dispose(); super.dispose();
} }
...@@ -265,6 +268,7 @@ class _RenderSnapshotWidget extends RenderProxyBox { ...@@ -265,6 +268,7 @@ class _RenderSnapshotWidget extends RenderProxyBox {
_disableSnapshotAttempt = false; _disableSnapshotAttempt = false;
_childRaster?.dispose(); _childRaster?.dispose();
_childRaster = null; _childRaster = null;
_childRasterSize = null;
markNeedsPaint(); markNeedsPaint();
} }
...@@ -296,19 +300,24 @@ class _RenderSnapshotWidget extends RenderProxyBox { ...@@ -296,19 +300,24 @@ class _RenderSnapshotWidget extends RenderProxyBox {
if (size.isEmpty) { if (size.isEmpty) {
_childRaster?.dispose(); _childRaster?.dispose();
_childRaster = null; _childRaster = null;
_childRasterSize = null;
return; return;
} }
if (!controller.allowSnapshotting || _disableSnapshotAttempt) { if (!controller.allowSnapshotting || _disableSnapshotAttempt) {
_childRaster?.dispose(); _childRaster?.dispose();
_childRaster = null; _childRaster = null;
_childRasterSize = null;
painter.paint(context, offset, size, super.paint); painter.paint(context, offset, size, super.paint);
return; return;
} }
_childRaster ??= _paintAndDetachToImage(); if (_childRaster == null) {
_childRaster = _paintAndDetachToImage();
_childRasterSize = size * devicePixelRatio;
}
if (_childRaster == null) { if (_childRaster == null) {
painter.paint(context, offset, size, super.paint); painter.paint(context, offset, size, super.paint);
} else { } 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 { ...@@ -356,11 +365,13 @@ abstract class SnapshotPainter extends ChangeNotifier {
/// [SnapshotPainter] paints the snapshot. This must account for the fact that the image /// [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 /// 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 /// 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 /// ```dart
/// void paint(PaintingContext context, Offset offset, Size size, ui.Image image, double pixelRatio) { /// void paint(PaintingContext context, Offset offset, Size size, ui.Image image, Size sourceSize, double pixelRatio) {
/// final Rect src = Rect.fromLTWH(0, 0, image.width.toDouble(), image.height.toDouble()); /// 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 Rect dst = Rect.fromLTWH(offset.dx, offset.dy, size.width, size.height);
/// final Paint paint = Paint() /// final Paint paint = Paint()
/// ..filterQuality = FilterQuality.low; /// ..filterQuality = FilterQuality.low;
...@@ -368,7 +379,7 @@ abstract class SnapshotPainter extends ChangeNotifier { ...@@ -368,7 +379,7 @@ abstract class SnapshotPainter extends ChangeNotifier {
/// } /// }
/// ``` /// ```
/// {@end-tool} /// {@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 /// Paint the child via [painter], applying any effects that would have been painted
/// in [SnapshotPainter.paintSnapshot]. /// in [SnapshotPainter.paintSnapshot].
...@@ -427,8 +438,8 @@ class _DefaultSnapshotPainter implements SnapshotPainter { ...@@ -427,8 +438,8 @@ class _DefaultSnapshotPainter implements SnapshotPainter {
} }
@override @override
void paintSnapshot(PaintingContext context, ui.Offset offset, ui.Size size, ui.Image image, double pixelRatio) { void paintSnapshot(PaintingContext context, ui.Offset offset, ui.Size size, ui.Image image, Size sourceSize, double pixelRatio) {
final Rect src = Rect.fromLTWH(0, 0, image.width.toDouble(), image.height.toDouble()); 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 Rect dst = Rect.fromLTWH(offset.dx, offset.dy, size.width, size.height);
final Paint paint = Paint() final Paint paint = Paint()
..filterQuality = FilterQuality.low; ..filterQuality = FilterQuality.low;
......
...@@ -9,8 +9,8 @@ ...@@ -9,8 +9,8 @@
import 'dart:ui' as ui; import 'dart:ui' as ui;
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
void main() { void main() {
...@@ -261,6 +261,42 @@ void main() { ...@@ -261,6 +261,42 @@ void main() {
expect(tester.takeException(), isNull); expect(tester.takeException(), isNull);
expect(tester.layers.last, isA<PlatformViewLayer>()); expect(tester.layers.last, isA<PlatformViewLayer>());
}, skip: kIsWeb); // TODO(jonahwilliams): https://github.com/flutter/flutter/issues/106689 }, 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<String> repaintBoundaryKey = ValueKey<String>('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 { class TestController extends SnapshotController {
...@@ -321,7 +357,7 @@ class TestPainter extends SnapshotPainter { ...@@ -321,7 +357,7 @@ class TestPainter extends SnapshotPainter {
} }
@override @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; count += 1;
lastImage = image; lastImage = image;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment