Unverified Commit 00495da0 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

Revert "[framework] use shader tiling instead of repeated calls to drawImage" (#124640)

Reverts flutter/flutter#119495

We'll managed to optimize almost all of the cases that made this slow. Actually, it will soon be harder to optimize shader tiling...
parent d1d426e5
......@@ -8,7 +8,6 @@ import 'dart:ui' as ui show FlutterView, Image;
import 'package:flutter/foundation.dart';
import 'package:flutter/scheduler.dart';
import 'package:vector_math/vector_math_64.dart';
import 'alignment.dart';
import 'basic_types.dart';
......@@ -405,61 +404,6 @@ void debugFlushLastFrameImageSizeInfo() {
}());
}
/// Information that describes how to tile an image for a given [ImageRepeat]
/// enum.
///
/// Used with [createTilingInfo].
@visibleForTesting
@immutable
class ImageTilingInfo {
/// Create a new [ImageTilingInfo] object.
const ImageTilingInfo({
required this.tmx,
required this.tmy,
required this.transform,
});
/// The tile mode for the x-axis.
final TileMode tmx;
/// The tile mode for the y-axis.
final TileMode tmy;
/// The transform to apply to the image shader.
final Matrix4 transform;
@override
String toString() {
if (!kDebugMode) {
return 'ImageTilingInfo';
}
return 'ImageTilingInfo($tmx, $tmy, $transform)';
}
}
/// Create the [ImageTilingInfo] for a given [ImageRepeat], canvas [rect],
/// [destinationRect], and [sourceRect].
@visibleForTesting
ImageTilingInfo createTilingInfo(ImageRepeat repeat, Rect rect, Rect destinationRect, Rect sourceRect) {
assert(repeat != ImageRepeat.noRepeat);
final TileMode tmx = (repeat == ImageRepeat.repeatX || repeat == ImageRepeat.repeat)
? TileMode.repeated
: TileMode.decal;
final TileMode tmy = (repeat == ImageRepeat.repeatY || repeat == ImageRepeat.repeat)
? TileMode.repeated
: TileMode.decal;
final Rect data = _generateImageTileRects(rect, destinationRect, repeat).first;
final Matrix4 transform = Matrix4.identity()
..scale(data.width / sourceRect.width, data.height / sourceRect.height)
..setTranslationRaw(data.topLeft.dx, data.topLeft.dy, 0);
return ImageTilingInfo(
tmx: tmx,
tmy: tmy,
transform: transform,
);
}
/// Paints an image into the given rectangle on the canvas.
///
/// The arguments have the following meanings:
......@@ -682,8 +626,7 @@ void paintImage({
if (needSave) {
canvas.save();
}
if (repeat != ImageRepeat.noRepeat && centerSlice != null) {
// Don't clip if an image shader is used.
if (repeat != ImageRepeat.noRepeat) {
canvas.clipRect(rect);
}
if (flipHorizontally) {
......@@ -699,12 +642,9 @@ void paintImage({
if (repeat == ImageRepeat.noRepeat) {
canvas.drawImageRect(image, sourceRect, destinationRect, paint);
} else {
final ImageTilingInfo info = createTilingInfo(repeat, rect, destinationRect, sourceRect);
final ImageShader shader = ImageShader(image, info.tmx, info.tmy, info.transform.storage, filterQuality: filterQuality);
canvas.drawRect(
rect,
paint..shader = shader
);
for (final Rect tileRect in _generateImageTileRects(rect, destinationRect, repeat)) {
canvas.drawImageRect(image, sourceRect, tileRect, paint);
}
}
} else {
canvas.scale(1 / scale);
......@@ -725,7 +665,7 @@ void paintImage({
}
}
List<Rect> _generateImageTileRects(Rect outputRect, Rect fundamentalRect, ImageRepeat repeat) {
Iterable<Rect> _generateImageTileRects(Rect outputRect, Rect fundamentalRect, ImageRepeat repeat) {
int startX = 0;
int startY = 0;
int stopX = 0;
......
......@@ -9,7 +9,6 @@ import 'package:fake_async/fake_async.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/painting.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:vector_math/vector_math_64.dart';
import '../image_data.dart';
import '../painting/mocks_for_image_cache.dart';
......@@ -679,8 +678,7 @@ void main() {
final TestCanvas canvas = TestCanvas();
// Paint a square image into an output rect that is twice as wide as it is
// tall. One copy of the image should be painted, aligned so that a repeating
// tile mode causes it to appear twice.
// tall. Two copies of the image should be painted, one next to the other.
const Rect outputRect = Rect.fromLTWH(30.0, 30.0, 400.0, 200.0);
final ui.Image image = await createTestImage(width: 100, height: 100);
......@@ -693,29 +691,34 @@ void main() {
repeat: ImageRepeat.repeatX,
);
final List<Invocation> calls = canvas.invocations.where((Invocation call) => call.memberName == #drawRect).toList();
const Size imageSize = Size(100.0, 100.0);
final List<Invocation> calls = canvas.invocations.where((Invocation call) => call.memberName == #drawImageRect).toList();
final Set<Rect> tileRects = <Rect>{};
expect(calls, hasLength(1));
final Invocation call = calls[0];
expect(calls, hasLength(2));
for (final Invocation call in calls) {
expect(call.isMethod, isTrue);
expect(call.positionalArguments, hasLength(2));
expect(call.positionalArguments, hasLength(4));
expect(call.positionalArguments[0], isA<ui.Image>());
// sourceRect should contain all pixels of the source image
expect(call.positionalArguments[1], Offset.zero & imageSize);
// A tiled image is drawn as a rect with a shader.
expect(call.positionalArguments[0], isA<Rect>());
expect(call.positionalArguments[1], isA<Paint>());
tileRects.add(call.positionalArguments[2] as Rect);
final Paint paint = call.positionalArguments[1] as Paint;
expect(call.positionalArguments[3], isA<Paint>());
}
expect(paint.shader, isA<ImageShader>());
expect(call.positionalArguments[0], outputRect);
expect(tileRects, <Rect>{const Rect.fromLTWH(30.0, 30.0, 200.0, 200.0), const Rect.fromLTWH(230.0, 30.0, 200.0, 200.0)});
});
test('paintImage with repeatY and fitWidth', () async {
final TestCanvas canvas = TestCanvas();
// Paint a square image into an output rect that is twice as tall as it is
// wide. One copy of the image should be painted, aligned so that a repeating
// tile mode causes it to appear twice.
// wide. Two copies of the image should be painted, one above the other.
const Rect outputRect = Rect.fromLTWH(30.0, 30.0, 200.0, 400.0);
final ui.Image image = await createTestImage(width: 100, height: 100);
......@@ -727,21 +730,28 @@ void main() {
fit: BoxFit.fitWidth,
repeat: ImageRepeat.repeatY,
);
final List<Invocation> calls = canvas.invocations.where((Invocation call) => call.memberName == #drawRect).toList();
expect(calls, hasLength(1));
final Invocation call = calls[0];
const Size imageSize = Size(100.0, 100.0);
final List<Invocation> calls = canvas.invocations.where((Invocation call) => call.memberName == #drawImageRect).toList();
final Set<Rect> tileRects = <Rect>{};
expect(calls, hasLength(2));
for (final Invocation call in calls) {
expect(call.isMethod, isTrue);
expect(call.positionalArguments, hasLength(2));
expect(call.positionalArguments, hasLength(4));
expect(call.positionalArguments[0], isA<ui.Image>());
// A tiled image is drawn as a rect with a shader.
expect(call.positionalArguments[0], isA<Rect>());
expect(call.positionalArguments[1], isA<Paint>());
// sourceRect should contain all pixels of the source image
expect(call.positionalArguments[1], Offset.zero & imageSize);
final Paint paint = call.positionalArguments[1] as Paint;
tileRects.add(call.positionalArguments[2] as Rect);
expect(call.positionalArguments[3], isA<Paint>());
}
expect(paint.shader, isA<ImageShader>());
expect(call.positionalArguments[0], outputRect);
expect(tileRects, <Rect>{const Rect.fromLTWH(30.0, 30.0, 200.0, 200.0), const Rect.fromLTWH(30.0, 230.0, 200.0, 200.0)});
});
test('DecorationImage scale test', () async {
......@@ -790,52 +800,4 @@ void main() {
info.dispose();
}, skip: kIsWeb); // https://github.com/flutter/flutter/issues/87442
test('Compute image tiling', () {
expect(() => createTilingInfo(ImageRepeat.noRepeat, Rect.zero, Rect.zero, Rect.zero), throwsAssertionError);
// These tests draw a 16x9 image into a 100x50 container with a destination
// size of and make assertions based on observed behavior and the original
// rectangles from https://github.com/flutter/flutter/pull/119495/
final ImageTilingInfo repeatX = createTilingInfo(
ImageRepeat.repeatX,
const Rect.fromLTRB(0.0, 0.0, 100.0, 50.0),
const Rect.fromLTRB(84.0, 0.0, 100.0, 9.0),
const Rect.fromLTRB(0.0, 0.0, 16.0, 9.0),
);
expect(repeatX.tmx, TileMode.repeated);
expect(repeatX.tmy, TileMode.decal);
expect(repeatX.transform, matrixMoreOrLessEquals(Matrix4.identity()
..scale(1.0, 1.0)
..setTranslationRaw(-12.0, 0.0, 0.0)
));
final ImageTilingInfo repeatY = createTilingInfo(
ImageRepeat.repeatY,
const Rect.fromLTRB(0.0, 0.0, 100.0, 50.0),
const Rect.fromLTRB(84.0, 0.0, 100.0, 9.0),
const Rect.fromLTRB(0.0, 0.0, 16.0, 9.0),
);
expect(repeatY.tmx, TileMode.decal);
expect(repeatY.tmy, TileMode.repeated);
expect(repeatY.transform, matrixMoreOrLessEquals(Matrix4.identity()
..scale(1.0, 1.0)
..setTranslationRaw(84.0, 0.0, 0.0)
));
final ImageTilingInfo repeat = createTilingInfo(
ImageRepeat.repeat,
const Rect.fromLTRB(0.0, 0.0, 100.0, 50.0),
const Rect.fromLTRB(84.0, 0.0, 100.0, 9.0),
const Rect.fromLTRB(0.0, 0.0, 16.0, 9.0),
);
expect(repeat.tmx, TileMode.repeated);
expect(repeat.tmy, TileMode.repeated);
expect(repeat.transform, matrixMoreOrLessEquals(Matrix4.identity()
..scale(1.0, 1.0)
..setTranslationRaw(-12.0, 0.0, 0.0)
));
});
}
......@@ -193,7 +193,7 @@ abstract class PaintPattern {
/// painting has completed, not at the time of the call. If the same [Paint]
/// object is reused multiple times, then this may not match the actual
/// arguments as they were seen by the method.
void rect({ Rect? rect, Color? color, double? strokeWidth, bool? hasMaskFilter, PaintingStyle? style, Matcher? shader });
void rect({ Rect? rect, Color? color, double? strokeWidth, bool? hasMaskFilter, PaintingStyle? style });
/// Indicates that a rounded rectangle clip is expected next.
///
......@@ -734,8 +734,8 @@ class _TestRecordingCanvasPatternMatcher extends _TestRecordingCanvasMatcher imp
}
@override
void rect({ Rect? rect, Color? color, double? strokeWidth, bool? hasMaskFilter, PaintingStyle? style, Matcher? shader }) {
_predicates.add(_RectPaintPredicate(rect: rect, color: color, strokeWidth: strokeWidth, hasMaskFilter: hasMaskFilter, style: style, shader: shader));
void rect({ Rect? rect, Color? color, double? strokeWidth, bool? hasMaskFilter, PaintingStyle? style }) {
_predicates.add(_RectPaintPredicate(rect: rect, color: color, strokeWidth: strokeWidth, hasMaskFilter: hasMaskFilter, style: style));
}
@override
......@@ -891,7 +891,6 @@ abstract class _DrawCommandPaintPredicate extends _PaintPredicate {
this.strokeWidth,
this.hasMaskFilter,
this.style,
this.shader,
this.strokeCap,
});
......@@ -903,7 +902,6 @@ abstract class _DrawCommandPaintPredicate extends _PaintPredicate {
final double? strokeWidth;
final bool? hasMaskFilter;
final PaintingStyle? style;
final Matcher? shader;
final StrokeCap? strokeCap;
String get methodName => _symbolName(symbol);
......@@ -939,9 +937,6 @@ abstract class _DrawCommandPaintPredicate extends _PaintPredicate {
if (style != null && paintArgument.style != style) {
throw 'It called $methodName with a paint whose style, ${paintArgument.style}, was not exactly the expected style ($style).';
}
if (shader != null && !shader!.matches(paintArgument.shader, <dynamic, dynamic>{})) {
throw 'It called $methodName with a paint whose shader, ${paintArgument.shader}, was not exactly the expected shader ($shader).';
}
if (strokeCap != null && paintArgument.strokeCap != strokeCap) {
throw 'It called $methodName with a paint whose strokeCap, ${paintArgument.strokeCap}, was not exactly the expected strokeCap ($strokeCap).';
}
......@@ -985,7 +980,6 @@ class _OneParameterPaintPredicate<T> extends _DrawCommandPaintPredicate {
required double? strokeWidth,
required bool? hasMaskFilter,
required PaintingStyle? style,
Matcher? shader,
}) : super(
symbol,
name,
......@@ -995,7 +989,6 @@ class _OneParameterPaintPredicate<T> extends _DrawCommandPaintPredicate {
strokeWidth: strokeWidth,
hasMaskFilter: hasMaskFilter,
style: style,
shader: shader,
);
final T? expected;
......@@ -1081,7 +1074,7 @@ class _TwoParameterPaintPredicate<T1, T2> extends _DrawCommandPaintPredicate {
}
class _RectPaintPredicate extends _OneParameterPaintPredicate<Rect> {
_RectPaintPredicate({ Rect? rect, Color? color, double? strokeWidth, bool? hasMaskFilter, PaintingStyle? style, Matcher? shader }) : super(
_RectPaintPredicate({ Rect? rect, Color? color, double? strokeWidth, bool? hasMaskFilter, PaintingStyle? style }) : super(
#drawRect,
'a rectangle',
expected: rect,
......@@ -1089,7 +1082,6 @@ class _RectPaintPredicate extends _OneParameterPaintPredicate<Rect> {
strokeWidth: strokeWidth,
hasMaskFilter: hasMaskFilter,
style: style,
shader: shader,
);
}
......
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