Skip to content
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

shaderWarmUp doesn't store shaders in PersistentCache #35923

Closed
gaaclarke opened this issue Jul 10, 2019 · 6 comments
Closed

shaderWarmUp doesn't store shaders in PersistentCache #35923

gaaclarke opened this issue Jul 10, 2019 · 6 comments
Labels
a: existing-apps Integration with existing apps via the add-to-app flow engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list platform-ios iOS applications specifically team-ios Owned by iOS platform team triaged-ios Triaged by iOS platform team

Comments

@gaaclarke
Copy link
Member

If you put a print statement in the DefaultShaderWarmUp.execute method and put a breakpoint at flutter::PersistentCache::store on an app that creates a FlutterEngine but doesn't create a FlutterViewController, you'll see that the DefaultShaderWarmUp is executed but nothing is stored in the PersistentCache.

I believe this is a great opportunity to make the first frame rendering faster on iOS, for the add-to-app case for sure, I'm not sure about the full Flutter case.

@gaaclarke gaaclarke added platform-ios iOS applications specifically a: existing-apps Integration with existing apps via the add-to-app flow labels Jul 10, 2019
@xster
Copy link
Member

xster commented Jul 10, 2019

We still manually request a frame in full-app cases before the first vsync to get a few milliseconds

@gaaclarke
Copy link
Member Author

This appears to be happening because the Rasterizer doesn't have a surface so a CPU surface is getting generated:
https://github.com/flutter/engine/blob/0cdfdce7179e82a95342b7a7349c5cada5fbf4cc/shell/common/rasterizer.cc#L133

The result is that the device is a SkBitmapDevice here:
https://github.com/google/skia/blob/215ff3325230557a35dce26174b3294b570afbf9/src/core/SkCanvas.cpp#L672

@gaaclarke
Copy link
Member Author

gaaclarke commented Jul 11, 2019

Here is the current setup:
shaderwarmup

What we can do is:

  1. Remove Rasterizer::MakeRasterSnapshot and put the logic into Picture::RasterizeToImage
  2. Switch that logic to instead of using the Rasterizer.surface_ to render, render on a surface that we've generated. If the IOManager has a GrContext we can use that to generate an offscreen gpu surface, otherwise create a CPU surface.
  3. Switch the rasterization to always happen on the IO thread instead of the GPU thread.

This is nice too because there is no reason that we should be generating these offscreen on the GPU thread and blocking regular rendering.

@jmagman jmagman added the engine flutter/engine repository. See also e: labels. label Jan 3, 2020
@jmagman jmagman added this to Engineer Reviewed in Add-to-app - iOS engine review Jan 9, 2020
@jmagman jmagman moved this from Engineer reviewed to Awaiting triage in Add-to-app - iOS engine review Jan 9, 2020
@jmagman jmagman moved this from Awaiting triage to Triage in Add-to-app - iOS engine review Jan 9, 2020
@gaaclarke gaaclarke moved this from Awaiting triage to Engineer reviewed in Add-to-app - iOS engine review Jan 17, 2020
@jmagman jmagman added this to Awaiting triage in Mobile - iOS engine review Feb 25, 2020
@jmagman jmagman removed this from Awaiting triage in Mobile - iOS engine review Feb 27, 2020
@jmagman jmagman added this to Awaiting triage in Mobile - iOS engine review Mar 10, 2020
@gaaclarke gaaclarke moved this from Awaiting triage to Engineer reviewed in Mobile - iOS engine review Apr 6, 2020
@kf6gpe kf6gpe added the P2 Important issues not at the top of the work list label Dec 9, 2020
@jmagman jmagman added this to Awaiting triage in iOS Platform - Add-to-app review Jun 8, 2021
@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-ios Owned by iOS platform team triaged-ios Triaged by iOS platform team labels Jul 8, 2023
@jmagman
Copy link
Member

jmagman commented Feb 8, 2024

Removed with #88455. @gaaclarke is there any actually work here post-impeller?

@gaaclarke
Copy link
Member Author

Removed with #88455. @gaaclarke is there any actually work here post-impeller?

Nope, impeller should make this obsolete.

iOS Platform - Add-to-app review automation moved this from Awaiting triage to Engineer reviewed Feb 8, 2024
Copy link

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 flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: existing-apps Integration with existing apps via the add-to-app flow engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list platform-ios iOS applications specifically team-ios Owned by iOS platform team triaged-ios Triaged by iOS platform team
Projects
Mobile - iOS engine review
  
Engineer reviewed
iOS Platform - Add-to-app review
  
Engineer reviewed
Add-to-app - iOS engine review
  
Engineer reviewed
Development

No branches or pull requests

6 participants