From c451b6b0d7a1148a1ce56d476dd57eaab386223f Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 29 Jul 2021 15:14:16 -0700 Subject: [PATCH] Revert "Perform no shader warm-up by default (#87126)" (#87238) This reverts commit 32c2e2bad9738798fc74e81133fffb8a84f751fb. --- .../test/image_cache_tracing_test.dart | 2 +- examples/layers/raw/shader_warm_up.dart | 35 ++++ .../smoketests/raw/shader_warm_up_test.dart | 14 ++ .../flutter/lib/src/painting/binding.dart | 20 +-- .../lib/src/painting/shader_warm_up.dart | 149 +++++++++++++++++- .../flutter/test/painting/binding_test.dart | 4 - .../test/painting/shader_warm_up_test.dart | 61 +++++++ 7 files changed, 265 insertions(+), 20 deletions(-) create mode 100644 examples/layers/raw/shader_warm_up.dart create mode 100644 examples/layers/test/smoketests/raw/shader_warm_up_test.dart create mode 100644 packages/flutter/test/painting/shader_warm_up_test.dart diff --git a/dev/tracing_tests/test/image_cache_tracing_test.dart b/dev/tracing_tests/test/image_cache_tracing_test.dart index 835b81ff11..0ef752d3f3 100644 --- a/dev/tracing_tests/test/image_cache_tracing_test.dart +++ b/dev/tracing_tests/test/image_cache_tracing_test.dart @@ -55,7 +55,7 @@ void main() { }, { 'name': 'listener', - 'args': {'parentId': '0', 'isolateId': isolateId} + 'args': {'parentId': '1', 'isolateId': isolateId} }, { 'name': 'ImageCache.clear', diff --git a/examples/layers/raw/shader_warm_up.dart b/examples/layers/raw/shader_warm_up.dart new file mode 100644 index 0000000000..35ac5bc4fa --- /dev/null +++ b/examples/layers/raw/shader_warm_up.dart @@ -0,0 +1,35 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// This example shows the draw operations to warm up the GPU shaders by default. + +import 'dart:ui' as ui; + +import 'package:flutter/material.dart'; + +Future beginFrame(Duration timeStamp) async { + // PAINT + final ui.PictureRecorder recorder = ui.PictureRecorder(); + const ui.Rect paintBounds = ui.Rect.fromLTRB(0, 0, 1000, 1000); + final ui.Canvas canvas = ui.Canvas(recorder, paintBounds); + final ui.Paint backgroundPaint = ui.Paint()..color = Colors.white; + canvas.drawRect(paintBounds, backgroundPaint); + await const DefaultShaderWarmUp( + drawCallSpacing: 80.0, + canvasSize: ui.Size(1024, 1024), + ).warmUpOnCanvas(canvas); + final ui.Picture picture = recorder.endRecording(); + + // COMPOSITE + final ui.SceneBuilder sceneBuilder = ui.SceneBuilder() + ..pushClipRect(paintBounds) + ..addPicture(ui.Offset.zero, picture) + ..pop(); + ui.window.render(sceneBuilder.build()); +} + +Future main() async { + ui.window.onBeginFrame = beginFrame; + ui.window.scheduleFrame(); +} diff --git a/examples/layers/test/smoketests/raw/shader_warm_up_test.dart b/examples/layers/test/smoketests/raw/shader_warm_up_test.dart new file mode 100644 index 0000000000..2a9152ebca --- /dev/null +++ b/examples/layers/test/smoketests/raw/shader_warm_up_test.dart @@ -0,0 +1,14 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// ignore: deprecated_member_use +import 'package:test_api/test_api.dart' hide TypeMatcher, isInstanceOf; + +import '../../../raw/shader_warm_up.dart' as demo; + +void main() { + test('layers smoketest for raw/shader_warm_up.dart', () { + demo.main(); + }); +} diff --git a/packages/flutter/lib/src/painting/binding.dart b/packages/flutter/lib/src/painting/binding.dart index 8f563066ed..905dbc902b 100644 --- a/packages/flutter/lib/src/painting/binding.dart +++ b/packages/flutter/lib/src/painting/binding.dart @@ -31,17 +31,17 @@ mixin PaintingBinding on BindingBase, ServicesBinding { /// [ShaderWarmUp] instance to be executed during [initInstances]. /// - /// Defaults to `null`, meaning no shader warm-up is done. Some platforms may - /// not support shader warm-up before at least one frame has been displayed. + /// Defaults to an instance of [DefaultShaderWarmUp]. /// /// If the application has scenes that require the compilation of complex - /// shaders, it may cause jank in the middle of an animation or interaction. - /// In that case, setting [shaderWarmUp] to a custom [ShaderWarmUp] before - /// creating the binding (usually before [runApp] for normal Flutter apps, and - /// before [enableFlutterDriverExtension] for Flutter driver tests) may help - /// if that object paints the difficult scene in its - /// [ShaderWarmUp.warmUpOnCanvas] method, as this allows Flutter to - /// pre-compile and cache the required shaders during startup. + /// shaders that are not covered by [DefaultShaderWarmUp], it may cause jank + /// in the middle of an animation or interaction. In that case, setting + /// [shaderWarmUp] to a custom [ShaderWarmUp] before creating the binding + /// (usually before [runApp] for normal Flutter apps, and before + /// [enableFlutterDriverExtension] for Flutter driver tests) may help if that + /// object paints the difficult scene in its [ShaderWarmUp.warmUpOnCanvas] + /// method, as this allows Flutter to pre-compile and cache the required + /// shaders during startup. /// /// Currently the warm-up happens synchronously on the raster thread which /// means the rendering of the first frame on the raster thread will be @@ -58,7 +58,7 @@ mixin PaintingBinding on BindingBase, ServicesBinding { /// /// * [ShaderWarmUp], the interface for implementing custom warm-up scenes. /// * - static ShaderWarmUp? shaderWarmUp; + static ShaderWarmUp? shaderWarmUp = const DefaultShaderWarmUp(); /// The singleton that implements the Flutter framework's image cache. /// diff --git a/packages/flutter/lib/src/painting/shader_warm_up.dart b/packages/flutter/lib/src/painting/shader_warm_up.dart index 890a3759c6..57ce493a19 100644 --- a/packages/flutter/lib/src/painting/shader_warm_up.dart +++ b/packages/flutter/lib/src/painting/shader_warm_up.dart @@ -17,11 +17,12 @@ import 'debug.dart'; /// jank in the middle of an animation. /// /// Therefore, we use this during the [PaintingBinding.initInstances] call to -/// move common shader compilations from animation time to startup time. If -/// needed, app developers can create a custom [ShaderWarmUp] subclass and -/// hand it to [PaintingBinding.shaderWarmUp] before -/// [PaintingBinding.initInstances] is called. Usually, that can be done before -/// calling [runApp]. +/// move common shader compilations from animation time to startup time. By +/// default, a [DefaultShaderWarmUp] is used. If needed, app developers can +/// create a custom [ShaderWarmUp] subclass and hand it to +/// [PaintingBinding.shaderWarmUp] (so it replaces [DefaultShaderWarmUp]) +/// before [PaintingBinding.initInstances] is called. Usually, that can be +/// done before calling [runApp]. /// /// To determine whether a draw operation is useful for warming up shaders, /// check whether it improves the slowest frame rasterization time. Also, @@ -101,3 +102,141 @@ abstract class ShaderWarmUp { picture.dispose(); } } + +/// Default way of warming up Skia shader compilations. +/// +/// The draw operations being warmed up here are decided according to Flutter +/// engineers' observation and experience based on the apps and the performance +/// issues seen so far. +/// +/// This is used for the default value of [PaintingBinding.shaderWarmUp]. +/// Consider setting that static property to a different value before the +/// binding is initialized to change the warm-up sequence. +/// +/// See also: +/// +/// * [ShaderWarmUp], the base class for shader warm-up objects. +/// * +class DefaultShaderWarmUp extends ShaderWarmUp { + /// Create an instance of the default shader warm-up logic. + /// + /// Since this constructor is `const`, [DefaultShaderWarmUp] can be used as + /// the default value of parameters. + const DefaultShaderWarmUp({ + this.drawCallSpacing = 0.0, + this.canvasSize = const ui.Size(100.0, 100.0), + }); + + /// Distance to place between draw calls for visualizing the draws for + /// debugging purposes (e.g. 80.0). + /// + /// Defaults to 0.0. + /// + /// When changing this value, the [canvasSize] must also be changed to + /// accommodate the bigger canvas. + final double drawCallSpacing; + + /// The [size] of the canvas required to paint the shapes in [warmUpOnCanvas]. + /// + /// When [drawCallSpacing] is 0.0, this should be at least 100.0 by 100.0. + final ui.Size canvasSize; + + @override + ui.Size get size => canvasSize; + + /// Trigger common draw operations on a canvas to warm up GPU shader + /// compilation cache. + @override + Future warmUpOnCanvas(ui.Canvas canvas) async { + const ui.RRect rrect = ui.RRect.fromLTRBXY(20.0, 20.0, 60.0, 60.0, 10.0, 10.0); + final ui.Path rrectPath = ui.Path()..addRRect(rrect); + final ui.Path circlePath = ui.Path()..addOval( + ui.Rect.fromCircle(center: const ui.Offset(40.0, 40.0), radius: 20.0), + ); + + // The following path is based on + // https://skia.org/user/api/SkCanvas_Reference#SkCanvas_drawPath + final ui.Path path = ui.Path(); + path.moveTo(20.0, 60.0); + path.quadraticBezierTo(60.0, 20.0, 60.0, 60.0); + path.close(); + path.moveTo(60.0, 20.0); + path.quadraticBezierTo(60.0, 60.0, 20.0, 60.0); + + final ui.Path convexPath = ui.Path(); + convexPath.moveTo(20.0, 30.0); + convexPath.lineTo(40.0, 20.0); + convexPath.lineTo(60.0, 30.0); + convexPath.lineTo(60.0, 60.0); + convexPath.lineTo(20.0, 60.0); + convexPath.close(); + + // Skia uses different shaders based on the kinds of paths being drawn and + // the associated paint configurations. According to our experience and + // tracing, drawing the following paths/paints generates various of + // shaders that are commonly used. + final List paths = [rrectPath, circlePath, path, convexPath]; + + final List paints = [ + ui.Paint() + ..isAntiAlias = true + ..style = ui.PaintingStyle.fill, + ui.Paint() + ..isAntiAlias = false + ..style = ui.PaintingStyle.fill, + ui.Paint() + ..isAntiAlias = true + ..style = ui.PaintingStyle.stroke + ..strokeWidth = 10, + ui.Paint() + ..isAntiAlias = true + ..style = ui.PaintingStyle.stroke + ..strokeWidth = 0.1, // hairline + ]; + + // Warm up path stroke and fill shaders. + for (int i = 0; i < paths.length; i += 1) { + canvas.save(); + for (final ui.Paint paint in paints) { + canvas.drawPath(paths[i], paint); + canvas.translate(drawCallSpacing, 0.0); + } + canvas.restore(); + canvas.translate(0.0, drawCallSpacing); + } + + // Warm up shadow shaders. + const ui.Color black = ui.Color(0xFF000000); + canvas.save(); + canvas.drawShadow(rrectPath, black, 10.0, true); + canvas.translate(drawCallSpacing, 0.0); + canvas.drawShadow(rrectPath, black, 10.0, false); + canvas.restore(); + + // Warm up text shaders. + canvas.translate(0.0, drawCallSpacing); + final ui.ParagraphBuilder paragraphBuilder = ui.ParagraphBuilder( + ui.ParagraphStyle(textDirection: ui.TextDirection.ltr), + )..pushStyle(ui.TextStyle(color: black))..addText('_'); + final ui.Paragraph paragraph = paragraphBuilder.build() + ..layout(const ui.ParagraphConstraints(width: 60.0)); + canvas.drawParagraph(paragraph, const ui.Offset(20.0, 20.0)); + + // Draw a rect inside a rrect with a non-trivial intersection. If the + // intersection is trivial (e.g., equals the rrect clip), Skia will optimize + // the clip out. + // + // Add an integral or fractional translation to trigger Skia's non-AA or AA + // optimizations (as did before in normal FillRectOp in rrect clip cases). + for (final double fraction in [0.0, 0.5]) { + canvas + ..save() + ..translate(fraction, fraction) + ..clipRRect(ui.RRect.fromLTRBR(8, 8, 328, 248, const ui.Radius.circular(16))) + ..drawRect(const ui.Rect.fromLTRB(10, 10, 320, 240), ui.Paint()) + ..restore(); + canvas.translate(drawCallSpacing, 0.0); + } + canvas.translate(0.0, drawCallSpacing); + } +} diff --git a/packages/flutter/test/painting/binding_test.dart b/packages/flutter/test/painting/binding_test.dart index 337591db32..3c3f0fa918 100644 --- a/packages/flutter/test/painting/binding_test.dart +++ b/packages/flutter/test/painting/binding_test.dart @@ -38,10 +38,6 @@ Future main() async { expect(binding.imageCache.clearCount, 1); expect(binding.imageCache.liveClearCount, 1); }); - - test('ShaderWarmUp is null by default', () { - expect(PaintingBinding.shaderWarmUp, null); - }); } class TestBindingBase implements BindingBase { diff --git a/packages/flutter/test/painting/shader_warm_up_test.dart b/packages/flutter/test/painting/shader_warm_up_test.dart new file mode 100644 index 0000000000..54e2d7b305 --- /dev/null +++ b/packages/flutter/test/painting/shader_warm_up_test.dart @@ -0,0 +1,61 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:ui' as ui; + +import 'package:flutter/foundation.dart'; +import 'package:flutter/painting.dart'; +import 'package:flutter_test/flutter_test.dart'; + +class TestCanvas implements Canvas { + TestCanvas(); + + final List invocations = []; + + @override + void noSuchMethod(Invocation invocation) { + invocations.add(invocation); + } +} + +void main() { + test('DefaultShaderWarmUp has expected canvas invocations', () { + final TestCanvas canvas = TestCanvas(); + const DefaultShaderWarmUp s = DefaultShaderWarmUp(); + s.warmUpOnCanvas(canvas); + + bool hasDrawRectAfterClipRRect = false; + for (int i = 0; i < canvas.invocations.length - 1; i += 1) { + if (canvas.invocations[i].memberName == #clipRRect && canvas.invocations[i + 1].memberName == #drawRect) { + hasDrawRectAfterClipRRect = true; + break; + } + } + + expect(hasDrawRectAfterClipRRect, true); + }); + + test('ShaderWarmUp.execute disposes the image and picture', () async { + const DefaultShaderWarmUp shaderWarmUp = DefaultShaderWarmUp(); + late ui.Picture capturedPicture; + late ui.Image capturedImage; + debugCaptureShaderWarmUpPicture = (ui.Picture picture) { + capturedPicture = picture; + expect(picture.approximateBytesUsed, greaterThan(0)); + return true; + }; + debugCaptureShaderWarmUpImage = (ui.Image image) { + capturedImage = image; + expect(image.width, 100); + expect(image.height, 100); + return true; + }; + await shaderWarmUp.execute(); + expect( + () => capturedPicture.approximateBytesUsed, + throwsA(isA().having((String message) => message, 'message', 'Object has been disposed.')), + ); + expect(capturedImage.debugDisposed, true); + }, skip: kIsWeb); // Browser doesn't support approximateBytesUsed and doesn't rasterize the picture at this time. +}