[Impeller] Enable playgrounds using a runtime instead of a build time…

… flag. (#40729) [Impeller] Enable playgrounds using a runtime instead of a build time flag.
flutter · Mar 29, 2023 · 86d5521 · 86d5521
1 parent 3b58a4d
commit 86d5521
Show file tree

Hide file tree

Showing 11 changed files with 12 additions and 47 deletions.
diff --git a/impeller/aiks/aiks_playground.cc b/impeller/aiks/aiks_playground.cc
@@ -22,7 +22,7 @@ bool AiksPlayground::OpenPlaygroundHere(const Picture& picture) {
 }
 
 bool AiksPlayground::OpenPlaygroundHere(AiksPlaygroundCallback callback) {
-  if (!Playground::is_enabled()) {
+  if (!switches_.enable_playground) {
     return true;
   }
 

diff --git a/impeller/display_list/display_list_playground.cc b/impeller/display_list/display_list_playground.cc
@@ -29,7 +29,7 @@ bool DisplayListPlayground::OpenPlaygroundHere(
 
 bool DisplayListPlayground::OpenPlaygroundHere(
     DisplayListPlaygroundCallback callback) {
-  if (!Playground::is_enabled()) {
+  if (!switches_.enable_playground) {
     return true;
   }
 

diff --git a/impeller/docs/faq.md b/impeller/docs/faq.md
@@ -99,12 +99,4 @@
   * Impeller is compiled into the Flutter engine. It is currently behind a flag
     as development progresses.
 * How do you run `impeller_unittests` with Playgrounds enabled?
-  * Playgrounds in the `impeller_unittests` harness can be enabled in one of
-    three ways:
-    * Edit `gn args` directly and add `impeller_enable_playground = true`.
-    * Add the `--enable-impeller-playground` flag to your `./flutter/tools/gn`
-      invocation.
-    * Set the `FLUTTER_IMPELLER_ENABLE_PLAYGROUND` to `1` before invoking
-      `./flutter/tools/gn`. Only do this if you frequently work with Playgrounds
-      and don't want to have to set the flags manually. Also, it would be a bad
-      idea to set this environment variable on CI.
+  * Specify the `--enable_playground` command-line option.
diff --git a/impeller/entity/entity_playground.cc b/impeller/entity/entity_playground.cc
@@ -15,7 +15,7 @@ EntityPlayground::EntityPlayground() = default;
 EntityPlayground::~EntityPlayground() = default;
 
 bool EntityPlayground::OpenPlaygroundHere(EntityPass& entity_pass) {
-  if (!Playground::is_enabled()) {
+  if (!switches_.enable_playground) {
     return true;
   }
 
@@ -31,7 +31,7 @@ bool EntityPlayground::OpenPlaygroundHere(EntityPass& entity_pass) {
 }
 
 bool EntityPlayground::OpenPlaygroundHere(Entity entity) {
-  if (!Playground::is_enabled()) {
+  if (!switches_.enable_playground) {
     return true;
   }
 
@@ -46,7 +46,7 @@ bool EntityPlayground::OpenPlaygroundHere(Entity entity) {
 }
 
 bool EntityPlayground::OpenPlaygroundHere(EntityPlaygroundCallback callback) {
-  if (!Playground::is_enabled()) {
+  if (!switches_.enable_playground) {
     return true;
   }
 

diff --git a/impeller/playground/BUILD.gn b/impeller/playground/BUILD.gn
@@ -58,8 +58,6 @@ impeller_component("playground") {
     public_deps += [ "../fixtures:shader_subgroup_fixtures" ]
   }
 
-  public_configs = [ ":playground_config" ]
-
   if (is_mac) {
     frameworks = [
       "AppKit.framework",
@@ -83,9 +81,3 @@ impeller_component("playground_test") {
     "//flutter/testing",
   ]
 }
-
-config("playground_config") {
-  if (impeller_enable_playground) {
-    defines = [ "IMPELLER_ENABLE_PLAYGROUND" ]
-  }
-}
diff --git a/impeller/playground/playground.cc b/impeller/playground/playground.cc
@@ -202,7 +202,7 @@ class AutoReleasePool {
 
 bool Playground::OpenPlaygroundHere(
     const Renderer::RenderCallback& render_callback) {
-  if (!is_enabled()) {
+  if (!switches_.enable_playground) {
     return true;
   }
 

diff --git a/impeller/playground/playground.h b/impeller/playground/playground.h
@@ -38,8 +38,6 @@ class Playground {
 
   virtual ~Playground();
 
-  static constexpr bool is_enabled() { return is_enabled_; }
-
   static bool ShouldOpenNewPlaygrounds();
 
   void SetupContext(PlaygroundBackend backend);
@@ -98,15 +96,9 @@ class Playground {
   virtual bool ShouldKeepRendering() const;
 
  private:
-#if IMPELLER_ENABLE_PLAYGROUND
-  static const bool is_enabled_ = true;
-#else
-  static const bool is_enabled_ = false;
-#endif  // IMPELLER_ENABLE_PLAYGROUND
+  struct GLFWInitializer;
 
   fml::TimeDelta start_time_;
-
-  struct GLFWInitializer;
   std::unique_ptr<GLFWInitializer> glfw_initializer_;
   std::unique_ptr<PlaygroundImpl> impl_;
   std::shared_ptr<Context> context_;

diff --git a/impeller/playground/switches.cc b/impeller/playground/switches.cc
@@ -11,9 +11,12 @@ namespace impeller {
 PlaygroundSwitches::PlaygroundSwitches() = default;
 
 PlaygroundSwitches::PlaygroundSwitches(const fml::CommandLine& args) {
+  enable_playground = args.HasOption("enable_playground");
   std::string timeout_str;
   if (args.GetOptionValue("playground_timeout_ms", &timeout_str)) {
     timeout = std::chrono::milliseconds(atoi(timeout_str.c_str()));
+    // Specifying a playground timeout implies you want to enable playgrounds.
+    enable_playground = true;
   }
   enable_vulkan_validation = args.HasOption("enable_vulkan_validation");
 }

diff --git a/impeller/playground/switches.h b/impeller/playground/switches.h
@@ -13,6 +13,7 @@
 namespace impeller {
 
 struct PlaygroundSwitches {
+  bool enable_playground = false;
   // If specified, the playgrounds will render for at least the duration
   // specified in the timeout. If the timeout is zero, exactly one frame will be
   // rendered in the playground.

diff --git a/impeller/tools/impeller.gni b/impeller/tools/impeller.gni
@@ -8,9 +8,6 @@ import("//flutter/impeller/tools/malioc.gni")
 import("//flutter/testing/testing.gni")
 
 declare_args() {
-  # Whether playgrounds are enabled for unit tests.
-  impeller_enable_playground = false
-
   # Whether the Metal backend is enabled.
   impeller_enable_metal = is_mac || is_ios
 

diff --git a/tools/gn b/tools/gn
@@ -546,11 +546,6 @@ def to_gn_args(args):
     gn_args['dart_use_mallinfo2'] = args.use_mallinfo2
 
   # Impeller flags.
-  if args.enable_impeller_playground:
-    gn_args['impeller_enable_playground'] = args.enable_impeller_playground
-  elif os.getenv('FLUTTER_IMPELLER_ENABLE_PLAYGROUND', '0') == '1':
-    gn_args['impeller_enable_playground'] = True
-
   if args.enable_impeller_3d:
     gn_args['impeller_enable_3d'] = True
 
@@ -977,13 +972,6 @@ def parse_args(args):
   )
 
   # Impeller flags.
-  parser.add_argument(
-      '--enable-impeller-playground',
-      default=False,
-      action='store_true',
-      help='Whether impeller unit tests run in playground mode.'
-  )
-
   parser.add_argument(
       '--enable-impeller-vulkan',
       default=False,