From 5e54c707e890917aca55c20de9df6e05d2bfe742 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 31 Aug 2020 12:17:11 -0700 Subject: [PATCH] Reland: Enable hybrid composition by default on Android (#20722) (#20864) This reverts commit 4de62c7c2659607acdc75ab8c1ccd305a3c6f9d1. --- common/settings.h | 9 --- flow/surface.cc | 4 ++ flow/surface.h | 2 + shell/common/rasterizer.cc | 4 ++ shell/common/switches.cc | 3 - shell/common/switches.h | 10 +--- shell/gpu/gpu_surface_gl.cc | 5 ++ shell/gpu/gpu_surface_gl.h | 3 + .../platform/android/android_shell_holder.cc | 58 +++++-------------- shell/platform/android/android_shell_holder.h | 8 --- shell/platform/android/android_surface_gl.cc | 3 - .../android/android_surface_software.cc | 3 - .../engine/loader/ApplicationInfoLoader.java | 8 +-- .../engine/loader/FlutterApplicationInfo.java | 7 +-- .../engine/loader/FlutterLoader.java | 3 - .../loader/ApplicationInfoLoaderTest.java | 3 - .../android/app/src/main/AndroidManifest.xml | 3 - 17 files changed, 36 insertions(+), 100 deletions(-) diff --git a/common/settings.h b/common/settings.h index ae52e4a73..6d7b07a2c 100644 --- a/common/settings.h +++ b/common/settings.h @@ -223,15 +223,6 @@ struct Settings { /// to log a timeline event that tracks the latency of engine startup. std::chrono::microseconds engine_start_timestamp = {}; - /// Whether the application claims that it uses the android embedded view for - /// platform views. - /// - /// A `true` value will result the raster task runner always run on the - /// platform thread. - // TODO(cyanlaz): Remove this when dynamic thread merging is done. - // https://github.com/flutter/flutter/issues/59930 - bool use_embedded_view = false; - std::string ToString() const; }; diff --git a/flow/surface.cc b/flow/surface.cc index a19ab9afe..7dbdfb067 100644 --- a/flow/surface.cc +++ b/flow/surface.cc @@ -18,4 +18,8 @@ std::unique_ptr Surface::MakeRenderContextCurrent() { return std::make_unique(true); } +bool Surface::ClearRenderContext() { + return false; +} + } // namespace flutter diff --git a/flow/surface.h b/flow/surface.h index 3009af650..610cc4323 100644 --- a/flow/surface.h +++ b/flow/surface.h @@ -33,6 +33,8 @@ class Surface { virtual std::unique_ptr MakeRenderContextCurrent(); + virtual bool ClearRenderContext(); + private: FML_DISALLOW_COPY_AND_ASSIGN(Surface); }; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 5bae45f00..8516d3cd5 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -178,6 +178,10 @@ void Rasterizer::Draw(fml::RefPtr> pipeline) { consume_result = PipelineConsumeResult::MoreAvailable; } + if (surface_ != nullptr) { + surface_->ClearRenderContext(); + } + // Merging the thread as we know the next `Draw` should be run on the platform // thread. if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr) { diff --git a/shell/common/switches.cc b/shell/common/switches.cc index 16494204d..de76aa1a1 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -229,9 +229,6 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { : "127.0.0.1"; } - settings.use_embedded_view = - command_line.HasOption(FlagForSwitch(Switch::UseEmbeddedView)); - // Set Observatory Port if (command_line.HasOption(FlagForSwitch(Switch::DeviceObservatoryPort))) { if (!GetSwitchValue(command_line, Switch::DeviceObservatoryPort, diff --git a/shell/common/switches.h b/shell/common/switches.h index 1110261f7..f33e27224 100644 --- a/shell/common/switches.h +++ b/shell/common/switches.h @@ -196,15 +196,7 @@ DEF_SWITCH( "Uses separate threads for the platform, UI, GPU and IO task runners. " "By default, a single thread is used for all task runners. Only available " "in the flutter_tester.") -// TODO(cyanlaz): Remove this when dynamic thread merging is done. -// https://github.com/flutter/flutter/issues/59930 -DEF_SWITCH(UseEmbeddedView, - "use-embedded-view", - "Whether an android application uses embedded views." - "This is a temporary flag to make the raster task runner runs on " - "the platform thread." - "This flag should be removed once the dynamic thread merging is " - "enabled on android.") + DEF_SWITCHES_END void PrintUsage(const std::string& executable_name); diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc index b7086654e..17cb60b7c 100644 --- a/shell/gpu/gpu_surface_gl.cc +++ b/shell/gpu/gpu_surface_gl.cc @@ -346,4 +346,9 @@ std::unique_ptr GPUSurfaceGL::MakeRenderContextCurrent() { return delegate_->GLContextMakeCurrent(); } +// |Surface| +bool GPUSurfaceGL::ClearRenderContext() { + return delegate_->GLContextClearCurrent(); +} + } // namespace flutter diff --git a/shell/gpu/gpu_surface_gl.h b/shell/gpu/gpu_surface_gl.h index aab085c92..326efcec3 100644 --- a/shell/gpu/gpu_surface_gl.h +++ b/shell/gpu/gpu_surface_gl.h @@ -48,6 +48,9 @@ class GPUSurfaceGL : public Surface { // |Surface| std::unique_ptr MakeRenderContextCurrent() override; + // |Surface| + bool ClearRenderContext() override; + private: GPUSurfaceGLDelegate* delegate_; sk_sp context_; diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 7272571d7..d58ad703f 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -28,8 +28,6 @@ static PlatformData GetDefaultPlatformData() { return platform_data; } -bool AndroidShellHolder::use_embedded_view; - AndroidShellHolder::AndroidShellHolder( flutter::Settings settings, std::shared_ptr jni_facade, @@ -102,47 +100,21 @@ AndroidShellHolder::AndroidShellHolder( ui_runner = thread_host_.ui_thread->GetTaskRunner(); io_runner = thread_host_.io_thread->GetTaskRunner(); } - if (settings.use_embedded_view) { - use_embedded_view = true; - // Embedded views requires the gpu and the platform views to be the same. - // The plan is to eventually dynamically merge the threads when there's a - // platform view in the layer tree. - // For now we use a fixed thread configuration with the same thread used as - // the gpu and platform task runner. - // TODO(amirh/chinmaygarde): remove this, and dynamically change the thread - // configuration. https://github.com/flutter/flutter/issues/23975 - // https://github.com/flutter/flutter/issues/59930 - flutter::TaskRunners task_runners(thread_label, // label - platform_runner, // platform - platform_runner, // raster - ui_runner, // ui - io_runner // io - ); - - shell_ = - Shell::Create(task_runners, // task runners - GetDefaultPlatformData(), // window data - settings_, // settings - on_create_platform_view, // platform view create callback - on_create_rasterizer // rasterizer create callback - ); - } else { - use_embedded_view = false; - flutter::TaskRunners task_runners(thread_label, // label - platform_runner, // platform - gpu_runner, // raster - ui_runner, // ui - io_runner // io - ); - - shell_ = - Shell::Create(task_runners, // task runners - GetDefaultPlatformData(), // window data - settings_, // settings - on_create_platform_view, // platform view create callback - on_create_rasterizer // rasterizer create callback - ); - } + + flutter::TaskRunners task_runners(thread_label, // label + platform_runner, // platform + gpu_runner, // raster + ui_runner, // ui + io_runner // io + ); + + shell_ = + Shell::Create(task_runners, // task runners + GetDefaultPlatformData(), // window data + settings_, // settings + on_create_platform_view, // platform view create callback + on_create_rasterizer // rasterizer create callback + ); platform_view_ = weak_platform_view; FML_DCHECK(platform_view_); diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index 28ad86108..932f12ff7 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -21,14 +21,6 @@ namespace flutter { class AndroidShellHolder { public: - // Whether the application sets to use embedded_view view - // `io.flutter.embedded_views_preview` flag. This can be static because it is - // determined by the application and it is safe when there are multiple - // `AndroidSurface`s. - // TODO(cyanglaz): remove this when dynamic thread merging is enabled on - // android. https://github.com/flutter/flutter/issues/59930 - static bool use_embedded_view; - AndroidShellHolder(flutter::Settings settings, std::shared_ptr jni_facade, bool is_background_view); diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index 97ba6186f..06e0c0a06 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -127,9 +127,6 @@ intptr_t AndroidSurfaceGL::GLContextFBO(GLFrameInfo frame_info) const { // |GPUSurfaceGLDelegate| ExternalViewEmbedder* AndroidSurfaceGL::GetExternalViewEmbedder() { - if (!AndroidShellHolder::use_embedded_view) { - return nullptr; - } return external_view_embedder_.get(); } diff --git a/shell/platform/android/android_surface_software.cc b/shell/platform/android/android_surface_software.cc index 1f99b967b..126f127bd 100644 --- a/shell/platform/android/android_surface_software.cc +++ b/shell/platform/android/android_surface_software.cc @@ -146,9 +146,6 @@ bool AndroidSurfaceSoftware::PresentBackingStore( // |GPUSurfaceSoftwareDelegate| ExternalViewEmbedder* AndroidSurfaceSoftware::GetExternalViewEmbedder() { - if (!AndroidShellHolder::use_embedded_view) { - return nullptr; - } return external_view_embedder_.get(); } diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/ApplicationInfoLoader.java b/shell/platform/android/io/flutter/embedding/engine/loader/ApplicationInfoLoader.java index c29ddd9f2..bc1f71af2 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/ApplicationInfoLoader.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/ApplicationInfoLoader.java @@ -79,11 +79,6 @@ final class ApplicationInfoLoader { return output.toString(); } - private static boolean getUseEmbeddedView(ApplicationInfo appInfo) { - Bundle bundle = appInfo.metaData; - return bundle != null && bundle.getBoolean("io.flutter.embedded_views_preview"); - } - private static void parseDomainConfig( XmlResourceParser xrp, JSONArray output, boolean inheritedCleartextPermitted) throws IOException, XmlPullParserException { @@ -155,7 +150,6 @@ final class ApplicationInfoLoader { getString(appInfo.metaData, PUBLIC_FLUTTER_ASSETS_DIR_KEY), getNetworkPolicy(appInfo, applicationContext), appInfo.nativeLibraryDir, - clearTextPermitted, - getUseEmbeddedView(appInfo)); + clearTextPermitted); } } diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterApplicationInfo.java b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterApplicationInfo.java index 3d5c2b10c..33a9e4488 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterApplicationInfo.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterApplicationInfo.java @@ -18,9 +18,6 @@ public final class FlutterApplicationInfo { final String domainNetworkPolicy; final String nativeLibraryDir; final boolean clearTextPermitted; - // TODO(cyanlaz): Remove this when dynamic thread merging is done. - // https://github.com/flutter/flutter/issues/59930 - final boolean useEmbeddedView; public FlutterApplicationInfo( String aotSharedLibraryName, @@ -29,8 +26,7 @@ public final class FlutterApplicationInfo { String flutterAssetsDir, String domainNetworkPolicy, String nativeLibraryDir, - boolean clearTextPermitted, - boolean useEmbeddedView) { + boolean clearTextPermitted) { this.aotSharedLibraryName = aotSharedLibraryName == null ? DEFAULT_AOT_SHARED_LIBRARY_NAME : aotSharedLibraryName; this.vmSnapshotData = vmSnapshotData == null ? DEFAULT_VM_SNAPSHOT_DATA : vmSnapshotData; @@ -41,6 +37,5 @@ public final class FlutterApplicationInfo { this.nativeLibraryDir = nativeLibraryDir; this.domainNetworkPolicy = domainNetworkPolicy == null ? "" : domainNetworkPolicy; this.clearTextPermitted = clearTextPermitted; - this.useEmbeddedView = useEmbeddedView; } } diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java index 8af9539c9..2b7c0879b 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java @@ -221,9 +221,6 @@ public class FlutterLoader { if (flutterApplicationInfo.domainNetworkPolicy != null) { shellArgs.add("--domain-network-policy=" + flutterApplicationInfo.domainNetworkPolicy); } - if (flutterApplicationInfo.useEmbeddedView) { - shellArgs.add("--use-embedded-view"); - } if (settings.getLogTag() != null) { shellArgs.add("--log-tag=" + settings.getLogTag()); } diff --git a/shell/platform/android/test/io/flutter/embedding/engine/loader/ApplicationInfoLoaderTest.java b/shell/platform/android/test/io/flutter/embedding/engine/loader/ApplicationInfoLoaderTest.java index 69ead8182..f17adb785 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/loader/ApplicationInfoLoaderTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/loader/ApplicationInfoLoaderTest.java @@ -47,7 +47,6 @@ public class ApplicationInfoLoaderTest { assertEquals("", info.domainNetworkPolicy); assertNull(info.nativeLibraryDir); assertEquals(true, info.clearTextPermitted); - assertEquals(false, info.useEmbeddedView); } @Config(shadows = {ApplicationInfoLoaderTest.ShadowNetworkSecurityPolicy.class}) @@ -92,7 +91,6 @@ public class ApplicationInfoLoaderTest { bundle.putString(ApplicationInfoLoader.PUBLIC_VM_SNAPSHOT_DATA_KEY, "testvmsnapshot"); bundle.putString(ApplicationInfoLoader.PUBLIC_ISOLATE_SNAPSHOT_DATA_KEY, "testisolatesnapshot"); bundle.putString(ApplicationInfoLoader.PUBLIC_FLUTTER_ASSETS_DIR_KEY, "testassets"); - bundle.putBoolean("io.flutter.embedded_views_preview", true); Context context = generateMockContext(bundle, null); FlutterApplicationInfo info = ApplicationInfoLoader.load(context); assertNotNull(info); @@ -102,7 +100,6 @@ public class ApplicationInfoLoaderTest { assertEquals("testassets", info.flutterAssetsDir); assertNull(info.nativeLibraryDir); assertEquals("", info.domainNetworkPolicy); - assertEquals(true, info.useEmbeddedView); } @Test diff --git a/testing/scenario_app/android/app/src/main/AndroidManifest.xml b/testing/scenario_app/android/app/src/main/AndroidManifest.xml index a580f17b7..1ddceaf51 100644 --- a/testing/scenario_app/android/app/src/main/AndroidManifest.xml +++ b/testing/scenario_app/android/app/src/main/AndroidManifest.xml @@ -35,9 +35,6 @@ - -- GitLab