-
Notifications
You must be signed in to change notification settings - Fork 28.5k
[Android] Severe jank with custom paint #92366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
JANK.-.2.7.0-3.0pre.MP4NO.JANK.-.2.6.0-5.2.pre.MP4 |
Hi @sidetraxaudio, Thanks for filing the issue. I can reproduce the issue on the latest
Heres the jank_report containing
for both stable and the master channel. Output videomasterjank.movstablestable.MOVflutter doctor -v
code sampleimport 'dart:typed_data';
import 'package:flutter/material.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';
void main() {
runApp(const MyApp());
}
class MyApp extends StatelessWidget {
const MyApp({Key? key}) : super(key: key);
@override
Widget build(BuildContext context) {
return const MaterialApp(
debugShowCheckedModeBanner: false,
home: MyHomePage(),
);
}
}
class MyHomePage extends StatelessWidget {
const MyHomePage({
Key? key,
}) : super(key: key);
@override
Widget build(BuildContext context) {
return Scaffold(
backgroundColor: Colors.transparent,
body: FutureBuilder(
future: loadGraph(),
builder: (context, AsyncSnapshot<Int16List> waveData) {
return (!waveData.hasData)
? const CircularProgressIndicator.adaptive()
: SizedBox(
width: MediaQuery.of(context).size.width,
height: MediaQuery.of(context).size.height,
child: SingleChildScrollView(
scrollDirection: Axis.horizontal,
child: SizedBox(
width: MediaQuery.of(context).size.width * 20,
height: MediaQuery.of(context).size.height,
child: RepaintBoundary(
child: CustomPaint(
willChange: false,
isComplex: true,
painter: PaintTest(
waveData: waveData.data!,
),
),
),
),
),
);
},
),
);
}
}
class PaintTest extends CustomPainter {
final Int16List waveData;
const PaintTest({
required this.waveData,
});
@override
void paint(Canvas canvas, Size size) {
final _height = size.height;
double x = 0;
double strokeSize = .5;
const zoomFactor = .5;
final Paint paintPos = Paint()
..color = Colors.pink
..strokeWidth = strokeSize
..isAntiAlias = false
..style = PaintingStyle.stroke;
final Paint paintNeg = Paint()
..color = Colors.pink
..strokeWidth = strokeSize
..isAntiAlias = false
..style = PaintingStyle.stroke;
final Paint paintZero = Paint()
..color = Colors.green
..strokeWidth = strokeSize
..isAntiAlias = false
..style = PaintingStyle.stroke;
int index = 0;
for (index = 0; index < waveData.length; index++) {
if (waveData[index].isNegative) {
canvas.drawLine(
Offset(x, _height * 1 / 2),
Offset(
x, _height * 1 / 2 - waveData[index] / 32768 * (_height / 2)),
paintPos);
x += zoomFactor;
} else {
(waveData[index] == 0)
? canvas.drawLine(Offset(x, _height * 1 / 2),
Offset(x, _height * 1 / 2 + 1), paintZero)
: canvas.drawLine(
Offset(x, _height * 1 / 2),
Offset(x,
_height * 1 / 2 - waveData[index] / 32767 * (_height / 2)),
paintNeg);
x += zoomFactor;
}
}
}
@override
bool shouldRepaint(CustomPainter oldDelegate) {
return false;
}
}
Future<Int16List> loadGraph() async {
//*_____________________________________________
//! LOAD ANY WAVE FILE - THIS IS 16bit 44.1 MONO
//*_____________________________________________
Int16List load16bitMonoWave = await rootBundle
.load('assets/twinkle_twinkle_little_star.wav')
.then((value) => value.buffer.asInt16List());
//! THIN WAVE FILE TO 1/20
Int16List waveDataThinned =
Int16List((load16bitMonoWave.length * 1 / 20).round());
int reducedIndex = 0;
for (int index = 0; index + 20 < load16bitMonoWave.length; index += 20) {
waveDataThinned[reducedIndex] = load16bitMonoWave[index];
reducedIndex++;
}
return waveDataThinned;
}
|
cc: @chinmaygarde |
@sidetraxaudio - it would be really helpful if you could further bisect this to a specific commit using |
Hi @maheshmnj it may be better to remove 'waveform' from title - in the example the CustomPaint is only drawing lines from a list of doubles and the problem will be evident in any process using the engine in a similar way. Someone drawing anything abstract from a dataset. Labelling it so specific may cause duplicates and stop other people from dropping by. Hi @chinmaygarde Re: Path() efficiency Yes you're certainly right for the simple example I provided to demonstrate the problem but in the actual development there requires far more varied output based on the incoming data on a sample by sample basis- there'd be a lot more paths and passes with many different paints. I'll be profiling and optimising on my refactoring pass over the app. I was originally surprised at how well it worked with so many paint passes. Hi @dnfield - I'll look into doing a git bisect as soon as I get a chance. Thanks gents. |
I tried bisecting the commits and looks like this commit introduced the jank. git bisect logs
Checking out 1 consecutive commit behind faaca13 it works fine. cc: @dnfield |
None of the skia commits in https://skia.googlesource.com/skia.git/+log/a20c60d9e5d5..c076abe32ab5 look suspicious, but that engine roll also enabled DisplayList by default. /cc @flar |
The |
Using a path would require more processing on the segments to compute the joins. That processing is great if you can see the joins and want them to have a given style, but when there are so many segments and you can't really see the joins, it is just wasted computations. |
@flar can you think of why #92366 (comment) would happen? |
You'd have to ask Skia. |
Ok, backing up a step -
My question is: can @flar think of any reason why enabling display list would have introduced jank here? |
Running the test case locally I'm seeing 100ms per call to the Dart Paint phase. This would be the phase during which the DisplayList is constructed. Using the supplied twinkle-twinkle WAV file I get 333k samples to render. Each is a drawLine call which is 24bytes per record which is an 8mb display list. The current buffer growth algorithm, copied from Skia's SkLiteDL is to bump it by 4k per buffer overrun which means that the buffer will be grown from 4k to 8m over 2000 reallocs. I'll check it's behavior with an exponential growth algorithm (and we could also consider saving the temp grow buffers from one frame to the next) and see how that impacts things. Looking at how SkPictureRecorder worked, it looks like it would double (starting from an initial buffer of 4 records) on each growth. |
The buffer growth was a red herring. Switching to a doubling algorithm decreased reallocs from 1985 to 12, but the time spent still averaged 150ms. |
To answer the question directly, you can verify that this is a DisplayList issue simply by running with it enabled and disabled, no bisecting or theorizing required:
As to why DisplayList is slower here, it is executing the same drawLine calls that were originally captured by the SkPicture and playing them back to Skia. There is likely an optimization inside SkPicture to batch the calls that are not being batched by the Painter. I tried manually batching them and performance improved 5x, but still not to the level of SkPicture. As far as I can tell from reading the code for the SkPictureRecorder it does not actually do any batching of the line calls, but there could be something deeper. I'm still investigating along those lines. |
It looks like SkPicture uses nested bounding boxes to quickly pinpoint the part of a picture that is visible when the picture greatly exceeds the clip. DisplayList doesn't have a mechanism like that. The SkCanvas draw calls often have a quick reject, but it has a tiny cost to be executed on every line in that picture. There are 333,775 lines in the waveform generated if I use the twinkle-twinkle WAV file from the jank report above. Of those, about 0.2% of them (1 out of 500) are rendered on any given frame on my Moto G4. I'll file a bug/feature_request? against DisplayList to cull by the bounds of the records which will hopefully bridge this gap. |
I created a new suggested implementation of the sample code that uses a scrolling ListView.builder to render the waveform section by section to avoid both the performance problems seen here as well as significantly reducing the memory usage to store the very large pictures. Some stats on frame times and the size of the Picture objects generated by the 3 implementations (data from a Moto G4): The DisplayList uses a lot less memory than SkPicture in all 3 implementations, but it does not perform well for the 2 cases that render the entire waveform in a single picture. Fortunately, it does perform equally on the last implementation that uses a ListView and both the SkPicture and DisplayList mechanisms perform better than any of the other approaches (and use a lot less memory as well). New example with 3 implementations
|
@flar This issue has been sitting for a while, do we want to close this or is it still beneficial to keep open? |
The underlying issue is now being tracked under #92740 The specific problem here has a workaround, so I'm going to close this issue. |
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
2 VIDEO FILES ATTACHED:
1x no jank w/2.6.0-5.2.pre and below
1x bad jank w/2.7.0-3.0pre
beta 2.6.0-5.2.pre and BELOW works perfect
beta 2.7.0-3.0.pre and ABOVE creates massive jank : custompaint in any kind of scrollview. Unsure if its a custompaint issue, scrollview issue or optimisation issue.
-confirmed 2 physical devices w/8 and 12gb ram
Int16List (pcm audio) int values are being thinned and custom-painted as an audio waveform inside horizontal singlechildscrollview. Prior to latest beta performance was exceptional. After latest beta (and including master ch releases) something is broken and screen redraws are as if they're not cached/rasterised at all irrespective of custompaint settings and are drawing on the fly???.
...
-Find a wave file to dump in assets
(suspect a complex paint object will suffice)
-Implement attached code.
-Switch between any release ABOVE 2.6.0-5.2.pre to see massive jank or BELOW (and including) to see perfect operation
...
-Scroll left or right to see massive jank above 2.6.0-5.2.pre
-Even scroll to the scrollview bounds and see jank on overscroll glow
Expected results:
-Smooth scrolling/rasterising/screen update as per prior release
Actual results:
-Massive jank
Please see attached video files.
Logs
The text was updated successfully, but these errors were encountered: