[Impeller] Cache tessellated data on impeller::Path objects.

[jonahwilliams]

For cases where we don't think we can make tessellation faster, fallback to caching the tessellated data so that at least subsequent frames are fast.

This helps flutter/flutter#140257 As the paths are stable, but flutter/flutter#141961 doesn't improve at all as there is animation in the contents (not the paths though). To make caching more effective we'd need to do something like utilize Skias Path ID or diff them or something like that. These could all be expensive.

See also: flutter/flutter#143077

[chinmaygarde]

I don't think we should do this till we get findings from the StC effort because of the performance cliff when the paths are animating.

[knopp]

I don't think we should do this till we get findings from the StC effort because of the performance cliff when the paths are animating.

It depends on use case though. In Superlist we have lot of SVG paths that don't change and the tessellation performance is one of the reasons we're stuck with Skia. We do not plan to animate these paths and are unlikely to experience a performance cliff.

[knopp]

What's the lifetime of impeller Path object? Is it held by the render pass? I haven't looked at the code recently, but in my first attempt on caching the tessellation data I used original Skia path generationId as key, because Skia paths are directly retained by dart objects.

The cache worked similarly to raster cache, unused paths were purged at the end of each frame.

[jonahwilliams]

The Path object lifecycle is tied to the display list that used it, so it should persist across frame as long as the pictures are stable : #50076 .

[knopp]

The Path object lifecycle is tied to the display list that used it, so it should persist across frame as long as the pictures are stable : #50076 .

My concern here is that I have no idea how likely it is for the display list to be invalidated (while still drawing the same SkPath). I assume there is a display list per repaint boundary? So for example an animation within the repaint boundary would invalidate the cached tessellated data?

[jonahwilliams]

Per picture layer in the framework, so a repaint boundary creates a new Picture layer. Now, we could make the caching more effective by using the Skia Path ID or something similar as a key - which would let you cache the path even if the display list isn't stable. I don't think this change contradicts that or makes it more difficult in any way.

[knopp]

I'm not opposed to caching the tesselation data, just trying to make sure I understand what the implications are. For example I think this will drastically improve static_path_tesselation macrobenchmark, but the cache will be completely invalidated when you add an animation to just one of the items. Maybe we should add a macrobenchmark variant that reflects this.

[jonahwilliams]

The animated tessellation benchmark already covers this case.

[knopp]

The animated tessellation benchmark already covers this case.

Looking at code, this is what happens in the painter:

class _CameraIconPainter extends CustomPainter {
  @override
  void paint(Canvas canvas, Size size) {
    final Matrix4 scale =
        Matrix4.diagonal3Values(size.width / 20, size.height / 20, 1.0);

    Path path;
    path = _path1.transform(scale.storage)..fillType = PathFillType.evenOdd;
    canvas.drawPath(path, Paint()..color = const Color(0xFFF84F39));

    path = _path2.transform(scale.storage)..fillType = PathFillType.evenOdd;
    canvas.drawPath(path, Paint()..color = const Color(0x60F84F39));
  }

The actually painted SkPath instances are short lived transformations of original SkPath. That means no amount of caching will help here when size is animated, which was the intention.

But, it also means that in this instance using the SkPath generation Id as cache key would be pretty useless. So we're still missing an example where the same SkPath is being painted in different pictures.

[jonahwilliams]

I don't think we're going to ever do content based hashing of paths as that would pretty expensive, especially for large animated paths which we already suffer with. So either the Picture layer is stable, or perhaps later on the path is stable.

We've also discused some sort of "Path.prepare()" style API that would let applications explicitly request that data for a path be frozen/cached, but that would depend on what route we end up taking with path tessellation.

[knopp]

I don't think we're going to ever do content based hashing of paths as that would pretty expensive, especially for large animated paths which we already suffer with. So either the Picture layer is stable, or perhaps later on the path is stable.

I woudn't suggest that. I remember having a prototype based on content caching and it was quite expensive with unclear benefit.

I think caching tessellation data in the impeller Path is a good first step, a second step could be to reuse same impeller Path instance for same SkPath (based on SkPath generation Id). That'd get us quite far.

[bdero]

This is great to have in our back pocket. Thanks!

[chinmaygarde]

This seems redundant after the SrC changes right?

[flutter-dashboard]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #50503 at sha c386f65