Skip to content

Commit

Permalink
Reland: Enable hybrid composition by default on Android (flutter#20722)…
Browse files Browse the repository at this point in the history
… (flutter#20864)

This reverts commit 4de62c7.
  • Loading branch information
Emmanuel Garcia authored Aug 31, 2020
1 parent 165abef commit 5e54c70
Show file tree
Hide file tree
Showing 17 changed files with 36 additions and 100 deletions.
9 changes: 0 additions & 9 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
4 changes: 4 additions & 0 deletions flow/surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,8 @@ std::unique_ptr<GLContextResult> Surface::MakeRenderContextCurrent() {
return std::make_unique<GLContextDefaultResult>(true);
}

bool Surface::ClearRenderContext() {
return false;
}

} // namespace flutter
2 changes: 2 additions & 0 deletions flow/surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class Surface {

virtual std::unique_ptr<GLContextResult> MakeRenderContextCurrent();

virtual bool ClearRenderContext();

private:
FML_DISALLOW_COPY_AND_ASSIGN(Surface);
};
Expand Down
4 changes: 4 additions & 0 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> 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) {
Expand Down
3 changes: 0 additions & 3 deletions shell/common/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 1 addition & 9 deletions shell/common/switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions shell/gpu/gpu_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -346,4 +346,9 @@ std::unique_ptr<GLContextResult> GPUSurfaceGL::MakeRenderContextCurrent() {
return delegate_->GLContextMakeCurrent();
}

// |Surface|
bool GPUSurfaceGL::ClearRenderContext() {
return delegate_->GLContextClearCurrent();
}

} // namespace flutter
3 changes: 3 additions & 0 deletions shell/gpu/gpu_surface_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class GPUSurfaceGL : public Surface {
// |Surface|
std::unique_ptr<GLContextResult> MakeRenderContextCurrent() override;

// |Surface|
bool ClearRenderContext() override;

private:
GPUSurfaceGLDelegate* delegate_;
sk_sp<GrDirectContext> context_;
Expand Down
58 changes: 15 additions & 43 deletions shell/platform/android/android_shell_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ static PlatformData GetDefaultPlatformData() {
return platform_data;
}

bool AndroidShellHolder::use_embedded_view;

AndroidShellHolder::AndroidShellHolder(
flutter::Settings settings,
std::shared_ptr<PlatformViewAndroidJNI> jni_facade,
Expand Down Expand Up @@ -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_);
Expand Down
8 changes: 0 additions & 8 deletions shell/platform/android/android_shell_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<PlatformViewAndroidJNI> jni_facade,
bool is_background_view);
Expand Down
3 changes: 0 additions & 3 deletions shell/platform/android/android_surface_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
3 changes: 0 additions & 3 deletions shell/platform/android/android_surface_software.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,6 @@ bool AndroidSurfaceSoftware::PresentBackingStore(

// |GPUSurfaceSoftwareDelegate|
ExternalViewEmbedder* AndroidSurfaceSoftware::GetExternalViewEmbedder() {
if (!AndroidShellHolder::use_embedded_view) {
return nullptr;
}
return external_view_embedder_.get();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,6 @@ private static String getNetworkPolicy(ApplicationInfo appInfo, Context context)
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 {
Expand Down Expand Up @@ -155,7 +150,6 @@ public static FlutterApplicationInfo load(@NonNull Context applicationContext) {
getString(appInfo.metaData, PUBLIC_FLUTTER_ASSETS_DIR_KEY),
getNetworkPolicy(appInfo, applicationContext),
appInfo.nativeLibraryDir,
clearTextPermitted,
getUseEmbeddedView(appInfo));
clearTextPermitted);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -29,8 +26,7 @@ public 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;
Expand All @@ -41,6 +37,5 @@ public FlutterApplicationInfo(
this.nativeLibraryDir = nativeLibraryDir;
this.domainNetworkPolicy = domainNetworkPolicy == null ? "" : domainNetworkPolicy;
this.clearTextPermitted = clearTextPermitted;
this.useEmbeddedView = useEmbeddedView;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,6 @@ public void ensureInitializationComplete(
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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public void itGeneratesCorrectApplicationInfoWithDefaultManifest() {
assertEquals("", info.domainNetworkPolicy);
assertNull(info.nativeLibraryDir);
assertEquals(true, info.clearTextPermitted);
assertEquals(false, info.useEmbeddedView);
}

@Config(shadows = {ApplicationInfoLoaderTest.ShadowNetworkSecurityPolicy.class})
Expand Down Expand Up @@ -92,7 +91,6 @@ public void itGeneratesCorrectApplicationInfoWithCustomValues() throws Exception
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);
Expand All @@ -102,7 +100,6 @@ public void itGeneratesCorrectApplicationInfoWithCustomValues() throws Exception
assertEquals("testassets", info.flutterAssetsDir);
assertNull(info.nativeLibraryDir);
assertEquals("", info.domainNetworkPolicy);
assertEquals(true, info.useEmbeddedView);
}

@Test
Expand Down
3 changes: 0 additions & 3 deletions testing/scenario_app/android/app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@
<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>
<meta-data
android:name="io.flutter.embedded_views_preview"
android:value="true" />
</application>
<uses-permission android:name="android.permission.INTERNET" />
</manifest>

0 comments on commit 5e54c70

Please sign in to comment.