From 35af6f8fc3fdbf68205d43bf190b1dc584b86bc6 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 22 Jun 2020 09:25:37 -0700 Subject: [PATCH 01/13] Revert "Revert "Add `GetBoundingRectAfterMutations` to EmbeddedViewParams to calculate the final bounding rect for platform view (#19170)" (#19204)" This reverts commit 9cecc5f115ebfd06251d6eebdbb532e462a01e07. --- ci/licenses_golden/licenses_flutter | 1 + flow/BUILD.gn | 1 + flow/embedded_view_params_unittests.cc | 98 +++++++++++++++++++ flow/embedded_views.h | 46 +++++++-- flow/layers/platform_view_layer.cc | 7 +- .../framework/Source/FlutterPlatformViews.mm | 7 +- shell/platform/embedder/embedder_layers.cc | 10 +- 7 files changed, 148 insertions(+), 22 deletions(-) create mode 100644 flow/embedded_view_params_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 242783e947b27..4cd541e0fed4f 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -26,6 +26,7 @@ FILE: ../../../flutter/common/task_runners.cc FILE: ../../../flutter/common/task_runners.h FILE: ../../../flutter/flow/compositor_context.cc FILE: ../../../flutter/flow/compositor_context.h +FILE: ../../../flutter/flow/embedded_view_params_unittests.cc FILE: ../../../flutter/flow/embedded_views.cc FILE: ../../../flutter/flow/embedded_views.h FILE: ../../../flutter/flow/gl_context_switch.cc diff --git a/flow/BUILD.gn b/flow/BUILD.gn index a8c3592ccecbf..85c55d2cfba0b 100644 --- a/flow/BUILD.gn +++ b/flow/BUILD.gn @@ -130,6 +130,7 @@ executable("flow_unittests") { testonly = true sources = [ + "embedded_view_params_unittests.cc", "flow_run_all_unittests.cc", "flow_test_utils.cc", "flow_test_utils.h", diff --git a/flow/embedded_view_params_unittests.cc b/flow/embedded_view_params_unittests.cc new file mode 100644 index 0000000000000..cb94936e17ddf --- /dev/null +++ b/flow/embedded_view_params_unittests.cc @@ -0,0 +1,98 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/flow/embedded_views.h" +#include "flutter/fml/logging.h" +#include "gtest/gtest.h" + +namespace flutter { +namespace testing { + +TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithNoMutations) { + MutatorsStack stack; + SkMatrix matrix; + + EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack); + const SkRect& rect = params.finalBoundingRect(); + ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), 0)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 1)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 1)); +} + +TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithScale) { + MutatorsStack stack; + SkMatrix matrix = SkMatrix::Scale(2, 2); + stack.PushTransform(matrix); + + EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack); + const SkRect& rect = params.finalBoundingRect(); + ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), 0)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 2)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 2)); +} + +TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithTranslate) { + MutatorsStack stack; + SkMatrix matrix = SkMatrix::MakeTrans(1, 1); + stack.PushTransform(matrix); + + EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack); + const SkRect& rect = params.finalBoundingRect(); + ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), 1)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 1)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 1)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 1)); +} + +TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithRotation90) { + MutatorsStack stack; + SkMatrix matrix; + matrix.setRotate(90); + stack.PushTransform(matrix); + + EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack); + const SkRect& rect = params.finalBoundingRect(); + + FML_DLOG(ERROR) << rect.x(); + ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), -1)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 1)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 1)); +} + +TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithRotation45) { + MutatorsStack stack; + SkMatrix matrix; + matrix.setRotate(45); + stack.PushTransform(matrix); + + EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack); + const SkRect& rect = params.finalBoundingRect(); + ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), -sqrt(2) / 2)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), sqrt(2))); + ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), sqrt(2))); +} + +TEST(EmbeddedViewParams, + GetBoundingRectAfterMutationsWithTranslateScaleAndRotation) { + SkMatrix matrix = SkMatrix::MakeTrans(2, 2); + matrix.preScale(3, 3); + matrix.preRotate(90); + + MutatorsStack stack; + stack.PushTransform(matrix); + + EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack); + const SkRect& rect = params.finalBoundingRect(); + ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), -1)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 2)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 3)); + ASSERT_TRUE(SkScalarNearlyEqual(rect.height(), 3)); +} + +} // namespace testing +} // namespace flutter diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 9cc90860101a8..3493f02be7415 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -186,21 +186,49 @@ class EmbeddedViewParams { public: EmbeddedViewParams() = default; + EmbeddedViewParams(SkMatrix matrix, + SkSize size_points, + MutatorsStack mutators_stack) + : matrix_(matrix), + size_points_(size_points), + mutators_stack_(mutators_stack) { + SkPath path; + SkRect starting_rect = SkRect::MakeSize(size_points); + path.addRect(starting_rect); + path.transform(matrix); + final_bounding_rect_ = path.computeTightBounds(); + } + EmbeddedViewParams(const EmbeddedViewParams& other) { - offsetPixels = other.offsetPixels; - sizePoints = other.sizePoints; - mutatorsStack = other.mutatorsStack; + size_points_ = other.size_points_; + mutators_stack_ = other.mutators_stack_; + matrix_ = other.matrix_; + final_bounding_rect_ = other.final_bounding_rect_; }; - SkPoint offsetPixels; - SkSize sizePoints; - MutatorsStack mutatorsStack; + // The original size of the platform view before any mutation matrix is + // applied. + const SkSize& sizePoints() const { return size_points_; }; + // The mutators stack contains the detailed step by step mutations for this + // platform view. + const MutatorsStack& mutatorsStack() const { return mutators_stack_; }; + // The bounding rect of the platform view after applying all the mutations. + // + // Clippings are ignored. + const SkRect& finalBoundingRect() const { return final_bounding_rect_; } bool operator==(const EmbeddedViewParams& other) const { - return offsetPixels == other.offsetPixels && - sizePoints == other.sizePoints && - mutatorsStack == other.mutatorsStack; + return size_points_ == other.size_points_ && + mutators_stack_ == other.mutators_stack_ && + final_bounding_rect_ == other.final_bounding_rect_ && + matrix_ == other.matrix_; } + + private: + SkMatrix matrix_; + SkSize size_points_; + MutatorsStack mutators_stack_; + SkRect final_bounding_rect_; }; enum class PostPrerollResult { kResubmitFrame, kSuccess }; diff --git a/flow/layers/platform_view_layer.cc b/flow/layers/platform_view_layer.cc index 81c40a99f0d89..551a270b16542 100644 --- a/flow/layers/platform_view_layer.cc +++ b/flow/layers/platform_view_layer.cc @@ -27,11 +27,8 @@ void PlatformViewLayer::Preroll(PrerollContext* context, } context->has_platform_view = true; std::unique_ptr params = - std::make_unique(); - params->offsetPixels = - SkPoint::Make(matrix.getTranslateX(), matrix.getTranslateY()); - params->sizePoints = size_; - params->mutatorsStack = context->mutators_stack; + std::make_unique(matrix, size_, + context->mutators_stack); context->view_embedder->PrerollCompositeEmbeddedView(view_id_, std::move(params)); } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index f590701e7e2cb..474e9f084c1ee 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -407,13 +407,14 @@ void FlutterPlatformViewsController::CompositeWithParams(int view_id, const EmbeddedViewParams& params) { - CGRect frame = CGRectMake(0, 0, params.sizePoints.width(), params.sizePoints.height()); + CGRect frame = CGRectMake(0, 0, params.sizePoints().width(), params.sizePoints().height()); UIView* touchInterceptor = touch_interceptors_[view_id].get(); touchInterceptor.layer.transform = CATransform3DIdentity; touchInterceptor.frame = frame; touchInterceptor.alpha = 1; - int currentClippingCount = CountClips(params.mutatorsStack); + const MutatorsStack& mutatorStack = params.mutatorsStack(); + int currentClippingCount = CountClips(mutatorStack); int previousClippingCount = clip_count_[view_id]; if (currentClippingCount != previousClippingCount) { clip_count_[view_id] = currentClippingCount; @@ -424,7 +425,7 @@ ReconstructClipViewsChain(currentClippingCount, touchInterceptor, oldPlatformViewRoot); root_views_[view_id] = fml::scoped_nsobject([newPlatformViewRoot retain]); } - ApplyMutators(params.mutatorsStack, touchInterceptor); + ApplyMutators(mutatorStack, touchInterceptor); } SkCanvas* FlutterPlatformViewsController::CompositeEmbeddedView(int view_id) { diff --git a/shell/platform/embedder/embedder_layers.cc b/shell/platform/embedder/embedder_layers.cc index 3b634235f820a..4716ea15683dd 100644 --- a/shell/platform/embedder/embedder_layers.cc +++ b/shell/platform/embedder/embedder_layers.cc @@ -108,7 +108,7 @@ void EmbedderLayers::PushPlatformViewLayer( view.struct_size = sizeof(FlutterPlatformView); view.identifier = identifier; - const auto& mutators = params.mutatorsStack; + const auto& mutators = params.mutatorsStack(); std::vector mutations_array; @@ -180,10 +180,10 @@ void EmbedderLayers::PushPlatformViewLayer( layer.platform_view = platform_views_referenced_.back().get(); const auto layer_bounds = - SkRect::MakeXYWH(params.offsetPixels.x(), // - params.offsetPixels.y(), // - params.sizePoints.width() * device_pixel_ratio_, // - params.sizePoints.height() * device_pixel_ratio_ // + SkRect::MakeXYWH(params.finalBoundingRect().x(), // + params.finalBoundingRect().y(), // + params.sizePoints().width() * device_pixel_ratio_, // + params.sizePoints().height() * device_pixel_ratio_ // ); const auto transformed_layer_bounds = From fb89d64808234c2e364d7ea83c3b5aba02036f01 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 22 Jun 2020 09:28:15 -0700 Subject: [PATCH 02/13] fix compile error --- .../external_view_embedder/external_view_embedder.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index 9485eedff5491..9d5dae31c89f1 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -63,10 +63,10 @@ SkRect AndroidExternalViewEmbedder::GetViewRect(int view_id) const { const EmbeddedViewParams& params = view_params_.at(view_id); // TODO(egarciad): The rect should be computed from the mutator stack. // https://github.com/flutter/flutter/issues/59821 - return SkRect::MakeXYWH(params.offsetPixels.x(), // - params.offsetPixels.y(), // - params.sizePoints.width() * device_pixel_ratio_, // - params.sizePoints.height() * device_pixel_ratio_ // + return SkRect::MakeXYWH(params.finalBoundingRect().x(), // + params.finalBoundingRect().y(), // + params.sizePoints().width() * device_pixel_ratio_, // + params.sizePoints().height() * device_pixel_ratio_ // ); } From 825ca5a08c5f2633919e122c461511344e30530b Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Mon, 22 Jun 2020 12:13:29 -0700 Subject: [PATCH 03/13] fix external_view_embedder_unittests --- .../external_view_embedder.cc | 4 +- .../external_view_embedder_unittests.cc | 45 ++++++++++++------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index 9d5dae31c89f1..f1f46ab21b49c 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -63,8 +63,8 @@ SkRect AndroidExternalViewEmbedder::GetViewRect(int view_id) const { const EmbeddedViewParams& params = view_params_.at(view_id); // TODO(egarciad): The rect should be computed from the mutator stack. // https://github.com/flutter/flutter/issues/59821 - return SkRect::MakeXYWH(params.finalBoundingRect().x(), // - params.finalBoundingRect().y(), // + return SkRect::MakeXYWH(params.finalBoundingRect().x(), // + params.finalBoundingRect().y(), // params.sizePoints().width() * device_pixel_ratio_, // params.sizePoints().height() * device_pixel_ratio_ // ); diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index bba1c126564a1..a0551a0a39dea 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -198,9 +198,11 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) { embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5, raster_thread_merger); - auto view_params = std::make_unique(); - view_params->offsetPixels = SkPoint::Make(10, 20); - view_params->sizePoints = SkSize::Make(30, 40); + MutatorsStack stack; + SkMatrix matrix = SkMatrix::MakeTrans(10, 20); + stack.PushTransform(matrix); + auto view_params = + std::make_unique(matrix, SkSize::Make(30, 40), stack); auto view_id = 0; embedder->PrerollCompositeEmbeddedView(view_id, std::move(view_params)); @@ -219,14 +221,21 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect__ChangedParams) { raster_thread_merger); auto view_id = 0; - auto view_params_1 = std::make_unique(); - view_params_1->offsetPixels = SkPoint::Make(10, 20); - view_params_1->sizePoints = SkSize::Make(30, 40); + + MutatorsStack stack1; + SkMatrix matrix1 = SkMatrix::MakeTrans(10, 20); + stack1.PushTransform(matrix1); + auto view_params_1 = std::make_unique( + matrix1, SkSize::Make(30, 40), stack1); + embedder->PrerollCompositeEmbeddedView(view_id, std::move(view_params_1)); - auto view_params_2 = std::make_unique(); - view_params_2->offsetPixels = SkPoint::Make(50, 60); - view_params_2->sizePoints = SkSize::Make(70, 80); + MutatorsStack stack2; + SkMatrix matrix2 = SkMatrix::MakeTrans(50, 60); + stack2.PushTransform(matrix2); + auto view_params_2 = std::make_unique( + matrix2, SkSize::Make(70, 80), stack2); + embedder->PrerollCompositeEmbeddedView(view_id, std::move(view_params_2)); ASSERT_EQ(SkRect::MakeXYWH(50, 60, 105, 120), embedder->GetViewRect(view_id)); @@ -281,11 +290,14 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__RecycleSurfaces) { embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger); // Add an Android view. - auto view_params_1 = std::make_unique(); - view_params_1->offsetPixels = SkPoint::Make(100, 100); + MutatorsStack stack1; + SkMatrix matrix1 = SkMatrix::MakeTrans(100, 100); + stack1.PushTransform(matrix1); // TODO(egarciad): Investigate why Flow applies the device pixel ratio to // the offsetPixels, but not the sizePoints. - view_params_1->sizePoints = SkSize::Make(200, 200); + auto view_params_1 = std::make_unique( + matrix1, SkSize::Make(200, 200), stack1); + embedder->PrerollCompositeEmbeddedView(0, std::move(view_params_1)); // This is the recording canvas flow writes to. auto canvas_1 = embedder->CompositeEmbeddedView(0); @@ -329,11 +341,14 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__RecycleSurfaces) { embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger); // Add an Android view. - auto view_params_1 = std::make_unique(); - view_params_1->offsetPixels = SkPoint::Make(100, 100); + MutatorsStack stack1; + SkMatrix matrix1 = SkMatrix::MakeTrans(100, 100); + stack1.PushTransform(matrix1); // TODO(egarciad): Investigate why Flow applies the device pixel ratio to // the offsetPixels, but not the sizePoints. - view_params_1->sizePoints = SkSize::Make(200, 200); + auto view_params_1 = std::make_unique( + matrix1, SkSize::Make(200, 200), stack1); + embedder->PrerollCompositeEmbeddedView(0, std::move(view_params_1)); // This is the recording canvas flow writes to. auto canvas_1 = embedder->CompositeEmbeddedView(0); From 86967e6704a3310521b3b3eb0397719fe583b28a Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 23 Jun 2020 10:41:00 -0700 Subject: [PATCH 04/13] draft --- common/settings.h | 2 + .../platform/android/android_shell_holder.cc | 45 ++++++++++++++----- shell/platform/android/flutter_main.cc | 8 ++-- shell/platform/android/flutter_main.h | 3 +- .../engine/loader/FlutterLoader.java | 8 +++- 5 files changed, 50 insertions(+), 16 deletions(-) diff --git a/common/settings.h b/common/settings.h index 52abe1994bd60..f963cd298ae5f 100644 --- a/common/settings.h +++ b/common/settings.h @@ -213,6 +213,8 @@ struct Settings { /// to log a timeline event that tracks the latency of engine startup. std::chrono::microseconds engine_start_timestamp = {}; + bool use_embedded_view = false; + std::string ToString() const; }; diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 62f8acf6fc8fd..a0ea1d0457410 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -101,20 +101,45 @@ AndroidShellHolder::AndroidShellHolder( ui_runner = thread_host_.ui_thread->GetTaskRunner(); io_runner = thread_host_.io_thread->GetTaskRunner(); } - flutter::TaskRunners task_runners(thread_label, // label - platform_runner, // platform - gpu_runner, // raster - ui_runner, // ui - io_runner // io - ); - - shell_ = + if (settings.use_embedded_view) { + // 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 GetDefaultWindowData(), // window data settings_, // settings on_create_platform_view, // platform view create callback on_create_rasterizer // rasterizer create callback ); + } else { + 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 + GetDefaultWindowData(), // 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_); @@ -122,7 +147,7 @@ AndroidShellHolder::AndroidShellHolder( is_valid_ = shell_ != nullptr; if (is_valid_) { - task_runners.GetRasterTaskRunner()->PostTask([]() { + shell_->GetTaskRunners().GetRasterTaskRunner()->PostTask([]() { // Android describes -8 as "most important display threads, for // compositing the screen and retrieving input events". Conservatively // set the raster thread to slightly lower priority than it. @@ -134,7 +159,7 @@ AndroidShellHolder::AndroidShellHolder( } } }); - task_runners.GetUITaskRunner()->PostTask([]() { + shell_->GetTaskRunners().GetUITaskRunner()->PostTask([]() { if (::setpriority(PRIO_PROCESS, gettid(), -1) != 0) { FML_LOG(ERROR) << "Failed to set UI task runner priority"; } diff --git a/shell/platform/android/flutter_main.cc b/shell/platform/android/flutter_main.cc index 1493bf4f0d161..24538e95274f2 100644 --- a/shell/platform/android/flutter_main.cc +++ b/shell/platform/android/flutter_main.cc @@ -63,7 +63,8 @@ void FlutterMain::Init(JNIEnv* env, jstring kernelPath, jstring appStoragePath, jstring engineCachesPath, - jlong initTimeMillis) { + jlong initTimeMillis, + jboolean useEmbeddedView) { std::vector args; args.push_back("flutter"); for (auto& arg : fml::jni::StringArrayToVector(env, jargs)) { @@ -99,6 +100,8 @@ void FlutterMain::Init(JNIEnv* env, } } + settings.use_embedded_view = useEmbeddedView; + settings.task_observer_add = [](intptr_t key, fml::closure callback) { fml::MessageLoop::GetCurrent().AddTaskObserver(key, std::move(callback)); }; @@ -165,8 +168,7 @@ bool FlutterMain::Register(JNIEnv* env) { static const JNINativeMethod methods[] = { { .name = "nativeInit", - .signature = "(Landroid/content/Context;[Ljava/lang/String;Ljava/" - "lang/String;Ljava/lang/String;Ljava/lang/String;J)V", + .signature = "(Landroid/content/Context;[Ljava/lang/String;Ljava/", .fnPtr = reinterpret_cast(&Init), }, { diff --git a/shell/platform/android/flutter_main.h b/shell/platform/android/flutter_main.h index 171800b3ec500..6a32a88bae909 100644 --- a/shell/platform/android/flutter_main.h +++ b/shell/platform/android/flutter_main.h @@ -36,7 +36,8 @@ class FlutterMain { jstring kernelPath, jstring appStoragePath, jstring engineCachesPath, - jlong initTimeMillis); + jlong initTimeMillis, + jboolean useEmbeddedView); void SetupObservatoryUriCallback(JNIEnv* env); 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 c57d8c86b8374..0c830f77991a9 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java @@ -201,7 +201,6 @@ public void ensureInitializationComplete( + applicationInfo.nativeLibraryDir + File.separator + DEFAULT_LIBRARY); - if (args != null) { Collections.addAll(shellArgs, args); } @@ -234,13 +233,18 @@ public void ensureInitializationComplete( } long initTimeMillis = SystemClock.uptimeMillis() - initStartTimestampMillis; + + Bundle bundle = applicationInfo.metaData; + boolean use_embedded_view = bundle.getBoolean("io.flutter.embedded_views_preview"); + FlutterJNI.nativeInit( applicationContext, shellArgs.toArray(new String[0]), kernelPath, result.appStoragePath, result.engineCachesPath, - initTimeMillis); + initTimeMillis, + use_embedded_view); initialized = true; } catch (Exception e) { From 63f4e4166128296b2b2e38afe50ec5fb75ada8ea Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 23 Jun 2020 10:42:07 -0700 Subject: [PATCH 05/13] add extra bool to nativeInit --- shell/platform/android/flutter_main.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/shell/platform/android/flutter_main.cc b/shell/platform/android/flutter_main.cc index 24538e95274f2..1493bf4f0d161 100644 --- a/shell/platform/android/flutter_main.cc +++ b/shell/platform/android/flutter_main.cc @@ -63,8 +63,7 @@ void FlutterMain::Init(JNIEnv* env, jstring kernelPath, jstring appStoragePath, jstring engineCachesPath, - jlong initTimeMillis, - jboolean useEmbeddedView) { + jlong initTimeMillis) { std::vector args; args.push_back("flutter"); for (auto& arg : fml::jni::StringArrayToVector(env, jargs)) { @@ -100,8 +99,6 @@ void FlutterMain::Init(JNIEnv* env, } } - settings.use_embedded_view = useEmbeddedView; - settings.task_observer_add = [](intptr_t key, fml::closure callback) { fml::MessageLoop::GetCurrent().AddTaskObserver(key, std::move(callback)); }; @@ -168,7 +165,8 @@ bool FlutterMain::Register(JNIEnv* env) { static const JNINativeMethod methods[] = { { .name = "nativeInit", - .signature = "(Landroid/content/Context;[Ljava/lang/String;Ljava/", + .signature = "(Landroid/content/Context;[Ljava/lang/String;Ljava/" + "lang/String;Ljava/lang/String;Ljava/lang/String;J)V", .fnPtr = reinterpret_cast(&Init), }, { From d3a554f81f013e3d95d72df2ee2725990c2e0e64 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 23 Jun 2020 12:13:23 -0700 Subject: [PATCH 06/13] fix nativeInit params --- common/settings.h | 5 +++++ shell/platform/android/android_shell_holder.cc | 2 ++ shell/platform/android/flutter_main.cc | 9 +++++++-- shell/platform/android/flutter_main.h | 2 ++ .../android/io/flutter/embedding/engine/FlutterJNI.java | 3 ++- .../flutter/embedding/engine/loader/FlutterLoader.java | 2 ++ 6 files changed, 20 insertions(+), 3 deletions(-) diff --git a/common/settings.h b/common/settings.h index f963cd298ae5f..ea4970747fbe9 100644 --- a/common/settings.h +++ b/common/settings.h @@ -213,6 +213,11 @@ struct Settings { /// to log a timeline event that tracks the latency of engine startup. std::chrono::microseconds engine_start_timestamp = {}; + /// Does the application claim 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/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index a0ea1d0457410..6109d1d05a5fe 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -102,6 +102,7 @@ AndroidShellHolder::AndroidShellHolder( io_runner = thread_host_.io_thread->GetTaskRunner(); } if (settings.use_embedded_view) { + FML_DLOG(ERROR) << "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. @@ -125,6 +126,7 @@ AndroidShellHolder::AndroidShellHolder( on_create_rasterizer // rasterizer create callback ); } else { + FML_DLOG(ERROR) << "use_embedded_view FALSE"; flutter::TaskRunners task_runners(thread_label, // label platform_runner, // platform gpu_runner, // raster diff --git a/shell/platform/android/flutter_main.cc b/shell/platform/android/flutter_main.cc index 1493bf4f0d161..70fcde2868084 100644 --- a/shell/platform/android/flutter_main.cc +++ b/shell/platform/android/flutter_main.cc @@ -63,7 +63,8 @@ void FlutterMain::Init(JNIEnv* env, jstring kernelPath, jstring appStoragePath, jstring engineCachesPath, - jlong initTimeMillis) { + jlong initTimeMillis, + jboolean useEmbeddedView) { std::vector args; args.push_back("flutter"); for (auto& arg : fml::jni::StringArrayToVector(env, jargs)) { @@ -73,6 +74,10 @@ void FlutterMain::Init(JNIEnv* env, auto settings = SettingsFromCommandLine(command_line); + // TODO(cyanlaz): Remove this when dynamic thread merging is done. + // https://github.com/flutter/flutter/issues/59930 + settings.use_embedded_view = useEmbeddedView; + int64_t init_time_micros = initTimeMillis * 1000; settings.engine_start_timestamp = std::chrono::microseconds(Dart_TimelineGetMicros() - init_time_micros); @@ -166,7 +171,7 @@ bool FlutterMain::Register(JNIEnv* env) { { .name = "nativeInit", .signature = "(Landroid/content/Context;[Ljava/lang/String;Ljava/" - "lang/String;Ljava/lang/String;Ljava/lang/String;J)V", + "lang/String;Ljava/lang/String;Ljava/lang/String;J;Z)V", .fnPtr = reinterpret_cast(&Init), }, { diff --git a/shell/platform/android/flutter_main.h b/shell/platform/android/flutter_main.h index 6a32a88bae909..097dd9f5bee26 100644 --- a/shell/platform/android/flutter_main.h +++ b/shell/platform/android/flutter_main.h @@ -29,6 +29,8 @@ class FlutterMain { FlutterMain(flutter::Settings settings); + // TODO(cyanlaz): Remove `useEmbeddedView` when dynamic thread merging is done. + // https://github.com/flutter/flutter/issues/59930 static void Init(JNIEnv* env, jclass clazz, jobject context, diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 5fb4e337c6ef5..5e43047bf4789 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -110,7 +110,8 @@ public static native void nativeInit( @Nullable String bundlePath, @NonNull String appStoragePath, @NonNull String engineCachesPath, - long initTimeMillis); + long initTimeMillis, + boolean useEmbeddedView); /** * Prefetch the default font manager provided by SkFontMgr::RefDefault() which is a process-wide 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 0c830f77991a9..50f4c9ffbda8e 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java @@ -234,6 +234,8 @@ public void ensureInitializationComplete( long initTimeMillis = SystemClock.uptimeMillis() - initStartTimestampMillis; + // TODO(cyanlaz): Remove this when dynamic thread merging is done. + // https://github.com/flutter/flutter/issues/59930 Bundle bundle = applicationInfo.metaData; boolean use_embedded_view = bundle.getBoolean("io.flutter.embedded_views_preview"); From 227e151dc59e16ce4ebbba723fac31db346c1b47 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 23 Jun 2020 13:45:40 -0700 Subject: [PATCH 07/13] parse use_embedded_view flag --- shell/common/switches.cc | 2 ++ shell/common/switches.h | 7 +++++++ shell/platform/android/android_shell_holder.cc | 10 +++++----- shell/platform/android/flutter_main.cc | 9 ++------- shell/platform/android/flutter_main.h | 5 +---- .../io/flutter/embedding/engine/FlutterJNI.java | 3 +-- .../flutter/embedding/engine/loader/FlutterLoader.java | 6 ++++-- shell/platform/android/platform_view_android.cc | 8 +++++--- shell/platform/android/platform_view_android.h | 6 ++++-- 9 files changed, 31 insertions(+), 25 deletions(-) diff --git a/shell/common/switches.cc b/shell/common/switches.cc index e3f6e985d4d74..235a5148410d3 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -227,6 +227,8 @@ 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 3bef7d645dde9..62d0683ae866d 100644 --- a/shell/common/switches.h +++ b/shell/common/switches.h @@ -189,6 +189,13 @@ 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/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 6109d1d05a5fe..2ebd606e0fa3e 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -57,13 +57,14 @@ AndroidShellHolder::AndroidShellHolder( fml::WeakPtr weak_platform_view; Shell::CreateCallback on_create_platform_view = - [is_background_view, &jni_facade, &weak_platform_view](Shell& shell) { + [is_background_view, &jni_facade, &weak_platform_view, use_embedded_view = settings.use_embedded_view](Shell& shell) { std::unique_ptr platform_view_android; if (is_background_view) { platform_view_android = std::make_unique( shell, // delegate shell.GetTaskRunners(), // task runners - jni_facade // JNI interop + jni_facade, // JNI interop + use_embedded_view ); } else { platform_view_android = std::make_unique( @@ -71,7 +72,8 @@ AndroidShellHolder::AndroidShellHolder( shell.GetTaskRunners(), // task runners jni_facade, // JNI interop shell.GetSettings() - .enable_software_rendering // use software rendering + .enable_software_rendering, // use software rendering, + use_embedded_view ); } weak_platform_view = platform_view_android->GetWeakPtr(); @@ -102,7 +104,6 @@ AndroidShellHolder::AndroidShellHolder( io_runner = thread_host_.io_thread->GetTaskRunner(); } if (settings.use_embedded_view) { - FML_DLOG(ERROR) << "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. @@ -126,7 +127,6 @@ AndroidShellHolder::AndroidShellHolder( on_create_rasterizer // rasterizer create callback ); } else { - FML_DLOG(ERROR) << "use_embedded_view FALSE"; flutter::TaskRunners task_runners(thread_label, // label platform_runner, // platform gpu_runner, // raster diff --git a/shell/platform/android/flutter_main.cc b/shell/platform/android/flutter_main.cc index 70fcde2868084..1493bf4f0d161 100644 --- a/shell/platform/android/flutter_main.cc +++ b/shell/platform/android/flutter_main.cc @@ -63,8 +63,7 @@ void FlutterMain::Init(JNIEnv* env, jstring kernelPath, jstring appStoragePath, jstring engineCachesPath, - jlong initTimeMillis, - jboolean useEmbeddedView) { + jlong initTimeMillis) { std::vector args; args.push_back("flutter"); for (auto& arg : fml::jni::StringArrayToVector(env, jargs)) { @@ -74,10 +73,6 @@ void FlutterMain::Init(JNIEnv* env, auto settings = SettingsFromCommandLine(command_line); - // TODO(cyanlaz): Remove this when dynamic thread merging is done. - // https://github.com/flutter/flutter/issues/59930 - settings.use_embedded_view = useEmbeddedView; - int64_t init_time_micros = initTimeMillis * 1000; settings.engine_start_timestamp = std::chrono::microseconds(Dart_TimelineGetMicros() - init_time_micros); @@ -171,7 +166,7 @@ bool FlutterMain::Register(JNIEnv* env) { { .name = "nativeInit", .signature = "(Landroid/content/Context;[Ljava/lang/String;Ljava/" - "lang/String;Ljava/lang/String;Ljava/lang/String;J;Z)V", + "lang/String;Ljava/lang/String;Ljava/lang/String;J)V", .fnPtr = reinterpret_cast(&Init), }, { diff --git a/shell/platform/android/flutter_main.h b/shell/platform/android/flutter_main.h index 097dd9f5bee26..171800b3ec500 100644 --- a/shell/platform/android/flutter_main.h +++ b/shell/platform/android/flutter_main.h @@ -29,8 +29,6 @@ class FlutterMain { FlutterMain(flutter::Settings settings); - // TODO(cyanlaz): Remove `useEmbeddedView` when dynamic thread merging is done. - // https://github.com/flutter/flutter/issues/59930 static void Init(JNIEnv* env, jclass clazz, jobject context, @@ -38,8 +36,7 @@ class FlutterMain { jstring kernelPath, jstring appStoragePath, jstring engineCachesPath, - jlong initTimeMillis, - jboolean useEmbeddedView); + jlong initTimeMillis); void SetupObservatoryUriCallback(JNIEnv* env); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 5e43047bf4789..5fb4e337c6ef5 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -110,8 +110,7 @@ public static native void nativeInit( @Nullable String bundlePath, @NonNull String appStoragePath, @NonNull String engineCachesPath, - long initTimeMillis, - boolean useEmbeddedView); + long initTimeMillis); /** * Prefetch the default font manager provided by SkFontMgr::RefDefault() which is a process-wide 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 50f4c9ffbda8e..de52d2fed7151 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java @@ -238,6 +238,9 @@ public void ensureInitializationComplete( // https://github.com/flutter/flutter/issues/59930 Bundle bundle = applicationInfo.metaData; boolean use_embedded_view = bundle.getBoolean("io.flutter.embedded_views_preview"); + if (use_embedded_view) { + shellArgs.add("--use-embedded-view"); + } FlutterJNI.nativeInit( applicationContext, @@ -245,8 +248,7 @@ public void ensureInitializationComplete( kernelPath, result.appStoragePath, result.engineCachesPath, - initTimeMillis, - use_embedded_view); + initTimeMillis); initialized = true; } catch (Exception e) { diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index c7aaf1ef6b3a7..44df75ac0b76f 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -51,7 +51,8 @@ PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, std::shared_ptr jni_facade, - bool use_software_rendering) + bool use_software_rendering, + bool use_embedded_view) : PlatformView(delegate, std::move(task_runners)), jni_facade_(jni_facade) { std::shared_ptr android_context; if (use_software_rendering) { @@ -70,7 +71,7 @@ PlatformViewAndroid::PlatformViewAndroid( FML_CHECK(android_context && android_context->IsValid()) << "Could not create an Android context."; - android_surface_ = SurfaceFactory(std::move(android_context), jni_facade); + android_surface_ = SurfaceFactory(std::move(android_context), jni_facade, use_embedded_view); FML_CHECK(android_surface_ && android_surface_->IsValid()) << "Could not create an OpenGL, Vulkan or Software surface to setup " "rendering."; @@ -79,7 +80,8 @@ PlatformViewAndroid::PlatformViewAndroid( PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, - std::shared_ptr jni_facade) + std::shared_ptr jni_facade, + bool use_embedded_view) : PlatformView(delegate, std::move(task_runners)), jni_facade_(jni_facade) {} diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index c0b6a1d79a4d8..7e52c57911be0 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -29,13 +29,15 @@ class PlatformViewAndroid final : public PlatformView { // background execution. PlatformViewAndroid(PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, - std::shared_ptr jni_facade); + std::shared_ptr jni_facade, + bool use_embedded_view); // Creates a PlatformViewAndroid with a rendering surface. PlatformViewAndroid(PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, std::shared_ptr jni_facade, - bool use_software_rendering); + bool use_software_rendering, + bool use_embedded_view); ~PlatformViewAndroid() override; From 994d91f87751e4109d00355056e2de13560e83a0 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 23 Jun 2020 14:00:59 -0700 Subject: [PATCH 08/13] cleanup --- shell/platform/android/android_shell_holder.cc | 8 +++----- .../external_view_embedder/external_view_embedder.h | 1 + shell/platform/android/platform_view_android.cc | 8 +++----- shell/platform/android/platform_view_android.h | 7 +++---- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 2ebd606e0fa3e..5da2b2af6b7f9 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -57,14 +57,13 @@ AndroidShellHolder::AndroidShellHolder( fml::WeakPtr weak_platform_view; Shell::CreateCallback on_create_platform_view = - [is_background_view, &jni_facade, &weak_platform_view, use_embedded_view = settings.use_embedded_view](Shell& shell) { + [is_background_view, &jni_facade, &weak_platform_view](Shell& shell) { std::unique_ptr platform_view_android; if (is_background_view) { platform_view_android = std::make_unique( shell, // delegate shell.GetTaskRunners(), // task runners - jni_facade, // JNI interop - use_embedded_view + jni_facade // JNI interop ); } else { platform_view_android = std::make_unique( @@ -72,8 +71,7 @@ AndroidShellHolder::AndroidShellHolder( shell.GetTaskRunners(), // task runners jni_facade, // JNI interop shell.GetSettings() - .enable_software_rendering, // use software rendering, - use_embedded_view + .enable_software_rendering // use software rendering, ); } weak_platform_view = platform_view_android->GetWeakPtr(); diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index 41b6e68f1155d..e25bdc358cb71 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -26,6 +26,7 @@ namespace flutter { /// class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { public: + AndroidExternalViewEmbedder( std::shared_ptr android_context, std::shared_ptr jni_facade, diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 44df75ac0b76f..c7aaf1ef6b3a7 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -51,8 +51,7 @@ PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, std::shared_ptr jni_facade, - bool use_software_rendering, - bool use_embedded_view) + bool use_software_rendering) : PlatformView(delegate, std::move(task_runners)), jni_facade_(jni_facade) { std::shared_ptr android_context; if (use_software_rendering) { @@ -71,7 +70,7 @@ PlatformViewAndroid::PlatformViewAndroid( FML_CHECK(android_context && android_context->IsValid()) << "Could not create an Android context."; - android_surface_ = SurfaceFactory(std::move(android_context), jni_facade, use_embedded_view); + android_surface_ = SurfaceFactory(std::move(android_context), jni_facade); FML_CHECK(android_surface_ && android_surface_->IsValid()) << "Could not create an OpenGL, Vulkan or Software surface to setup " "rendering."; @@ -80,8 +79,7 @@ PlatformViewAndroid::PlatformViewAndroid( PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, - std::shared_ptr jni_facade, - bool use_embedded_view) + std::shared_ptr jni_facade) : PlatformView(delegate, std::move(task_runners)), jni_facade_(jni_facade) {} diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 7e52c57911be0..2e1a76c5467f6 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -29,15 +29,13 @@ class PlatformViewAndroid final : public PlatformView { // background execution. PlatformViewAndroid(PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, - std::shared_ptr jni_facade, - bool use_embedded_view); + std::shared_ptr jni_facade); // Creates a PlatformViewAndroid with a rendering surface. PlatformViewAndroid(PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, std::shared_ptr jni_facade, - bool use_software_rendering, - bool use_embedded_view); + bool use_software_rendering); ~PlatformViewAndroid() override; @@ -77,6 +75,7 @@ class PlatformViewAndroid final : public PlatformView { const fml::jni::JavaObjectWeakGlobalRef& surface_texture); private: + const std::shared_ptr jni_facade_; std::unique_ptr android_surface_; From 9bdbd7f162ce7d40e5fb41af82ef634088905f52 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 23 Jun 2020 14:51:17 -0700 Subject: [PATCH 09/13] no external view embedder if flag not set --- shell/platform/android/android_shell_holder.cc | 4 ++++ shell/platform/android/android_shell_holder.h | 9 +++++++++ shell/platform/android/android_surface_gl.cc | 4 ++++ shell/platform/android/android_surface_software.cc | 4 ++++ shell/platform/android/platform_view_android.h | 1 - shell/platform/android/surface/android_surface.h | 1 + 6 files changed, 22 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 5da2b2af6b7f9..2300e2f44f5c6 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -28,6 +28,8 @@ static WindowData GetDefaultWindowData() { return window_data; } +bool AndroidShellHolder::use_embedded_view; + AndroidShellHolder::AndroidShellHolder( flutter::Settings settings, std::shared_ptr jni_facade, @@ -102,6 +104,7 @@ AndroidShellHolder::AndroidShellHolder( 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. @@ -125,6 +128,7 @@ AndroidShellHolder::AndroidShellHolder( on_create_rasterizer // rasterizer create callback ); } else { + use_embedded_view = false; flutter::TaskRunners task_runners(thread_label, // label platform_runner, // platform gpu_runner, // raster diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index 107e93f18672a..faddbc54346e2 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -20,7 +20,16 @@ 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 f4bd9e6747907..c8cc3d2d1183e 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -8,6 +8,7 @@ #include "flutter/fml/logging.h" #include "flutter/fml/memory/ref_ptr.h" +#include "flutter/shell/platform/android/android_shell_holder.h" namespace flutter { @@ -122,6 +123,9 @@ intptr_t AndroidSurfaceGL::GLContextFBO() 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 7face67038d0a..3bc52e317279a 100644 --- a/shell/platform/android/android_surface_software.cc +++ b/shell/platform/android/android_surface_software.cc @@ -11,6 +11,7 @@ #include "flutter/fml/platform/android/jni_weak_ref.h" #include "flutter/fml/platform/android/scoped_java_ref.h" #include "flutter/fml/trace_event.h" +#include "flutter/shell/platform/android/android_shell_holder.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" namespace flutter { @@ -145,6 +146,9 @@ 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/platform_view_android.h b/shell/platform/android/platform_view_android.h index 2e1a76c5467f6..39a0ce19380a4 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -118,7 +118,6 @@ class PlatformViewAndroid final : public PlatformView { FML_DISALLOW_COPY_AND_ASSIGN(PlatformViewAndroid); }; - } // namespace flutter #endif // SHELL_PLATFORM_ANDROID_PLATFORM_VIEW_ANDROID_H_ diff --git a/shell/platform/android/surface/android_surface.h b/shell/platform/android/surface/android_surface.h index 26604f4ff7717..928127e1364cf 100644 --- a/shell/platform/android/surface/android_surface.h +++ b/shell/platform/android/surface/android_surface.h @@ -16,6 +16,7 @@ namespace flutter { class AndroidSurface { public: + using Factory = std::function( std::shared_ptr android_context, std::shared_ptr jni_facade)>; From ddd34bb4243b8815f869cd88fb5e17d346b8131f Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Tue, 23 Jun 2020 14:54:23 -0700 Subject: [PATCH 10/13] cleanup --- shell/platform/android/android_shell_holder.cc | 4 ++-- .../android/external_view_embedder/external_view_embedder.h | 1 - shell/platform/android/platform_view_android.h | 1 - shell/platform/android/surface/android_surface.h | 1 - 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 2300e2f44f5c6..c12623f33a7da 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -65,7 +65,7 @@ AndroidShellHolder::AndroidShellHolder( platform_view_android = std::make_unique( shell, // delegate shell.GetTaskRunners(), // task runners - jni_facade // JNI interop + jni_facade // JNI interop ); } else { platform_view_android = std::make_unique( @@ -73,7 +73,7 @@ AndroidShellHolder::AndroidShellHolder( shell.GetTaskRunners(), // task runners jni_facade, // JNI interop shell.GetSettings() - .enable_software_rendering // use software rendering, + .enable_software_rendering // use software rendering ); } weak_platform_view = platform_view_android->GetWeakPtr(); diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index e25bdc358cb71..41b6e68f1155d 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -26,7 +26,6 @@ namespace flutter { /// class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { public: - AndroidExternalViewEmbedder( std::shared_ptr android_context, std::shared_ptr jni_facade, diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 39a0ce19380a4..c7b22fb17070e 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -75,7 +75,6 @@ class PlatformViewAndroid final : public PlatformView { const fml::jni::JavaObjectWeakGlobalRef& surface_texture); private: - const std::shared_ptr jni_facade_; std::unique_ptr android_surface_; diff --git a/shell/platform/android/surface/android_surface.h b/shell/platform/android/surface/android_surface.h index 928127e1364cf..26604f4ff7717 100644 --- a/shell/platform/android/surface/android_surface.h +++ b/shell/platform/android/surface/android_surface.h @@ -16,7 +16,6 @@ namespace flutter { class AndroidSurface { public: - using Factory = std::function( std::shared_ptr android_context, std::shared_ptr jni_facade)>; From a37f942dc7475016540e2827b2f78795c4228527 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 24 Jun 2020 13:40:54 -0700 Subject: [PATCH 11/13] review fixes --- common/settings.h | 2 +- shell/platform/android/android_shell_holder.h | 1 - shell/platform/android/platform_view_android.h | 1 + testing/scenario_app/android/app/src/main/AndroidManifest.xml | 3 +++ 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/common/settings.h b/common/settings.h index ea4970747fbe9..25af94a806438 100644 --- a/common/settings.h +++ b/common/settings.h @@ -213,7 +213,7 @@ struct Settings { /// to log a timeline event that tracks the latency of engine startup. std::chrono::microseconds engine_start_timestamp = {}; - /// Does the application claim that it uses the android embedded view for platform views. + /// 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. diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index faddbc54346e2..4bc2eb5999b72 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -20,7 +20,6 @@ namespace flutter { class AndroidShellHolder { - public: // Whether the application sets to use embedded_view view diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index c7b22fb17070e..55d40522c8ca8 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -116,6 +116,7 @@ class PlatformViewAndroid final : public PlatformView { void FireFirstFrameCallback(); FML_DISALLOW_COPY_AND_ASSIGN(PlatformViewAndroid); + }; } // namespace flutter diff --git a/testing/scenario_app/android/app/src/main/AndroidManifest.xml b/testing/scenario_app/android/app/src/main/AndroidManifest.xml index 65ff9e909329b..a7416851ccd0d 100644 --- a/testing/scenario_app/android/app/src/main/AndroidManifest.xml +++ b/testing/scenario_app/android/app/src/main/AndroidManifest.xml @@ -28,6 +28,9 @@ + From 0f922f82d41fdcecc08801bf2bf60a8b4b974d15 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 24 Jun 2020 13:48:14 -0700 Subject: [PATCH 12/13] formatting --- common/settings.h | 6 ++-- shell/common/switches.cc | 3 +- shell/common/switches.h | 6 ++-- .../platform/android/android_shell_holder.cc | 34 +++++++++---------- shell/platform/android/android_shell_holder.h | 10 +++--- .../platform/android/platform_view_android.h | 1 - 6 files changed, 32 insertions(+), 28 deletions(-) diff --git a/common/settings.h b/common/settings.h index 25af94a806438..bf615379637e5 100644 --- a/common/settings.h +++ b/common/settings.h @@ -213,9 +213,11 @@ 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. + /// 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. + /// 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; diff --git a/shell/common/switches.cc b/shell/common/switches.cc index 235a5148410d3..34c4c4ee56f46 100644 --- a/shell/common/switches.cc +++ b/shell/common/switches.cc @@ -227,7 +227,8 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) { : "127.0.0.1"; } - settings.use_embedded_view = command_line.HasOption(FlagForSwitch(Switch::UseEmbeddedView)); + settings.use_embedded_view = + command_line.HasOption(FlagForSwitch(Switch::UseEmbeddedView)); // Set Observatory Port if (command_line.HasOption(FlagForSwitch(Switch::DeviceObservatoryPort))) { diff --git a/shell/common/switches.h b/shell/common/switches.h index 62d0683ae866d..8506bc7e72496 100644 --- a/shell/common/switches.h +++ b/shell/common/switches.h @@ -194,8 +194,10 @@ DEF_SWITCH( 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.") + "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/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index c12623f33a7da..c9667dcb7edf3 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -108,25 +108,25 @@ AndroidShellHolder::AndroidShellHolder( // 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 + // 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 + platform_runner, // raster ui_runner, // ui io_runner // io ); shell_ = - Shell::Create(task_runners, // task runners - GetDefaultWindowData(), // window data - settings_, // settings - on_create_platform_view, // platform view create callback - on_create_rasterizer // rasterizer create callback - ); + Shell::Create(task_runners, // task runners + GetDefaultWindowData(), // 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 @@ -137,12 +137,12 @@ AndroidShellHolder::AndroidShellHolder( ); shell_ = - Shell::Create(task_runners, // task runners - GetDefaultWindowData(), // window data - settings_, // settings - on_create_platform_view, // platform view create callback - on_create_rasterizer // rasterizer create callback - ); + Shell::Create(task_runners, // task runners + GetDefaultWindowData(), // window data + settings_, // settings + on_create_platform_view, // platform view create callback + on_create_rasterizer // rasterizer create callback + ); } platform_view_ = weak_platform_view; diff --git a/shell/platform/android/android_shell_holder.h b/shell/platform/android/android_shell_holder.h index 4bc2eb5999b72..6fb6695801733 100644 --- a/shell/platform/android/android_shell_holder.h +++ b/shell/platform/android/android_shell_holder.h @@ -21,12 +21,12 @@ 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 + // `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, diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 55d40522c8ca8..c7b22fb17070e 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -116,7 +116,6 @@ class PlatformViewAndroid final : public PlatformView { void FireFirstFrameCallback(); FML_DISALLOW_COPY_AND_ASSIGN(PlatformViewAndroid); - }; } // namespace flutter From 66a2f6fe88be37fd21589164f00bcb0a87bc0c44 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Wed, 24 Jun 2020 14:11:34 -0700 Subject: [PATCH 13/13] null check --- .../io/flutter/embedding/engine/loader/FlutterLoader.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 de52d2fed7151..9156a64ef061e 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java @@ -237,9 +237,11 @@ public void ensureInitializationComplete( // TODO(cyanlaz): Remove this when dynamic thread merging is done. // https://github.com/flutter/flutter/issues/59930 Bundle bundle = applicationInfo.metaData; - boolean use_embedded_view = bundle.getBoolean("io.flutter.embedded_views_preview"); - if (use_embedded_view) { - shellArgs.add("--use-embedded-view"); + if (bundle != null) { + boolean use_embedded_view = bundle.getBoolean("io.flutter.embedded_views_preview"); + if (use_embedded_view) { + shellArgs.add("--use-embedded-view"); + } } FlutterJNI.nativeInit(