From 85f3f1f22937671a7ff2b4bf399ca13211c49e88 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 11 Jun 2020 19:10:44 -0700 Subject: [PATCH 01/28] Wire up external view embedder --- shell/platform/android/BUILD.gn | 3 +- shell/platform/android/context/BUILD.gn | 18 +++ .../android/{ => context}/android_context.cc | 19 +-- .../android/{ => context}/android_context.h | 1 - .../android/external_view_embedder/BUILD.gn | 3 + .../external_view_embedder.cc | 144 ++++++++++++++++-- .../external_view_embedder.h | 34 ++++- .../external_view_embedder/surface_pool.cc | 16 ++ .../external_view_embedder/surface_pool.h | 73 +++++++++ 9 files changed, 281 insertions(+), 30 deletions(-) create mode 100644 shell/platform/android/context/BUILD.gn rename shell/platform/android/{ => context}/android_context.cc (72%) rename shell/platform/android/{ => context}/android_context.h (95%) create mode 100644 shell/platform/android/external_view_embedder/surface_pool.cc create mode 100644 shell/platform/android/external_view_embedder/surface_pool.h diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index f68249194b751..78c50e108508f 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -23,8 +23,6 @@ shared_library("flutter_shell_native") { sources = [ "$root_build_dir/flutter_icu/icudtl.o", - "android_context.cc", - "android_context.h", "android_context_gl.cc", "android_context_gl.h", "android_environment_gl.cc", @@ -67,6 +65,7 @@ shared_library("flutter_shell_native") { "//flutter/runtime", "//flutter/runtime:libdart", "//flutter/shell/common", + "//flutter/shell/platform/android/context", "//flutter/shell/platform/android/external_view_embedder", "//flutter/shell/platform/android/jni", "//third_party/skia", diff --git a/shell/platform/android/context/BUILD.gn b/shell/platform/android/context/BUILD.gn new file mode 100644 index 0000000000000..f510fbdd649df --- /dev/null +++ b/shell/platform/android/context/BUILD.gn @@ -0,0 +1,18 @@ +# 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. + +import("//flutter/common/config.gni") + +source_set("context") { + sources = [ + "android_context.cc", + "android_context.h", + ] + + public_configs = [ "//flutter:config" ] + + deps = [ + "//flutter/fml", + ] +} diff --git a/shell/platform/android/android_context.cc b/shell/platform/android/context/android_context.cc similarity index 72% rename from shell/platform/android/android_context.cc rename to shell/platform/android/context/android_context.cc index c59f56186d900..bf244b00ac794 100644 --- a/shell/platform/android/android_context.cc +++ b/shell/platform/android/context/android_context.cc @@ -4,19 +4,13 @@ #include "flutter/shell/platform/android/android_context.h" +#if OS_ANDROID #include "flutter/shell/platform/android/android_context_gl.h" #include "flutter/shell/platform/android/android_environment_gl.h" +#endif namespace flutter { -std::shared_ptr CreateContextGL() { - auto context_gl = std::make_shared( - AndroidRenderingAPI::kOpenGLES, - fml::MakeRefCounted()); - FML_CHECK(context_gl->IsValid()) << "Could not create an Android context GL."; - return context_gl; -} - AndroidContext::AndroidContext(AndroidRenderingAPI rendering_api) : rendering_api_(rendering_api) {} @@ -28,9 +22,16 @@ AndroidRenderingAPI AndroidContext::RenderingApi() const { std::shared_ptr AndroidContext::Create( AndroidRenderingAPI rendering_api) { +#if OS_ANDROID if (rendering_api == AndroidRenderingAPI::kOpenGLES) { - return CreateContextGL(); + auto context_gl = std::make_shared( + AndroidRenderingAPI::kOpenGLES, + fml::MakeRefCounted()); + FML_CHECK(context_gl->IsValid()) + << "Could not create an Android context GL."; + return context_gl; } +#endif return std::make_shared(rendering_api); } diff --git a/shell/platform/android/android_context.h b/shell/platform/android/context/android_context.h similarity index 95% rename from shell/platform/android/android_context.h rename to shell/platform/android/context/android_context.h index 313d5377c40fa..b14eae344c058 100644 --- a/shell/platform/android/android_context.h +++ b/shell/platform/android/context/android_context.h @@ -8,7 +8,6 @@ #include "flutter/fml/macros.h" #include "flutter/fml/memory/ref_counted.h" #include "flutter/fml/memory/ref_ptr.h" -#include "flutter/shell/common/platform_view.h" namespace flutter { diff --git a/shell/platform/android/external_view_embedder/BUILD.gn b/shell/platform/android/external_view_embedder/BUILD.gn index 5145e00fa8972..07faeb5221354 100644 --- a/shell/platform/android/external_view_embedder/BUILD.gn +++ b/shell/platform/android/external_view_embedder/BUILD.gn @@ -14,6 +14,8 @@ source_set("external_view_embedder") { sources = [ "external_view_embedder.cc", "external_view_embedder.h", + "surface_pool.cc", + "surface_pool.h", ] public_configs = [ "//flutter:config" ] @@ -22,6 +24,7 @@ source_set("external_view_embedder") { "//flutter/common", "//flutter/flow", "//flutter/fml", + "//flutter/shell/platform/android/context", "//flutter/shell/platform/android/jni", "//third_party/skia", ] 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 4e42ac662f77f..f4199e03f9227 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -9,21 +9,34 @@ namespace flutter { AndroidExternalViewEmbedder::AndroidExternalViewEmbedder( - std::shared_ptr jni_facade) - : ExternalViewEmbedder(), jni_facade_(jni_facade) {} + std::shared_ptr jni_facade, + std::shared_ptr android_context) + : ExternalViewEmbedder(), + jni_facade_(jni_facade), + android_context_(android_context), + surface_pool_(std::make_unique()) {} // |ExternalViewEmbedder| void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView( int view_id, std::unique_ptr params) { - // TODO(egarciad): Implement hybrid composition. - // https://github.com/flutter/flutter/issues/55270 + auto rtree_factory = RTreeFactory(); + view_rtrees_[view_id] = rtree_factory.getInstance(); + TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView"); picture_recorders_[view_id] = std::make_unique(); - picture_recorders_[view_id]->beginRecording(SkRect::Make(frame_size_)); + picture_recorders_[view_id]->beginRecording(SkRect::Make(frame_size_), + &rtree_factory); composition_order_.push_back(view_id); + // Update params. + if (view_params_.count(view_id) == 1 && + view_params_[view_id] == *params.get()) { + // Do nothing if the params didn't change. + return; + } + view_params_[view_id] = EmbeddedViewParams(*params.get()); } // |ExternalViewEmbedder| @@ -33,8 +46,6 @@ SkCanvas* AndroidExternalViewEmbedder::CompositeEmbeddedView(int view_id) { // |ExternalViewEmbedder| std::vector AndroidExternalViewEmbedder::GetCurrentCanvases() { - // TODO(egarciad): Implement hybrid composition. - // https://github.com/flutter/flutter/issues/55270 std::vector canvases; for (size_t i = 0; i < composition_order_.size(); i++) { int64_t view_id = composition_order_[i]; @@ -47,19 +58,128 @@ std::vector AndroidExternalViewEmbedder::GetCurrentCanvases() { bool AndroidExternalViewEmbedder::SubmitFrame( GrContext* context, std::unique_ptr frame) { - // TODO(egarciad): Implement hybrid composition. - // https://github.com/flutter/flutter/issues/55270 TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::SubmitFrame"); + if (should_run_rasterizer_on_platform_thread_) { // Don't submit the current frame if the frame will be resubmitted. return true; } + + std::map> overlay_layers; + std::map> pictures; + SkCanvas* background_canvas = frame->SkiaCanvas(); + + // Restore the clip context after exiting this method since it's changed + // below. + SkAutoCanvasRestore save(background_canvas, /*doSave=*/true); + for (size_t i = 0; i < composition_order_.size(); i++) { int64_t view_id = composition_order_[i]; - frame->SkiaCanvas()->drawPicture( - picture_recorders_[view_id]->finishRecordingAsPicture()); + SkPoint view_offset = view_params_[view_id].offsetPixels; + SkSize view_size = view_params_[view_id].sizePoints; + + // Display the platform view. If it's already displayed, then it's + // just positioned and sized. + jni_facade_->FlutterViewOnDisplayPlatformView(view_id, // + view_offset.x(), // + view_offset.y(), // + view_size.width(), // + view_size.height() // + ); + + sk_sp rtree = view_rtrees_[view_id]; + // Determinate if Flutter UI intersects with any of the previous + // platform views stacked by z position. + // + // This is done by querying the r-tree that holds the records for the + // picture recorder corresponding to the flow layers added after a platform + // view layer. + for (size_t j = i + 1; j > 0; j--) { + int64_t current_view_id = composition_order_[j - 1]; + SkRect current_view_rect = GetPlatformViewRect(current_view_id); + // Each rect corresponds to a native view that renders Flutter UI. + std::list intersection_rects = + rtree->searchNonOverlappingDrawnRects(current_view_rect); + auto allocation_size = intersection_rects.size(); + // Limit the number of native views, so it doesn't grow for ever. + // + // In this case, the rects are merged into a single one that is the union + // of all the rects. + if (allocation_size > kMaxLayerAllocations) { + SkRect joined_rect; + for (const SkRect& rect : intersection_rects) { + joined_rect.join(rect); + } + intersection_rects.clear(); + intersection_rects.push_back(joined_rect); + } + for (SkRect& intersection_rect : intersection_rects) { + // Get the intersection rect between the current rect + // and the platform view rect. + // joined_rect.intersect(platform_view_rect); + // Subpixels in the platform may not align with the canvas subpixels. + // + // To workaround it, round the floating point bounds and make the rect + // slighly larger. For example, {0.3, 0.5, 3.1, 4.7} becomes {0, 0, 4, + // 5}. + intersection_rect.setLTRB(std::floor(intersection_rect.left()), // + std::floor(intersection_rect.top()), // + std::ceil(intersection_rect.right()), // + std::ceil(intersection_rect.bottom()) // + ); + // Clip the background canvas, so it doesn't contain any of the pixels + // drawn on the overlay layer. + background_canvas->clipRect(joined_rect, SkClipOp::kDifference); + } + overlay_layers[current_view_id] = intersection_rects; + } + pictures[view_id] = picture_recorders_[view_id]->finishRecordingAsPicture(); + background_canvas->drawPicture(pictures[view_id]); } - return frame->Submit(); + // Submit the background canvas frame before switching the GL context to + // the surfaces above. + frame->Submit(); + + for (size_t i = 0; i < composition_order_.size(); i++) { + int64_t view_id = composition_order_[i]; + for (SkRect& overlay_rect : overlay_layers[view_id]) { + CreateSurfaceIfNeeded(context, // + view_id, // + pictures[view_id], // + overlay_rect, // + ) + ->Submit(); + } + } + return true; +} + +// |ExternalViewEmbedder| +std::unique_ptr +AndroidExternalViewEmbedder::CreateSurfaceIfNeeded(GrContext* context, + int64_t view_id, + sk_sp picture, + SkRect& rect) { + std::shared_ptr layer = + surface_pool_->GetLayer(context, jni_facade_, android_context_); + + std::unique_ptr frame = + layer->surface->AcquireFrame(frame_size_); + // Display the overlay surface. If it's already displayed, then it's + // just positioned and sized. + jni_facade_->FlutterViewDisplayOverlaySurface(layer->id, // + rect.x(), // + rect.y(), // + rect.width(), // + rect.height() // + ); + SkCanvas* overlay_canvas = frame->SkiaCanvas(); + overlay_canvas->clear(SK_ColorTRANSPARENT); + // Offset the picture since its absolute position on the scene is determined + // by the position of the overlay view. + overlay_canvas->translate(-rect.x(), -rect.y()); + overlay_canvas->drawPicture(picture); + return frame; } // |ExternalViewEmbedder| 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 6cfa82a8d58e1..0576e18252227 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -6,7 +6,8 @@ #define FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_H_ #include "flutter/flow/embedded_views.h" - +#include "flutter/shell/platform/android/context/android_context.h" +#include "flutter/shell/platform/android/external_view_embedder/surface_pool.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" #include "third_party/skia/include/core/SkPictureRecorder.h" @@ -15,7 +16,8 @@ namespace flutter { class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { public: AndroidExternalViewEmbedder( - std::shared_ptr jni_facade); + std::shared_ptr jni_facade, + std::shared_ptr android_context); // |ExternalViewEmbedder| void PrerollCompositeEmbeddedView( @@ -52,9 +54,6 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { fml::RefPtr raster_thread_merger) override; private: - // Allows to call methods in Java. - const std::shared_ptr jni_facade_; - // The number of frames the rasterizer task runner will continue // to run on the platform thread after no platform view is rendered. // @@ -62,6 +61,15 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // where the platform view might be momentarily off the screen. static const int kDefaultMergedLeaseDuration = 10; + // Allows to call methods in Java. + const std::shared_ptr jni_facade_; + + // Provides metadata to the Android surfaces. + const std::shared_ptr android_context_; + + // Holds surfaces. Allows to recycle surfaces or allocate new ones. + const std::unique_ptr surface_pool_; + // Whether the rasterizer task runner should run on the platform thread. // When this is true, the current frame is cancelled and resubmitted. bool should_run_rasterizer_on_platform_thread_ = false; @@ -78,8 +86,22 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // of the last leaf node in the layer tree. std::map> picture_recorders_; - /// Resets the state. + // The params for a platform view, which contains the size, position and + // mutation stack. + std::map view_params_; + + // The r-tree that captures the operations for the picture recorders. + std::map> view_rtrees_; + + // Resets the state. void Reset(); + + // Creates a Surface when needed or recycles an existing one. + // Finally, draws the picture on the frame's canvas. + std::unique_ptr CreateSurfaceIfNeeded(GrContext* context, + int64_t view_id, + sk_sp picture, + SkRect& rect); }; } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc new file mode 100644 index 0000000000000..e16229e20be59 --- /dev/null +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -0,0 +1,16 @@ +// 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/shell/platform/android/external_view_embedder/surface_pool.h" + +namespace flutter { + +OverlayLayer::OverlayLayer(long id, + std::unique_ptr android_surface, + std::unique_ptr surface) + : id(id), + android_surface(std::move(android_surface)), + surface(std::move(surface)){}; + +} diff --git a/shell/platform/android/external_view_embedder/surface_pool.h b/shell/platform/android/external_view_embedder/surface_pool.h new file mode 100644 index 0000000000000..327373b2b84b7 --- /dev/null +++ b/shell/platform/android/external_view_embedder/surface_pool.h @@ -0,0 +1,73 @@ +// 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. + +#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_SURFACE_POOL_H_ +#define FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_SURFACE_POOL_H_ + +#include "flutter/flow/surface.h" +#include "flutter/shell/platform/android/android_surface.h" +#include "flutter/shell/platform/android/context/android_context.h" + +namespace flutter { + +struct OverlayLayer { + OverlayLayer(long id, + std::unique_ptr android_surface, + std::unique_ptr surface); + + ~OverlayLayer() = default; + + const long id; + + const std::unique_ptr android_surface; + + const std::unique_ptr surface; + + // The `GrContext` that is currently used by the overlay surfaces. + // We track this to know when the GrContext for the Flutter app has changed + // so we can update the overlay with the new context. + GrContext* gr_context; +}; + +// This class isn't thread safe. +class SurfacePool { + public: + SurfacePool() = default; + ~SurfacePool() = default; + + // Gets a layer from the pool if available, or allocates a new one. + // Finally, it marks the layer as used. That is, it increments + // `available_layer_index_`. + std::shared_ptr GetLayer( + GrContext* gr_context, + std::shared_ptr jni_facade, + std::shared_ptr android_context); + + // Gets the layers in the pool that aren't currently used. + // This method doesn't mark the layers as unused. + std::vector> GetUnusedLayers(); + + // Marks the layers in the pool as available for reuse. + void RecycleLayers(); + + private: + // The index of the entry in the layers_ vector that determines the beginning + // of the unused layers. For example, consider the following vector: + // _____ + // | 0 | + /// |---| + /// | 1 | <-- `available_layer_index_` + /// |---| + /// | 2 | + /// |---| + /// + /// This indicates that entries starting from 1 can be reused meanwhile the + /// entry at position 0 cannot be reused. + size_t available_layer_index_ = 0; + std::vector> layers_; +} + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_SURFACE_POOL_H_ From cc09a8b37d6f3441906e72360bbe56c19131b8cc Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 12 Jun 2020 09:17:16 -0700 Subject: [PATCH 02/28] Format --- .../external_view_embedder.cc | 3 +- .../external_view_embedder/surface_pool.cc | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 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 f4199e03f9227..9c62b8b02ae71 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -30,10 +30,9 @@ void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView( &rtree_factory); composition_order_.push_back(view_id); - // Update params. + // Update params if they changed. if (view_params_.count(view_id) == 1 && view_params_[view_id] == *params.get()) { - // Do nothing if the params didn't change. return; } view_params_[view_id] = EmbeddedViewParams(*params.get()); diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index e16229e20be59..91963a820731a 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -13,4 +13,52 @@ OverlayLayer::OverlayLayer(long id, android_surface(std::move(android_surface)), surface(std::move(surface)){}; +std::shared_ptr SurfacePool::GetLayer( + GrContext* gr_context, + std::shared_ptr jni_facade, + std::shared_ptr android_context) { + // Allocate a new surface if there isn't one available. + if (available_layer_index_ >= layers_.size()) { + std::unique_ptr android_surface = + AndroidSurface::Create(android_context, jni_facade); + // TODO(egarciad): jni_facade->FlutterViewCreateOverlaySurface(..) + // Then, android_surface->SetNativeWindow(overlay_layer->GetWindow()) + // https://github.com/flutter/flutter/issues/55270 + std::unique_ptr surface = + android_surface->CreateGPUSurface(gr_context); + std::shared_ptr layer = + std::make_shared(overlay_layer->GetId(), // + std::move(android_surface), // + std::move(surface) // + ); + layer->gr_context = gr_context; + layers_.push_back(layer); + } + std::shared_ptr layer = layers_[available_layer_index_]; + // Since the surfaces are recycled, it's possible that the GrContext is + // different. + if (gr_context != layer->gr_context) { + layer->gr_context = gr_context; + // The overlay already exists, but the GrContext was changed so we need to + // recreate the rendering surface with the new GrContext. + std::unique_ptr surface = + layer->android_surface->CreateGPUSurface(gr_context); + layer->surface = std::move(surface); + } + available_layer_index_++; + return layer; } + +void SurfacePool::RecycleLayers() { + available_layer_index_ = 0; +} + +std::vector> SurfacePool::GetUnusedLayers() { + std::vector> results; + for (size_t i = available_layer_index_; i < layers_.size(); i++) { + results.push_back(layers_[i]); + } + return results; +} + +} // namespace flutter From aecc85f7f1322f97d62cfe00fd5a5a244e6a548d Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 12 Jun 2020 09:41:31 -0700 Subject: [PATCH 03/28] Move AndroidSurface --- shell/platform/android/BUILD.gn | 3 +- shell/platform/android/android_surface.cc | 5 ++ .../android/external_view_embedder/BUILD.gn | 1 + shell/platform/android/surface/BUILD.gn | 20 ++++++++ .../android/surface/android_surface.cc | 46 +++++++++++++++++++ .../android/surface/android_surface.h | 43 +++++++++++++++++ 6 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 shell/platform/android/surface/BUILD.gn create mode 100644 shell/platform/android/surface/android_surface.cc create mode 100644 shell/platform/android/surface/android_surface.h diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index 78c50e108508f..434de659ba634 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -33,8 +33,6 @@ shared_library("flutter_shell_native") { "android_native_window.h", "android_shell_holder.cc", "android_shell_holder.h", - "android_surface.cc", - "android_surface.h", "android_surface_gl.cc", "android_surface_gl.h", "android_surface_software.cc", @@ -68,6 +66,7 @@ shared_library("flutter_shell_native") { "//flutter/shell/platform/android/context", "//flutter/shell/platform/android/external_view_embedder", "//flutter/shell/platform/android/jni", + "//flutter/shell/platform/android/surface", "//third_party/skia", ] diff --git a/shell/platform/android/android_surface.cc b/shell/platform/android/android_surface.cc index e50f79180b78b..3aa5f1db089e8 100644 --- a/shell/platform/android/android_surface.cc +++ b/shell/platform/android/android_surface.cc @@ -6,17 +6,22 @@ #include +#if OS_ANDROID #include "flutter/shell/platform/android/android_surface_gl.h" #include "flutter/shell/platform/android/android_surface_software.h" #if SHELL_ENABLE_VULKAN #include "flutter/shell/platform/android/android_surface_vulkan.h" #endif // SHELL_ENABLE_VULKAN +#endif // OS_ANDROID namespace flutter { std::unique_ptr AndroidSurface::Create( std::shared_ptr android_context, std::shared_ptr jni_facade) { +#if !OS_ANDROID + return nullptr; +#endif // OS_ANDROID std::unique_ptr surface; switch (android_context->RenderingApi()) { case AndroidRenderingAPI::kSoftware: diff --git a/shell/platform/android/external_view_embedder/BUILD.gn b/shell/platform/android/external_view_embedder/BUILD.gn index 07faeb5221354..cbd8ceb90ddbe 100644 --- a/shell/platform/android/external_view_embedder/BUILD.gn +++ b/shell/platform/android/external_view_embedder/BUILD.gn @@ -26,6 +26,7 @@ source_set("external_view_embedder") { "//flutter/fml", "//flutter/shell/platform/android/context", "//flutter/shell/platform/android/jni", + "//flutter/shell/platform/android/surface", "//third_party/skia", ] } diff --git a/shell/platform/android/surface/BUILD.gn b/shell/platform/android/surface/BUILD.gn new file mode 100644 index 0000000000000..8cdfb1c612d67 --- /dev/null +++ b/shell/platform/android/surface/BUILD.gn @@ -0,0 +1,20 @@ +# 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. + +import("//flutter/common/config.gni") + +source_set("surface") { + sources = [ + "android_surface.cc", + "android_surface.h", + ] + + public_configs = [ "//flutter:config" ] + + deps = [ + "//flutter/fml", + "//flutter/flow", + "//third_party/skia", + ] +} diff --git a/shell/platform/android/surface/android_surface.cc b/shell/platform/android/surface/android_surface.cc new file mode 100644 index 0000000000000..94dbbe1cee4ce --- /dev/null +++ b/shell/platform/android/surface/android_surface.cc @@ -0,0 +1,46 @@ +// 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/shell/platform/android/android_surface.h" + +#include + +#if OS_ANDROID +#include "flutter/shell/platform/android/android_surface_gl.h" +#include "flutter/shell/platform/android/android_surface_software.h" +#if SHELL_ENABLE_VULKAN +#include "flutter/shell/platform/android/android_surface_vulkan.h" +#endif // SHELL_ENABLE_VULKAN +#endif // OS_ANDROID + +namespace flutter { + +std::unique_ptr AndroidSurface::Create( + std::shared_ptr android_context, + std::shared_ptr jni_facade) { +#if !OS_ANDROID + return nullptr; +#endif // OS_ANDROID + std::unique_ptr surface; + switch (android_context->RenderingApi()) { + case AndroidRenderingAPI::kSoftware: + surface = std::make_unique(jni_facade); + break; + case AndroidRenderingAPI::kOpenGLES: + surface = std::make_unique(android_context, jni_facade); + break; + case AndroidRenderingAPI::kVulkan: +#if SHELL_ENABLE_VULKAN + surface = std::make_unique(jni_facade); +#endif // SHELL_ENABLE_VULKAN + break; + } + FML_CHECK(surface); + return surface->IsValid() ? std::move(surface) : nullptr; + ; +} + +AndroidSurface::~AndroidSurface() = default; + +} // namespace flutter diff --git a/shell/platform/android/surface/android_surface.h b/shell/platform/android/surface/android_surface.h new file mode 100644 index 0000000000000..7914f58573c2e --- /dev/null +++ b/shell/platform/android/surface/android_surface.h @@ -0,0 +1,43 @@ +// 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. + +#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_H_ +#define FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_H_ + +#include "flutter/flow/surface.h" +#include "flutter/fml/macros.h" +#include "flutter/shell/platform/android/android_native_window.h" +#include "flutter/shell/platform/android/context/android_context.h" +#include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "third_party/skia/include/core/SkSize.h" + +namespace flutter { + +class AndroidSurface { + public: + static std::unique_ptr Create( + std::shared_ptr android_context, + std::shared_ptr jni_facade); + + virtual ~AndroidSurface(); + + virtual bool IsValid() const = 0; + + virtual void TeardownOnScreenContext() = 0; + + virtual std::unique_ptr CreateGPUSurface( + GrContext* gr_context = nullptr) = 0; + + virtual bool OnScreenSurfaceResize(const SkISize& size) = 0; + + virtual bool ResourceContextMakeCurrent() = 0; + + virtual bool ResourceContextClearCurrent() = 0; + + virtual bool SetNativeWindow(fml::RefPtr window) = 0; +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_H_ From 7dd77ee1ff5b6abff285c018bd8269017e1e52cb Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Sat, 13 Jun 2020 13:05:37 -0700 Subject: [PATCH 04/28] Compile with host toolchain --- shell/platform/android/BUILD.gn | 2 - shell/platform/android/android_context_gl.h | 4 +- .../platform/android/android_shell_holder.cc | 1 + shell/platform/android/android_surface.cc | 46 ------------------ shell/platform/android/android_surface.h | 48 ------------------- shell/platform/android/android_surface_gl.cc | 4 +- shell/platform/android/android_surface_gl.h | 2 +- .../android/android_surface_software.cc | 5 +- .../android/android_surface_software.h | 5 +- .../android/android_surface_vulkan.cc | 5 +- .../platform/android/android_surface_vulkan.h | 6 +-- .../android/context/android_context.cc | 22 ++------- .../android/context/android_context.h | 5 +- .../external_view_embedder.cc | 29 +++++++---- .../external_view_embedder.h | 23 +++++++-- .../external_view_embedder_unittests.cc | 10 ++-- .../external_view_embedder/surface_pool.cc | 15 ++++-- .../external_view_embedder/surface_pool.h | 36 +++++++------- .../platform/android/platform_view_android.cc | 47 ++++++++++++++++-- .../platform/android/platform_view_android.h | 4 +- shell/platform/android/surface/BUILD.gn | 6 ++- .../{ => surface}/android_native_window.cc | 8 +++- .../{ => surface}/android_native_window.h | 10 +++- .../android/surface/android_surface.cc | 37 +------------- .../android/surface/android_surface.h | 4 +- 25 files changed, 165 insertions(+), 219 deletions(-) delete mode 100644 shell/platform/android/android_surface.cc delete mode 100644 shell/platform/android/android_surface.h rename shell/platform/android/{ => surface}/android_native_window.cc (80%) rename shell/platform/android/{ => surface}/android_native_window.h (87%) diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index 434de659ba634..b2c144e53ec24 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -29,8 +29,6 @@ shared_library("flutter_shell_native") { "android_environment_gl.h", "android_external_texture_gl.cc", "android_external_texture_gl.h", - "android_native_window.cc", - "android_native_window.h", "android_shell_holder.cc", "android_shell_holder.h", "android_surface_gl.cc", diff --git a/shell/platform/android/android_context_gl.h b/shell/platform/android/android_context_gl.h index 6b68d295a0835..1b7b5f411a8d3 100644 --- a/shell/platform/android/android_context_gl.h +++ b/shell/platform/android/android_context_gl.h @@ -9,9 +9,9 @@ #include "flutter/fml/memory/ref_counted.h" #include "flutter/fml/memory/ref_ptr.h" #include "flutter/shell/common/platform_view.h" -#include "flutter/shell/platform/android/android_context.h" #include "flutter/shell/platform/android/android_environment_gl.h" -#include "flutter/shell/platform/android/android_native_window.h" +#include "flutter/shell/platform/android/context/android_context.h" +#include "flutter/shell/platform/android/surface/android_native_window.h" #include "third_party/skia/include/core/SkSize.h" namespace flutter { diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index e99c76e961ed1..62f8acf6fc8fd 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -16,6 +16,7 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/message_loop.h" +#include "flutter/fml/platform/android/jni_util.h" #include "flutter/shell/common/rasterizer.h" #include "flutter/shell/platform/android/platform_view_android.h" diff --git a/shell/platform/android/android_surface.cc b/shell/platform/android/android_surface.cc deleted file mode 100644 index 3aa5f1db089e8..0000000000000 --- a/shell/platform/android/android_surface.cc +++ /dev/null @@ -1,46 +0,0 @@ -// 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/shell/platform/android/android_surface.h" - -#include - -#if OS_ANDROID -#include "flutter/shell/platform/android/android_surface_gl.h" -#include "flutter/shell/platform/android/android_surface_software.h" -#if SHELL_ENABLE_VULKAN -#include "flutter/shell/platform/android/android_surface_vulkan.h" -#endif // SHELL_ENABLE_VULKAN -#endif // OS_ANDROID - -namespace flutter { - -std::unique_ptr AndroidSurface::Create( - std::shared_ptr android_context, - std::shared_ptr jni_facade) { -#if !OS_ANDROID - return nullptr; -#endif // OS_ANDROID - std::unique_ptr surface; - switch (android_context->RenderingApi()) { - case AndroidRenderingAPI::kSoftware: - surface = std::make_unique(jni_facade); - break; - case AndroidRenderingAPI::kOpenGLES: - surface = std::make_unique(android_context, jni_facade); - break; - case AndroidRenderingAPI::kVulkan: -#if SHELL_ENABLE_VULKAN - surface = std::make_unique(jni_facade); -#endif // SHELL_ENABLE_VULKAN - break; - } - FML_CHECK(surface); - return surface->IsValid() ? std::move(surface) : nullptr; - ; -} - -AndroidSurface::~AndroidSurface() = default; - -} // namespace flutter diff --git a/shell/platform/android/android_surface.h b/shell/platform/android/android_surface.h deleted file mode 100644 index 4997588b6a649..0000000000000 --- a/shell/platform/android/android_surface.h +++ /dev/null @@ -1,48 +0,0 @@ -// 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. - -#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_H_ -#define FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_H_ - -#include - -#include "flutter/flow/surface.h" -#include "flutter/fml/macros.h" -#include "flutter/fml/platform/android/jni_util.h" -#include "flutter/fml/platform/android/jni_weak_ref.h" -#include "flutter/shell/common/platform_view.h" -#include "flutter/shell/platform/android/android_context.h" -#include "flutter/shell/platform/android/android_native_window.h" -#include "flutter/shell/platform/android/jni/platform_view_android_jni.h" -#include "third_party/skia/include/core/SkSize.h" - -namespace flutter { - -class AndroidSurface { - public: - static std::unique_ptr Create( - std::shared_ptr android_context, - std::shared_ptr jni_facade); - - virtual ~AndroidSurface(); - - virtual bool IsValid() const = 0; - - virtual void TeardownOnScreenContext() = 0; - - virtual std::unique_ptr CreateGPUSurface( - GrContext* gr_context = nullptr) = 0; - - virtual bool OnScreenSurfaceResize(const SkISize& size) = 0; - - virtual bool ResourceContextMakeCurrent() = 0; - - virtual bool ResourceContextClearCurrent() = 0; - - virtual bool SetNativeWindow(fml::RefPtr window) = 0; -}; - -} // namespace flutter - -#endif // FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_H_ diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index 8b91867dd3f0e..99d1bfa45d4c7 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -24,8 +24,8 @@ AndroidSurfaceGL::AndroidSurfaceGL( if (!offscreen_surface_->IsValid()) { offscreen_surface_ = nullptr; } - external_view_embedder_ = - std::make_unique(jni_facade); + external_view_embedder_ = std::make_unique( + android_context, jni_facade, nullptr); } AndroidSurfaceGL::~AndroidSurfaceGL() = default; diff --git a/shell/platform/android/android_surface_gl.h b/shell/platform/android/android_surface_gl.h index faf30e0b8afc8..092e53de970fd 100644 --- a/shell/platform/android/android_surface_gl.h +++ b/shell/platform/android/android_surface_gl.h @@ -12,9 +12,9 @@ #include "flutter/shell/gpu/gpu_surface_gl.h" #include "flutter/shell/platform/android/android_context_gl.h" #include "flutter/shell/platform/android/android_environment_gl.h" -#include "flutter/shell/platform/android/android_surface.h" #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "flutter/shell/platform/android/surface/android_surface.h" namespace flutter { diff --git a/shell/platform/android/android_surface_software.cc b/shell/platform/android/android_surface_software.cc index 56d9e3bc542fd..91e848da0883e 100644 --- a/shell/platform/android/android_surface_software.cc +++ b/shell/platform/android/android_surface_software.cc @@ -37,11 +37,12 @@ bool GetSkColorType(int32_t buffer_format, } // anonymous namespace AndroidSurfaceSoftware::AndroidSurfaceSoftware( + std::shared_ptr android_context, std::shared_ptr jni_facade) { GetSkColorType(WINDOW_FORMAT_RGBA_8888, &target_color_type_, &target_alpha_type_); - external_view_embedder_ = - std::make_unique(jni_facade); + external_view_embedder_ = std::make_unique( + android_context, jni_facade, nullptr); } AndroidSurfaceSoftware::~AndroidSurfaceSoftware() = default; diff --git a/shell/platform/android/android_surface_software.h b/shell/platform/android/android_surface_software.h index 00a4b245af7d2..762b318193d2d 100644 --- a/shell/platform/android/android_surface_software.h +++ b/shell/platform/android/android_surface_software.h @@ -9,16 +9,17 @@ #include "flutter/fml/platform/android/jni_weak_ref.h" #include "flutter/fml/platform/android/scoped_java_ref.h" #include "flutter/shell/gpu/gpu_surface_software.h" -#include "flutter/shell/platform/android/android_surface.h" #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "flutter/shell/platform/android/surface/android_surface.h" namespace flutter { class AndroidSurfaceSoftware final : public AndroidSurface, public GPUSurfaceSoftwareDelegate { public: - AndroidSurfaceSoftware(std::shared_ptr jni_facade); + AndroidSurfaceSoftware(std::shared_ptr android_context, + std::shared_ptr jni_facade); ~AndroidSurfaceSoftware() override; diff --git a/shell/platform/android/android_surface_vulkan.cc b/shell/platform/android/android_surface_vulkan.cc index 3d802a1f1ba42..9b506f40ebb57 100644 --- a/shell/platform/android/android_surface_vulkan.cc +++ b/shell/platform/android/android_surface_vulkan.cc @@ -13,10 +13,11 @@ namespace flutter { AndroidSurfaceVulkan::AndroidSurfaceVulkan( + std::shared_ptr android_context, std::shared_ptr jni_facade) : proc_table_(fml::MakeRefCounted()) { - external_view_embedder_ = - std::make_unique(jni_facade); + external_view_embedder_ = std::make_unique( + android_context, jni_facade, nullptr); } AndroidSurfaceVulkan::~AndroidSurfaceVulkan() = default; diff --git a/shell/platform/android/android_surface_vulkan.h b/shell/platform/android/android_surface_vulkan.h index 2778a7ec10003..3c2ac555bd57b 100644 --- a/shell/platform/android/android_surface_vulkan.h +++ b/shell/platform/android/android_surface_vulkan.h @@ -9,10 +9,9 @@ #include #include "flutter/fml/macros.h" #include "flutter/shell/gpu/gpu_surface_vulkan_delegate.h" -#include "flutter/shell/platform/android/android_native_window.h" -#include "flutter/shell/platform/android/android_surface.h" #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "flutter/shell/platform/android/surface/android_surface.h" #include "flutter/vulkan/vulkan_window.h" namespace flutter { @@ -20,7 +19,8 @@ namespace flutter { class AndroidSurfaceVulkan : public AndroidSurface, public GPUSurfaceVulkanDelegate { public: - AndroidSurfaceVulkan(std::shared_ptr jni_facade); + AndroidSurfaceVulkan(std::shared_ptr android_context, + std::shared_ptr jni_facade); ~AndroidSurfaceVulkan() override; diff --git a/shell/platform/android/context/android_context.cc b/shell/platform/android/context/android_context.cc index bf244b00ac794..4ff6550c15abb 100644 --- a/shell/platform/android/context/android_context.cc +++ b/shell/platform/android/context/android_context.cc @@ -2,12 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/shell/platform/android/android_context.h" - -#if OS_ANDROID -#include "flutter/shell/platform/android/android_context_gl.h" -#include "flutter/shell/platform/android/android_environment_gl.h" -#endif +#include "flutter/shell/platform/android/context/android_context.h" namespace flutter { @@ -20,19 +15,8 @@ AndroidRenderingAPI AndroidContext::RenderingApi() const { return rendering_api_; } -std::shared_ptr AndroidContext::Create( - AndroidRenderingAPI rendering_api) { -#if OS_ANDROID - if (rendering_api == AndroidRenderingAPI::kOpenGLES) { - auto context_gl = std::make_shared( - AndroidRenderingAPI::kOpenGLES, - fml::MakeRefCounted()); - FML_CHECK(context_gl->IsValid()) - << "Could not create an Android context GL."; - return context_gl; - } -#endif - return std::make_shared(rendering_api); +bool AndroidContext::IsValid() const { + return true; } } // namespace flutter diff --git a/shell/platform/android/context/android_context.h b/shell/platform/android/context/android_context.h index b14eae344c058..37fa60ef6f3c4 100644 --- a/shell/platform/android/context/android_context.h +++ b/shell/platform/android/context/android_context.h @@ -26,11 +26,10 @@ class AndroidContext { ~AndroidContext(); - static std::shared_ptr Create( - AndroidRenderingAPI rendering_api); - AndroidRenderingAPI RenderingApi() const; + bool IsValid() const; + private: const AndroidRenderingAPI rendering_api_; 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 9c62b8b02ae71..b5f9f80e8e061 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -9,11 +9,13 @@ namespace flutter { AndroidExternalViewEmbedder::AndroidExternalViewEmbedder( + std::shared_ptr android_context, std::shared_ptr jni_facade, - std::shared_ptr android_context) + AndroidSurface::Factory surface_factory) : ExternalViewEmbedder(), - jni_facade_(jni_facade), - android_context_(android_context), + android_context_(std::move(android_context)), + jni_facade_(std::move(jni_facade)), + surface_factory_(surface_factory), surface_pool_(std::make_unique()) {} // |ExternalViewEmbedder| @@ -53,6 +55,16 @@ std::vector AndroidExternalViewEmbedder::GetCurrentCanvases() { return canvases; } +SkRect AndroidExternalViewEmbedder::GetViewRect(int view_id) { + EmbeddedViewParams* params = &view_params_[view_id]; + FML_CHECK(params != nullptr); + return SkRect::MakeXYWH(params->offsetPixels.x(), // + params->offsetPixels.y(), // + params->sizePoints.width() * device_pixel_ratio_, // + params->sizePoints.height() * device_pixel_ratio_ // + ); +} + // |ExternalViewEmbedder| bool AndroidExternalViewEmbedder::SubmitFrame( GrContext* context, @@ -95,7 +107,7 @@ bool AndroidExternalViewEmbedder::SubmitFrame( // view layer. for (size_t j = i + 1; j > 0; j--) { int64_t current_view_id = composition_order_[j - 1]; - SkRect current_view_rect = GetPlatformViewRect(current_view_id); + SkRect current_view_rect = GetViewRect(current_view_id); // Each rect corresponds to a native view that renders Flutter UI. std::list intersection_rects = rtree->searchNonOverlappingDrawnRects(current_view_rect); @@ -128,7 +140,7 @@ bool AndroidExternalViewEmbedder::SubmitFrame( ); // Clip the background canvas, so it doesn't contain any of the pixels // drawn on the overlay layer. - background_canvas->clipRect(joined_rect, SkClipOp::kDifference); + background_canvas->clipRect(intersection_rect, SkClipOp::kDifference); } overlay_layers[current_view_id] = intersection_rects; } @@ -145,7 +157,7 @@ bool AndroidExternalViewEmbedder::SubmitFrame( CreateSurfaceIfNeeded(context, // view_id, // pictures[view_id], // - overlay_rect, // + overlay_rect // ) ->Submit(); } @@ -159,8 +171,8 @@ AndroidExternalViewEmbedder::CreateSurfaceIfNeeded(GrContext* context, int64_t view_id, sk_sp picture, SkRect& rect) { - std::shared_ptr layer = - surface_pool_->GetLayer(context, jni_facade_, android_context_); + std::shared_ptr layer = surface_pool_->GetLayer( + context, android_context_, jni_facade_, surface_factory_); std::unique_ptr frame = layer->surface->AcquireFrame(frame_size_); @@ -224,6 +236,7 @@ void AndroidExternalViewEmbedder::BeginFrame(SkISize frame_size, double device_pixel_ratio) { Reset(); frame_size_ = frame_size; + device_pixel_ratio_ = device_pixel_ratio; } // |ExternalViewEmbedder| 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 0576e18252227..7f6037109028e 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -6,6 +6,7 @@ #define FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_H_ #include "flutter/flow/embedded_views.h" +#include "flutter/flow/rtree.h" #include "flutter/shell/platform/android/context/android_context.h" #include "flutter/shell/platform/android/external_view_embedder/surface_pool.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" @@ -16,8 +17,9 @@ namespace flutter { class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { public: AndroidExternalViewEmbedder( + std::shared_ptr android_context, std::shared_ptr jni_facade, - std::shared_ptr android_context); + AndroidSurface::Factory surface_factory); // |ExternalViewEmbedder| void PrerollCompositeEmbeddedView( @@ -54,6 +56,8 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { fml::RefPtr raster_thread_merger) override; private: + static const int kMaxLayerAllocations = 2; + // The number of frames the rasterizer task runner will continue // to run on the platform thread after no platform view is rendered. // @@ -61,11 +65,14 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // where the platform view might be momentarily off the screen. static const int kDefaultMergedLeaseDuration = 10; + // Provides metadata to the Android surfaces. + const std::shared_ptr android_context_; + // Allows to call methods in Java. const std::shared_ptr jni_facade_; - // Provides metadata to the Android surfaces. - const std::shared_ptr android_context_; + // Allows to create surfaces. + const AndroidSurface::Factory surface_factory_; // Holds surfaces. Allows to recycle surfaces or allocate new ones. const std::unique_ptr surface_pool_; @@ -74,9 +81,13 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // When this is true, the current frame is cancelled and resubmitted. bool should_run_rasterizer_on_platform_thread_ = false; - // The size of the background canvas. + // The size of the root canvas. SkISize frame_size_; + // The pixel ratio used to determinate the size of a platform view layer + // relative to the device layout system. + double device_pixel_ratio_; + // The order of composition. Each entry contains a unique id for the platform // view. std::vector composition_order_; @@ -102,6 +113,10 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { int64_t view_id, sk_sp picture, SkRect& rect); + + // Gets the rect based on the device pixel ratio of a platform view displayed + // on the screen. + SkRect GetViewRect(int view_id); }; } // namespace flutter 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 d0ef824cb6724..d7671a89077ff 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 @@ -11,7 +11,7 @@ namespace flutter { namespace testing { TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { - auto embedder = new AndroidExternalViewEmbedder(nullptr); + auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); @@ -27,7 +27,7 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { } TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) { - auto embedder = new AndroidExternalViewEmbedder(nullptr); + auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); @@ -39,7 +39,7 @@ TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) { } TEST(AndroidExternalViewEmbedder, CancelFrame) { - auto embedder = new AndroidExternalViewEmbedder(nullptr); + auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); @@ -50,7 +50,7 @@ TEST(AndroidExternalViewEmbedder, CancelFrame) { } TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { - auto embedder = new AndroidExternalViewEmbedder(nullptr); + auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); auto platform_thread = new fml::Thread("platform"); auto rasterizer_thread = new fml::Thread("rasterizer"); auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); @@ -82,7 +82,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { } TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { - auto embedder = new AndroidExternalViewEmbedder(nullptr); + auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); auto platform_thread = new fml::Thread("platform"); auto rasterizer_thread = new fml::Thread("rasterizer"); auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index 91963a820731a..7583fd5e2e206 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -6,28 +6,35 @@ namespace flutter { -OverlayLayer::OverlayLayer(long id, +OverlayLayer::OverlayLayer(int id, std::unique_ptr android_surface, std::unique_ptr surface) : id(id), android_surface(std::move(android_surface)), surface(std::move(surface)){}; +OverlayLayer::~OverlayLayer() = default; + +SurfacePool::SurfacePool() = default; + +SurfacePool::~SurfacePool() = default; + std::shared_ptr SurfacePool::GetLayer( GrContext* gr_context, + std::shared_ptr android_context, std::shared_ptr jni_facade, - std::shared_ptr android_context) { + AndroidSurface::Factory surface_factory) { // Allocate a new surface if there isn't one available. if (available_layer_index_ >= layers_.size()) { std::unique_ptr android_surface = - AndroidSurface::Create(android_context, jni_facade); + surface_factory(android_context, jni_facade); // TODO(egarciad): jni_facade->FlutterViewCreateOverlaySurface(..) // Then, android_surface->SetNativeWindow(overlay_layer->GetWindow()) // https://github.com/flutter/flutter/issues/55270 std::unique_ptr surface = android_surface->CreateGPUSurface(gr_context); std::shared_ptr layer = - std::make_shared(overlay_layer->GetId(), // + std::make_shared(0, // std::move(android_surface), // std::move(surface) // ); diff --git a/shell/platform/android/external_view_embedder/surface_pool.h b/shell/platform/android/external_view_embedder/surface_pool.h index 327373b2b84b7..e6c541fd46db8 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.h +++ b/shell/platform/android/external_view_embedder/surface_pool.h @@ -6,23 +6,23 @@ #define FLUTTER_SHELL_PLATFORM_ANDROID_EXTERNAL_VIEW_EMBEDDER_SURFACE_POOL_H_ #include "flutter/flow/surface.h" -#include "flutter/shell/platform/android/android_surface.h" #include "flutter/shell/platform/android/context/android_context.h" +#include "flutter/shell/platform/android/surface/android_surface.h" namespace flutter { struct OverlayLayer { - OverlayLayer(long id, + OverlayLayer(int id, std::unique_ptr android_surface, std::unique_ptr surface); - ~OverlayLayer() = default; + ~OverlayLayer(); - const long id; + const int id; const std::unique_ptr android_surface; - const std::unique_ptr surface; + std::unique_ptr surface; // The `GrContext` that is currently used by the overlay surfaces. // We track this to know when the GrContext for the Flutter app has changed @@ -33,16 +33,18 @@ struct OverlayLayer { // This class isn't thread safe. class SurfacePool { public: - SurfacePool() = default; - ~SurfacePool() = default; + SurfacePool(); + + ~SurfacePool(); // Gets a layer from the pool if available, or allocates a new one. // Finally, it marks the layer as used. That is, it increments // `available_layer_index_`. std::shared_ptr GetLayer( GrContext* gr_context, + std::shared_ptr android_context, std::shared_ptr jni_facade, - std::shared_ptr android_context); + AndroidSurface::Factory surface_factory); // Gets the layers in the pool that aren't currently used. // This method doesn't mark the layers as unused. @@ -56,17 +58,17 @@ class SurfacePool { // of the unused layers. For example, consider the following vector: // _____ // | 0 | - /// |---| - /// | 1 | <-- `available_layer_index_` - /// |---| - /// | 2 | - /// |---| - /// - /// This indicates that entries starting from 1 can be reused meanwhile the - /// entry at position 0 cannot be reused. + // |---| + // | 1 | <-- `available_layer_index_` + // |---| + // | 2 | + // |---| + // + // This indicates that entries starting from 1 can be reused meanwhile the + // entry at position 0 cannot be reused. size_t available_layer_index_ = 0; std::vector> layers_; -} +}; } // namespace flutter diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 0cae52e370c4d..23a123a15ebdc 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -10,15 +10,45 @@ #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/shell/common/shell_io_manager.h" #include "flutter/shell/gpu/gpu_surface_gl_delegate.h" -#include "flutter/shell/platform/android/android_context.h" +#include "flutter/shell/platform/android/android_context_gl.h" #include "flutter/shell/platform/android/android_external_texture_gl.h" #include "flutter/shell/platform/android/android_surface_gl.h" +#include "flutter/shell/platform/android/android_surface_software.h" + +#if SHELL_ENABLE_VULKAN +#include "flutter/shell/platform/android/android_surface_vulkan.h" +#endif + +#include "flutter/shell/platform/android/context/android_context.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" #include "flutter/shell/platform/android/platform_message_response_android.h" #include "flutter/shell/platform/android/vsync_waiter_android.h" namespace flutter { +std::unique_ptr SurfaceFactory( + std::shared_ptr android_context, + std::shared_ptr jni_facade) { + std::unique_ptr surface; + switch (android_context->RenderingApi()) { + case AndroidRenderingAPI::kSoftware: + surface = + std::make_unique(android_context, jni_facade); + break; + case AndroidRenderingAPI::kOpenGLES: + surface = std::make_unique(android_context, jni_facade); + break; + case AndroidRenderingAPI::kVulkan: +#if SHELL_ENABLE_VULKAN + surface = + std::make_unique(android_context, jni_facade); +#endif // SHELL_ENABLE_VULKAN + break; + } + FML_CHECK(surface); + return surface->IsValid() ? std::move(surface) : nullptr; +} + PlatformViewAndroid::PlatformViewAndroid( PlatformView::Delegate& delegate, flutter::TaskRunners task_runners, @@ -27,15 +57,22 @@ PlatformViewAndroid::PlatformViewAndroid( : PlatformView(delegate, std::move(task_runners)), jni_facade_(jni_facade) { std::shared_ptr android_context; if (use_software_rendering) { - android_context = AndroidContext::Create(AndroidRenderingAPI::kSoftware); + android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); } else { #if SHELL_ENABLE_VULKAN - android_context = AndroidContext::Create(AndroidRenderingAPI::kVulkan); + android_context = + std::make_shared(AndroidRenderingAPI::kVulkan); #else // SHELL_ENABLE_VULKAN - android_context = AndroidContext::Create(AndroidRenderingAPI::kOpenGLES); + android_context = std::make_shared( + AndroidRenderingAPI::kOpenGLES, + fml::MakeRefCounted()); #endif // SHELL_ENABLE_VULKAN } - android_surface_ = AndroidSurface::Create(android_context, jni_facade); + FML_CHECK(android_context->IsValid()) + << "Could not create an Android context."; + + android_surface_ = SurfaceFactory(android_context, jni_facade); FML_CHECK(android_surface_) << "Could not create an OpenGL, Vulkan or Software surface to setup " "rendering."; diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 9f859a1dcdaf5..856c64862166a 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -15,9 +15,9 @@ #include "flutter/fml/platform/android/scoped_java_ref.h" #include "flutter/lib/ui/window/platform_message.h" #include "flutter/shell/common/platform_view.h" -#include "flutter/shell/platform/android/android_native_window.h" -#include "flutter/shell/platform/android/android_surface.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "flutter/shell/platform/android/surface/android_native_window.h" +#include "flutter/shell/platform/android/surface/android_surface.h" namespace flutter { diff --git a/shell/platform/android/surface/BUILD.gn b/shell/platform/android/surface/BUILD.gn index 8cdfb1c612d67..1cd92c79d15b0 100644 --- a/shell/platform/android/surface/BUILD.gn +++ b/shell/platform/android/surface/BUILD.gn @@ -2,12 +2,12 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -import("//flutter/common/config.gni") - source_set("surface") { sources = [ "android_surface.cc", "android_surface.h", + "android_native_window.cc", + "android_native_window.h", ] public_configs = [ "//flutter:config" ] @@ -16,5 +16,7 @@ source_set("surface") { "//flutter/fml", "//flutter/flow", "//third_party/skia", + "//flutter/shell/platform/android/context", + "//flutter/shell/platform/android/jni", ] } diff --git a/shell/platform/android/android_native_window.cc b/shell/platform/android/surface/android_native_window.cc similarity index 80% rename from shell/platform/android/android_native_window.cc rename to shell/platform/android/surface/android_native_window.cc index fac43fa89d45b..e701b986f48e9 100644 --- a/shell/platform/android/android_native_window.cc +++ b/shell/platform/android/surface/android_native_window.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/shell/platform/android/android_native_window.h" +#include "flutter/shell/platform/android/surface/android_native_window.h" namespace flutter { @@ -10,7 +10,9 @@ AndroidNativeWindow::AndroidNativeWindow(Handle window) : window_(window) {} AndroidNativeWindow::~AndroidNativeWindow() { if (window_ != nullptr) { +#if OS_ANDROID ANativeWindow_release(window_); +#endif // OS_ANDROID window_ = nullptr; } } @@ -24,9 +26,13 @@ AndroidNativeWindow::Handle AndroidNativeWindow::handle() const { } SkISize AndroidNativeWindow::GetSize() const { +#if OS_ANDROID return window_ == nullptr ? SkISize::Make(0, 0) : SkISize::Make(ANativeWindow_getWidth(window_), ANativeWindow_getHeight(window_)); +#else // OS_ANDROID + return SkISize::Make(0, 0); +#endif // OS_ANDROID } } // namespace flutter diff --git a/shell/platform/android/android_native_window.h b/shell/platform/android/surface/android_native_window.h similarity index 87% rename from shell/platform/android/android_native_window.h rename to shell/platform/android/surface/android_native_window.h index 7e9626d63d8ea..bcb1c19e5a292 100644 --- a/shell/platform/android/android_native_window.h +++ b/shell/platform/android/surface/android_native_window.h @@ -5,10 +5,14 @@ #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_NATIVE_WINDOW_H_ #define FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_NATIVE_WINDOW_H_ +#include "flutter/fml/build_config.h" + +#if OS_ANDROID #include +#endif // OS_ANDROID + #include "flutter/fml/macros.h" #include "flutter/fml/memory/ref_counted.h" -#include "flutter/fml/memory/ref_ptr.h" #include "third_party/skia/include/core/SkSize.h" namespace flutter { @@ -16,7 +20,11 @@ namespace flutter { class AndroidNativeWindow : public fml::RefCountedThreadSafe { public: +#if OS_ANDROID using Handle = ANativeWindow*; +#else // OS_ANDROID + using Handle = std::nullptr_t; +#endif // OS_ANDROID bool IsValid() const; diff --git a/shell/platform/android/surface/android_surface.cc b/shell/platform/android/surface/android_surface.cc index 94dbbe1cee4ce..1f5032d8917cf 100644 --- a/shell/platform/android/surface/android_surface.cc +++ b/shell/platform/android/surface/android_surface.cc @@ -2,45 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/shell/platform/android/android_surface.h" - -#include - -#if OS_ANDROID -#include "flutter/shell/platform/android/android_surface_gl.h" -#include "flutter/shell/platform/android/android_surface_software.h" -#if SHELL_ENABLE_VULKAN -#include "flutter/shell/platform/android/android_surface_vulkan.h" -#endif // SHELL_ENABLE_VULKAN -#endif // OS_ANDROID +#include "flutter/shell/platform/android/surface/android_surface.h" namespace flutter { -std::unique_ptr AndroidSurface::Create( - std::shared_ptr android_context, - std::shared_ptr jni_facade) { -#if !OS_ANDROID - return nullptr; -#endif // OS_ANDROID - std::unique_ptr surface; - switch (android_context->RenderingApi()) { - case AndroidRenderingAPI::kSoftware: - surface = std::make_unique(jni_facade); - break; - case AndroidRenderingAPI::kOpenGLES: - surface = std::make_unique(android_context, jni_facade); - break; - case AndroidRenderingAPI::kVulkan: -#if SHELL_ENABLE_VULKAN - surface = std::make_unique(jni_facade); -#endif // SHELL_ENABLE_VULKAN - break; - } - FML_CHECK(surface); - return surface->IsValid() ? std::move(surface) : nullptr; - ; -} - AndroidSurface::~AndroidSurface() = default; } // namespace flutter diff --git a/shell/platform/android/surface/android_surface.h b/shell/platform/android/surface/android_surface.h index 7914f58573c2e..2daae44da6850 100644 --- a/shell/platform/android/surface/android_surface.h +++ b/shell/platform/android/surface/android_surface.h @@ -7,16 +7,16 @@ #include "flutter/flow/surface.h" #include "flutter/fml/macros.h" -#include "flutter/shell/platform/android/android_native_window.h" #include "flutter/shell/platform/android/context/android_context.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "flutter/shell/platform/android/surface/android_native_window.h" #include "third_party/skia/include/core/SkSize.h" namespace flutter { class AndroidSurface { public: - static std::unique_ptr Create( + typedef std::unique_ptr (*Factory)( std::shared_ptr android_context, std::shared_ptr jni_facade); From 56fd07a2bce9fcce58a15ba6f0e88fb30875057d Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Sat, 13 Jun 2020 13:17:54 -0700 Subject: [PATCH 05/28] Pass surface factory --- shell/platform/android/android_surface_gl.cc | 5 +++-- shell/platform/android/android_surface_gl.h | 3 ++- shell/platform/android/android_surface_software.cc | 5 +++-- shell/platform/android/android_surface_software.h | 3 ++- shell/platform/android/android_surface_vulkan.cc | 5 +++-- shell/platform/android/android_surface_vulkan.h | 3 ++- shell/platform/android/platform_view_android.cc | 13 +++++++------ 7 files changed, 22 insertions(+), 15 deletions(-) diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index 99d1bfa45d4c7..2c3ea5fd1a8d4 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -13,7 +13,8 @@ namespace flutter { AndroidSurfaceGL::AndroidSurfaceGL( std::shared_ptr android_context, - std::shared_ptr jni_facade) + std::shared_ptr jni_facade, + AndroidSurface::Factory surface_factory) : native_window_(nullptr), onscreen_surface_(nullptr), offscreen_surface_(nullptr) { @@ -25,7 +26,7 @@ AndroidSurfaceGL::AndroidSurfaceGL( offscreen_surface_ = nullptr; } external_view_embedder_ = std::make_unique( - android_context, jni_facade, nullptr); + android_context, jni_facade, surface_factory); } AndroidSurfaceGL::~AndroidSurfaceGL() = default; diff --git a/shell/platform/android/android_surface_gl.h b/shell/platform/android/android_surface_gl.h index 092e53de970fd..45d57e03ea489 100644 --- a/shell/platform/android/android_surface_gl.h +++ b/shell/platform/android/android_surface_gl.h @@ -22,7 +22,8 @@ class AndroidSurfaceGL final : public GPUSurfaceGLDelegate, public AndroidSurface { public: AndroidSurfaceGL(std::shared_ptr android_context, - std::shared_ptr jni_facade); + std::shared_ptr jni_facade, + AndroidSurface::Factory surface_factory); ~AndroidSurfaceGL() override; diff --git a/shell/platform/android/android_surface_software.cc b/shell/platform/android/android_surface_software.cc index 91e848da0883e..ffddb33c73141 100644 --- a/shell/platform/android/android_surface_software.cc +++ b/shell/platform/android/android_surface_software.cc @@ -38,11 +38,12 @@ bool GetSkColorType(int32_t buffer_format, AndroidSurfaceSoftware::AndroidSurfaceSoftware( std::shared_ptr android_context, - std::shared_ptr jni_facade) { + std::shared_ptr jni_facade, + AndroidSurface::Factory surface_factory) { GetSkColorType(WINDOW_FORMAT_RGBA_8888, &target_color_type_, &target_alpha_type_); external_view_embedder_ = std::make_unique( - android_context, jni_facade, nullptr); + android_context, jni_facade, surface_factory); } AndroidSurfaceSoftware::~AndroidSurfaceSoftware() = default; diff --git a/shell/platform/android/android_surface_software.h b/shell/platform/android/android_surface_software.h index 762b318193d2d..97de7f3556801 100644 --- a/shell/platform/android/android_surface_software.h +++ b/shell/platform/android/android_surface_software.h @@ -19,7 +19,8 @@ class AndroidSurfaceSoftware final : public AndroidSurface, public GPUSurfaceSoftwareDelegate { public: AndroidSurfaceSoftware(std::shared_ptr android_context, - std::shared_ptr jni_facade); + std::shared_ptr jni_facade, + AndroidSurface::Factory surface_factory); ~AndroidSurfaceSoftware() override; diff --git a/shell/platform/android/android_surface_vulkan.cc b/shell/platform/android/android_surface_vulkan.cc index 9b506f40ebb57..acb9f08681aae 100644 --- a/shell/platform/android/android_surface_vulkan.cc +++ b/shell/platform/android/android_surface_vulkan.cc @@ -14,10 +14,11 @@ namespace flutter { AndroidSurfaceVulkan::AndroidSurfaceVulkan( std::shared_ptr android_context, - std::shared_ptr jni_facade) + std::shared_ptr jni_facade, + AndroidSurface::Factory surface_factory) : proc_table_(fml::MakeRefCounted()) { external_view_embedder_ = std::make_unique( - android_context, jni_facade, nullptr); + android_context, jni_facade, surface_factory); } AndroidSurfaceVulkan::~AndroidSurfaceVulkan() = default; diff --git a/shell/platform/android/android_surface_vulkan.h b/shell/platform/android/android_surface_vulkan.h index 3c2ac555bd57b..64bc5e2a63b4f 100644 --- a/shell/platform/android/android_surface_vulkan.h +++ b/shell/platform/android/android_surface_vulkan.h @@ -20,7 +20,8 @@ class AndroidSurfaceVulkan : public AndroidSurface, public GPUSurfaceVulkanDelegate { public: AndroidSurfaceVulkan(std::shared_ptr android_context, - std::shared_ptr jni_facade); + std::shared_ptr jni_facade, + AndroidSurface::Factory surface_factory); ~AndroidSurfaceVulkan() override; diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 23a123a15ebdc..f06b4d40a48d9 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -17,7 +17,7 @@ #if SHELL_ENABLE_VULKAN #include "flutter/shell/platform/android/android_surface_vulkan.h" -#endif +#endif // SHELL_ENABLE_VULKAN #include "flutter/shell/platform/android/context/android_context.h" #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" @@ -32,16 +32,17 @@ std::unique_ptr SurfaceFactory( std::unique_ptr surface; switch (android_context->RenderingApi()) { case AndroidRenderingAPI::kSoftware: - surface = - std::make_unique(android_context, jni_facade); + surface = std::make_unique( + android_context, jni_facade, SurfaceFactory); break; case AndroidRenderingAPI::kOpenGLES: - surface = std::make_unique(android_context, jni_facade); + surface = std::make_unique(android_context, jni_facade, + SurfaceFactory); break; case AndroidRenderingAPI::kVulkan: #if SHELL_ENABLE_VULKAN - surface = - std::make_unique(android_context, jni_facade); + surface = std::make_unique( + android_context, jni_facade, SurfaceFactory); #endif // SHELL_ENABLE_VULKAN break; } From 58b3e971e41fe7a1787cd5c4c66f103b48e91c70 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Sat, 13 Jun 2020 13:20:25 -0700 Subject: [PATCH 06/28] Remove imports --- shell/platform/android/context/android_context.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/shell/platform/android/context/android_context.h b/shell/platform/android/context/android_context.h index 37fa60ef6f3c4..627cbd8150855 100644 --- a/shell/platform/android/context/android_context.h +++ b/shell/platform/android/context/android_context.h @@ -6,8 +6,6 @@ #define FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_CONTEXT_H_ #include "flutter/fml/macros.h" -#include "flutter/fml/memory/ref_counted.h" -#include "flutter/fml/memory/ref_ptr.h" namespace flutter { From b3540a9636308eb2a8424a8ac75f129dffa9c082 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Sat, 13 Jun 2020 13:25:28 -0700 Subject: [PATCH 07/28] Move shared pointers --- shell/platform/android/android_surface_gl.cc | 2 +- shell/platform/android/android_surface_software.cc | 2 +- shell/platform/android/android_surface_vulkan.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index 2c3ea5fd1a8d4..e97bab69cf677 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -26,7 +26,7 @@ AndroidSurfaceGL::AndroidSurfaceGL( offscreen_surface_ = nullptr; } external_view_embedder_ = std::make_unique( - android_context, jni_facade, surface_factory); + std::move(android_context), std::move(jni_facade), surface_factory); } AndroidSurfaceGL::~AndroidSurfaceGL() = default; diff --git a/shell/platform/android/android_surface_software.cc b/shell/platform/android/android_surface_software.cc index ffddb33c73141..fedf4b7812418 100644 --- a/shell/platform/android/android_surface_software.cc +++ b/shell/platform/android/android_surface_software.cc @@ -43,7 +43,7 @@ AndroidSurfaceSoftware::AndroidSurfaceSoftware( GetSkColorType(WINDOW_FORMAT_RGBA_8888, &target_color_type_, &target_alpha_type_); external_view_embedder_ = std::make_unique( - android_context, jni_facade, surface_factory); + std::move(android_context), std::move(jni_facade), surface_factory); } AndroidSurfaceSoftware::~AndroidSurfaceSoftware() = default; diff --git a/shell/platform/android/android_surface_vulkan.cc b/shell/platform/android/android_surface_vulkan.cc index acb9f08681aae..affd4c2b8d574 100644 --- a/shell/platform/android/android_surface_vulkan.cc +++ b/shell/platform/android/android_surface_vulkan.cc @@ -18,7 +18,7 @@ AndroidSurfaceVulkan::AndroidSurfaceVulkan( AndroidSurface::Factory surface_factory) : proc_table_(fml::MakeRefCounted()) { external_view_embedder_ = std::make_unique( - android_context, jni_facade, surface_factory); + std::move(android_context), std::move(jni_facade), surface_factory); } AndroidSurfaceVulkan::~AndroidSurfaceVulkan() = default; From 1e896bd0451a5d9a8ad371dea6d8b5593d82ab44 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Sat, 13 Jun 2020 13:26:41 -0700 Subject: [PATCH 08/28] gn format --- shell/platform/android/surface/BUILD.gn | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/platform/android/surface/BUILD.gn b/shell/platform/android/surface/BUILD.gn index 1cd92c79d15b0..bc4431dabdb54 100644 --- a/shell/platform/android/surface/BUILD.gn +++ b/shell/platform/android/surface/BUILD.gn @@ -4,19 +4,19 @@ source_set("surface") { sources = [ - "android_surface.cc", - "android_surface.h", "android_native_window.cc", "android_native_window.h", + "android_surface.cc", + "android_surface.h", ] public_configs = [ "//flutter:config" ] deps = [ - "//flutter/fml", "//flutter/flow", - "//third_party/skia", + "//flutter/fml", "//flutter/shell/platform/android/context", "//flutter/shell/platform/android/jni", + "//third_party/skia", ] } From 7fcbb34ced29734613203dcf4e165ec83074743d Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 15 Jun 2020 10:58:03 -0700 Subject: [PATCH 09/28] Update license --- ci/licenses_golden/licenses_flutter | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 2770d2953015b..237c00d0803b6 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -645,8 +645,6 @@ FILE: ../../../flutter/shell/gpu/gpu_surface_vulkan.h FILE: ../../../flutter/shell/gpu/gpu_surface_vulkan_delegate.cc FILE: ../../../flutter/shell/gpu/gpu_surface_vulkan_delegate.h FILE: ../../../flutter/shell/platform/android/AndroidManifest.xml -FILE: ../../../flutter/shell/platform/android/android_context.cc -FILE: ../../../flutter/shell/platform/android/android_context.h FILE: ../../../flutter/shell/platform/android/android_context_gl.cc FILE: ../../../flutter/shell/platform/android/android_context_gl.h FILE: ../../../flutter/shell/platform/android/android_environment_gl.cc @@ -654,12 +652,8 @@ FILE: ../../../flutter/shell/platform/android/android_environment_gl.h FILE: ../../../flutter/shell/platform/android/android_exports.lst FILE: ../../../flutter/shell/platform/android/android_external_texture_gl.cc FILE: ../../../flutter/shell/platform/android/android_external_texture_gl.h -FILE: ../../../flutter/shell/platform/android/android_native_window.cc -FILE: ../../../flutter/shell/platform/android/android_native_window.h FILE: ../../../flutter/shell/platform/android/android_shell_holder.cc FILE: ../../../flutter/shell/platform/android/android_shell_holder.h -FILE: ../../../flutter/shell/platform/android/android_surface.cc -FILE: ../../../flutter/shell/platform/android/android_surface.h FILE: ../../../flutter/shell/platform/android/android_surface_gl.cc FILE: ../../../flutter/shell/platform/android/android_surface_gl.h FILE: ../../../flutter/shell/platform/android/android_surface_software.cc @@ -668,9 +662,13 @@ FILE: ../../../flutter/shell/platform/android/android_surface_vulkan.cc FILE: ../../../flutter/shell/platform/android/android_surface_vulkan.h FILE: ../../../flutter/shell/platform/android/apk_asset_provider.cc FILE: ../../../flutter/shell/platform/android/apk_asset_provider.h +FILE: ../../../flutter/shell/platform/android/context/android_context.cc +FILE: ../../../flutter/shell/platform/android/context/android_context.h FILE: ../../../flutter/shell/platform/android/external_view_embedder/external_view_embedder.cc FILE: ../../../flutter/shell/platform/android/external_view_embedder/external_view_embedder.h FILE: ../../../flutter/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +FILE: ../../../flutter/shell/platform/android/external_view_embedder/surface_pool.cc +FILE: ../../../flutter/shell/platform/android/external_view_embedder/surface_pool.h FILE: ../../../flutter/shell/platform/android/flutter_main.cc FILE: ../../../flutter/shell/platform/android/flutter_main.h FILE: ../../../flutter/shell/platform/android/io/flutter/Log.java @@ -793,6 +791,10 @@ FILE: ../../../flutter/shell/platform/android/platform_view_android.h FILE: ../../../flutter/shell/platform/android/platform_view_android_jni_impl.cc FILE: ../../../flutter/shell/platform/android/platform_view_android_jni_impl.h FILE: ../../../flutter/shell/platform/android/robolectric.properties +FILE: ../../../flutter/shell/platform/android/surface/android_native_window.cc +FILE: ../../../flutter/shell/platform/android/surface/android_native_window.h +FILE: ../../../flutter/shell/platform/android/surface/android_surface.cc +FILE: ../../../flutter/shell/platform/android/surface/android_surface.h FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.cc FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/basic_message_channel_unittests.cc From a586e170550cf77809cd8f6bdbccb8c3b818a084 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 15 Jun 2020 11:32:40 -0700 Subject: [PATCH 10/28] Make external view embedder const --- shell/platform/android/android_surface_gl.cc | 12 +++++++----- shell/platform/android/android_surface_gl.h | 5 +++-- shell/platform/android/android_surface_software.cc | 8 +++++--- shell/platform/android/android_surface_software.h | 3 ++- shell/platform/android/android_surface_vulkan.cc | 9 +++++---- shell/platform/android/android_surface_vulkan.h | 3 ++- 6 files changed, 24 insertions(+), 16 deletions(-) diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index e97bab69cf677..12ba27e7a9210 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -15,18 +15,20 @@ AndroidSurfaceGL::AndroidSurfaceGL( std::shared_ptr android_context, std::shared_ptr jni_facade, AndroidSurface::Factory surface_factory) - : native_window_(nullptr), + : external_view_embedder_(std::make_unique( + std::move(android_context), + std::move(jni_facade), + surface_factory)), + android_context_( + std::static_pointer_cast(android_context)), + native_window_(nullptr), onscreen_surface_(nullptr), offscreen_surface_(nullptr) { - android_context_ = - std::static_pointer_cast(android_context); // Acquire the offscreen surface. offscreen_surface_ = android_context_->CreateOffscreenSurface(); if (!offscreen_surface_->IsValid()) { offscreen_surface_ = nullptr; } - external_view_embedder_ = std::make_unique( - std::move(android_context), std::move(jni_facade), surface_factory); } AndroidSurfaceGL::~AndroidSurfaceGL() = default; diff --git a/shell/platform/android/android_surface_gl.h b/shell/platform/android/android_surface_gl.h index 45d57e03ea489..35a25a252c1ea 100644 --- a/shell/platform/android/android_surface_gl.h +++ b/shell/platform/android/android_surface_gl.h @@ -64,9 +64,10 @@ class AndroidSurfaceGL final : public GPUSurfaceGLDelegate, ExternalViewEmbedder* GetExternalViewEmbedder() override; private: + const std::unique_ptr external_view_embedder_; + const std::shared_ptr android_context_; + fml::RefPtr native_window_; - std::unique_ptr external_view_embedder_; - std::shared_ptr android_context_; std::unique_ptr onscreen_surface_; std::unique_ptr offscreen_surface_; diff --git a/shell/platform/android/android_surface_software.cc b/shell/platform/android/android_surface_software.cc index fedf4b7812418..07362e74a59e2 100644 --- a/shell/platform/android/android_surface_software.cc +++ b/shell/platform/android/android_surface_software.cc @@ -39,11 +39,13 @@ bool GetSkColorType(int32_t buffer_format, AndroidSurfaceSoftware::AndroidSurfaceSoftware( std::shared_ptr android_context, std::shared_ptr jni_facade, - AndroidSurface::Factory surface_factory) { + AndroidSurface::Factory surface_factory) + : external_view_embedder_(std::make_unique( + std::move(android_context), + std::move(jni_facade), + surface_factory)) { GetSkColorType(WINDOW_FORMAT_RGBA_8888, &target_color_type_, &target_alpha_type_); - external_view_embedder_ = std::make_unique( - std::move(android_context), std::move(jni_facade), surface_factory); } AndroidSurfaceSoftware::~AndroidSurfaceSoftware() = default; diff --git a/shell/platform/android/android_surface_software.h b/shell/platform/android/android_surface_software.h index 97de7f3556801..1f5fb885dbf88 100644 --- a/shell/platform/android/android_surface_software.h +++ b/shell/platform/android/android_surface_software.h @@ -55,11 +55,12 @@ class AndroidSurfaceSoftware final : public AndroidSurface, ExternalViewEmbedder* GetExternalViewEmbedder() override; private: + const std::unique_ptr external_view_embedder_; + sk_sp sk_surface_; fml::RefPtr native_window_; SkColorType target_color_type_; SkAlphaType target_alpha_type_; - std::unique_ptr external_view_embedder_; FML_DISALLOW_COPY_AND_ASSIGN(AndroidSurfaceSoftware); }; diff --git a/shell/platform/android/android_surface_vulkan.cc b/shell/platform/android/android_surface_vulkan.cc index affd4c2b8d574..b672070771de8 100644 --- a/shell/platform/android/android_surface_vulkan.cc +++ b/shell/platform/android/android_surface_vulkan.cc @@ -16,10 +16,11 @@ AndroidSurfaceVulkan::AndroidSurfaceVulkan( std::shared_ptr android_context, std::shared_ptr jni_facade, AndroidSurface::Factory surface_factory) - : proc_table_(fml::MakeRefCounted()) { - external_view_embedder_ = std::make_unique( - std::move(android_context), std::move(jni_facade), surface_factory); -} + : proc_table_(fml::MakeRefCounted()), + external_view_embedder_(std::make_unique( + std::move(android_context), + std::move(jni_facade), + surface_factory)) {} AndroidSurfaceVulkan::~AndroidSurfaceVulkan() = default; diff --git a/shell/platform/android/android_surface_vulkan.h b/shell/platform/android/android_surface_vulkan.h index 64bc5e2a63b4f..3491d55acbcb8 100644 --- a/shell/platform/android/android_surface_vulkan.h +++ b/shell/platform/android/android_surface_vulkan.h @@ -53,9 +53,10 @@ class AndroidSurfaceVulkan : public AndroidSurface, fml::RefPtr vk() override; private: + const std::unique_ptr external_view_embedder_; + fml::RefPtr proc_table_; fml::RefPtr native_window_; - std::unique_ptr external_view_embedder_; FML_DISALLOW_COPY_AND_ASSIGN(AndroidSurfaceVulkan); }; From d5bb5ff83f5f943624687fca3a5fc35e7d537fbb Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 15 Jun 2020 19:15:18 -0700 Subject: [PATCH 11/28] Add unit tests --- BUILD.gn | 1 + ci/licenses_golden/licenses_flutter | 6 + .../android/external_view_embedder/BUILD.gn | 8 +- .../external_view_embedder.cc | 5 +- .../external_view_embedder.h | 8 +- .../external_view_embedder_unittests.cc | 37 +++++- .../surface_pool_unittests.cc | 108 ++++++++++++++++++ shell/platform/android/jni/BUILD.gn | 35 ++++++ shell/platform/android/jni/mock_jni.cc | 63 ++++++++++ shell/platform/android/jni/mock_jni.h | 81 +++++++++++++ .../platform/android/jni/mock_jni_unittest.cc | 28 +++++ .../android/jni/platform_view_android_jni.h | 6 +- shell/platform/android/surface/BUILD.gn | 15 +++ .../android/surface/android_surface.h | 5 +- .../android/surface/mock_android_surface.cc | 70 ++++++++++++ .../android/surface/mock_android_surface.h | 69 +++++++++++ testing/run_tests.py | 2 + 17 files changed, 530 insertions(+), 17 deletions(-) create mode 100644 shell/platform/android/external_view_embedder/surface_pool_unittests.cc create mode 100644 shell/platform/android/jni/mock_jni.cc create mode 100644 shell/platform/android/jni/mock_jni.h create mode 100644 shell/platform/android/jni/mock_jni_unittest.cc create mode 100644 shell/platform/android/surface/mock_android_surface.cc create mode 100644 shell/platform/android/surface/mock_android_surface.h diff --git a/BUILD.gn b/BUILD.gn index 19dce340cd183..d6d634a9a69d0 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -75,6 +75,7 @@ group("flutter") { "//flutter/runtime:runtime_unittests", "//flutter/shell/common:shell_unittests", "//flutter/shell/platform/android/external_view_embedder:android_external_view_embedder_unittests", + "//flutter/shell/platform/android/jni:jni_unittests", "//flutter/shell/platform/embedder:embedder_unittests", "//flutter/testing:testing_unittests", "//flutter/third_party/txt:txt_unittests", diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 237c00d0803b6..2b16d70eed33f 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -669,6 +669,7 @@ FILE: ../../../flutter/shell/platform/android/external_view_embedder/external_vi FILE: ../../../flutter/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc FILE: ../../../flutter/shell/platform/android/external_view_embedder/surface_pool.cc FILE: ../../../flutter/shell/platform/android/external_view_embedder/surface_pool.h +FILE: ../../../flutter/shell/platform/android/external_view_embedder/surface_pool_unittests.cc FILE: ../../../flutter/shell/platform/android/flutter_main.cc FILE: ../../../flutter/shell/platform/android/flutter_main.h FILE: ../../../flutter/shell/platform/android/io/flutter/Log.java @@ -781,6 +782,9 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterRunArgument FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterView.java FILE: ../../../flutter/shell/platform/android/io/flutter/view/TextureRegistry.java FILE: ../../../flutter/shell/platform/android/io/flutter/view/VsyncWaiter.java +FILE: ../../../flutter/shell/platform/android/jni/mock_jni.cc +FILE: ../../../flutter/shell/platform/android/jni/mock_jni.h +FILE: ../../../flutter/shell/platform/android/jni/mock_jni_unittest.cc FILE: ../../../flutter/shell/platform/android/jni/platform_view_android_jni.cc FILE: ../../../flutter/shell/platform/android/jni/platform_view_android_jni.h FILE: ../../../flutter/shell/platform/android/library_loader.cc @@ -795,6 +799,8 @@ FILE: ../../../flutter/shell/platform/android/surface/android_native_window.cc FILE: ../../../flutter/shell/platform/android/surface/android_native_window.h FILE: ../../../flutter/shell/platform/android/surface/android_surface.cc FILE: ../../../flutter/shell/platform/android/surface/android_surface.h +FILE: ../../../flutter/shell/platform/android/surface/mock_android_surface.cc +FILE: ../../../flutter/shell/platform/android/surface/mock_android_surface.h FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.cc FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/basic_message_channel_unittests.cc diff --git a/shell/platform/android/external_view_embedder/BUILD.gn b/shell/platform/android/external_view_embedder/BUILD.gn index cbd8ceb90ddbe..e6c6eb866e1f0 100644 --- a/shell/platform/android/external_view_embedder/BUILD.gn +++ b/shell/platform/android/external_view_embedder/BUILD.gn @@ -2,12 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -import("//build/config/android/config.gni") import("//flutter/common/config.gni") -import("//flutter/shell/config.gni") -import("//flutter/shell/gpu/gpu.gni") -import("//flutter/shell/version/version.gni") - import("//flutter/testing/testing.gni") source_set("external_view_embedder") { @@ -40,11 +35,14 @@ executable("android_external_view_embedder_unittests") { sources = [ "external_view_embedder_unittests.cc", + "surface_pool_unittests.cc", ] deps = [ ":external_view_embedder", ":external_view_embedder_fixtures", + "//flutter/shell/platform/android/jni:mock_jni", + "//flutter/shell/platform/android/surface:mock_surface", "//flutter/testing", "//flutter/testing:dart", "//flutter/testing:skia", 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 b5f9f80e8e061..858d9492579ad 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -42,7 +42,10 @@ void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView( // |ExternalViewEmbedder| SkCanvas* AndroidExternalViewEmbedder::CompositeEmbeddedView(int view_id) { - return picture_recorders_[view_id]->getRecordingCanvas(); + if (picture_recorders_.count(view_id) == 1) { + return picture_recorders_[view_id]->getRecordingCanvas(); + } + return nullptr; } // |ExternalViewEmbedder| 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 7f6037109028e..53eae0e925eb8 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -55,6 +55,10 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { void EndFrame( fml::RefPtr raster_thread_merger) override; + // Gets the rect based on the device pixel ratio of a platform view displayed + // on the screen. + SkRect GetViewRect(int view_id); + private: static const int kMaxLayerAllocations = 2; @@ -113,10 +117,6 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { int64_t view_id, sk_sp picture, SkRect& rect); - - // Gets the rect based on the device pixel ratio of a platform view displayed - // on the screen. - SkRect GetViewRect(int view_id); }; } // namespace flutter 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 d7671a89077ff..4bcfb9ade5dae 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 @@ -29,13 +29,15 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) { auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + ASSERT_EQ(nullptr, embedder->CompositeEmbeddedView(0)); embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); - ASSERT_TRUE(embedder->CompositeEmbeddedView(0) != nullptr); + ASSERT_NE(nullptr, embedder->CompositeEmbeddedView(0)); + ASSERT_EQ(nullptr, embedder->CompositeEmbeddedView(1)); embedder->PrerollCompositeEmbeddedView( 1, std::make_unique()); - ASSERT_TRUE(embedder->CompositeEmbeddedView(1) != nullptr); + ASSERT_NE(nullptr, embedder->CompositeEmbeddedView(1)); } TEST(AndroidExternalViewEmbedder, CancelFrame) { @@ -100,5 +102,36 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { ASSERT_FALSE(raster_thread_merger->IsMerged()); } +TEST(AndroidExternalViewEmbedder, PlatformViewRect) { + auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5); + + auto view_params = std::make_unique(); + view_params->offsetPixels = SkPoint::Make(10, 20); + view_params->sizePoints = SkSize::Make(30, 40); + + auto view_id = 0; + embedder->PrerollCompositeEmbeddedView(view_id, std::move(view_params)); + ASSERT_EQ(SkRect::MakeXYWH(10, 20, 45, 60), embedder->GetViewRect(view_id)); +} + +TEST(AndroidExternalViewEmbedder, PlatformViewRect__ChangedParams) { + auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5); + + 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); + 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); + embedder->PrerollCompositeEmbeddedView(view_id, std::move(view_params_2)); + + ASSERT_EQ(SkRect::MakeXYWH(50, 60, 105, 120), embedder->GetViewRect(view_id)); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc new file mode 100644 index 0000000000000..f0c09529e678f --- /dev/null +++ b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc @@ -0,0 +1,108 @@ +// 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/shell/platform/android/external_view_embedder/surface_pool.h" +#include "flutter/shell/platform/android/jni/mock_jni.h" +#include "flutter/shell/platform/android/surface/mock_android_surface.h" +#include "gtest/gtest.h" +#include "third_party/skia/include/gpu/GrContext.h" + +namespace flutter { +namespace testing { + +TEST(SurfacePool, GetLayer__AllocateOneLayer) { + auto pool = new SurfacePool(); + + auto gr_context = GrContext::MakeMock(nullptr); + auto android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); + auto mock_jni = std::make_shared(); + auto surface_factory = + [](std::shared_ptr android_context, + std::shared_ptr jni_facade) { + return std::make_unique(/*id=*/123); + }; + auto layer = pool->GetLayer(gr_context.get(), android_context, mock_jni, + surface_factory); + + ASSERT_NE(nullptr, layer); + ASSERT_EQ(gr_context.get(), layer->gr_context); + ASSERT_NE(nullptr, layer->surface); + ASSERT_EQ(gr_context.get(), layer->surface->GetContext()); + ASSERT_EQ( + 123, + static_cast(layer->android_surface.get())->GetId()); +} + +TEST(SurfacePool, GetUnusedLayers) { + auto pool = new SurfacePool(); + + auto gr_context = GrContext::MakeMock(nullptr); + auto android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); + auto mock_jni = std::make_shared(); + auto surface_factory = + [](std::shared_ptr android_context, + std::shared_ptr jni_facade) { + return std::make_unique(/*id=*/123); + }; + auto layer = pool->GetLayer(gr_context.get(), android_context, mock_jni, + surface_factory); + ASSERT_EQ(0UL, pool->GetUnusedLayers().size()); + + pool->RecycleLayers(); + ASSERT_EQ(1UL, pool->GetUnusedLayers().size()); + ASSERT_EQ(layer, pool->GetUnusedLayers()[0]); +} + +TEST(SurfacePool, GetLayer__Recycle) { + auto pool = new SurfacePool(); + + auto gr_context_1 = GrContext::MakeMock(nullptr); + auto android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); + auto mock_jni = std::make_shared(); + auto surface_factory = + [](std::shared_ptr android_context, + std::shared_ptr jni_facade) { + return std::make_unique(/*id=*/123); + }; + auto layer_1 = pool->GetLayer(gr_context_1.get(), android_context, mock_jni, + surface_factory); + + pool->RecycleLayers(); + + auto gr_context_2 = GrContext::MakeMock(nullptr); + auto layer_2 = pool->GetLayer(gr_context_2.get(), android_context, mock_jni, + surface_factory); + ASSERT_NE(nullptr, layer_1); + ASSERT_EQ(layer_1, layer_2); + ASSERT_EQ(gr_context_2.get(), layer_1->gr_context); + ASSERT_EQ(gr_context_2.get(), layer_2->gr_context); +} + +TEST(SurfacePool, GetLayer__AllocateTwoLayers) { + auto pool = new SurfacePool(); + + auto gr_context_1 = GrContext::MakeMock(nullptr); + auto android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); + auto mock_jni = std::make_shared(); + auto surface_factory = + [](std::shared_ptr android_context, + std::shared_ptr jni_facade) { + return std::make_unique(/*id=*/123); + }; + auto layer_1 = pool->GetLayer(gr_context_1.get(), android_context, mock_jni, + surface_factory); + auto layer_2 = pool->GetLayer(gr_context_1.get(), android_context, mock_jni, + surface_factory); + + ASSERT_NE(nullptr, layer_1); + ASSERT_NE(nullptr, layer_2); + ASSERT_NE(layer_1, layer_2); +} + +} // namespace testing +} // namespace flutter diff --git a/shell/platform/android/jni/BUILD.gn b/shell/platform/android/jni/BUILD.gn index 700dfb5c6676e..5db8dee0e5fa0 100644 --- a/shell/platform/android/jni/BUILD.gn +++ b/shell/platform/android/jni/BUILD.gn @@ -3,6 +3,7 @@ # found in the LICENSE file. import("//flutter/common/config.gni") +import("//flutter/testing/testing.gni") source_set("jni") { sources = [ @@ -18,3 +19,37 @@ source_set("jni") { "//third_party/skia", ] } + +source_set("mock_jni") { + sources = [ + "mock_jni.cc", + "mock_jni.h", + ] + + public_configs = [ "//flutter:config" ] + + deps = [ + ":jni", + "//third_party/skia", + ] +} + +test_fixtures("jni_fixtures") { + fixtures = [] +} + +executable("jni_unittests") { + testonly = true + + sources = [ + "mock_jni_unittest.cc", + ] + + deps = [ + ":jni", + ":jni_fixtures", + ":mock_jni", + "//flutter/testing", + "//flutter/testing:dart", + ] +} diff --git a/shell/platform/android/jni/mock_jni.cc b/shell/platform/android/jni/mock_jni.cc new file mode 100644 index 0000000000000..334a0ba08b742 --- /dev/null +++ b/shell/platform/android/jni/mock_jni.cc @@ -0,0 +1,63 @@ +// 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/shell/platform/android/jni/mock_jni.h" + +namespace flutter { + +MockJNI::MockJNI() = default; + +MockJNI::~MockJNI() = default; + +void MockJNI::FlutterViewHandlePlatformMessage( + fml::RefPtr message, + int responseId) { + jni_calls_.push_back( + FlutterViewHandlePlatformMessageCall{message, responseId}); +} + +void MockJNI::FlutterViewHandlePlatformMessageResponse( + int responseId, + std::unique_ptr data) {} + +void MockJNI::FlutterViewUpdateSemantics(std::vector buffer, + std::vector strings) {} + +void MockJNI::FlutterViewUpdateCustomAccessibilityActions( + std::vector actions_buffer, + std::vector strings) {} + +void MockJNI::FlutterViewOnFirstFrame() {} + +void MockJNI::FlutterViewOnPreEngineRestart() {} + +void MockJNI::SurfaceTextureAttachToGLContext(JavaWeakGlobalRef surface_texture, + int textureId) {} + +void MockJNI::SurfaceTextureUpdateTexImage(JavaWeakGlobalRef surface_texture) {} + +void MockJNI::SurfaceTextureGetTransformMatrix( + JavaWeakGlobalRef surface_texture, + SkMatrix& transform) {} + +void MockJNI::SurfaceTextureDetachFromGLContext( + JavaWeakGlobalRef surface_texture) {} + +void MockJNI::FlutterViewOnDisplayPlatformView(int view_id, + int x, + int y, + int width, + int height) {} + +void MockJNI::FlutterViewDisplayOverlaySurface(int surface_id, + int x, + int y, + int width, + int height) {} + +std::vector& MockJNI::GetCalls() { + return jni_calls_; +} + +} // namespace flutter diff --git a/shell/platform/android/jni/mock_jni.h b/shell/platform/android/jni/mock_jni.h new file mode 100644 index 0000000000000..d868f37e50e94 --- /dev/null +++ b/shell/platform/android/jni/mock_jni.h @@ -0,0 +1,81 @@ +// 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. + +#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_JNI_H_ +#define FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_JNI_H_ + +#include + +#include "flutter/shell/platform/android/jni/platform_view_android_jni.h" + +namespace flutter { + +//------------------------------------------------------------------------------ +/// Mock for |PlatformViewAndroidJNI|. This implementation can be used in unit +/// tests without requiring the Android toolchain. +/// +class MockJNI final : public PlatformViewAndroidJNI { + public: + struct FlutterViewHandlePlatformMessageCall { + fml::RefPtr message; + int responseId; + }; + + using JNICall = std::variant; + + MockJNI(); + + ~MockJNI() override; + + void FlutterViewHandlePlatformMessage( + fml::RefPtr message, + int responseId) override; + + void FlutterViewHandlePlatformMessageResponse( + int responseId, + std::unique_ptr data) override; + + void FlutterViewUpdateSemantics(std::vector buffer, + std::vector strings) override; + + void FlutterViewUpdateCustomAccessibilityActions( + std::vector actions_buffer, + std::vector strings) override; + + void FlutterViewOnFirstFrame() override; + + void FlutterViewOnPreEngineRestart() override; + + void SurfaceTextureAttachToGLContext(JavaWeakGlobalRef surface_texture, + int textureId) override; + + void SurfaceTextureUpdateTexImage(JavaWeakGlobalRef surface_texture) override; + + void SurfaceTextureGetTransformMatrix(JavaWeakGlobalRef surface_texture, + SkMatrix& transform) override; + + void SurfaceTextureDetachFromGLContext( + JavaWeakGlobalRef surface_texture) override; + + void FlutterViewOnDisplayPlatformView(int view_id, + int x, + int y, + int width, + int height) override; + + void FlutterViewDisplayOverlaySurface(int surface_id, + int x, + int y, + int width, + int height) override; + // Gets the JNI calls. + std::vector& GetCalls(); + + private: + std::vector jni_calls_; +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_JNI_H_ diff --git a/shell/platform/android/jni/mock_jni_unittest.cc b/shell/platform/android/jni/mock_jni_unittest.cc new file mode 100644 index 0000000000000..d227516594cd2 --- /dev/null +++ b/shell/platform/android/jni/mock_jni_unittest.cc @@ -0,0 +1,28 @@ +// 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/shell/platform/android/jni/mock_jni.h" +#include "gtest/gtest.h" + +namespace flutter { +namespace testing { + +TEST(MockJNI, FlutterViewHandlePlatformMessage) { + auto mock_jni = std::make_unique(); + + auto message = + fml::MakeRefCounted("", nullptr); + auto response_id = 1; + mock_jni->FlutterViewHandlePlatformMessage(message, response_id); + + auto calls = mock_jni->GetCalls(); + ASSERT_EQ(1UL, calls.size()); + + auto platform_message_call = + std::get(calls[0]); + ASSERT_EQ("", platform_message_call.message->channel()); +} + +} // namespace testing +} // namespace flutter diff --git a/shell/platform/android/jni/platform_view_android_jni.h b/shell/platform/android/jni/platform_view_android_jni.h index eed89a1347845..80269c08598ce 100644 --- a/shell/platform/android/jni/platform_view_android_jni.h +++ b/shell/platform/android/jni/platform_view_android_jni.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_PLATFORM_VIEW_ANDROID_JNI_H_ -#define FLUTTER_SHELL_PLATFORM_ANDROID_PLATFORM_VIEW_ANDROID_JNI_H_ +#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_JNI_PLATFORM_VIEW_ANDROID_JNI_H_ +#define FLUTTER_SHELL_PLATFORM_ANDROID_JNI_PLATFORM_VIEW_ANDROID_JNI_H_ #include "flutter/fml/macros.h" #include "flutter/fml/mapping.h" @@ -134,4 +134,4 @@ class PlatformViewAndroidJNI { } // namespace flutter -#endif // FLUTTER_SHELL_PLATFORM_ANDROID_PLATFORM_VIEW_ANDROID_JNI_H_ +#endif // FLUTTER_SHELL_PLATFORM_ANDROID_JNI_PLATFORM_VIEW_ANDROID_JNI_H_ diff --git a/shell/platform/android/surface/BUILD.gn b/shell/platform/android/surface/BUILD.gn index bc4431dabdb54..95c9f210d1a97 100644 --- a/shell/platform/android/surface/BUILD.gn +++ b/shell/platform/android/surface/BUILD.gn @@ -20,3 +20,18 @@ source_set("surface") { "//third_party/skia", ] } + +source_set("mock_surface") { + sources = [ + "mock_android_surface.cc", + "mock_android_surface.h", + ] + + public_configs = [ "//flutter:config" ] + + deps = [ + ":surface", + "//flutter/shell/gpu:gpu_surface_gl", + "//third_party/skia", + ] +} diff --git a/shell/platform/android/surface/android_surface.h b/shell/platform/android/surface/android_surface.h index 2daae44da6850..053eb94e999ce 100644 --- a/shell/platform/android/surface/android_surface.h +++ b/shell/platform/android/surface/android_surface.h @@ -16,9 +16,10 @@ namespace flutter { class AndroidSurface { public: - typedef std::unique_ptr (*Factory)( + typedef std::function( std::shared_ptr android_context, - std::shared_ptr jni_facade); + std::shared_ptr jni_facade)> + Factory; virtual ~AndroidSurface(); diff --git a/shell/platform/android/surface/mock_android_surface.cc b/shell/platform/android/surface/mock_android_surface.cc new file mode 100644 index 0000000000000..e8c68125e7af1 --- /dev/null +++ b/shell/platform/android/surface/mock_android_surface.cc @@ -0,0 +1,70 @@ +// 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/shell/platform/android/surface/mock_android_surface.h" + +#include "flutter/shell/gpu/gpu_surface_gl.h" + +namespace flutter { + +MockAndroidSurface::MockAndroidSurface(int id) : id_(id){}; + +MockAndroidSurface::~MockAndroidSurface() = default; + +bool MockAndroidSurface::IsValid() const { + return true; +} + +void MockAndroidSurface::TeardownOnScreenContext() {} + +std::unique_ptr MockAndroidSurface::CreateGPUSurface( + GrContext* gr_context) { + if (gr_context) { + return std::make_unique(sk_ref_sp(gr_context), this, true); + } + return std::make_unique(this, true); +} + +bool MockAndroidSurface::OnScreenSurfaceResize(const SkISize& size) { + return true; +} + +bool MockAndroidSurface::ResourceContextMakeCurrent() { + return true; +} + +bool MockAndroidSurface::ResourceContextClearCurrent() { + return true; +} + +bool MockAndroidSurface::SetNativeWindow( + fml::RefPtr window) { + return true; +} + +std::unique_ptr MockAndroidSurface::GLContextMakeCurrent() { + return std::make_unique(/*static_result=*/true); +} + +bool MockAndroidSurface::GLContextClearCurrent() { + return true; +} + +bool MockAndroidSurface::GLContextPresent() { + return true; +} + +intptr_t MockAndroidSurface::GLContextFBO() const { + return 0; +} + +ExternalViewEmbedder* MockAndroidSurface::GetExternalViewEmbedder() { + return nullptr; +} + +int MockAndroidSurface::GetId() const { + return id_; +} + +} // namespace flutter diff --git a/shell/platform/android/surface/mock_android_surface.h b/shell/platform/android/surface/mock_android_surface.h new file mode 100644 index 0000000000000..cdc5b4e1e9b1b --- /dev/null +++ b/shell/platform/android/surface/mock_android_surface.h @@ -0,0 +1,69 @@ +// 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. + +#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_MOCK_ANDROID_SURFACE_H_ +#define FLUTTER_SHELL_PLATFORM_ANDROID_MOCK_ANDROID_SURFACE_H_ + +#include "flutter/shell/gpu/gpu_surface_gl.h" +#include "flutter/shell/platform/android/surface/android_surface.h" + +namespace flutter { + +//------------------------------------------------------------------------------ +/// Mock for |AndroidSurface|. This implementation can be used in unit +/// tests without requiring the Android toolchain. +/// +class MockAndroidSurface final : public GPUSurfaceGLDelegate, + public AndroidSurface { + public: + MockAndroidSurface(int id); + + ~MockAndroidSurface() override; + + // |AndroidSurface| + bool IsValid() const override; + + // |AndroidSurface| + void TeardownOnScreenContext() override; + + // |AndroidSurface| + std::unique_ptr CreateGPUSurface(GrContext* gr_context) override; + + // |AndroidSurface| + bool OnScreenSurfaceResize(const SkISize& size) override; + + // |AndroidSurface| + bool ResourceContextMakeCurrent() override; + + // |AndroidSurface| + bool ResourceContextClearCurrent() override; + + // |AndroidSurface| + bool SetNativeWindow(fml::RefPtr window) override; + + // |GPUSurfaceGLDelegate| + std::unique_ptr GLContextMakeCurrent() override; + + // |GPUSurfaceGLDelegate| + bool GLContextClearCurrent() override; + + // |GPUSurfaceGLDelegate| + bool GLContextPresent() override; + + // |GPUSurfaceGLDelegate| + intptr_t GLContextFBO() const override; + + // |GPUSurfaceGLDelegate| + ExternalViewEmbedder* GetExternalViewEmbedder() override; + + // Used in unit tests. + int GetId() const; + + private: + int id_; +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_ANDROID_MOCK_ANDROID_SURFACE_H_ diff --git a/testing/run_tests.py b/testing/run_tests.py index c0f6e9938b64f..ac4c48225b686 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -139,6 +139,8 @@ def RunCCTests(build_dir, filter): RunEngineExecutable(build_dir, 'testing_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'jni_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'android_external_view_embedder_unittests', filter, shuffle_flags) # These unit-tests are Objective-C and can only run on Darwin. From 7be0bd22fdd424b8a045a76927057dadeb52b34f Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Mon, 15 Jun 2020 20:50:11 -0700 Subject: [PATCH 12/28] Add OverlayMetadata --- .../external_view_embedder/surface_pool.cc | 6 ++++ .../external_view_embedder/surface_pool.h | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index 7583fd5e2e206..2ec541706839e 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -6,6 +6,12 @@ namespace flutter { +OverlayMetadata::OverlayMetadata(int id, + fml::RefPtr window) + : id(id), window(std::move(window)) {} + +OverlayMetadata::~OverlayMetadata() = default; + OverlayLayer::OverlayLayer(int id, std::unique_ptr android_surface, std::unique_ptr surface) diff --git a/shell/platform/android/external_view_embedder/surface_pool.h b/shell/platform/android/external_view_embedder/surface_pool.h index e6c541fd46db8..e64afbfa510b9 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.h +++ b/shell/platform/android/external_view_embedder/surface_pool.h @@ -11,6 +11,29 @@ namespace flutter { +//------------------------------------------------------------------------------ +/// The metadata returned from Java which is converted into an |OverlayLayer| +/// by |SurfacePool|. +/// +struct OverlayMetadata { + OverlayMetadata(int id, fml::RefPtr window); + + ~OverlayMetadata(); + + // A unique id to identify the overlay when it gets recycled. + const int id; + + // Holds a reference to the native window. That is, an `ANativeWindow`, + // which is the C counterpart of the `android.view.Surface` object in Java. + const fml::RefPtr window; +}; + +//------------------------------------------------------------------------------ +/// An Overlay layer represents an `android.view.View` in the C side. +/// +/// The `id` is used to uniquely identify the layer and recycle it between +/// frames. +/// struct OverlayLayer { OverlayLayer(int id, std::unique_ptr android_surface, @@ -18,15 +41,20 @@ struct OverlayLayer { ~OverlayLayer(); + // A unique id to identify the overlay when it gets recycled. const int id; + // A GPU surface. const std::unique_ptr android_surface; + // A GPU surface. This may change when the overlay is recycled. std::unique_ptr surface; // The `GrContext` that is currently used by the overlay surfaces. // We track this to know when the GrContext for the Flutter app has changed // so we can update the overlay with the new context. + // + // This may change when the overlay is recycled. GrContext* gr_context; }; From a6adf291fbd3c650084c87338722eee7100f7e49 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Tue, 16 Jun 2020 23:59:35 -0700 Subject: [PATCH 13/28] Use gmock --- shell/gpu/gpu_surface_gl_delegate.cc | 94 ++++++------ .../android/external_view_embedder/BUILD.gn | 2 + .../external_view_embedder.cc | 2 + .../external_view_embedder_unittests.cc | 60 +++++++- .../surface_pool_unittests.cc | 105 +++++++++---- shell/platform/android/jni/BUILD.gn | 6 +- shell/platform/android/jni/mock_jni.cc | 79 ---------- shell/platform/android/jni/mock_jni.h | 139 ++++++++---------- .../platform/android/jni/mock_jni_unittest.cc | 11 +- shell/platform/android/surface/BUILD.gn | 9 +- .../android/surface/mock_android_surface.cc | 41 ------ .../android/surface/mock_android_surface.h | 38 ++--- testing/BUILD.gn | 1 + 13 files changed, 277 insertions(+), 310 deletions(-) delete mode 100644 shell/platform/android/jni/mock_jni.cc diff --git a/shell/gpu/gpu_surface_gl_delegate.cc b/shell/gpu/gpu_surface_gl_delegate.cc index 91efd5130ddcb..024201e710b4f 100644 --- a/shell/gpu/gpu_surface_gl_delegate.cc +++ b/shell/gpu/gpu_surface_gl_delegate.cc @@ -29,62 +29,62 @@ GPUSurfaceGLDelegate::GLProcResolver GPUSurfaceGLDelegate::GetGLProcResolver() return nullptr; } -static bool IsProcResolverOpenGLES( - GPUSurfaceGLDelegate::GLProcResolver proc_resolver) { - // Version string prefix that identifies an OpenGL ES implementation. -#define GPU_GL_VERSION 0x1F02 - constexpr char kGLESVersionPrefix[] = "OpenGL ES"; +// static bool IsProcResolverOpenGLES( +// GPUSurfaceGLDelegate::GLProcResolver proc_resolver) { +// // Version string prefix that identifies an OpenGL ES implementation. +// #define GPU_GL_VERSION 0x1F02 +// constexpr char kGLESVersionPrefix[] = "OpenGL ES"; - using GLGetStringProc = const char* (*)(uint32_t); +// using GLGetStringProc = const char* (*)(uint32_t); - GLGetStringProc gl_get_string = - reinterpret_cast(proc_resolver("glGetString")); +// GLGetStringProc gl_get_string = +// reinterpret_cast(proc_resolver("glGetString")); - FML_CHECK(gl_get_string) - << "The GL proc resolver could not resolve glGetString"; +// FML_CHECK(gl_get_string) +// << "The GL proc resolver could not resolve glGetString"; - const char* gl_version_string = gl_get_string(GPU_GL_VERSION); +// const char* gl_version_string = gl_get_string(GPU_GL_VERSION); - FML_CHECK(gl_version_string) - << "The GL proc resolver's glGetString(GL_VERSION) failed"; +// FML_CHECK(gl_version_string) +// << "The GL proc resolver's glGetString(GL_VERSION) failed"; - return strncmp(gl_version_string, kGLESVersionPrefix, - strlen(kGLESVersionPrefix)) == 0; -} +// return strncmp(gl_version_string, kGLESVersionPrefix, +// strlen(kGLESVersionPrefix)) == 0; +// } static sk_sp CreateGLInterface( GPUSurfaceGLDelegate::GLProcResolver proc_resolver) { - if (proc_resolver == nullptr) { - // If there is no custom proc resolver, ask Skia to guess the native - // interface. This often leads to interesting results on most platforms. - return GrGLMakeNativeInterface(); - } - - struct ProcResolverContext { - GPUSurfaceGLDelegate::GLProcResolver resolver; - }; - - ProcResolverContext context = {proc_resolver}; - - GrGLGetProc gl_get_proc = [](void* context, - const char gl_proc_name[]) -> GrGLFuncPtr { - auto proc_resolver_context = - reinterpret_cast(context); - return reinterpret_cast( - proc_resolver_context->resolver(gl_proc_name)); - }; - - // glGetString indicates an OpenGL ES interface. - if (IsProcResolverOpenGLES(proc_resolver)) { - return GrGLMakeAssembledGLESInterface(&context, gl_get_proc); - } - - // Fallback to OpenGL. - if (auto interface = GrGLMakeAssembledGLInterface(&context, gl_get_proc)) { - return interface; - } - - FML_LOG(ERROR) << "Could not create a valid GL interface."; + // if (proc_resolver == nullptr) { + // // If there is no custom proc resolver, ask Skia to guess the native + // // interface. This often leads to interesting results on most platforms. + // return GrGLMakeNativeInterface(); + // } + + // struct ProcResolverContext { + // GPUSurfaceGLDelegate::GLProcResolver resolver; + // }; + + // ProcResolverContext context = {proc_resolver}; + + // GrGLGetProc gl_get_proc = [](void* context, + // const char gl_proc_name[]) -> GrGLFuncPtr { + // auto proc_resolver_context = + // reinterpret_cast(context); + // return reinterpret_cast( + // proc_resolver_context->resolver(gl_proc_name)); + // }; + + // // glGetString indicates an OpenGL ES interface. + // if (IsProcResolverOpenGLES(proc_resolver)) { + // return GrGLMakeAssembledGLESInterface(&context, gl_get_proc); + // } + + // // Fallback to OpenGL. + // if (auto interface = GrGLMakeAssembledGLInterface(&context, gl_get_proc)) { + // return interface; + // } + + // FML_LOG(ERROR) << "Could not create a valid GL interface."; return nullptr; } diff --git a/shell/platform/android/external_view_embedder/BUILD.gn b/shell/platform/android/external_view_embedder/BUILD.gn index e6c6eb866e1f0..f37a6cd83eaf8 100644 --- a/shell/platform/android/external_view_embedder/BUILD.gn +++ b/shell/platform/android/external_view_embedder/BUILD.gn @@ -41,10 +41,12 @@ executable("android_external_view_embedder_unittests") { deps = [ ":external_view_embedder", ":external_view_embedder_fixtures", + "//flutter/shell/gpu:gpu_surface_gl", "//flutter/shell/platform/android/jni:mock_jni", "//flutter/shell/platform/android/surface:mock_surface", "//flutter/testing", "//flutter/testing:dart", "//flutter/testing:skia", + "//third_party/googletest:gmock", ] } 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 04262487a01a7..23b92419f804e 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -240,6 +240,7 @@ void AndroidExternalViewEmbedder::BeginFrame(SkISize frame_size, Reset(); frame_size_ = frame_size; device_pixel_ratio_ = device_pixel_ratio; + jni_facade_->FlutterViewBeginFrame(); } // |ExternalViewEmbedder| @@ -254,6 +255,7 @@ void AndroidExternalViewEmbedder::EndFrame( raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); should_run_rasterizer_on_platform_thread_ = false; } + jni_facade_->FlutterViewEndFrame(); } } // namespace flutter 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 4bcfb9ade5dae..6b9336080d4d5 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 @@ -5,14 +5,22 @@ #include "flutter/fml/raster_thread_merger.h" #include "flutter/fml/thread.h" #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" +#include "flutter/shell/platform/android/jni/mock_jni.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" namespace flutter { namespace testing { +using ::testing::Mock; + TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { - auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + auto mock_jni = std::make_shared(); + + EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + auto embedder = + std::make_unique(nullptr, mock_jni, nullptr); embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); embedder->PrerollCompositeEmbeddedView( @@ -26,8 +34,28 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { ASSERT_EQ(SkISize::Make(10, 20), canvases[1]->getBaseLayerSize()); } +TEST(AndroidExternalViewEmbedder, GetCurrentCanvases__CompositeOrder) { + auto mock_jni = std::make_shared(); + EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + + auto embedder = + std::make_unique(nullptr, mock_jni, nullptr); + embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); + + embedder->PrerollCompositeEmbeddedView( + 0, std::make_unique()); + embedder->PrerollCompositeEmbeddedView( + 1, std::make_unique()); + + auto canvases = embedder->GetCurrentCanvases(); + ASSERT_EQ(2UL, canvases.size()); + ASSERT_EQ(embedder->CompositeEmbeddedView(0), canvases[0]); + ASSERT_EQ(embedder->CompositeEmbeddedView(1), canvases[1]); +} + TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) { - auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + auto embedder = + std::make_unique(nullptr, nullptr, nullptr); ASSERT_EQ(nullptr, embedder->CompositeEmbeddedView(0)); embedder->PrerollCompositeEmbeddedView( @@ -41,7 +69,8 @@ TEST(AndroidExternalViewEmbedder, CompositeEmbeddedView) { } TEST(AndroidExternalViewEmbedder, CancelFrame) { - auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + auto embedder = + std::make_unique(nullptr, nullptr, nullptr); embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); @@ -52,7 +81,12 @@ TEST(AndroidExternalViewEmbedder, CancelFrame) { } TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { - auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + auto mock_jni = std::make_shared(); + EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + EXPECT_CALL(*mock_jni, FlutterViewEndFrame()); + + auto embedder = + std::make_unique(nullptr, mock_jni, nullptr); auto platform_thread = new fml::Thread("platform"); auto rasterizer_thread = new fml::Thread("rasterizer"); auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); @@ -84,7 +118,11 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { } TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { - auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + auto mock_jni = std::make_shared(); + EXPECT_CALL(*mock_jni, FlutterViewEndFrame()); + + auto embedder = + std::make_unique(nullptr, mock_jni, nullptr); auto platform_thread = new fml::Thread("platform"); auto rasterizer_thread = new fml::Thread("rasterizer"); auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); @@ -103,7 +141,11 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { } TEST(AndroidExternalViewEmbedder, PlatformViewRect) { - auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + auto mock_jni = std::make_shared(); + EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + + auto embedder = + std::make_unique(nullptr, mock_jni, nullptr); embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5); auto view_params = std::make_unique(); @@ -116,7 +158,11 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) { } TEST(AndroidExternalViewEmbedder, PlatformViewRect__ChangedParams) { - auto embedder = new AndroidExternalViewEmbedder(nullptr, nullptr, nullptr); + auto mock_jni = std::make_shared(); + EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + + auto embedder = + std::make_unique(nullptr, mock_jni, nullptr); embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5); auto view_id = 0; diff --git a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc index f0c09529e678f..75bae85bc0a7d 100644 --- a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc +++ b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc @@ -2,80 +2,116 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "flutter/fml/make_copyable.h" #include "flutter/shell/platform/android/external_view_embedder/surface_pool.h" #include "flutter/shell/platform/android/jni/mock_jni.h" #include "flutter/shell/platform/android/surface/mock_android_surface.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "third_party/skia/include/gpu/GrContext.h" namespace flutter { namespace testing { +using ::testing::ByMove; +using ::testing::Return; + TEST(SurfacePool, GetLayer__AllocateOneLayer) { - auto pool = new SurfacePool(); + auto pool = std::make_unique(); auto gr_context = GrContext::MakeMock(nullptr); auto android_context = std::make_shared(AndroidRenderingAPI::kSoftware); + auto mock_jni = std::make_shared(); + auto window = fml::MakeRefCounted(nullptr); + EXPECT_CALL(*mock_jni, FlutterViewCreateOverlaySurface()) + .WillOnce(Return( + ByMove(std::make_unique( + 0, window)))); + auto surface_factory = - [](std::shared_ptr android_context, - std::shared_ptr jni_facade) { - return std::make_unique(/*id=*/123); + [gr_context, window](std::shared_ptr android_context, + std::shared_ptr jni_facade) { + auto mock_surface = std::make_unique(); + EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); + EXPECT_CALL(*mock_surface, SetNativeWindow(window)); + return mock_surface; }; auto layer = pool->GetLayer(gr_context.get(), android_context, mock_jni, surface_factory); ASSERT_NE(nullptr, layer); ASSERT_EQ(gr_context.get(), layer->gr_context); - ASSERT_NE(nullptr, layer->surface); - ASSERT_EQ(gr_context.get(), layer->surface->GetContext()); - ASSERT_EQ( - 123, - static_cast(layer->android_surface.get())->GetId()); } TEST(SurfacePool, GetUnusedLayers) { - auto pool = new SurfacePool(); + auto pool = std::make_unique(); auto gr_context = GrContext::MakeMock(nullptr); auto android_context = std::make_shared(AndroidRenderingAPI::kSoftware); + auto mock_jni = std::make_shared(); + auto window = fml::MakeRefCounted(nullptr); + EXPECT_CALL(*mock_jni, FlutterViewCreateOverlaySurface()) + .WillOnce(Return( + ByMove(std::make_unique( + 0, window)))); + auto surface_factory = - [](std::shared_ptr android_context, - std::shared_ptr jni_facade) { - return std::make_unique(/*id=*/123); + [gr_context, window](std::shared_ptr android_context, + std::shared_ptr jni_facade) { + auto mock_surface = std::make_unique(); + EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); + EXPECT_CALL(*mock_surface, SetNativeWindow(window)); + return mock_surface; }; auto layer = pool->GetLayer(gr_context.get(), android_context, mock_jni, surface_factory); ASSERT_EQ(0UL, pool->GetUnusedLayers().size()); pool->RecycleLayers(); + ASSERT_EQ(1UL, pool->GetUnusedLayers().size()); ASSERT_EQ(layer, pool->GetUnusedLayers()[0]); } TEST(SurfacePool, GetLayer__Recycle) { - auto pool = new SurfacePool(); + auto pool = std::make_unique(); auto gr_context_1 = GrContext::MakeMock(nullptr); + auto mock_jni = std::make_shared(); + auto window = fml::MakeRefCounted(nullptr); + EXPECT_CALL(*mock_jni, FlutterViewCreateOverlaySurface()) + .WillOnce(Return( + ByMove(std::make_unique( + 0, window)))); + auto android_context = std::make_shared(AndroidRenderingAPI::kSoftware); - auto mock_jni = std::make_shared(); + + auto gr_context_2 = GrContext::MakeMock(nullptr); auto surface_factory = - [](std::shared_ptr android_context, - std::shared_ptr jni_facade) { - return std::make_unique(/*id=*/123); + [gr_context_1, gr_context_2, window]( + std::shared_ptr android_context, + std::shared_ptr jni_facade) { + auto mock_surface = std::make_unique(); + // Allocate two GPU surfaces for each gr context. + EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context_1.get())); + EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context_2.get())); + // Set the native window once. + EXPECT_CALL(*mock_surface, SetNativeWindow(window)); + return mock_surface; }; auto layer_1 = pool->GetLayer(gr_context_1.get(), android_context, mock_jni, surface_factory); pool->RecycleLayers(); - auto gr_context_2 = GrContext::MakeMock(nullptr); auto layer_2 = pool->GetLayer(gr_context_2.get(), android_context, mock_jni, surface_factory); + ASSERT_NE(nullptr, layer_1); ASSERT_EQ(layer_1, layer_2); ASSERT_EQ(gr_context_2.get(), layer_1->gr_context); @@ -83,25 +119,40 @@ TEST(SurfacePool, GetLayer__Recycle) { } TEST(SurfacePool, GetLayer__AllocateTwoLayers) { - auto pool = new SurfacePool(); + auto pool = std::make_unique(); - auto gr_context_1 = GrContext::MakeMock(nullptr); + auto gr_context = GrContext::MakeMock(nullptr); auto android_context = std::make_shared(AndroidRenderingAPI::kSoftware); + auto mock_jni = std::make_shared(); + auto window = fml::MakeRefCounted(nullptr); + EXPECT_CALL(*mock_jni, FlutterViewCreateOverlaySurface()) + .Times(2) + .WillOnce(Return( + ByMove(std::make_unique( + 0, window)))) + .WillOnce(Return( + ByMove(std::make_unique( + 1, window)))); + auto surface_factory = - [](std::shared_ptr android_context, - std::shared_ptr jni_facade) { - return std::make_unique(/*id=*/123); + [gr_context, window](std::shared_ptr android_context, + std::shared_ptr jni_facade) { + auto mock_surface = std::make_unique(); + EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); + EXPECT_CALL(*mock_surface, SetNativeWindow(window)); + return mock_surface; }; - auto layer_1 = pool->GetLayer(gr_context_1.get(), android_context, mock_jni, + auto layer_1 = pool->GetLayer(gr_context.get(), android_context, mock_jni, surface_factory); - auto layer_2 = pool->GetLayer(gr_context_1.get(), android_context, mock_jni, + auto layer_2 = pool->GetLayer(gr_context.get(), android_context, mock_jni, surface_factory); - ASSERT_NE(nullptr, layer_1); ASSERT_NE(nullptr, layer_2); ASSERT_NE(layer_1, layer_2); + ASSERT_EQ(0, layer_1->id); + ASSERT_EQ(1, layer_2->id); } } // namespace testing diff --git a/shell/platform/android/jni/BUILD.gn b/shell/platform/android/jni/BUILD.gn index b45780bf65053..199af2bbeb963 100644 --- a/shell/platform/android/jni/BUILD.gn +++ b/shell/platform/android/jni/BUILD.gn @@ -22,8 +22,9 @@ source_set("jni") { } source_set("mock_jni") { + testonly = true + sources = [ - "mock_jni.cc", "mock_jni.h", ] @@ -31,7 +32,6 @@ source_set("mock_jni") { deps = [ ":jni", - "//third_party/skia", ] } @@ -47,10 +47,10 @@ executable("jni_unittests") { ] deps = [ - ":jni", ":jni_fixtures", ":mock_jni", "//flutter/testing", "//flutter/testing:dart", + "//flutter/testing:skia", ] } diff --git a/shell/platform/android/jni/mock_jni.cc b/shell/platform/android/jni/mock_jni.cc deleted file mode 100644 index 64e6d2eb67fbd..0000000000000 --- a/shell/platform/android/jni/mock_jni.cc +++ /dev/null @@ -1,79 +0,0 @@ -// 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/shell/platform/android/jni/mock_jni.h" - -namespace flutter { - -MockJNI::MockJNI() = default; - -MockJNI::~MockJNI() = default; - -void MockJNI::FlutterViewHandlePlatformMessage( - fml::RefPtr message, - int responseId) { - jni_calls_.push_back( - FlutterViewHandlePlatformMessageCall{message, responseId}); -} - -void MockJNI::FlutterViewHandlePlatformMessageResponse( - int responseId, - std::unique_ptr data) {} - -void MockJNI::FlutterViewUpdateSemantics(std::vector buffer, - std::vector strings) {} - -void MockJNI::FlutterViewUpdateCustomAccessibilityActions( - std::vector actions_buffer, - std::vector strings) {} - -void MockJNI::FlutterViewOnFirstFrame() {} - -void MockJNI::FlutterViewOnPreEngineRestart() {} - -void MockJNI::SurfaceTextureAttachToGLContext(JavaWeakGlobalRef surface_texture, - int textureId) {} - -void MockJNI::SurfaceTextureUpdateTexImage(JavaWeakGlobalRef surface_texture) {} - -void MockJNI::SurfaceTextureGetTransformMatrix( - JavaWeakGlobalRef surface_texture, - SkMatrix& transform) {} - -void MockJNI::SurfaceTextureDetachFromGLContext( - JavaWeakGlobalRef surface_texture) {} - -void MockJNI::FlutterViewOnDisplayPlatformView(int view_id, - int x, - int y, - int width, - int height) {} - -void MockJNI::FlutterViewDisplayOverlaySurface(int surface_id, - int x, - int y, - int width, - int height) {} - -void MockJNI::FlutterViewBeginFrame() {} - -void MockJNI::FlutterViewEndFrame() {} - -std::unique_ptr -MockJNI::FlutterViewCreateOverlaySurface() { - auto window = fml::MakeRefCounted(nullptr); - auto metadata = std::make_unique( - overlay_surface_id_, window); - - jni_calls_.push_back(FlutterViewCreateOverlaySurfaceCall{metadata.get()}); - - overlay_surface_id_++; - return metadata; -} - -std::vector& MockJNI::GetCalls() { - return jni_calls_; -} - -} // namespace flutter diff --git a/shell/platform/android/jni/mock_jni.h b/shell/platform/android/jni/mock_jni.h index 9e6aa094b75e7..b8ec7193c0331 100644 --- a/shell/platform/android/jni/mock_jni.h +++ b/shell/platform/android/jni/mock_jni.h @@ -5,9 +5,8 @@ #ifndef FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_JNI_H_ #define FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_JNI_H_ -#include - #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" +#include "gmock/gmock.h" namespace flutter { @@ -15,79 +14,71 @@ namespace flutter { /// Mock for |PlatformViewAndroidJNI|. This implementation can be used in unit /// tests without requiring the Android toolchain. /// -class MockJNI final : public PlatformViewAndroidJNI { +class MockJNI : public PlatformViewAndroidJNI { public: - struct FlutterViewHandlePlatformMessageCall { - fml::RefPtr message; - int responseId; - }; - - struct FlutterViewCreateOverlaySurfaceCall { - PlatformViewAndroidJNI::OverlayMetadata* metadata; - }; - - using JNICall = std::variant; - - MockJNI(); - - ~MockJNI() override; - - void FlutterViewHandlePlatformMessage( - fml::RefPtr message, - int responseId) override; - - void FlutterViewHandlePlatformMessageResponse( - int responseId, - std::unique_ptr data) override; - - void FlutterViewUpdateSemantics(std::vector buffer, - std::vector strings) override; - - void FlutterViewUpdateCustomAccessibilityActions( - std::vector actions_buffer, - std::vector strings) override; - - void FlutterViewOnFirstFrame() override; - - void FlutterViewOnPreEngineRestart() override; - - void SurfaceTextureAttachToGLContext(JavaWeakGlobalRef surface_texture, - int textureId) override; - - void SurfaceTextureUpdateTexImage(JavaWeakGlobalRef surface_texture) override; - - void SurfaceTextureGetTransformMatrix(JavaWeakGlobalRef surface_texture, - SkMatrix& transform) override; - - void SurfaceTextureDetachFromGLContext( - JavaWeakGlobalRef surface_texture) override; - - void FlutterViewOnDisplayPlatformView(int view_id, - int x, - int y, - int width, - int height) override; - - void FlutterViewDisplayOverlaySurface(int surface_id, - int x, - int y, - int width, - int height) override; - - void FlutterViewBeginFrame() override; - - void FlutterViewEndFrame() override; - - std::unique_ptr - FlutterViewCreateOverlaySurface() override; - - // Gets the JNI calls. - std::vector& GetCalls(); - - private: - std::vector jni_calls_; - int overlay_surface_id_ = 0; + MOCK_METHOD(void, + FlutterViewHandlePlatformMessage, + (fml::RefPtr message, int responseId), + (override)); + + MOCK_METHOD(void, + FlutterViewHandlePlatformMessageResponse, + (int responseId, std::unique_ptr data), + (override)); + + MOCK_METHOD(void, + FlutterViewUpdateSemantics, + (std::vector buffer, std::vector strings), + (override)); + + MOCK_METHOD(void, + FlutterViewUpdateCustomAccessibilityActions, + (std::vector actions_buffer, + std::vector strings), + (override)); + + MOCK_METHOD(void, FlutterViewOnFirstFrame, (), (override)); + + MOCK_METHOD(void, FlutterViewOnPreEngineRestart, (), (override)); + + MOCK_METHOD(void, + SurfaceTextureAttachToGLContext, + (JavaWeakGlobalRef surface_texture, int textureId), + (override)); + + MOCK_METHOD(void, + SurfaceTextureUpdateTexImage, + (JavaWeakGlobalRef surface_texture), + (override)); + + MOCK_METHOD(void, + SurfaceTextureGetTransformMatrix, + (JavaWeakGlobalRef surface_texture, SkMatrix& transform), + (override)); + + MOCK_METHOD(void, + SurfaceTextureDetachFromGLContext, + (JavaWeakGlobalRef surface_texture), + (override)); + + MOCK_METHOD(void, + FlutterViewOnDisplayPlatformView, + (int view_id, int x, int y, int width, int height), + (override)); + + MOCK_METHOD(void, + FlutterViewDisplayOverlaySurface, + (int surface_id, int x, int y, int width, int height), + (override)); + + MOCK_METHOD(void, FlutterViewBeginFrame, (), (override)); + + MOCK_METHOD(void, FlutterViewEndFrame, (), (override)); + + MOCK_METHOD(std::unique_ptr, + FlutterViewCreateOverlaySurface, + (), + (override)); }; } // namespace flutter diff --git a/shell/platform/android/jni/mock_jni_unittest.cc b/shell/platform/android/jni/mock_jni_unittest.cc index d227516594cd2..f52ba3cfcbf80 100644 --- a/shell/platform/android/jni/mock_jni_unittest.cc +++ b/shell/platform/android/jni/mock_jni_unittest.cc @@ -3,25 +3,22 @@ // found in the LICENSE file. #include "flutter/shell/platform/android/jni/mock_jni.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" namespace flutter { namespace testing { TEST(MockJNI, FlutterViewHandlePlatformMessage) { - auto mock_jni = std::make_unique(); + MockJNI mock_jni; auto message = fml::MakeRefCounted("", nullptr); auto response_id = 1; - mock_jni->FlutterViewHandlePlatformMessage(message, response_id); - auto calls = mock_jni->GetCalls(); - ASSERT_EQ(1UL, calls.size()); + EXPECT_CALL(mock_jni, FlutterViewHandlePlatformMessage(message, response_id)); - auto platform_message_call = - std::get(calls[0]); - ASSERT_EQ("", platform_message_call.message->channel()); + mock_jni.FlutterViewHandlePlatformMessage(message, response_id); } } // namespace testing diff --git a/shell/platform/android/surface/BUILD.gn b/shell/platform/android/surface/BUILD.gn index 76ba5be655c0c..6e64c3bc7c2fb 100644 --- a/shell/platform/android/surface/BUILD.gn +++ b/shell/platform/android/surface/BUILD.gn @@ -35,16 +35,23 @@ source_set("native_window") { } source_set("mock_surface") { + testonly = true + sources = [ "mock_android_surface.cc", "mock_android_surface.h", ] - public_configs = [ "//flutter:config" ] + public_configs = [ + "//flutter:config", + "//third_party/googletest:gmock_config", + ] deps = [ ":surface", "//flutter/shell/gpu:gpu_surface_gl", + "//third_party/googletest:gmock", + "//third_party/googletest:gtest", "//third_party/skia", ] } diff --git a/shell/platform/android/surface/mock_android_surface.cc b/shell/platform/android/surface/mock_android_surface.cc index e8c68125e7af1..29360ac198012 100644 --- a/shell/platform/android/surface/mock_android_surface.cc +++ b/shell/platform/android/surface/mock_android_surface.cc @@ -4,45 +4,8 @@ #include "flutter/shell/platform/android/surface/mock_android_surface.h" -#include "flutter/shell/gpu/gpu_surface_gl.h" - namespace flutter { -MockAndroidSurface::MockAndroidSurface(int id) : id_(id){}; - -MockAndroidSurface::~MockAndroidSurface() = default; - -bool MockAndroidSurface::IsValid() const { - return true; -} - -void MockAndroidSurface::TeardownOnScreenContext() {} - -std::unique_ptr MockAndroidSurface::CreateGPUSurface( - GrContext* gr_context) { - if (gr_context) { - return std::make_unique(sk_ref_sp(gr_context), this, true); - } - return std::make_unique(this, true); -} - -bool MockAndroidSurface::OnScreenSurfaceResize(const SkISize& size) { - return true; -} - -bool MockAndroidSurface::ResourceContextMakeCurrent() { - return true; -} - -bool MockAndroidSurface::ResourceContextClearCurrent() { - return true; -} - -bool MockAndroidSurface::SetNativeWindow( - fml::RefPtr window) { - return true; -} - std::unique_ptr MockAndroidSurface::GLContextMakeCurrent() { return std::make_unique(/*static_result=*/true); } @@ -63,8 +26,4 @@ ExternalViewEmbedder* MockAndroidSurface::GetExternalViewEmbedder() { return nullptr; } -int MockAndroidSurface::GetId() const { - return id_; -} - } // namespace flutter diff --git a/shell/platform/android/surface/mock_android_surface.h b/shell/platform/android/surface/mock_android_surface.h index cdc5b4e1e9b1b..c775fef4afe9e 100644 --- a/shell/platform/android/surface/mock_android_surface.h +++ b/shell/platform/android/surface/mock_android_surface.h @@ -7,6 +7,7 @@ #include "flutter/shell/gpu/gpu_surface_gl.h" #include "flutter/shell/platform/android/surface/android_surface.h" +#include "gmock/gmock.h" namespace flutter { @@ -17,30 +18,25 @@ namespace flutter { class MockAndroidSurface final : public GPUSurfaceGLDelegate, public AndroidSurface { public: - MockAndroidSurface(int id); + MOCK_METHOD(bool, IsValid, (), (const, override)); - ~MockAndroidSurface() override; + MOCK_METHOD(void, TeardownOnScreenContext, (), (override)); - // |AndroidSurface| - bool IsValid() const override; + MOCK_METHOD(std::unique_ptr, + CreateGPUSurface, + (GrContext * gr_context), + (override)); - // |AndroidSurface| - void TeardownOnScreenContext() override; + MOCK_METHOD(bool, OnScreenSurfaceResize, (const SkISize& size), (override)); - // |AndroidSurface| - std::unique_ptr CreateGPUSurface(GrContext* gr_context) override; + MOCK_METHOD(bool, ResourceContextMakeCurrent, (), (override)); - // |AndroidSurface| - bool OnScreenSurfaceResize(const SkISize& size) override; + MOCK_METHOD(bool, ResourceContextClearCurrent, (), (override)); - // |AndroidSurface| - bool ResourceContextMakeCurrent() override; - - // |AndroidSurface| - bool ResourceContextClearCurrent() override; - - // |AndroidSurface| - bool SetNativeWindow(fml::RefPtr window) override; + MOCK_METHOD(bool, + SetNativeWindow, + (fml::RefPtr window), + (override)); // |GPUSurfaceGLDelegate| std::unique_ptr GLContextMakeCurrent() override; @@ -56,12 +52,6 @@ class MockAndroidSurface final : public GPUSurfaceGLDelegate, // |GPUSurfaceGLDelegate| ExternalViewEmbedder* GetExternalViewEmbedder() override; - - // Used in unit tests. - int GetId() const; - - private: - int id_; }; } // namespace flutter diff --git a/testing/BUILD.gn b/testing/BUILD.gn index 1fbfa4b3d3874..0a8f690bb080f 100644 --- a/testing/BUILD.gn +++ b/testing/BUILD.gn @@ -18,6 +18,7 @@ source_set("testing_lib") { public_deps = [ "//flutter/fml", + "//third_party/googletest:gmock", "//third_party/googletest:gtest", ] public_configs = [ "//flutter:config" ] From 56fd6a21c8178bd543dcc8d4f0fee06f85ec9268 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 17 Jun 2020 21:44:49 -0700 Subject: [PATCH 14/28] Unit test --- shell/platform/android/BUILD.gn | 1 + .../android/external_view_embedder/BUILD.gn | 4 +- .../external_view_embedder.cc | 23 +- .../external_view_embedder.h | 2 +- .../external_view_embedder_unittests.cc | 204 ++++++++++++++++-- .../external_view_embedder/surface_pool.cc | 2 +- .../external_view_embedder/surface_pool.h | 2 +- .../surface_pool_unittests.cc | 40 ++-- shell/platform/android/jni/BUILD.gn | 8 +- .../android/jni/{mock_jni.h => jni_mock.h} | 8 +- ...k_jni_unittest.cc => jni_mock_unittest.cc} | 10 +- .../android/platform_view_android_jni_impl.cc | 6 +- shell/platform/android/surface/BUILD.gn | 6 +- ...oid_surface.cc => android_surface_mock.cc} | 12 +- ...droid_surface.h => android_surface_mock.h} | 8 +- 15 files changed, 251 insertions(+), 85 deletions(-) rename shell/platform/android/jni/{mock_jni.h => jni_mock.h} (92%) rename shell/platform/android/jni/{mock_jni_unittest.cc => jni_mock_unittest.cc} (60%) rename shell/platform/android/surface/{mock_android_surface.cc => android_surface_mock.cc} (52%) rename shell/platform/android/surface/{mock_android_surface.h => android_surface_mock.h} (86%) diff --git a/shell/platform/android/BUILD.gn b/shell/platform/android/BUILD.gn index 4f6a5bd0e92cb..a548a3e3edec7 100644 --- a/shell/platform/android/BUILD.gn +++ b/shell/platform/android/BUILD.gn @@ -65,6 +65,7 @@ shared_library("flutter_shell_native") { "//flutter/shell/platform/android/external_view_embedder", "//flutter/shell/platform/android/jni", "//flutter/shell/platform/android/surface", + "//flutter/shell/platform/android/surface:native_window", "//third_party/skia", ] diff --git a/shell/platform/android/external_view_embedder/BUILD.gn b/shell/platform/android/external_view_embedder/BUILD.gn index f37a6cd83eaf8..555e504e29489 100644 --- a/shell/platform/android/external_view_embedder/BUILD.gn +++ b/shell/platform/android/external_view_embedder/BUILD.gn @@ -42,8 +42,8 @@ executable("android_external_view_embedder_unittests") { ":external_view_embedder", ":external_view_embedder_fixtures", "//flutter/shell/gpu:gpu_surface_gl", - "//flutter/shell/platform/android/jni:mock_jni", - "//flutter/shell/platform/android/surface:mock_surface", + "//flutter/shell/platform/android/jni:jni_mock", + "//flutter/shell/platform/android/surface:surface_mock", "//flutter/testing", "//flutter/testing:dart", "//flutter/testing:skia", 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 23b92419f804e..3b70f5dd8d627 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -11,7 +11,7 @@ namespace flutter { AndroidExternalViewEmbedder::AndroidExternalViewEmbedder( std::shared_ptr android_context, std::shared_ptr jni_facade, - AndroidSurface::Factory surface_factory) + const AndroidSurface::Factory& surface_factory) : ExternalViewEmbedder(), android_context_(std::move(android_context)), jni_facade_(std::move(jni_facade)), @@ -22,11 +22,11 @@ AndroidExternalViewEmbedder::AndroidExternalViewEmbedder( void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView( int view_id, std::unique_ptr params) { - auto rtree_factory = RTreeFactory(); - view_rtrees_[view_id] = rtree_factory.getInstance(); - TRACE_EVENT0("flutter", "AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView"); + + auto rtree_factory = RTreeFactory(); + view_rtrees_[view_id] = rtree_factory.getInstance(); picture_recorders_[view_id] = std::make_unique(); picture_recorders_[view_id]->beginRecording(SkRect::Make(frame_size_), &rtree_factory); @@ -89,18 +89,18 @@ bool AndroidExternalViewEmbedder::SubmitFrame( for (size_t i = 0; i < composition_order_.size(); i++) { int64_t view_id = composition_order_[i]; - SkPoint view_offset = view_params_[view_id].offsetPixels; - SkSize view_size = view_params_[view_id].sizePoints; + SkRect view_rect = GetViewRect(view_id); // Display the platform view. If it's already displayed, then it's // just positioned and sized. jni_facade_->FlutterViewOnDisplayPlatformView(view_id, // - view_offset.x(), // - view_offset.y(), // - view_size.width(), // - view_size.height() // + view_rect.x(), // + view_rect.y(), // + view_rect.width(), // + view_rect.height() // ); + pictures[view_id] = picture_recorders_[view_id]->finishRecordingAsPicture(); sk_sp rtree = view_rtrees_[view_id]; // Determinate if Flutter UI intersects with any of the previous // platform views stacked by z position. @@ -115,6 +115,7 @@ bool AndroidExternalViewEmbedder::SubmitFrame( std::list intersection_rects = rtree->searchNonOverlappingDrawnRects(current_view_rect); auto allocation_size = intersection_rects.size(); + // Limit the number of native views, so it doesn't grow for ever. // // In this case, the rects are merged into a single one that is the union @@ -147,7 +148,6 @@ bool AndroidExternalViewEmbedder::SubmitFrame( } overlay_layers[current_view_id] = intersection_rects; } - pictures[view_id] = picture_recorders_[view_id]->finishRecordingAsPicture(); background_canvas->drawPicture(pictures[view_id]); } // Submit the background canvas frame before switching the GL context to @@ -255,6 +255,7 @@ void AndroidExternalViewEmbedder::EndFrame( raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); should_run_rasterizer_on_platform_thread_ = false; } + surface_pool_->RecycleLayers(); jni_facade_->FlutterViewEndFrame(); } 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 53eae0e925eb8..019d6932a0c66 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -19,7 +19,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { AndroidExternalViewEmbedder( std::shared_ptr android_context, std::shared_ptr jni_facade, - AndroidSurface::Factory surface_factory); + const AndroidSurface::Factory& surface_factory); // |ExternalViewEmbedder| void PrerollCompositeEmbeddedView( 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 6b9336080d4d5..23eb430cf1f11 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 @@ -2,25 +2,53 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "flutter/flow/surface.h" #include "flutter/fml/raster_thread_merger.h" #include "flutter/fml/thread.h" #include "flutter/shell/platform/android/external_view_embedder/external_view_embedder.h" -#include "flutter/shell/platform/android/jni/mock_jni.h" +#include "flutter/shell/platform/android/jni/jni_mock.h" +#include "flutter/shell/platform/android/surface/android_surface_mock.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "third_party/skia/include/gpu/GrContext.h" namespace flutter { namespace testing { -using ::testing::Mock; +using ::testing::ByMove; +using ::testing::Return; + +class SurfaceMock : public Surface { + public: + MOCK_METHOD(bool, IsValid, (), (override)); + + MOCK_METHOD(std::unique_ptr, + AcquireFrame, + (const SkISize& size), + (override)); + + MOCK_METHOD(SkMatrix, GetRootTransformation, (), (const, override)); + + MOCK_METHOD(GrContext*, GetContext, (), (override)); + + MOCK_METHOD(flutter::ExternalViewEmbedder*, + GetExternalViewEmbedder, + (), + (override)); + + MOCK_METHOD(std::unique_ptr, + MakeRenderContextCurrent, + (), + (override)); +}; TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { - auto mock_jni = std::make_shared(); + auto jni_mock = std::make_shared(); - EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); auto embedder = - std::make_unique(nullptr, mock_jni, nullptr); + std::make_unique(nullptr, jni_mock, nullptr); embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); embedder->PrerollCompositeEmbeddedView( @@ -35,11 +63,11 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { } TEST(AndroidExternalViewEmbedder, GetCurrentCanvases__CompositeOrder) { - auto mock_jni = std::make_shared(); - EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + auto jni_mock = std::make_shared(); + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); auto embedder = - std::make_unique(nullptr, mock_jni, nullptr); + std::make_unique(nullptr, jni_mock, nullptr); embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); embedder->PrerollCompositeEmbeddedView( @@ -81,12 +109,10 @@ TEST(AndroidExternalViewEmbedder, CancelFrame) { } TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { - auto mock_jni = std::make_shared(); - EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); - EXPECT_CALL(*mock_jni, FlutterViewEndFrame()); + auto jni_mock = std::make_shared(); auto embedder = - std::make_unique(nullptr, mock_jni, nullptr); + std::make_unique(nullptr, jni_mock, nullptr); auto platform_thread = new fml::Thread("platform"); auto rasterizer_thread = new fml::Thread("rasterizer"); auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); @@ -97,6 +123,7 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { platform_queue_id, rasterizer_queue_id); ASSERT_FALSE(raster_thread_merger->IsMerged()); + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); // Push a platform view. embedder->PrerollCompositeEmbeddedView( @@ -106,7 +133,9 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { ASSERT_EQ(PostPrerollResult::kResubmitFrame, postpreroll_result); ASSERT_TRUE(embedder->SubmitFrame(nullptr, nullptr)); + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(raster_thread_merger); + ASSERT_TRUE(raster_thread_merger->IsMerged()); int pending_frames = 0; @@ -118,11 +147,10 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { } TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { - auto mock_jni = std::make_shared(); - EXPECT_CALL(*mock_jni, FlutterViewEndFrame()); + auto jni_mock = std::make_shared(); auto embedder = - std::make_unique(nullptr, mock_jni, nullptr); + std::make_unique(nullptr, jni_mock, nullptr); auto platform_thread = new fml::Thread("platform"); auto rasterizer_thread = new fml::Thread("rasterizer"); auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); @@ -136,16 +164,19 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { PostPrerollResult result = embedder->PostPrerollAction(raster_thread_merger); ASSERT_EQ(PostPrerollResult::kSuccess, result); + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); embedder->EndFrame(raster_thread_merger); + ASSERT_FALSE(raster_thread_merger->IsMerged()); } TEST(AndroidExternalViewEmbedder, PlatformViewRect) { - auto mock_jni = std::make_shared(); - EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + auto jni_mock = std::make_shared(); auto embedder = - std::make_unique(nullptr, mock_jni, nullptr); + std::make_unique(nullptr, jni_mock, nullptr); + + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5); auto view_params = std::make_unique(); @@ -158,11 +189,12 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) { } TEST(AndroidExternalViewEmbedder, PlatformViewRect__ChangedParams) { - auto mock_jni = std::make_shared(); - EXPECT_CALL(*mock_jni, FlutterViewBeginFrame()); + auto jni_mock = std::make_shared(); auto embedder = - std::make_unique(nullptr, mock_jni, nullptr); + std::make_unique(nullptr, jni_mock, nullptr); + + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5); auto view_id = 0; @@ -179,5 +211,135 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect__ChangedParams) { ASSERT_EQ(SkRect::MakeXYWH(50, 60, 105, 120), embedder->GetViewRect(view_id)); } +TEST(AndroidExternalViewEmbedder, SubmitFrame__RecycleSurfaces) { + auto jni_mock = std::make_shared(); + auto android_context = + std::make_shared(AndroidRenderingAPI::kSoftware); + + auto window = fml::MakeRefCounted(nullptr); + auto gr_context = GrContext::MakeMock(nullptr); + auto frame_size = SkISize::Make(1000, 1000); + auto surface_factory = + [gr_context, window, frame_size]( + std::shared_ptr android_context, + std::shared_ptr jni_facade) { + auto android_surface_mock = std::make_unique(); + auto surface_mock = std::make_unique(); + auto surface_frame_1 = std::make_unique( + SkSurface::MakeNull(1000, 1000), false, + [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { + return true; + }); + auto surface_frame_2 = std::make_unique( + SkSurface::MakeNull(1000, 1000), false, + [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { + return true; + }); + + EXPECT_CALL(*surface_mock, AcquireFrame(frame_size)) + .Times(2 /* frames */) + .WillOnce(Return(ByMove(std::move(surface_frame_1)))) + .WillOnce(Return(ByMove(std::move(surface_frame_2)))); + + EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get())) + .WillOnce(Return(ByMove(std::move(surface_mock)))); + + EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); + return android_surface_mock; + }; + auto embedder = std::make_unique( + android_context, jni_mock, surface_factory); + + // ------------------ First frame ------------------ // + { + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); + embedder->BeginFrame(frame_size, nullptr, 1.5); + + // Add an Android view. + auto view_params_1 = std::make_unique(); + view_params_1->offsetPixels = SkPoint::Make(100, 100); + // 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); + embedder->PrerollCompositeEmbeddedView(0, std::move(view_params_1)); + // This is the recording canvas flow writes to. + auto canvas_1 = embedder->CompositeEmbeddedView(0); + + auto rect_paint = SkPaint(); + rect_paint.setColor(SkColors::kCyan); + rect_paint.setStyle(SkPaint::Style::kFill_Style); + + // This simulates Flutter UI that doesn't intersect with the Android view. + canvas_1->drawRect(SkRect::MakeXYWH(0, 0, 50, 50), rect_paint); + // This simulates Flutter UI that intersects with the Android view. + canvas_1->drawRect(SkRect::MakeXYWH(50, 50, 200, 200), rect_paint); + canvas_1->drawRect(SkRect::MakeXYWH(150, 150, 100, 100), rect_paint); + + // Create a new overlay surface. + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) + .WillOnce(Return( + ByMove(std::make_unique( + 0, window)))); + // The JNI call to display the Android view. + EXPECT_CALL(*jni_mock, + FlutterViewOnDisplayPlatformView(0, 100, 100, 300, 300)); + // The JNI call to display the overlay surface. + EXPECT_CALL(*jni_mock, + FlutterViewDisplayOverlaySurface(0, 50, 50, 200, 200)); + + auto surface_frame = + std::make_unique(SkSurface::MakeNull(1000, 1000), false, + [](const SurfaceFrame& surface_frame, + SkCanvas* canvas) { return true; }); + + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); + + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); + embedder->EndFrame(nullptr); + } + + // ------------------ Second frame ------------------ // + { + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); + embedder->BeginFrame(frame_size, nullptr, 1.5); + + // Add an Android view. + auto view_params_1 = std::make_unique(); + view_params_1->offsetPixels = SkPoint::Make(100, 100); + // 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); + embedder->PrerollCompositeEmbeddedView(0, std::move(view_params_1)); + // This is the recording canvas flow writes to. + auto canvas_1 = embedder->CompositeEmbeddedView(0); + + auto rect_paint = SkPaint(); + rect_paint.setColor(SkColors::kCyan); + rect_paint.setStyle(SkPaint::Style::kFill_Style); + + // This simulates Flutter UI that doesn't intersect with the Android view. + canvas_1->drawRect(SkRect::MakeXYWH(0, 0, 50, 50), rect_paint); + // This simulates Flutter UI that intersects with the Android view. + canvas_1->drawRect(SkRect::MakeXYWH(50, 50, 200, 200), rect_paint); + canvas_1->drawRect(SkRect::MakeXYWH(150, 150, 100, 100), rect_paint); + + // Don't create a new overlay surface since it's recycled from the first + // frame. + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()).Times(0); + // The JNI call to display the Android view. + EXPECT_CALL(*jni_mock, + FlutterViewOnDisplayPlatformView(0, 100, 100, 300, 300)); + // The JNI call to display the overlay surface. + EXPECT_CALL(*jni_mock, + FlutterViewDisplayOverlaySurface(0, 50, 50, 200, 200)); + + auto surface_frame = + std::make_unique(SkSurface::MakeNull(1000, 1000), false, + [](const SurfaceFrame& surface_frame, + SkCanvas* canvas) { return true; }); + embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); + } +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index 819ee31fbe246..f0b818e804e61 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -23,7 +23,7 @@ std::shared_ptr SurfacePool::GetLayer( GrContext* gr_context, std::shared_ptr android_context, std::shared_ptr jni_facade, - AndroidSurface::Factory surface_factory) { + const AndroidSurface::Factory& surface_factory) { // Allocate a new surface if there isn't one available. if (available_layer_index_ >= layers_.size()) { std::unique_ptr android_surface = diff --git a/shell/platform/android/external_view_embedder/surface_pool.h b/shell/platform/android/external_view_embedder/surface_pool.h index a631aa4e9de64..88bf01ead1d50 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.h +++ b/shell/platform/android/external_view_embedder/surface_pool.h @@ -55,7 +55,7 @@ class SurfacePool { GrContext* gr_context, std::shared_ptr android_context, std::shared_ptr jni_facade, - AndroidSurface::Factory surface_factory); + const AndroidSurface::Factory& surface_factory); // Gets the layers in the pool that aren't currently used. // This method doesn't mark the layers as unused. diff --git a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc index 75bae85bc0a7d..5393c81f02159 100644 --- a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc +++ b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc @@ -4,8 +4,8 @@ #include "flutter/fml/make_copyable.h" #include "flutter/shell/platform/android/external_view_embedder/surface_pool.h" -#include "flutter/shell/platform/android/jni/mock_jni.h" -#include "flutter/shell/platform/android/surface/mock_android_surface.h" +#include "flutter/shell/platform/android/jni/jni_mock.h" +#include "flutter/shell/platform/android/surface/android_surface_mock.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "third_party/skia/include/gpu/GrContext.h" @@ -23,9 +23,9 @@ TEST(SurfacePool, GetLayer__AllocateOneLayer) { auto android_context = std::make_shared(AndroidRenderingAPI::kSoftware); - auto mock_jni = std::make_shared(); + auto jni_mock = std::make_shared(); auto window = fml::MakeRefCounted(nullptr); - EXPECT_CALL(*mock_jni, FlutterViewCreateOverlaySurface()) + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) .WillOnce(Return( ByMove(std::make_unique( 0, window)))); @@ -33,12 +33,12 @@ TEST(SurfacePool, GetLayer__AllocateOneLayer) { auto surface_factory = [gr_context, window](std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto mock_surface = std::make_unique(); + auto mock_surface = std::make_unique(); EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); EXPECT_CALL(*mock_surface, SetNativeWindow(window)); return mock_surface; }; - auto layer = pool->GetLayer(gr_context.get(), android_context, mock_jni, + auto layer = pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); ASSERT_NE(nullptr, layer); @@ -52,9 +52,9 @@ TEST(SurfacePool, GetUnusedLayers) { auto android_context = std::make_shared(AndroidRenderingAPI::kSoftware); - auto mock_jni = std::make_shared(); + auto jni_mock = std::make_shared(); auto window = fml::MakeRefCounted(nullptr); - EXPECT_CALL(*mock_jni, FlutterViewCreateOverlaySurface()) + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) .WillOnce(Return( ByMove(std::make_unique( 0, window)))); @@ -62,12 +62,12 @@ TEST(SurfacePool, GetUnusedLayers) { auto surface_factory = [gr_context, window](std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto mock_surface = std::make_unique(); + auto mock_surface = std::make_unique(); EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); EXPECT_CALL(*mock_surface, SetNativeWindow(window)); return mock_surface; }; - auto layer = pool->GetLayer(gr_context.get(), android_context, mock_jni, + auto layer = pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); ASSERT_EQ(0UL, pool->GetUnusedLayers().size()); @@ -81,9 +81,9 @@ TEST(SurfacePool, GetLayer__Recycle) { auto pool = std::make_unique(); auto gr_context_1 = GrContext::MakeMock(nullptr); - auto mock_jni = std::make_shared(); + auto jni_mock = std::make_shared(); auto window = fml::MakeRefCounted(nullptr); - EXPECT_CALL(*mock_jni, FlutterViewCreateOverlaySurface()) + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) .WillOnce(Return( ByMove(std::make_unique( 0, window)))); @@ -96,7 +96,7 @@ TEST(SurfacePool, GetLayer__Recycle) { [gr_context_1, gr_context_2, window]( std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto mock_surface = std::make_unique(); + auto mock_surface = std::make_unique(); // Allocate two GPU surfaces for each gr context. EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context_1.get())); EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context_2.get())); @@ -104,12 +104,12 @@ TEST(SurfacePool, GetLayer__Recycle) { EXPECT_CALL(*mock_surface, SetNativeWindow(window)); return mock_surface; }; - auto layer_1 = pool->GetLayer(gr_context_1.get(), android_context, mock_jni, + auto layer_1 = pool->GetLayer(gr_context_1.get(), android_context, jni_mock, surface_factory); pool->RecycleLayers(); - auto layer_2 = pool->GetLayer(gr_context_2.get(), android_context, mock_jni, + auto layer_2 = pool->GetLayer(gr_context_2.get(), android_context, jni_mock, surface_factory); ASSERT_NE(nullptr, layer_1); @@ -125,9 +125,9 @@ TEST(SurfacePool, GetLayer__AllocateTwoLayers) { auto android_context = std::make_shared(AndroidRenderingAPI::kSoftware); - auto mock_jni = std::make_shared(); + auto jni_mock = std::make_shared(); auto window = fml::MakeRefCounted(nullptr); - EXPECT_CALL(*mock_jni, FlutterViewCreateOverlaySurface()) + EXPECT_CALL(*jni_mock, FlutterViewCreateOverlaySurface()) .Times(2) .WillOnce(Return( ByMove(std::make_unique( @@ -139,14 +139,14 @@ TEST(SurfacePool, GetLayer__AllocateTwoLayers) { auto surface_factory = [gr_context, window](std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto mock_surface = std::make_unique(); + auto mock_surface = std::make_unique(); EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); EXPECT_CALL(*mock_surface, SetNativeWindow(window)); return mock_surface; }; - auto layer_1 = pool->GetLayer(gr_context.get(), android_context, mock_jni, + auto layer_1 = pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); - auto layer_2 = pool->GetLayer(gr_context.get(), android_context, mock_jni, + auto layer_2 = pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); ASSERT_NE(nullptr, layer_1); ASSERT_NE(nullptr, layer_2); diff --git a/shell/platform/android/jni/BUILD.gn b/shell/platform/android/jni/BUILD.gn index 199af2bbeb963..4929ed62646ae 100644 --- a/shell/platform/android/jni/BUILD.gn +++ b/shell/platform/android/jni/BUILD.gn @@ -21,11 +21,11 @@ source_set("jni") { ] } -source_set("mock_jni") { +source_set("jni_mock") { testonly = true sources = [ - "mock_jni.h", + "jni_mock.h", ] public_configs = [ "//flutter:config" ] @@ -43,12 +43,12 @@ executable("jni_unittests") { testonly = true sources = [ - "mock_jni_unittest.cc", + "jni_mock_unittest.cc", ] deps = [ ":jni_fixtures", - ":mock_jni", + ":jni_mock", "//flutter/testing", "//flutter/testing:dart", "//flutter/testing:skia", diff --git a/shell/platform/android/jni/mock_jni.h b/shell/platform/android/jni/jni_mock.h similarity index 92% rename from shell/platform/android/jni/mock_jni.h rename to shell/platform/android/jni/jni_mock.h index b8ec7193c0331..54d3297e0d3bd 100644 --- a/shell/platform/android/jni/mock_jni.h +++ b/shell/platform/android/jni/jni_mock.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_JNI_H_ -#define FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_JNI_H_ +#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_H_ +#define FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_H_ #include "flutter/shell/platform/android/jni/platform_view_android_jni.h" #include "gmock/gmock.h" @@ -14,7 +14,7 @@ namespace flutter { /// Mock for |PlatformViewAndroidJNI|. This implementation can be used in unit /// tests without requiring the Android toolchain. /// -class MockJNI : public PlatformViewAndroidJNI { +class JNIMock final : public PlatformViewAndroidJNI { public: MOCK_METHOD(void, FlutterViewHandlePlatformMessage, @@ -83,4 +83,4 @@ class MockJNI : public PlatformViewAndroidJNI { } // namespace flutter -#endif // FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_JNI_H_ +#endif // FLUTTER_SHELL_PLATFORM_ANDROID_JNI_MOCK_H_ diff --git a/shell/platform/android/jni/mock_jni_unittest.cc b/shell/platform/android/jni/jni_mock_unittest.cc similarity index 60% rename from shell/platform/android/jni/mock_jni_unittest.cc rename to shell/platform/android/jni/jni_mock_unittest.cc index f52ba3cfcbf80..738c49ce8d730 100644 --- a/shell/platform/android/jni/mock_jni_unittest.cc +++ b/shell/platform/android/jni/jni_mock_unittest.cc @@ -2,23 +2,23 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/shell/platform/android/jni/mock_jni.h" +#include "flutter/shell/platform/android/jni/jni_mock.h" #include "gmock/gmock.h" #include "gtest/gtest.h" namespace flutter { namespace testing { -TEST(MockJNI, FlutterViewHandlePlatformMessage) { - MockJNI mock_jni; +TEST(JNIMock, FlutterViewHandlePlatformMessage) { + JNIMock mock; auto message = fml::MakeRefCounted("", nullptr); auto response_id = 1; - EXPECT_CALL(mock_jni, FlutterViewHandlePlatformMessage(message, response_id)); + EXPECT_CALL(mock, FlutterViewHandlePlatformMessage(message, response_id)); - mock_jni.FlutterViewHandlePlatformMessage(message, response_id); + mock.FlutterViewHandlePlatformMessage(message, response_id); } } // namespace testing diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 14285e73af272..7937934a48da8 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -1105,13 +1105,15 @@ PlatformViewAndroidJNIImpl::FlutterViewCreateOverlaySurface() { auto java_object = java_object_.get(env); if (java_object.is_null()) { - return; + return nullptr; } env->CallVoidMethod(java_object.obj(), g_create_overlay_surface_method); FML_CHECK(CheckException(env)); - return nullptr; + // TODO(egarciad): Wire this up. + // https://github.com/flutter/flutter/issues/55270 + return std::make_unique(0, nullptr); } } // namespace flutter diff --git a/shell/platform/android/surface/BUILD.gn b/shell/platform/android/surface/BUILD.gn index 6e64c3bc7c2fb..a5ba5305d68a3 100644 --- a/shell/platform/android/surface/BUILD.gn +++ b/shell/platform/android/surface/BUILD.gn @@ -34,12 +34,12 @@ source_set("native_window") { ] } -source_set("mock_surface") { +source_set("surface_mock") { testonly = true sources = [ - "mock_android_surface.cc", - "mock_android_surface.h", + "android_surface_mock.cc", + "android_surface_mock.h", ] public_configs = [ diff --git a/shell/platform/android/surface/mock_android_surface.cc b/shell/platform/android/surface/android_surface_mock.cc similarity index 52% rename from shell/platform/android/surface/mock_android_surface.cc rename to shell/platform/android/surface/android_surface_mock.cc index 29360ac198012..5b0e7adad8cdd 100644 --- a/shell/platform/android/surface/mock_android_surface.cc +++ b/shell/platform/android/surface/android_surface_mock.cc @@ -2,27 +2,27 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/shell/platform/android/surface/mock_android_surface.h" +#include "flutter/shell/platform/android/surface/android_surface_mock.h" namespace flutter { -std::unique_ptr MockAndroidSurface::GLContextMakeCurrent() { +std::unique_ptr AndroidSurfaceMock::GLContextMakeCurrent() { return std::make_unique(/*static_result=*/true); } -bool MockAndroidSurface::GLContextClearCurrent() { +bool AndroidSurfaceMock::GLContextClearCurrent() { return true; } -bool MockAndroidSurface::GLContextPresent() { +bool AndroidSurfaceMock::GLContextPresent() { return true; } -intptr_t MockAndroidSurface::GLContextFBO() const { +intptr_t AndroidSurfaceMock::GLContextFBO() const { return 0; } -ExternalViewEmbedder* MockAndroidSurface::GetExternalViewEmbedder() { +ExternalViewEmbedder* AndroidSurfaceMock::GetExternalViewEmbedder() { return nullptr; } diff --git a/shell/platform/android/surface/mock_android_surface.h b/shell/platform/android/surface/android_surface_mock.h similarity index 86% rename from shell/platform/android/surface/mock_android_surface.h rename to shell/platform/android/surface/android_surface_mock.h index c775fef4afe9e..23c3083567c50 100644 --- a/shell/platform/android/surface/mock_android_surface.h +++ b/shell/platform/android/surface/android_surface_mock.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_MOCK_ANDROID_SURFACE_H_ -#define FLUTTER_SHELL_PLATFORM_ANDROID_MOCK_ANDROID_SURFACE_H_ +#ifndef FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_MOCK_H_ +#define FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_MOCK_H_ #include "flutter/shell/gpu/gpu_surface_gl.h" #include "flutter/shell/platform/android/surface/android_surface.h" @@ -15,7 +15,7 @@ namespace flutter { /// Mock for |AndroidSurface|. This implementation can be used in unit /// tests without requiring the Android toolchain. /// -class MockAndroidSurface final : public GPUSurfaceGLDelegate, +class AndroidSurfaceMock final : public GPUSurfaceGLDelegate, public AndroidSurface { public: MOCK_METHOD(bool, IsValid, (), (const, override)); @@ -56,4 +56,4 @@ class MockAndroidSurface final : public GPUSurfaceGLDelegate, } // namespace flutter -#endif // FLUTTER_SHELL_PLATFORM_ANDROID_MOCK_ANDROID_SURFACE_H_ +#endif // FLUTTER_SHELL_PLATFORM_ANDROID_ANDROID_SURFACE_MOCK_H_ From a0e371e06394f9293ed32b14bb33eb41f830370a Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 17 Jun 2020 21:47:40 -0700 Subject: [PATCH 15/28] Revert file change --- shell/gpu/gpu_surface_gl_delegate.cc | 94 ++++++++++++++-------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/shell/gpu/gpu_surface_gl_delegate.cc b/shell/gpu/gpu_surface_gl_delegate.cc index 024201e710b4f..91efd5130ddcb 100644 --- a/shell/gpu/gpu_surface_gl_delegate.cc +++ b/shell/gpu/gpu_surface_gl_delegate.cc @@ -29,62 +29,62 @@ GPUSurfaceGLDelegate::GLProcResolver GPUSurfaceGLDelegate::GetGLProcResolver() return nullptr; } -// static bool IsProcResolverOpenGLES( -// GPUSurfaceGLDelegate::GLProcResolver proc_resolver) { -// // Version string prefix that identifies an OpenGL ES implementation. -// #define GPU_GL_VERSION 0x1F02 -// constexpr char kGLESVersionPrefix[] = "OpenGL ES"; +static bool IsProcResolverOpenGLES( + GPUSurfaceGLDelegate::GLProcResolver proc_resolver) { + // Version string prefix that identifies an OpenGL ES implementation. +#define GPU_GL_VERSION 0x1F02 + constexpr char kGLESVersionPrefix[] = "OpenGL ES"; -// using GLGetStringProc = const char* (*)(uint32_t); + using GLGetStringProc = const char* (*)(uint32_t); -// GLGetStringProc gl_get_string = -// reinterpret_cast(proc_resolver("glGetString")); + GLGetStringProc gl_get_string = + reinterpret_cast(proc_resolver("glGetString")); -// FML_CHECK(gl_get_string) -// << "The GL proc resolver could not resolve glGetString"; + FML_CHECK(gl_get_string) + << "The GL proc resolver could not resolve glGetString"; -// const char* gl_version_string = gl_get_string(GPU_GL_VERSION); + const char* gl_version_string = gl_get_string(GPU_GL_VERSION); -// FML_CHECK(gl_version_string) -// << "The GL proc resolver's glGetString(GL_VERSION) failed"; + FML_CHECK(gl_version_string) + << "The GL proc resolver's glGetString(GL_VERSION) failed"; -// return strncmp(gl_version_string, kGLESVersionPrefix, -// strlen(kGLESVersionPrefix)) == 0; -// } + return strncmp(gl_version_string, kGLESVersionPrefix, + strlen(kGLESVersionPrefix)) == 0; +} static sk_sp CreateGLInterface( GPUSurfaceGLDelegate::GLProcResolver proc_resolver) { - // if (proc_resolver == nullptr) { - // // If there is no custom proc resolver, ask Skia to guess the native - // // interface. This often leads to interesting results on most platforms. - // return GrGLMakeNativeInterface(); - // } - - // struct ProcResolverContext { - // GPUSurfaceGLDelegate::GLProcResolver resolver; - // }; - - // ProcResolverContext context = {proc_resolver}; - - // GrGLGetProc gl_get_proc = [](void* context, - // const char gl_proc_name[]) -> GrGLFuncPtr { - // auto proc_resolver_context = - // reinterpret_cast(context); - // return reinterpret_cast( - // proc_resolver_context->resolver(gl_proc_name)); - // }; - - // // glGetString indicates an OpenGL ES interface. - // if (IsProcResolverOpenGLES(proc_resolver)) { - // return GrGLMakeAssembledGLESInterface(&context, gl_get_proc); - // } - - // // Fallback to OpenGL. - // if (auto interface = GrGLMakeAssembledGLInterface(&context, gl_get_proc)) { - // return interface; - // } - - // FML_LOG(ERROR) << "Could not create a valid GL interface."; + if (proc_resolver == nullptr) { + // If there is no custom proc resolver, ask Skia to guess the native + // interface. This often leads to interesting results on most platforms. + return GrGLMakeNativeInterface(); + } + + struct ProcResolverContext { + GPUSurfaceGLDelegate::GLProcResolver resolver; + }; + + ProcResolverContext context = {proc_resolver}; + + GrGLGetProc gl_get_proc = [](void* context, + const char gl_proc_name[]) -> GrGLFuncPtr { + auto proc_resolver_context = + reinterpret_cast(context); + return reinterpret_cast( + proc_resolver_context->resolver(gl_proc_name)); + }; + + // glGetString indicates an OpenGL ES interface. + if (IsProcResolverOpenGLES(proc_resolver)) { + return GrGLMakeAssembledGLESInterface(&context, gl_get_proc); + } + + // Fallback to OpenGL. + if (auto interface = GrGLMakeAssembledGLInterface(&context, gl_get_proc)) { + return interface; + } + + FML_LOG(ERROR) << "Could not create a valid GL interface."; return nullptr; } From 07fbc72feabf4d0fa1788b38cec1a644499a34da Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 17 Jun 2020 21:58:47 -0700 Subject: [PATCH 16/28] Update licences --- ci/licenses_golden/licenses_flutter | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 861a42291e772..6140284e3b82d 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -784,9 +784,9 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterRunArgument FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterView.java FILE: ../../../flutter/shell/platform/android/io/flutter/view/TextureRegistry.java FILE: ../../../flutter/shell/platform/android/io/flutter/view/VsyncWaiter.java -FILE: ../../../flutter/shell/platform/android/jni/mock_jni.cc -FILE: ../../../flutter/shell/platform/android/jni/mock_jni.h FILE: ../../../flutter/shell/platform/android/jni/mock_jni_unittest.cc +FILE: ../../../flutter/shell/platform/android/jni/jni_mock.h +FILE: ../../../flutter/shell/platform/android/jni/jni_mock_unittest.cc FILE: ../../../flutter/shell/platform/android/jni/platform_view_android_jni.cc FILE: ../../../flutter/shell/platform/android/jni/platform_view_android_jni.h FILE: ../../../flutter/shell/platform/android/library_loader.cc @@ -801,8 +801,8 @@ FILE: ../../../flutter/shell/platform/android/surface/android_native_window.cc FILE: ../../../flutter/shell/platform/android/surface/android_native_window.h FILE: ../../../flutter/shell/platform/android/surface/android_surface.cc FILE: ../../../flutter/shell/platform/android/surface/android_surface.h -FILE: ../../../flutter/shell/platform/android/surface/mock_android_surface.cc -FILE: ../../../flutter/shell/platform/android/surface/mock_android_surface.h +FILE: ../../../flutter/shell/platform/android/surface/android_surface_mock.cc +FILE: ../../../flutter/shell/platform/android/surface/android_surface_mock.h FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.cc FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.h FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/basic_message_channel_unittests.cc From 66bb8139a673798cbdfa8f7633d2d4c1ff50207c Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 17 Jun 2020 22:04:58 -0700 Subject: [PATCH 17/28] Make const and pass by ref --- shell/platform/android/android_surface_gl.cc | 2 +- shell/platform/android/android_surface_gl.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index 12ba27e7a9210..b8666ac66cb87 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -14,7 +14,7 @@ namespace flutter { AndroidSurfaceGL::AndroidSurfaceGL( std::shared_ptr android_context, std::shared_ptr jni_facade, - AndroidSurface::Factory surface_factory) + const AndroidSurface::Factory& surface_factory) : external_view_embedder_(std::make_unique( std::move(android_context), std::move(jni_facade), diff --git a/shell/platform/android/android_surface_gl.h b/shell/platform/android/android_surface_gl.h index 35a25a252c1ea..7c682171d12c4 100644 --- a/shell/platform/android/android_surface_gl.h +++ b/shell/platform/android/android_surface_gl.h @@ -23,7 +23,7 @@ class AndroidSurfaceGL final : public GPUSurfaceGLDelegate, public: AndroidSurfaceGL(std::shared_ptr android_context, std::shared_ptr jni_facade, - AndroidSurface::Factory surface_factory); + const AndroidSurface::Factory& surface_factory); ~AndroidSurfaceGL() override; From cf943f81524797c639a52a6012d028a6251230d2 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 17 Jun 2020 22:08:24 -0700 Subject: [PATCH 18/28] Remove config --- shell/platform/android/surface/BUILD.gn | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/android/surface/BUILD.gn b/shell/platform/android/surface/BUILD.gn index a5ba5305d68a3..efac83534d6a2 100644 --- a/shell/platform/android/surface/BUILD.gn +++ b/shell/platform/android/surface/BUILD.gn @@ -44,7 +44,6 @@ source_set("surface_mock") { public_configs = [ "//flutter:config", - "//third_party/googletest:gmock_config", ] deps = [ From 98a861f653557217f977cfd7b688a27ddef785ae Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 17 Jun 2020 22:11:03 -0700 Subject: [PATCH 19/28] Remove dependency --- shell/platform/android/external_view_embedder/BUILD.gn | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/android/external_view_embedder/BUILD.gn b/shell/platform/android/external_view_embedder/BUILD.gn index 555e504e29489..06765a97a13d0 100644 --- a/shell/platform/android/external_view_embedder/BUILD.gn +++ b/shell/platform/android/external_view_embedder/BUILD.gn @@ -47,6 +47,5 @@ executable("android_external_view_embedder_unittests") { "//flutter/testing", "//flutter/testing:dart", "//flutter/testing:skia", - "//third_party/googletest:gmock", ] } From 99f4d817e4468bb99e9fb3eb9d82bde73fdb298a Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 17 Jun 2020 22:25:26 -0700 Subject: [PATCH 20/28] gn format --- shell/platform/android/surface/BUILD.gn | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/shell/platform/android/surface/BUILD.gn b/shell/platform/android/surface/BUILD.gn index efac83534d6a2..7764b932a9eac 100644 --- a/shell/platform/android/surface/BUILD.gn +++ b/shell/platform/android/surface/BUILD.gn @@ -42,9 +42,7 @@ source_set("surface_mock") { "android_surface_mock.h", ] - public_configs = [ - "//flutter:config", - ] + public_configs = [ "//flutter:config" ] deps = [ ":surface", From 1d4fef7492490c2f07e7e8161692b1c822682215 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Wed, 17 Jun 2020 22:51:22 -0700 Subject: [PATCH 21/28] Update license --- ci/licenses_golden/licenses_flutter | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 39188933c72d9..242783e947b27 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -785,7 +785,6 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterRunArgument FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterView.java FILE: ../../../flutter/shell/platform/android/io/flutter/view/TextureRegistry.java FILE: ../../../flutter/shell/platform/android/io/flutter/view/VsyncWaiter.java -FILE: ../../../flutter/shell/platform/android/jni/mock_jni_unittest.cc FILE: ../../../flutter/shell/platform/android/jni/jni_mock.h FILE: ../../../flutter/shell/platform/android/jni/jni_mock_unittest.cc FILE: ../../../flutter/shell/platform/android/jni/platform_view_android_jni.cc From f336612e707ad610148d2d531ab42e563251af23 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 18 Jun 2020 13:45:44 -0700 Subject: [PATCH 22/28] Fix CI --- BUILD.gn | 4 ++-- shell/platform/android/android_surface_vulkan.cc | 6 +++--- testing/run_tests.py | 9 ++++----- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index d6d634a9a69d0..fe269de24cfae 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -74,8 +74,6 @@ group("flutter") { "//flutter/lib/ui:ui_unittests", "//flutter/runtime:runtime_unittests", "//flutter/shell/common:shell_unittests", - "//flutter/shell/platform/android/external_view_embedder:android_external_view_embedder_unittests", - "//flutter/shell/platform/android/jni:jni_unittests", "//flutter/shell/platform/embedder:embedder_unittests", "//flutter/testing:testing_unittests", "//flutter/third_party/txt:txt_unittests", @@ -86,6 +84,8 @@ group("flutter") { "//flutter/fml:fml_benchmarks", "//flutter/lib/ui:ui_benchmarks", "//flutter/shell/common:shell_benchmarks", + "//flutter/shell/platform/android/external_view_embedder:android_external_view_embedder_unittests", + "//flutter/shell/platform/android/jni:jni_unittests", "//flutter/third_party/txt:txt_benchmarks", ] } diff --git a/shell/platform/android/android_surface_vulkan.cc b/shell/platform/android/android_surface_vulkan.cc index b672070771de8..fea06f09274c8 100644 --- a/shell/platform/android/android_surface_vulkan.cc +++ b/shell/platform/android/android_surface_vulkan.cc @@ -16,11 +16,11 @@ AndroidSurfaceVulkan::AndroidSurfaceVulkan( std::shared_ptr android_context, std::shared_ptr jni_facade, AndroidSurface::Factory surface_factory) - : proc_table_(fml::MakeRefCounted()), - external_view_embedder_(std::make_unique( + : external_view_embedder_(std::make_unique( std::move(android_context), std::move(jni_facade), - surface_factory)) {} + surface_factory)), + proc_table_(fml::MakeRefCounted()) {} AndroidSurfaceVulkan::~AndroidSurfaceVulkan() = default; diff --git a/testing/run_tests.py b/testing/run_tests.py index a270fba412fd7..b799030aba56c 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -131,18 +131,17 @@ def RunCCTests(build_dir, filter): RunEngineExecutable(build_dir, 'runtime_unittests', filter, shuffle_flags) - # https://github.com/flutter/flutter/issues/36295 if not IsWindows(): + # https://github.com/flutter/flutter/issues/36295 RunEngineExecutable(build_dir, 'shell_unittests', filter, shuffle_flags) + # https://github.com/google/googletest/issues/2490 + RunEngineExecutable(build_dir, 'android_external_view_embedder_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'jni_unittests', filter, shuffle_flags) RunEngineExecutable(build_dir, 'ui_unittests', filter, shuffle_flags) RunEngineExecutable(build_dir, 'testing_unittests', filter, shuffle_flags) - RunEngineExecutable(build_dir, 'jni_unittests', filter, shuffle_flags) - - RunEngineExecutable(build_dir, 'android_external_view_embedder_unittests', filter, shuffle_flags) - # These unit-tests are Objective-C and can only run on Darwin. if IsMac(): RunEngineExecutable(build_dir, 'flutter_channels_unittests', filter, shuffle_flags) From f47e7185dcd3a60f9a00a6a4aed4bad46e712979 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 18 Jun 2020 18:14:13 -0700 Subject: [PATCH 23/28] Feedback --- .../external_view_embedder.cc | 48 ++++++++++--------- .../external_view_embedder.h | 19 ++++++-- .../external_view_embedder_unittests.cc | 7 ++- .../external_view_embedder/surface_pool.cc | 5 ++ .../surface_pool_unittests.cc | 40 +++++++++------- .../platform/android/platform_view_android.cc | 2 +- 6 files changed, 75 insertions(+), 46 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 3b70f5dd8d627..aa4a8cc7cb669 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -26,24 +26,24 @@ void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView( "AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView"); auto rtree_factory = RTreeFactory(); - view_rtrees_[view_id] = rtree_factory.getInstance(); - picture_recorders_[view_id] = std::make_unique(); - picture_recorders_[view_id]->beginRecording(SkRect::Make(frame_size_), - &rtree_factory); + view_rtrees_.insert({view_id, rtree_factory.getInstance()}); + picture_recorders_.insert({view_id, std::make_unique()}); + picture_recorders_.at(view_id)->beginRecording(SkRect::Make(frame_size_), + &rtree_factory); composition_order_.push_back(view_id); - // Update params if they changed. + // Update params only if they changed. if (view_params_.count(view_id) == 1 && - view_params_[view_id] == *params.get()) { + view_params_.at(view_id) == *params.get()) { return; } - view_params_[view_id] = EmbeddedViewParams(*params.get()); + view_params_.insert_or_assign(view_id, EmbeddedViewParams(*params.get())); } // |ExternalViewEmbedder| SkCanvas* AndroidExternalViewEmbedder::CompositeEmbeddedView(int view_id) { if (picture_recorders_.count(view_id) == 1) { - return picture_recorders_[view_id]->getRecordingCanvas(); + return picture_recorders_.at(view_id)->getRecordingCanvas(); } return nullptr; } @@ -53,14 +53,15 @@ std::vector AndroidExternalViewEmbedder::GetCurrentCanvases() { std::vector canvases; for (size_t i = 0; i < composition_order_.size(); i++) { int64_t view_id = composition_order_[i]; - canvases.push_back(picture_recorders_[view_id]->getRecordingCanvas()); + canvases.push_back(picture_recorders_.at(view_id)->getRecordingCanvas()); } return canvases; } -SkRect AndroidExternalViewEmbedder::GetViewRect(int view_id) { - EmbeddedViewParams* params = &view_params_[view_id]; +SkRect AndroidExternalViewEmbedder::GetViewRect(int view_id) const { + const EmbeddedViewParams* params = &(view_params_.at(view_id)); FML_CHECK(params); + // TODO(egarciad): The rect should be computed from the mutator stack. return SkRect::MakeXYWH(params->offsetPixels.x(), // params->offsetPixels.y(), // params->sizePoints.width() * device_pixel_ratio_, // @@ -79,8 +80,8 @@ bool AndroidExternalViewEmbedder::SubmitFrame( return true; } - std::map> overlay_layers; - std::map> pictures; + std::unordered_map> overlay_layers; + std::unordered_map> pictures; SkCanvas* background_canvas = frame->SkiaCanvas(); // Restore the clip context after exiting this method since it's changed @@ -100,8 +101,11 @@ bool AndroidExternalViewEmbedder::SubmitFrame( view_rect.height() // ); - pictures[view_id] = picture_recorders_[view_id]->finishRecordingAsPicture(); - sk_sp rtree = view_rtrees_[view_id]; + sk_sp picture = + picture_recorders_.at(view_id)->finishRecordingAsPicture(); + FML_CHECK(picture); + pictures.insert({view_id, picture}); + sk_sp rtree = view_rtrees_.at(view_id); // Determinate if Flutter UI intersects with any of the previous // platform views stacked by z position. // @@ -146,9 +150,9 @@ bool AndroidExternalViewEmbedder::SubmitFrame( // drawn on the overlay layer. background_canvas->clipRect(intersection_rect, SkClipOp::kDifference); } - overlay_layers[current_view_id] = intersection_rects; + overlay_layers.insert({current_view_id, intersection_rects}); } - background_canvas->drawPicture(pictures[view_id]); + background_canvas->drawPicture(pictures.at(view_id)); } // Submit the background canvas frame before switching the GL context to // the surfaces above. @@ -156,11 +160,11 @@ bool AndroidExternalViewEmbedder::SubmitFrame( for (size_t i = 0; i < composition_order_.size(); i++) { int64_t view_id = composition_order_[i]; - for (SkRect& overlay_rect : overlay_layers[view_id]) { - CreateSurfaceIfNeeded(context, // - view_id, // - pictures[view_id], // - overlay_rect // + for (SkRect& overlay_rect : overlay_layers.at(view_id)) { + CreateSurfaceIfNeeded(context, // + view_id, // + pictures.at(view_id), // + overlay_rect // ) ->Submit(); } 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 019d6932a0c66..166509b780396 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -14,6 +14,16 @@ namespace flutter { +//------------------------------------------------------------------------------ +/// Allows to embed Android views into a Flutter application. +/// +/// This class calls Java methods via |PlatformViewAndroidJNI| to manage the +/// lifecycle of the Android view corresponding to |flutter::PlatformViewLayer|. +/// +/// It also orchestrates overlay surfaces. These are Android views +/// that render above (by Z order) the Android view corresponding to +/// |flutter::PlatformViewLayer|. +/// class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { public: AndroidExternalViewEmbedder( @@ -57,7 +67,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // Gets the rect based on the device pixel ratio of a platform view displayed // on the screen. - SkRect GetViewRect(int view_id); + SkRect GetViewRect(int view_id) const; private: static const int kMaxLayerAllocations = 2; @@ -99,14 +109,15 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // The platform view's picture recorder keyed off the platform view id, which // contains any subsequent operation until the next platform view or the end // of the last leaf node in the layer tree. - std::map> picture_recorders_; + std::unordered_map> + picture_recorders_; // The params for a platform view, which contains the size, position and // mutation stack. - std::map view_params_; + std::unordered_map view_params_; // The r-tree that captures the operations for the picture recorders. - std::map> view_rtrees_; + std::unordered_map> view_rtrees_; // Resets the state. void Reset(); 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 23eb430cf1f11..03a33a8ef29be 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 @@ -223,8 +223,6 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__RecycleSurfaces) { [gr_context, window, frame_size]( std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto android_surface_mock = std::make_unique(); - auto surface_mock = std::make_unique(); auto surface_frame_1 = std::make_unique( SkSurface::MakeNull(1000, 1000), false, [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { @@ -236,15 +234,20 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__RecycleSurfaces) { return true; }); + auto surface_mock = std::make_unique(); EXPECT_CALL(*surface_mock, AcquireFrame(frame_size)) .Times(2 /* frames */) .WillOnce(Return(ByMove(std::move(surface_frame_1)))) .WillOnce(Return(ByMove(std::move(surface_frame_2)))); + auto android_surface_mock = std::make_unique(); + EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); + EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get())) .WillOnce(Return(ByMove(std::move(surface_mock)))); EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); + return android_surface_mock; }; auto embedder = std::make_unique( diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index f0b818e804e61..5252e3df21e34 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -28,6 +28,11 @@ std::shared_ptr SurfacePool::GetLayer( if (available_layer_index_ >= layers_.size()) { std::unique_ptr android_surface = surface_factory(android_context, jni_facade); + + FML_CHECK(android_surface && android_surface->IsValid()) + << "Could not create an OpenGL, Vulkan or Software surface to setup " + "rendering."; + std::unique_ptr java_metadata = jni_facade->FlutterViewCreateOverlaySurface(); diff --git a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc index 5393c81f02159..5e3d20afc25be 100644 --- a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc +++ b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc @@ -33,10 +33,11 @@ TEST(SurfacePool, GetLayer__AllocateOneLayer) { auto surface_factory = [gr_context, window](std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto mock_surface = std::make_unique(); - EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); - EXPECT_CALL(*mock_surface, SetNativeWindow(window)); - return mock_surface; + auto android_surface_mock = std::make_unique(); + EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get())); + EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); + EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); + return android_surface_mock; }; auto layer = pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); @@ -62,10 +63,11 @@ TEST(SurfacePool, GetUnusedLayers) { auto surface_factory = [gr_context, window](std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto mock_surface = std::make_unique(); - EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); - EXPECT_CALL(*mock_surface, SetNativeWindow(window)); - return mock_surface; + auto android_surface_mock = std::make_unique(); + EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get())); + EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); + EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); + return android_surface_mock; }; auto layer = pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); @@ -96,13 +98,16 @@ TEST(SurfacePool, GetLayer__Recycle) { [gr_context_1, gr_context_2, window]( std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto mock_surface = std::make_unique(); + auto android_surface_mock = std::make_unique(); // Allocate two GPU surfaces for each gr context. - EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context_1.get())); - EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context_2.get())); + EXPECT_CALL(*android_surface_mock, + CreateGPUSurface(gr_context_1.get())); + EXPECT_CALL(*android_surface_mock, + CreateGPUSurface(gr_context_2.get())); // Set the native window once. - EXPECT_CALL(*mock_surface, SetNativeWindow(window)); - return mock_surface; + EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); + EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); + return android_surface_mock; }; auto layer_1 = pool->GetLayer(gr_context_1.get(), android_context, jni_mock, surface_factory); @@ -139,10 +144,11 @@ TEST(SurfacePool, GetLayer__AllocateTwoLayers) { auto surface_factory = [gr_context, window](std::shared_ptr android_context, std::shared_ptr jni_facade) { - auto mock_surface = std::make_unique(); - EXPECT_CALL(*mock_surface, CreateGPUSurface(gr_context.get())); - EXPECT_CALL(*mock_surface, SetNativeWindow(window)); - return mock_surface; + auto android_surface_mock = std::make_unique(); + EXPECT_CALL(*android_surface_mock, CreateGPUSurface(gr_context.get())); + EXPECT_CALL(*android_surface_mock, SetNativeWindow(window)); + EXPECT_CALL(*android_surface_mock, IsValid()).WillOnce(Return(true)); + return android_surface_mock; }; auto layer_1 = pool->GetLayer(gr_context.get(), android_context, jni_mock, surface_factory); diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 1fa59abef5011..51eb01fee297b 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -74,7 +74,7 @@ PlatformViewAndroid::PlatformViewAndroid( << "Could not create an Android context."; android_surface_ = SurfaceFactory(android_context, jni_facade); - FML_CHECK(android_surface_) + FML_CHECK(android_surface_ && android_surface_->IsValid()) << "Could not create an OpenGL, Vulkan or Software surface to setup " "rendering."; } From 887a6b3b2ce6e9e2f5643c85bdd9df70862426d8 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 18 Jun 2020 18:24:23 -0700 Subject: [PATCH 24/28] Link to issue in TODO --- .../android/external_view_embedder/external_view_embedder.cc | 1 + 1 file changed, 1 insertion(+) 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 aa4a8cc7cb669..9e9c06e30b2c5 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -62,6 +62,7 @@ SkRect AndroidExternalViewEmbedder::GetViewRect(int view_id) const { const EmbeddedViewParams* params = &(view_params_.at(view_id)); FML_CHECK(params); // 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_, // From 73635168708e0842c6ec295c262c8336ef0bda28 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Thu, 18 Jun 2020 18:38:13 -0700 Subject: [PATCH 25/28] typedef -> using --- shell/platform/android/surface/android_surface.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/shell/platform/android/surface/android_surface.h b/shell/platform/android/surface/android_surface.h index 053eb94e999ce..26604f4ff7717 100644 --- a/shell/platform/android/surface/android_surface.h +++ b/shell/platform/android/surface/android_surface.h @@ -16,10 +16,9 @@ namespace flutter { class AndroidSurface { public: - typedef std::function( + using Factory = std::function( std::shared_ptr android_context, - std::shared_ptr jni_facade)> - Factory; + std::shared_ptr jni_facade)>; virtual ~AndroidSurface(); From fed997794941c56aed849b7fc3c04a3f3a2cdcbe Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 19 Jun 2020 16:42:07 -0700 Subject: [PATCH 26/28] Address Jason feedback --- .../external_view_embedder.cc | 37 ++++++++----------- .../external_view_embedder.h | 2 +- .../external_view_embedder/surface_pool.cc | 7 ++-- .../external_view_embedder/surface_pool.h | 2 +- .../surface_pool_unittests.cc | 9 +++-- 5 files changed, 28 insertions(+), 29 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 9e9c06e30b2c5..33afabde78e86 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -28,9 +28,10 @@ void AndroidExternalViewEmbedder::PrerollCompositeEmbeddedView( auto rtree_factory = RTreeFactory(); view_rtrees_.insert({view_id, rtree_factory.getInstance()}); - picture_recorders_.insert({view_id, std::make_unique()}); - picture_recorders_.at(view_id)->beginRecording(SkRect::Make(frame_size_), - &rtree_factory); + auto picture_recorder = std::make_unique(); + picture_recorder->beginRecording(SkRect::Make(frame_size_), &rtree_factory); + + picture_recorders_.insert({view_id, std::move(picture_recorder)}); composition_order_.push_back(view_id); // Update params only if they changed. if (view_params_.count(view_id) == 1 && @@ -59,14 +60,13 @@ std::vector AndroidExternalViewEmbedder::GetCurrentCanvases() { } SkRect AndroidExternalViewEmbedder::GetViewRect(int view_id) const { - const EmbeddedViewParams* params = &(view_params_.at(view_id)); - FML_CHECK(params); + 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.offsetPixels.x(), // + params.offsetPixels.y(), // + params.sizePoints.width() * device_pixel_ratio_, // + params.sizePoints.height() * device_pixel_ratio_ // ); } @@ -113,15 +113,15 @@ bool AndroidExternalViewEmbedder::SubmitFrame( // This is done by querying the r-tree that holds the records for the // picture recorder corresponding to the flow layers added after a platform // view layer. - for (size_t j = i + 1; j > 0; j--) { - int64_t current_view_id = composition_order_[j - 1]; + for (ssize_t j = i; j >= 0; j--) { + int64_t current_view_id = composition_order_[j]; SkRect current_view_rect = GetViewRect(current_view_id); // Each rect corresponds to a native view that renders Flutter UI. std::list intersection_rects = rtree->searchNonOverlappingDrawnRects(current_view_rect); auto allocation_size = intersection_rects.size(); - // Limit the number of native views, so it doesn't grow for ever. + // Limit the number of native views, so it doesn't grow forever. // // In this case, the rects are merged into a single one that is the union // of all the rects. @@ -142,11 +142,7 @@ bool AndroidExternalViewEmbedder::SubmitFrame( // To workaround it, round the floating point bounds and make the rect // slighly larger. For example, {0.3, 0.5, 3.1, 4.7} becomes {0, 0, 4, // 5}. - intersection_rect.setLTRB(std::floor(intersection_rect.left()), // - std::floor(intersection_rect.top()), // - std::ceil(intersection_rect.right()), // - std::ceil(intersection_rect.bottom()) // - ); + intersection_rect.set(intersection_rect.roundOut()); // Clip the background canvas, so it doesn't contain any of the pixels // drawn on the overlay layer. background_canvas->clipRect(intersection_rect, SkClipOp::kDifference); @@ -159,9 +155,8 @@ bool AndroidExternalViewEmbedder::SubmitFrame( // the surfaces above. frame->Submit(); - for (size_t i = 0; i < composition_order_.size(); i++) { - int64_t view_id = composition_order_[i]; - for (SkRect& overlay_rect : overlay_layers.at(view_id)) { + for (int64_t view_id : composition_order_) { + for (const SkRect& overlay_rect : overlay_layers.at(view_id)) { CreateSurfaceIfNeeded(context, // view_id, // pictures.at(view_id), // @@ -178,7 +173,7 @@ std::unique_ptr AndroidExternalViewEmbedder::CreateSurfaceIfNeeded(GrContext* context, int64_t view_id, sk_sp picture, - SkRect& rect) { + const SkRect& rect) { std::shared_ptr layer = surface_pool_->GetLayer( context, android_context_, jni_facade_, surface_factory_); 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 166509b780396..e343230855f3f 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -127,7 +127,7 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { std::unique_ptr CreateSurfaceIfNeeded(GrContext* context, int64_t view_id, sk_sp picture, - SkRect& rect); + const SkRect& rect); }; } // namespace flutter diff --git a/shell/platform/android/external_view_embedder/surface_pool.cc b/shell/platform/android/external_view_embedder/surface_pool.cc index 5252e3df21e34..8258234686b73 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.cc +++ b/shell/platform/android/external_view_embedder/surface_pool.cc @@ -24,6 +24,7 @@ std::shared_ptr SurfacePool::GetLayer( std::shared_ptr android_context, std::shared_ptr jni_facade, const AndroidSurface::Factory& surface_factory) { + intptr_t gr_context_key = reinterpret_cast(gr_context); // Allocate a new surface if there isn't one available. if (available_layer_index_ >= layers_.size()) { std::unique_ptr android_surface = @@ -47,14 +48,14 @@ std::shared_ptr SurfacePool::GetLayer( std::move(android_surface), // std::move(surface) // ); - layer->gr_context = gr_context; + layer->gr_context_key = gr_context_key; layers_.push_back(layer); } std::shared_ptr layer = layers_[available_layer_index_]; // Since the surfaces are recycled, it's possible that the GrContext is // different. - if (gr_context != layer->gr_context) { - layer->gr_context = gr_context; + if (gr_context_key != layer->gr_context_key) { + layer->gr_context_key = gr_context_key; // The overlay already exists, but the GrContext was changed so we need to // recreate the rendering surface with the new GrContext. std::unique_ptr surface = diff --git a/shell/platform/android/external_view_embedder/surface_pool.h b/shell/platform/android/external_view_embedder/surface_pool.h index 88bf01ead1d50..e4eee52d66042 100644 --- a/shell/platform/android/external_view_embedder/surface_pool.h +++ b/shell/platform/android/external_view_embedder/surface_pool.h @@ -38,7 +38,7 @@ struct OverlayLayer { // so we can update the overlay with the new context. // // This may change when the overlay is recycled. - GrContext* gr_context; + intptr_t gr_context_key; }; // This class isn't thread safe. diff --git a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc index 5e3d20afc25be..72488c9cb03aa 100644 --- a/shell/platform/android/external_view_embedder/surface_pool_unittests.cc +++ b/shell/platform/android/external_view_embedder/surface_pool_unittests.cc @@ -43,7 +43,8 @@ TEST(SurfacePool, GetLayer__AllocateOneLayer) { surface_factory); ASSERT_NE(nullptr, layer); - ASSERT_EQ(gr_context.get(), layer->gr_context); + ASSERT_EQ(reinterpret_cast(gr_context.get()), + layer->gr_context_key); } TEST(SurfacePool, GetUnusedLayers) { @@ -119,8 +120,10 @@ TEST(SurfacePool, GetLayer__Recycle) { ASSERT_NE(nullptr, layer_1); ASSERT_EQ(layer_1, layer_2); - ASSERT_EQ(gr_context_2.get(), layer_1->gr_context); - ASSERT_EQ(gr_context_2.get(), layer_2->gr_context); + ASSERT_EQ(reinterpret_cast(gr_context_2.get()), + layer_1->gr_context_key); + ASSERT_EQ(reinterpret_cast(gr_context_2.get()), + layer_2->gr_context_key); } TEST(SurfacePool, GetLayer__AllocateTwoLayers) { From b7e117e9f24f9edcb19d92f70e56ac2aa771fceb Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Fri, 19 Jun 2020 21:16:05 -0700 Subject: [PATCH 27/28] Fix issues --- shell/platform/android/android_surface_gl.cc | 8 +++---- .../android/android_surface_software.cc | 8 +++---- .../android/android_surface_vulkan.cc | 8 +++---- .../external_view_embedder.cc | 4 ++-- .../platform/android/platform_view_android.cc | 23 ++++++++----------- 5 files changed, 24 insertions(+), 27 deletions(-) diff --git a/shell/platform/android/android_surface_gl.cc b/shell/platform/android/android_surface_gl.cc index b8666ac66cb87..f4bd9e6747907 100644 --- a/shell/platform/android/android_surface_gl.cc +++ b/shell/platform/android/android_surface_gl.cc @@ -15,10 +15,10 @@ AndroidSurfaceGL::AndroidSurfaceGL( std::shared_ptr android_context, std::shared_ptr jni_facade, const AndroidSurface::Factory& surface_factory) - : external_view_embedder_(std::make_unique( - std::move(android_context), - std::move(jni_facade), - surface_factory)), + : external_view_embedder_( + std::make_unique(android_context, + jni_facade, + surface_factory)), android_context_( std::static_pointer_cast(android_context)), native_window_(nullptr), diff --git a/shell/platform/android/android_surface_software.cc b/shell/platform/android/android_surface_software.cc index 07362e74a59e2..7face67038d0a 100644 --- a/shell/platform/android/android_surface_software.cc +++ b/shell/platform/android/android_surface_software.cc @@ -40,10 +40,10 @@ AndroidSurfaceSoftware::AndroidSurfaceSoftware( std::shared_ptr android_context, std::shared_ptr jni_facade, AndroidSurface::Factory surface_factory) - : external_view_embedder_(std::make_unique( - std::move(android_context), - std::move(jni_facade), - surface_factory)) { + : external_view_embedder_( + std::make_unique(android_context, + jni_facade, + surface_factory)) { GetSkColorType(WINDOW_FORMAT_RGBA_8888, &target_color_type_, &target_alpha_type_); } diff --git a/shell/platform/android/android_surface_vulkan.cc b/shell/platform/android/android_surface_vulkan.cc index fea06f09274c8..4c428e931637c 100644 --- a/shell/platform/android/android_surface_vulkan.cc +++ b/shell/platform/android/android_surface_vulkan.cc @@ -16,10 +16,10 @@ AndroidSurfaceVulkan::AndroidSurfaceVulkan( std::shared_ptr android_context, std::shared_ptr jni_facade, AndroidSurface::Factory surface_factory) - : external_view_embedder_(std::make_unique( - std::move(android_context), - std::move(jni_facade), - surface_factory)), + : external_view_embedder_( + std::make_unique(android_context, + jni_facade, + surface_factory)), proc_table_(fml::MakeRefCounted()) {} AndroidSurfaceVulkan::~AndroidSurfaceVulkan() = default; 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 33afabde78e86..36b8a65cd2dbd 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -13,8 +13,8 @@ AndroidExternalViewEmbedder::AndroidExternalViewEmbedder( std::shared_ptr jni_facade, const AndroidSurface::Factory& surface_factory) : ExternalViewEmbedder(), - android_context_(std::move(android_context)), - jni_facade_(std::move(jni_facade)), + android_context_(android_context), + jni_facade_(jni_facade), surface_factory_(surface_factory), surface_pool_(std::make_unique()) {} diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 51eb01fee297b..c7aaf1ef6b3a7 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -29,25 +29,22 @@ namespace flutter { std::unique_ptr SurfaceFactory( std::shared_ptr android_context, std::shared_ptr jni_facade) { - std::unique_ptr surface; + FML_CHECK(SurfaceFactory); switch (android_context->RenderingApi()) { case AndroidRenderingAPI::kSoftware: - surface = std::make_unique( + return std::make_unique( android_context, jni_facade, SurfaceFactory); - break; case AndroidRenderingAPI::kOpenGLES: - surface = std::make_unique(android_context, jni_facade, - SurfaceFactory); - break; + return std::make_unique(android_context, jni_facade, + SurfaceFactory); case AndroidRenderingAPI::kVulkan: #if SHELL_ENABLE_VULKAN - surface = std::make_unique( - android_context, jni_facade, SurfaceFactory); + return std::make_unique(android_context, jni_facade, + SurfaceFactory); #endif // SHELL_ENABLE_VULKAN - break; + return nullptr; } - FML_CHECK(surface); - return surface->IsValid() ? std::move(surface) : nullptr; + return nullptr; } PlatformViewAndroid::PlatformViewAndroid( @@ -70,10 +67,10 @@ PlatformViewAndroid::PlatformViewAndroid( fml::MakeRefCounted()); #endif // SHELL_ENABLE_VULKAN } - FML_CHECK(android_context->IsValid()) + FML_CHECK(android_context && android_context->IsValid()) << "Could not create an Android context."; - android_surface_ = SurfaceFactory(android_context, jni_facade); + 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."; From 8aac6f5d2be6fb33f1192b8c84d44c3f46c853f7 Mon Sep 17 00:00:00 2001 From: Emmanuel Garcia Date: Sat, 20 Jun 2020 09:45:03 -0700 Subject: [PATCH 28/28] Pass thread merger to BeginFrame --- flow/embedded_views.h | 8 +- fml/raster_thread_merger.cc | 11 ++- fml/raster_thread_merger.h | 5 +- fml/raster_thread_merger_unittests.cc | 6 ++ shell/common/rasterizer.cc | 6 +- .../shell_test_external_view_embedder.cc | 8 +- .../shell_test_external_view_embedder.h | 8 +- .../external_view_embedder.cc | 18 +++- .../external_view_embedder.h | 8 +- .../external_view_embedder_unittests.cc | 97 +++++++++++++------ shell/platform/darwin/ios/ios_surface.h | 6 +- shell/platform/darwin/ios/ios_surface.mm | 5 +- .../embedder_external_view_embedder.cc | 8 +- .../embedder_external_view_embedder.h | 8 +- 14 files changed, 142 insertions(+), 60 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index 8bbef5a0a3f4d..9cc90860101a8 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -227,9 +227,11 @@ class ExternalViewEmbedder { // sets the stage for the next pre-roll. virtual void CancelFrame() = 0; - virtual void BeginFrame(SkISize frame_size, - GrContext* context, - double device_pixel_ratio) = 0; + virtual void BeginFrame( + SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) = 0; virtual void PrerollCompositeEmbeddedView( int view_id, diff --git a/fml/raster_thread_merger.cc b/fml/raster_thread_merger.cc index 718b41ce327d3..b424c04b2754e 100644 --- a/fml/raster_thread_merger.cc +++ b/fml/raster_thread_merger.cc @@ -28,12 +28,15 @@ void RasterThreadMerger::MergeWithLease(size_t lease_term) { } } -bool RasterThreadMerger::IsOnRasterizingThread() { - const auto current_queue_id = MessageLoop::GetCurrentTaskQueueId(); +bool RasterThreadMerger::IsOnPlatformThread() const { + return MessageLoop::GetCurrentTaskQueueId() == platform_queue_id_; +} + +bool RasterThreadMerger::IsOnRasterizingThread() const { if (is_merged_) { - return current_queue_id == platform_queue_id_; + return IsOnPlatformThread(); } else { - return current_queue_id == gpu_queue_id_; + return !IsOnPlatformThread(); } } diff --git a/fml/raster_thread_merger.h b/fml/raster_thread_merger.h index 1e12e3c41d9b8..7c0318ff77d26 100644 --- a/fml/raster_thread_merger.h +++ b/fml/raster_thread_merger.h @@ -44,7 +44,10 @@ class RasterThreadMerger // Returns true if the current thread owns rasterizing. // When the threads are merged, platform thread owns rasterizing. // When un-merged, raster thread owns rasterizing. - bool IsOnRasterizingThread(); + bool IsOnRasterizingThread() const; + + // Returns true if the current thread is the platform thread. + bool IsOnPlatformThread() const; private: static const int kLeaseNotSet; diff --git a/fml/raster_thread_merger_unittests.cc b/fml/raster_thread_merger_unittests.cc index 40ae9ba5351e2..a182723a79191 100644 --- a/fml/raster_thread_merger_unittests.cc +++ b/fml/raster_thread_merger_unittests.cc @@ -92,12 +92,14 @@ TEST(RasterThreadMerger, IsNotOnRasterizingThread) { loop1->GetTaskRunner()->PostTask([&]() { ASSERT_FALSE(raster_thread_merger_->IsOnRasterizingThread()); + ASSERT_TRUE(raster_thread_merger_->IsOnPlatformThread()); ASSERT_EQ(fml::MessageLoop::GetCurrentTaskQueueId(), qid1); pre_merge.CountDown(); }); loop2->GetTaskRunner()->PostTask([&]() { ASSERT_TRUE(raster_thread_merger_->IsOnRasterizingThread()); + ASSERT_FALSE(raster_thread_merger_->IsOnPlatformThread()); ASSERT_EQ(fml::MessageLoop::GetCurrentTaskQueueId(), qid2); pre_merge.CountDown(); }); @@ -108,6 +110,7 @@ TEST(RasterThreadMerger, IsNotOnRasterizingThread) { loop1->GetTaskRunner()->PostTask([&]() { ASSERT_TRUE(raster_thread_merger_->IsOnRasterizingThread()); + ASSERT_TRUE(raster_thread_merger_->IsOnPlatformThread()); ASSERT_EQ(fml::MessageLoop::GetCurrentTaskQueueId(), qid1); post_merge.CountDown(); }); @@ -116,6 +119,7 @@ TEST(RasterThreadMerger, IsNotOnRasterizingThread) { // this will be false since this is going to be run // on loop1 really. ASSERT_TRUE(raster_thread_merger_->IsOnRasterizingThread()); + ASSERT_TRUE(raster_thread_merger_->IsOnPlatformThread()); ASSERT_EQ(fml::MessageLoop::GetCurrentTaskQueueId(), qid1); post_merge.CountDown(); }); @@ -126,12 +130,14 @@ TEST(RasterThreadMerger, IsNotOnRasterizingThread) { loop1->GetTaskRunner()->PostTask([&]() { ASSERT_FALSE(raster_thread_merger_->IsOnRasterizingThread()); + ASSERT_TRUE(raster_thread_merger_->IsOnPlatformThread()); ASSERT_EQ(fml::MessageLoop::GetCurrentTaskQueueId(), qid1); post_unmerge.CountDown(); }); loop2->GetTaskRunner()->PostTask([&]() { ASSERT_TRUE(raster_thread_merger_->IsOnRasterizingThread()); + ASSERT_FALSE(raster_thread_merger_->IsOnPlatformThread()); ASSERT_EQ(fml::MessageLoop::GetCurrentTaskQueueId(), qid2); post_unmerge.CountDown(); }); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 076c42937076f..afcce2960a6a6 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -391,9 +391,9 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) { SkCanvas* embedder_root_canvas = nullptr; if (external_view_embedder != nullptr) { - external_view_embedder->BeginFrame(layer_tree.frame_size(), - surface_->GetContext(), - layer_tree.device_pixel_ratio()); + external_view_embedder->BeginFrame( + layer_tree.frame_size(), surface_->GetContext(), + layer_tree.device_pixel_ratio(), raster_thread_merger_); embedder_root_canvas = external_view_embedder->GetRootCanvas(); } diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index 3930ab7b61717..306c1b0016b44 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -6,9 +6,11 @@ namespace flutter { void ShellTestExternalViewEmbedder::CancelFrame() {} // |ExternalViewEmbedder| -void ShellTestExternalViewEmbedder::BeginFrame(SkISize frame_size, - GrContext* context, - double device_pixel_ratio) {} +void ShellTestExternalViewEmbedder::BeginFrame( + SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) {} // |ExternalViewEmbedder| void ShellTestExternalViewEmbedder::PrerollCompositeEmbeddedView( diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 765ae5c8334e7..da96503a9bb46 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -29,9 +29,11 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { void CancelFrame() override; // |ExternalViewEmbedder| - void BeginFrame(SkISize frame_size, - GrContext* context, - double device_pixel_ratio) override; + void BeginFrame( + SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) override; // |ExternalViewEmbedder| void PrerollCompositeEmbeddedView( 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 36b8a65cd2dbd..9485eedff5491 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -234,13 +234,18 @@ void AndroidExternalViewEmbedder::Reset() { } // |ExternalViewEmbedder| -void AndroidExternalViewEmbedder::BeginFrame(SkISize frame_size, - GrContext* context, - double device_pixel_ratio) { +void AndroidExternalViewEmbedder::BeginFrame( + SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) { Reset(); frame_size_ = frame_size; device_pixel_ratio_ = device_pixel_ratio; - jni_facade_->FlutterViewBeginFrame(); + // JNI method must be called on the platform thread. + if (raster_thread_merger->IsOnPlatformThread()) { + jni_facade_->FlutterViewBeginFrame(); + } } // |ExternalViewEmbedder| @@ -256,7 +261,10 @@ void AndroidExternalViewEmbedder::EndFrame( should_run_rasterizer_on_platform_thread_ = false; } surface_pool_->RecycleLayers(); - jni_facade_->FlutterViewEndFrame(); + // JNI method must be called on the platform thread. + if (raster_thread_merger->IsOnPlatformThread()) { + jni_facade_->FlutterViewEndFrame(); + } } } // namespace flutter 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 e343230855f3f..41b6e68f1155d 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -54,9 +54,11 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { SkCanvas* GetRootCanvas() override; // |ExternalViewEmbedder| - void BeginFrame(SkISize frame_size, - GrContext* context, - double device_pixel_ratio) override; + void BeginFrame( + SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) override; // |ExternalViewEmbedder| void CancelFrame() override; 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 03a33a8ef29be..bba1c126564a1 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 @@ -42,14 +42,41 @@ class SurfaceMock : public Surface { (override)); }; +fml::RefPtr GetThreadMergerFromPlatformThread() { + auto rasterizer_thread = new fml::Thread("rasterizer"); + auto rasterizer_queue_id = + rasterizer_thread->GetTaskRunner()->GetTaskQueueId(); + + // Assume the current thread is the platform thread. + fml::MessageLoop::EnsureInitializedForCurrentThread(); + auto platform_queue_id = fml::MessageLoop::GetCurrentTaskQueueId(); + + return fml::MakeRefCounted(platform_queue_id, + rasterizer_queue_id); +} + +fml::RefPtr GetThreadMergerFromRasterThread() { + auto platform_thread = new fml::Thread("rasterizer"); + auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); + + // Assume the current thread is the raster thread. + fml::MessageLoop::EnsureInitializedForCurrentThread(); + auto rasterizer_queue_id = fml::MessageLoop::GetCurrentTaskQueueId(); + + return fml::MakeRefCounted(platform_queue_id, + rasterizer_queue_id); +} + TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { auto jni_mock = std::make_shared(); - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); - auto embedder = std::make_unique(nullptr, jni_mock, nullptr); - embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); + auto raster_thread_merger = GetThreadMergerFromPlatformThread(); + + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); + embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0, + raster_thread_merger); embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); @@ -64,11 +91,14 @@ TEST(AndroidExternalViewEmbedder, GetCurrentCanvases) { TEST(AndroidExternalViewEmbedder, GetCurrentCanvases__CompositeOrder) { auto jni_mock = std::make_shared(); - EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); auto embedder = std::make_unique(nullptr, jni_mock, nullptr); - embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); + auto raster_thread_merger = GetThreadMergerFromPlatformThread(); + + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); + embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0, + raster_thread_merger); embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); @@ -110,21 +140,15 @@ TEST(AndroidExternalViewEmbedder, CancelFrame) { TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { auto jni_mock = std::make_shared(); - auto embedder = std::make_unique(nullptr, jni_mock, nullptr); - auto platform_thread = new fml::Thread("platform"); - auto rasterizer_thread = new fml::Thread("rasterizer"); - auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); - auto rasterizer_queue_id = - rasterizer_thread->GetTaskRunner()->GetTaskQueueId(); - auto raster_thread_merger = fml::MakeRefCounted( - platform_queue_id, rasterizer_queue_id); + auto raster_thread_merger = GetThreadMergerFromPlatformThread(); ASSERT_FALSE(raster_thread_merger->IsMerged()); EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); - embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0); + embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0, + raster_thread_merger); // Push a platform view. embedder->PrerollCompositeEmbeddedView( 0, std::make_unique()); @@ -148,17 +172,10 @@ TEST(AndroidExternalViewEmbedder, RasterizerRunsOnPlatformThread) { TEST(AndroidExternalViewEmbedder, RasterizerRunsOnRasterizerThread) { auto jni_mock = std::make_shared(); - auto embedder = std::make_unique(nullptr, jni_mock, nullptr); - auto platform_thread = new fml::Thread("platform"); - auto rasterizer_thread = new fml::Thread("rasterizer"); - auto platform_queue_id = platform_thread->GetTaskRunner()->GetTaskQueueId(); - auto rasterizer_queue_id = - rasterizer_thread->GetTaskRunner()->GetTaskQueueId(); - auto raster_thread_merger = fml::MakeRefCounted( - platform_queue_id, rasterizer_queue_id); + auto raster_thread_merger = GetThreadMergerFromPlatformThread(); ASSERT_FALSE(raster_thread_merger->IsMerged()); PostPrerollResult result = embedder->PostPrerollAction(raster_thread_merger); @@ -175,9 +192,11 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect) { auto embedder = std::make_unique(nullptr, jni_mock, nullptr); + auto raster_thread_merger = GetThreadMergerFromPlatformThread(); EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); - embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5); + 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); @@ -193,9 +212,11 @@ TEST(AndroidExternalViewEmbedder, PlatformViewRect__ChangedParams) { auto embedder = std::make_unique(nullptr, jni_mock, nullptr); + auto raster_thread_merger = GetThreadMergerFromPlatformThread(); EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); - embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5); + embedder->BeginFrame(SkISize::Make(100, 100), nullptr, 1.5, + raster_thread_merger); auto view_id = 0; auto view_params_1 = std::make_unique(); @@ -252,11 +273,12 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__RecycleSurfaces) { }; auto embedder = std::make_unique( android_context, jni_mock, surface_factory); + auto raster_thread_merger = GetThreadMergerFromPlatformThread(); // ------------------ First frame ------------------ // { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); - embedder->BeginFrame(frame_size, nullptr, 1.5); + embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger); // Add an Android view. auto view_params_1 = std::make_unique(); @@ -298,13 +320,13 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__RecycleSurfaces) { embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); - embedder->EndFrame(nullptr); + embedder->EndFrame(raster_thread_merger); } // ------------------ Second frame ------------------ // { EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()); - embedder->BeginFrame(frame_size, nullptr, 1.5); + embedder->BeginFrame(frame_size, nullptr, 1.5, raster_thread_merger); // Add an Android view. auto view_params_1 = std::make_unique(); @@ -341,8 +363,29 @@ TEST(AndroidExternalViewEmbedder, SubmitFrame__RecycleSurfaces) { [](const SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; }); embedder->SubmitFrame(gr_context.get(), std::move(surface_frame)); + + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()); + embedder->EndFrame(raster_thread_merger); } } +TEST(AndroidExternalViewEmbedder, DoesNotCallJNIPlatformThreadOnlyMethods) { + auto jni_mock = std::make_shared(); + + auto embedder = + std::make_unique(nullptr, jni_mock, nullptr); + + // While on the raster thread, don't make JNI calls as these methods can only + // run on the platform thread. + auto raster_thread_merger = GetThreadMergerFromRasterThread(); + + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0); + embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0, + raster_thread_merger); + + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()).Times(0); + embedder->EndFrame(raster_thread_merger); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/darwin/ios/ios_surface.h b/shell/platform/darwin/ios/ios_surface.h index fc795b0e62fb1..5fb6f78f6d7cc 100644 --- a/shell/platform/darwin/ios/ios_surface.h +++ b/shell/platform/darwin/ios/ios_surface.h @@ -62,7 +62,11 @@ class IOSSurface : public ExternalViewEmbedder { void CancelFrame() override; // |ExternalViewEmbedder| - void BeginFrame(SkISize frame_size, GrContext* context, double device_pixel_ratio) override; + void BeginFrame(SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) override; + // |ExternalViewEmbedder| void PrerollCompositeEmbeddedView(int view_id, std::unique_ptr params) override; diff --git a/shell/platform/darwin/ios/ios_surface.mm b/shell/platform/darwin/ios/ios_surface.mm index 36598c1b96a2b..26eca6cf26b90 100644 --- a/shell/platform/darwin/ios/ios_surface.mm +++ b/shell/platform/darwin/ios/ios_surface.mm @@ -94,7 +94,10 @@ bool IsIosEmbeddedViewsPreviewEnabled() { } // |ExternalViewEmbedder| -void IOSSurface::BeginFrame(SkISize frame_size, GrContext* context, double device_pixel_ratio) { +void IOSSurface::BeginFrame(SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) { TRACE_EVENT0("flutter", "IOSSurface::BeginFrame"); FML_CHECK(platform_views_controller_ != nullptr); platform_views_controller_->SetFrameSize(frame_size); diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index 752efec53998c..fcb8067e2a9e0 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -47,9 +47,11 @@ void EmbedderExternalViewEmbedder::CancelFrame() { } // |ExternalViewEmbedder| -void EmbedderExternalViewEmbedder::BeginFrame(SkISize frame_size, - GrContext* context, - double device_pixel_ratio) { +void EmbedderExternalViewEmbedder::BeginFrame( + SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) { Reset(); pending_frame_size_ = frame_size; diff --git a/shell/platform/embedder/embedder_external_view_embedder.h b/shell/platform/embedder/embedder_external_view_embedder.h index c78e39fd75415..6fc0eec0c9ca6 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.h +++ b/shell/platform/embedder/embedder_external_view_embedder.h @@ -73,9 +73,11 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { void CancelFrame() override; // |ExternalViewEmbedder| - void BeginFrame(SkISize frame_size, - GrContext* context, - double device_pixel_ratio) override; + void BeginFrame( + SkISize frame_size, + GrContext* context, + double device_pixel_ratio, + fml::RefPtr raster_thread_merger) override; // |ExternalViewEmbedder| void PrerollCompositeEmbeddedView(