Unverified Commit d9ef7df9 authored by amirh's avatar amirh Committed by GitHub

Use RRects instead of Paths when possible in Material. (#14404)

0672055a changed the Material widget to
always use Paths for representing the outline.
These paths are later used for clipping and drawing a shadow.
This changed introduced a performance regression, see:
https://github.com/flutter/flutter/issues/14403

We did not expect a path that is a rounded rectangle to be less
performant than a rounded rectangle, as Skia should be able to tell the
path is just a rounded rectangle.
Until we find a solution for this regression, we keep using RRect when
we can represent the shape with it.
parent b54b576c
...@@ -295,6 +295,14 @@ class _MaterialState extends State<Material> with TickerProviderStateMixin { ...@@ -295,6 +295,14 @@ class _MaterialState extends State<Material> with TickerProviderStateMixin {
if (widget.type == MaterialType.transparency) if (widget.type == MaterialType.transparency)
return _clipToShape(shape: shape, contents: contents); return _clipToShape(shape: shape, contents: contents);
// PhysicalModel performs better than PhysicalShape, so we use it when
// possible.
// This is not expected, and we do this as a temporary workaround until the
// shape performance regression is resolved, see:
// https://github.com/flutter/flutter/issues/14403
if (shape.runtimeType == CircleBorder || shape.runtimeType == RoundedRectangleBorder)
return _physicalModelInterior(contents, shape, backgroundColor);
return new _MaterialInterior( return new _MaterialInterior(
curve: Curves.fastOutSlowIn, curve: Curves.fastOutSlowIn,
...@@ -305,10 +313,45 @@ class _MaterialState extends State<Material> with TickerProviderStateMixin { ...@@ -305,10 +313,45 @@ class _MaterialState extends State<Material> with TickerProviderStateMixin {
shadowColor: widget.shadowColor, shadowColor: widget.shadowColor,
child: contents, child: contents,
); );
}
Widget _physicalModelInterior(Widget contents, ShapeBorder shape, Color backgroundColor) {
assert(shape.runtimeType == CircleBorder || shape.runtimeType == RoundedRectangleBorder);
BoxShape boxShape;
BorderRadius borderRadius;
if (shape.runtimeType == CircleBorder) {
boxShape = BoxShape.circle;
borderRadius = BorderRadius.zero;
} else {
final RoundedRectangleBorder border = shape;
boxShape = BoxShape.rectangle;
borderRadius = border.borderRadius;
}
return new AnimatedPhysicalModel(
curve: Curves.fastOutSlowIn,
duration: kThemeChangeDuration,
shape: boxShape,
borderRadius: borderRadius,
elevation: widget.elevation,
color: backgroundColor,
shadowColor: widget.shadowColor,
animateColor: false,
child: contents,
);
} }
static Widget _clipToShape({ShapeBorder shape, Widget contents}) { static Widget _clipToShape({ShapeBorder shape, Widget contents}) {
// ClipRRect performs better than ClipPath, so we use it when possible.
// This is not expected, and we do this as a temporary workaround until the
// shape performance regression is resolved, see:
// https://github.com/flutter/flutter/issues/14403
if (shape.runtimeType == RoundedRectangleBorder) {
final RoundedRectangleBorder border = shape;
return new ClipRRect(
borderRadius: border.borderRadius,
child: contents,
);
}
return new ClipPath( return new ClipPath(
child: contents, child: contents,
clipper: new ShapeBorderClipper( clipper: new ShapeBorderClipper(
......
...@@ -140,17 +140,17 @@ void main() { ...@@ -140,17 +140,17 @@ void main() {
), ),
), ),
); );
expect(tester.renderObject<RenderProxyBox>(find.byType(PhysicalShape)).child, paintsNothing); expect(tester.renderObject<RenderProxyBox>(find.byType(PhysicalModel)).child, paintsNothing);
await tester.tap(find.byType(InkWell)); await tester.tap(find.byType(InkWell));
await tester.pump(); await tester.pump();
await tester.pump(const Duration(milliseconds: 10)); await tester.pump(const Duration(milliseconds: 10));
expect(tester.renderObject<RenderProxyBox>(find.byType(PhysicalShape)).child, paints..circle()); expect(tester.renderObject<RenderProxyBox>(find.byType(PhysicalModel)).child, paints..circle());
await tester.drag(find.byType(ListView), const Offset(0.0, -1000.0)); await tester.drag(find.byType(ListView), const Offset(0.0, -1000.0));
await tester.pump(const Duration(milliseconds: 10)); await tester.pump(const Duration(milliseconds: 10));
await tester.drag(find.byType(ListView), const Offset(0.0, 1000.0)); await tester.drag(find.byType(ListView), const Offset(0.0, 1000.0));
await tester.pump(const Duration(milliseconds: 10)); await tester.pump(const Duration(milliseconds: 10));
expect( expect(
tester.renderObject<RenderProxyBox>(find.byType(PhysicalShape)).child, tester.renderObject<RenderProxyBox>(find.byType(PhysicalModel)).child,
keepAlive ? (paints..circle()) : paintsNothing, keepAlive ? (paints..circle()) : paintsNothing,
); );
} }
......
...@@ -29,8 +29,8 @@ Widget buildMaterial( ...@@ -29,8 +29,8 @@ Widget buildMaterial(
); );
} }
RenderPhysicalShape getShadow(WidgetTester tester) { RenderPhysicalModel getShadow(WidgetTester tester) {
return tester.renderObject(find.byType(PhysicalShape)); return tester.renderObject(find.byType(PhysicalModel));
} }
class PaintRecorder extends CustomPainter { class PaintRecorder extends CustomPainter {
...@@ -122,23 +122,23 @@ void main() { ...@@ -122,23 +122,23 @@ void main() {
// a kThemeChangeDuration time interval. // a kThemeChangeDuration time interval.
await tester.pumpWidget(buildMaterial(elevation: 0.0)); await tester.pumpWidget(buildMaterial(elevation: 0.0));
final RenderPhysicalShape modelA = getShadow(tester); final RenderPhysicalModel modelA = getShadow(tester);
expect(modelA.elevation, equals(0.0)); expect(modelA.elevation, equals(0.0));
await tester.pumpWidget(buildMaterial(elevation: 9.0)); await tester.pumpWidget(buildMaterial(elevation: 9.0));
final RenderPhysicalShape modelB = getShadow(tester); final RenderPhysicalModel modelB = getShadow(tester);
expect(modelB.elevation, equals(0.0)); expect(modelB.elevation, equals(0.0));
await tester.pump(const Duration(milliseconds: 1)); await tester.pump(const Duration(milliseconds: 1));
final RenderPhysicalShape modelC = getShadow(tester); final RenderPhysicalModel modelC = getShadow(tester);
expect(modelC.elevation, closeTo(0.0, 0.001)); expect(modelC.elevation, closeTo(0.0, 0.001));
await tester.pump(kThemeChangeDuration ~/ 2); await tester.pump(kThemeChangeDuration ~/ 2);
final RenderPhysicalShape modelD = getShadow(tester); final RenderPhysicalModel modelD = getShadow(tester);
expect(modelD.elevation, isNot(closeTo(0.0, 0.001))); expect(modelD.elevation, isNot(closeTo(0.0, 0.001)));
await tester.pump(kThemeChangeDuration); await tester.pump(kThemeChangeDuration);
final RenderPhysicalShape modelE = getShadow(tester); final RenderPhysicalModel modelE = getShadow(tester);
expect(modelE.elevation, equals(9.0)); expect(modelE.elevation, equals(9.0));
}); });
...@@ -147,23 +147,23 @@ void main() { ...@@ -147,23 +147,23 @@ void main() {
// a kThemeChangeDuration time interval. // a kThemeChangeDuration time interval.
await tester.pumpWidget(buildMaterial(shadowColor: const Color(0xFF00FF00))); await tester.pumpWidget(buildMaterial(shadowColor: const Color(0xFF00FF00)));
final RenderPhysicalShape modelA = getShadow(tester); final RenderPhysicalModel modelA = getShadow(tester);
expect(modelA.shadowColor, equals(const Color(0xFF00FF00))); expect(modelA.shadowColor, equals(const Color(0xFF00FF00)));
await tester.pumpWidget(buildMaterial(shadowColor: const Color(0xFFFF0000))); await tester.pumpWidget(buildMaterial(shadowColor: const Color(0xFFFF0000)));
final RenderPhysicalShape modelB = getShadow(tester); final RenderPhysicalModel modelB = getShadow(tester);
expect(modelB.shadowColor, equals(const Color(0xFF00FF00))); expect(modelB.shadowColor, equals(const Color(0xFF00FF00)));
await tester.pump(const Duration(milliseconds: 1)); await tester.pump(const Duration(milliseconds: 1));
final RenderPhysicalShape modelC = getShadow(tester); final RenderPhysicalModel modelC = getShadow(tester);
expect(modelC.shadowColor, within<Color>(distance: 1, from: const Color(0xFF00FF00))); expect(modelC.shadowColor, within<Color>(distance: 1, from: const Color(0xFF00FF00)));
await tester.pump(kThemeChangeDuration ~/ 2); await tester.pump(kThemeChangeDuration ~/ 2);
final RenderPhysicalShape modelD = getShadow(tester); final RenderPhysicalModel modelD = getShadow(tester);
expect(modelD.shadowColor, isNot(within<Color>(distance: 1, from: const Color(0xFF00FF00)))); expect(modelD.shadowColor, isNot(within<Color>(distance: 1, from: const Color(0xFF00FF00))));
await tester.pump(kThemeChangeDuration); await tester.pump(kThemeChangeDuration);
final RenderPhysicalShape modelE = getShadow(tester); final RenderPhysicalModel modelE = getShadow(tester);
expect(modelE.shadowColor, equals(const Color(0xFFFF0000))); expect(modelE.shadowColor, equals(const Color(0xFFFF0000)));
}); });
...@@ -178,7 +178,7 @@ void main() { ...@@ -178,7 +178,7 @@ void main() {
) )
); );
expect(find.byKey(materialKey), clipsWithBoundingRect); expect(find.byKey(materialKey), clipsWithBoundingRRect(borderRadius: BorderRadius.zero));
}); });
testWidgets('clips to rounded rect when borderRadius provided', (WidgetTester tester) async { testWidgets('clips to rounded rect when borderRadius provided', (WidgetTester tester) async {
......
...@@ -110,6 +110,14 @@ class TestRecordingPaintingContext implements PaintingContext { ...@@ -110,6 +110,14 @@ class TestRecordingPaintingContext implements PaintingContext {
canvas.restore(); canvas.restore();
} }
@override
void pushClipRRect(bool needsCompositing, Offset offset, Rect bounds, RRect clipRRect, PaintingContextCallback painter) {
canvas.save();
canvas.clipRRect(clipRRect.shift(offset));
painter(this, offset);
canvas.restore();
}
@override @override
void pushClipPath(bool needsCompositing, Offset offset, Rect bounds, Path clipPath, PaintingContextCallback painter) { void pushClipPath(bool needsCompositing, Offset offset, Rect bounds, Path clipPath, PaintingContextCallback painter) {
canvas canvas
......
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