Put Metal renderer selection behind runtime flag and plist opt-in. #13056
Conversation
| return false; | ||
| } | ||
|
|
||
| // If the application want to use metal on a per run basis with disregard for version checks or |
| return true; | ||
| } | ||
|
|
||
| // This is just a version we picked that is easy to support and has all necessary Metal features. |
There was a problem hiding this comment.
May be worth mentioning here that we picked this because of Skia.
There was a problem hiding this comment.
Someone reading this later might wonder, right? The constraint for lowering this is on Skia rather than on the engine team directly.
| #if FLUTTER_SHELL_ENABLE_METAL | ||
| return [CAMetalLayer class]; | ||
| if (UseMetalRenderer()) { | ||
| return [CAMetalLayer class]; | ||
| } else { | ||
| return [CAEAGLLayer class]; | ||
| } |
There was a problem hiding this comment.
nit: we could clean this block up if UserMetalRenderer was guarded interiorly to just return false if this define isn't set.
We also need to do this in FlutterOverlayView for platform views.
| } | ||
|
|
||
| #if FLUTTER_SHELL_ENABLE_METAL | ||
| static bool UseMetalRenderer() { |
There was a problem hiding this comment.
If we can't make the various combinations in this method mutually exclusive, perhaps we should print some warnings for people who mess up and put conflicting ones in the project.
For example, in --disable-metal, check if they specified --force-metal or have the plist value set, and FML_LOG(WARNING) that we're disabling metal because this flag is set even though the other ones are.
There was a problem hiding this comment.
I was doing that but realized I was over-engineering it. This is not meant to be used by folks outside the team. In fact, this won't even be available in default builds (you still need to build locally for metal). This is just so I can test Metal in different apps without modifying the plist (test stuff like external textures, texture upload perf etc.). All command line arguments will be removed when this gets into the default builds.
Even if this were in vended builds, the only way to specify this is via command line arguments specified in Xcode schemes. That is, people can't vend binaries for others to test that depend on functionality set by command line args.
There was a problem hiding this comment.
Ohh I was mixing this up with shell arguments.
It might be nice to have a shell argument to force metal to be disabled, similar to having a shell argument to force software rendering.
This is fine though.
|
LUCI failure has been reverted. Landing. |
git@github.com:flutter/engine.git/compare/cef6751dba87...e78c3f5 git log cef6751..e78c3f5 --no-merges --oneline 2019-10-11 iska.kaushik@gmail.com [dart_aot_runner] Add support for generating dart_aot snapshots (flutter/engine#13071) 2019-10-10 chinmaygarde@gmail.com Put Metal renderer selection behind runtime flag and plist opt-in. (flutter/engine#13056) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
git@github.com:flutter/engine.git/compare/cef6751dba87...e78c3f5 git log cef6751..e78c3f5 --no-merges --oneline 2019-10-11 iska.kaushik@gmail.com [dart_aot_runner] Add support for generating dart_aot snapshots (flutter/engine#13071) 2019-10-10 chinmaygarde@gmail.com Put Metal renderer selection behind runtime flag and plist opt-in. (flutter/engine#13056) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
3 participants
Fixes flutter/flutter#42396