From 25e56802da54e67100ce7c70a23e28221544f494 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 10 Feb 2020 14:56:12 -0800 Subject: [PATCH 1/3] Lift restriction that embedders may not trample the render thread OpenGL context in composition callbacks. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During the implementation of custom compositor integration, the embedder gets callbacks on the render thread to prepare render targets (framebuffers, textures, etc) for the engine to render into, callbacks to present these render targets along with platform managed contents, and, callbacks to collect render targets once they can no longer be recycled by the engine in subsequent frames. During these callbacks, the engine mandates the OpenGL state on the render thread be preserved. This restriction has been the source of hard to isolate issues where the embedder trampled on the OpenGL bindings state in the callback but failed to restore state before control went back to the engine. Due to the nature of the OpenGL API, such errors are easy to make and overlook. This patch lifts the restriction from the embedder. Embedders may now freely work with the OpenGL state in custom compositor callbacks and the engine will make sure to disregard OpenGL bindings when control flows back to it. Disregarding current OpenGL state has a certain performance penalty and the majority of this patch handles refactoring various engine embedder components such that this happens only once per frame. The most trivial version of this patch would reset context bindings on every transition of control flow from the embedder to the engine. However, that naive approach would have necessitated more than 50 binding resets in existing unit-test cases (depending on the number of platform view interleaving levels and render target recycling hit rates). In this implementation, bindings will be reset only once per frame and this does not depend on the number of platform views in the scene. The majority of this patch is a refactoring of engine subsystems used in `ExternalViewEmbedder::SubmitFrame` which is thoroughly documented with each opportunity for the embedder to invalidate OpenGL state tagged. The refactoring also enables the implementation of the following optimizations to engine behavior which should aid in reducing the memory needed for the creation of render targets. These optimization include: * The engine will only ask the embedder for render targets in which it expects to render into. This was a quirk in the way in which root and non-root render targets were handled. The engine could require the embedder to create a render target but then realize it didn’t have anything to render into it. In the presentation callback, it would skip that render target. But the embedder still had to allocate that extra render target. This will no longer be the case and should reduce memory use. * The engine may now skip always realizing (via the embedder render target creation callback) and presenting the root render target. This was also a side effect of the same quirk. Previously, the engine would always ask the embedder to present the root render target even if it was empty. Since this is no longer the case, few render targets should be allocated which will reduce memory consumption. * The engine will now ask the embedder to collect unused render targets before it asks it to create new ones. The previous behavior was to ask the embedder for new targets and then collect old ones. This would cause spikes in memory use when the size of the render targets would change. These memory use spikes should now be troughs. * The previous render target cache also considered the platform view ID in cache viability considerations (instead of just the size of the render target). This was a bug which has been fixed. This should lead to better cache utilization in some situations. These optimizations are now codified in unit-tests and the updated test expectations are a result of these optimizations now being in place. Fixes https://github.com/flutter/flutter/issues/50751 Fixes https://github.com/flutter/flutter/issues/46911 Fixes https://github.com/flutter/flutter/issues/43778 Fixes https://buganizer.corp.google.com/issues/146142979 --- ci/licenses_golden/licenses_flutter | 7 + fml/BUILD.gn | 2 + fml/hash_combine.h | 38 ++ fml/hash_combine_unittests.cc | 23 ++ shell/common/canvas_spy.h | 6 +- shell/platform/embedder/BUILD.gn | 5 + .../embedder/embedder_external_view.cc | 107 ++++++ .../embedder/embedder_external_view.h | 117 +++++++ .../embedder_external_view_embedder.cc | 327 ++++++++---------- .../embedder_external_view_embedder.h | 51 +-- shell/platform/embedder/embedder_include2.c | 9 + shell/platform/embedder/embedder_layers.cc | 3 +- .../embedder/embedder_render_target_cache.cc | 62 ++++ .../embedder/embedder_render_target_cache.h | 57 +++ shell/platform/embedder/fixtures/main.dart | 40 +++ .../tests/embedder_test_compositor.cc | 43 ++- .../embedder/tests/embedder_test_compositor.h | 20 +- .../embedder/tests/embedder_unittests.cc | 180 ++++++++-- 18 files changed, 834 insertions(+), 263 deletions(-) create mode 100644 fml/hash_combine.h create mode 100644 fml/hash_combine_unittests.cc create mode 100644 shell/platform/embedder/embedder_external_view.cc create mode 100644 shell/platform/embedder/embedder_external_view.h create mode 100644 shell/platform/embedder/embedder_include2.c create mode 100644 shell/platform/embedder/embedder_render_target_cache.cc create mode 100644 shell/platform/embedder/embedder_render_target_cache.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index f8cba6e7c66a6..b80d4ac5b606b 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -126,6 +126,8 @@ FILE: ../../../flutter/fml/file_unittest.cc FILE: ../../../flutter/fml/gpu_thread_merger.cc FILE: ../../../flutter/fml/gpu_thread_merger.h FILE: ../../../flutter/fml/gpu_thread_merger_unittests.cc +FILE: ../../../flutter/fml/hash_combine.h +FILE: ../../../flutter/fml/hash_combine_unittests.cc FILE: ../../../flutter/fml/icu_util.cc FILE: ../../../flutter/fml/icu_util.h FILE: ../../../flutter/fml/log_level.h @@ -913,15 +915,20 @@ FILE: ../../../flutter/shell/platform/embedder/embedder_engine.cc FILE: ../../../flutter/shell/platform/embedder/embedder_engine.h FILE: ../../../flutter/shell/platform/embedder/embedder_external_texture_gl.cc FILE: ../../../flutter/shell/platform/embedder/embedder_external_texture_gl.h +FILE: ../../../flutter/shell/platform/embedder/embedder_external_view.cc +FILE: ../../../flutter/shell/platform/embedder/embedder_external_view.h FILE: ../../../flutter/shell/platform/embedder/embedder_external_view_embedder.cc FILE: ../../../flutter/shell/platform/embedder/embedder_external_view_embedder.h FILE: ../../../flutter/shell/platform/embedder/embedder_include.c +FILE: ../../../flutter/shell/platform/embedder/embedder_include2.c FILE: ../../../flutter/shell/platform/embedder/embedder_layers.cc FILE: ../../../flutter/shell/platform/embedder/embedder_layers.h FILE: ../../../flutter/shell/platform/embedder/embedder_platform_message_response.cc FILE: ../../../flutter/shell/platform/embedder/embedder_platform_message_response.h FILE: ../../../flutter/shell/platform/embedder/embedder_render_target.cc FILE: ../../../flutter/shell/platform/embedder/embedder_render_target.h +FILE: ../../../flutter/shell/platform/embedder/embedder_render_target_cache.cc +FILE: ../../../flutter/shell/platform/embedder/embedder_render_target_cache.h FILE: ../../../flutter/shell/platform/embedder/embedder_safe_access.h FILE: ../../../flutter/shell/platform/embedder/embedder_surface.cc FILE: ../../../flutter/shell/platform/embedder/embedder_surface.h diff --git a/fml/BUILD.gn b/fml/BUILD.gn index b218025f2450f..d8c7a95d682ef 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -26,6 +26,7 @@ source_set("fml") { "file.h", "gpu_thread_merger.cc", "gpu_thread_merger.h", + "hash_combine.h", "icu_util.cc", "icu_util.h", "log_level.h", @@ -232,6 +233,7 @@ executable("fml_unittests") { "command_line_unittest.cc", "file_unittest.cc", "gpu_thread_merger_unittests.cc", + "hash_combine_unittests.cc", "memory/ref_counted_unittest.cc", "memory/weak_ptr_unittest.cc", "message_loop_task_queues_merge_unmerge_unittests.cc", diff --git a/fml/hash_combine.h b/fml/hash_combine.h new file mode 100644 index 0000000000000..e06f7d33f7c13 --- /dev/null +++ b/fml/hash_combine.h @@ -0,0 +1,38 @@ +// 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_FML_HASH_COMBINE_H_ +#define FLUTTER_FML_HASH_COMBINE_H_ + +#include + +namespace fml { + +template +constexpr void HashCombineSeed(std::size_t& seed, Type arg) { + seed ^= std::hash{}(arg) + 0x9e3779b9 + (seed << 6) + (seed >> 2); +} + +template +constexpr void HashCombineSeed(std::size_t& seed, + Type arg, + Rest... other_args) { + HashCombineSeed(seed, arg); + HashCombineSeed(seed, other_args...); +} + +constexpr std::size_t HashCombine() { + return 0xdabbad00; +} + +template +constexpr std::size_t HashCombine(Type... args) { + std::size_t seed = HashCombine(); + HashCombineSeed(seed, args...); + return seed; +} + +} // namespace fml + +#endif // FLUTTER_FML_HASH_COMBINE_H_ diff --git a/fml/hash_combine_unittests.cc b/fml/hash_combine_unittests.cc new file mode 100644 index 0000000000000..230e35c04dc52 --- /dev/null +++ b/fml/hash_combine_unittests.cc @@ -0,0 +1,23 @@ +// 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/fml/hash_combine.h" +#include "flutter/testing/testing.h" + +namespace fml { +namespace testing { + +TEST(HashCombineTest, CanHash) { + ASSERT_EQ(HashCombine(), HashCombine()); + ASSERT_EQ(HashCombine("Hello"), HashCombine("Hello")); + ASSERT_NE(HashCombine("Hello"), HashCombine("World")); + ASSERT_EQ(HashCombine("Hello", "World"), HashCombine("Hello", "World")); + ASSERT_NE(HashCombine("World", "Hello"), HashCombine("Hello", "World")); + ASSERT_EQ(HashCombine(12u), HashCombine(12u)); + ASSERT_NE(HashCombine(12u), HashCombine(12.0f)); + ASSERT_EQ(HashCombine('a'), HashCombine('a')); +} + +} // namespace testing +} // namespace fml diff --git a/shell/common/canvas_spy.h b/shell/common/canvas_spy.h index e9b721e3912be..bc9da2bd72ae2 100644 --- a/shell/common/canvas_spy.h +++ b/shell/common/canvas_spy.h @@ -26,8 +26,8 @@ class CanvasSpy { public: CanvasSpy(SkCanvas* target_canvas); - //------------------------------------------------------------------------------ - /// @brief Returns true if any non trasnparent content has been drawn + //---------------------------------------------------------------------------- + /// @brief Returns true if any non transparent content has been drawn /// into /// the spying canvas. Note that this class does tries to detect /// empty canvases but in some cases may return true even for @@ -35,7 +35,7 @@ class CanvasSpy { /// canvas). bool DidDrawIntoCanvas(); - //------------------------------------------------------------------------------ + //---------------------------------------------------------------------------- /// @brief The returned canvas delegate all operations to the target /// canvas /// while spying on them. diff --git a/shell/platform/embedder/BUILD.gn b/shell/platform/embedder/BUILD.gn index 7c49cc2b681f3..7215622912c76 100644 --- a/shell/platform/embedder/BUILD.gn +++ b/shell/platform/embedder/BUILD.gn @@ -29,15 +29,20 @@ template("embedder_source_set") { "embedder_engine.h", "embedder_external_texture_gl.cc", "embedder_external_texture_gl.h", + "embedder_external_view.cc", + "embedder_external_view.h", "embedder_external_view_embedder.cc", "embedder_external_view_embedder.h", "embedder_include.c", + "embedder_include2.c", "embedder_layers.cc", "embedder_layers.h", "embedder_platform_message_response.cc", "embedder_platform_message_response.h", "embedder_render_target.cc", "embedder_render_target.h", + "embedder_render_target_cache.cc", + "embedder_render_target_cache.h", "embedder_safe_access.h", "embedder_surface.cc", "embedder_surface.h", diff --git a/shell/platform/embedder/embedder_external_view.cc b/shell/platform/embedder/embedder_external_view.cc new file mode 100644 index 0000000000000..5119d550b0362 --- /dev/null +++ b/shell/platform/embedder/embedder_external_view.cc @@ -0,0 +1,107 @@ +// 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/embedder/embedder_external_view.h" +#include "flutter/fml/trace_event.h" +#include "flutter/shell/common/canvas_spy.h" + +namespace flutter { + +static SkISize TransformedSurfaceSize(const SkISize& size, + const SkMatrix& transformation) { + const auto source_rect = SkRect::MakeWH(size.width(), size.height()); + const auto transformed_rect = transformation.mapRect(source_rect); + return SkISize::Make(transformed_rect.width(), transformed_rect.height()); +} + +EmbedderExternalView::EmbedderExternalView( + const SkISize& frame_size, + const SkMatrix& surface_transformation) + : EmbedderExternalView(frame_size, surface_transformation, {}, nullptr) {} + +EmbedderExternalView::EmbedderExternalView( + const SkISize& frame_size, + const SkMatrix& surface_transformation, + ViewIdentifier view_identifier, + std::unique_ptr params) + : render_surface_size_( + TransformedSurfaceSize(frame_size, surface_transformation)), + surface_transformation_(surface_transformation), + view_identifier_(view_identifier), + embedded_view_params_(std::move(params)), + recorder_(std::make_unique()), + canvas_spy_(std::make_unique( + recorder_->beginRecording(frame_size.width(), frame_size.height()))) { +} + +EmbedderExternalView::~EmbedderExternalView() = default; + +EmbedderExternalView::RenderTargetDescriptor +EmbedderExternalView::CreateRenderTargetDescriptor() const { + return {render_surface_size_}; +} + +SkCanvas* EmbedderExternalView::GetCanvas() const { + return canvas_spy_->GetSpyingCanvas(); +} + +SkISize EmbedderExternalView::GetRenderSurfaceSize() const { + return render_surface_size_; +} + +bool EmbedderExternalView::IsRootView() const { + return !HasPlatformView(); +} + +bool EmbedderExternalView::HasPlatformView() const { + return view_identifier_.platform_view_id.has_value(); +} + +bool EmbedderExternalView::HasEngineRenderedContents() const { + return canvas_spy_->DidDrawIntoCanvas(); +} + +EmbedderExternalView::ViewIdentifier EmbedderExternalView::GetViewIdentifier() + const { + return view_identifier_; +} + +const EmbeddedViewParams* EmbedderExternalView::GetEmbeddedViewParams() const { + return embedded_view_params_.get(); +} + +bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target) { + TRACE_EVENT0("flutter", "EmbedderExternalView::Render"); + + FML_DCHECK(HasEngineRenderedContents()) + << "Unnecessary asked to render into a render target when there was " + "nothing to render."; + + auto picture = recorder_->finishRecordingAsPicture(); + if (!picture) { + return false; + } + + auto surface = render_target.GetRenderSurface(); + if (!surface) { + return false; + } + + FML_DCHECK(SkISize::Make(surface->width(), surface->height()) == + render_surface_size_); + + auto canvas = surface->getCanvas(); + if (!canvas) { + return false; + } + + canvas->setMatrix(surface_transformation_); + canvas->clear(SK_ColorTRANSPARENT); + canvas->drawPicture(picture); + canvas->flush(); + + return true; +} + +} // namespace flutter diff --git a/shell/platform/embedder/embedder_external_view.h b/shell/platform/embedder/embedder_external_view.h new file mode 100644 index 0000000000000..508bbd9a75b6d --- /dev/null +++ b/shell/platform/embedder/embedder_external_view.h @@ -0,0 +1,117 @@ +// 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_EMBEDDER_EMBEDDER_EXTERNAL_VIEW_H_ +#define FLUTTER_SHELL_PLATFORM_EMBEDDER_EMBEDDER_EXTERNAL_VIEW_H_ + +#include +#include + +#include "flutter/flow/embedded_views.h" +#include "flutter/fml/hash_combine.h" +#include "flutter/fml/macros.h" +#include "flutter/shell/common/canvas_spy.h" +#include "flutter/shell/platform/embedder/embedder_render_target.h" +#include "third_party/skia/include/core/SkPictureRecorder.h" + +namespace flutter { + +class EmbedderExternalView { + public: + using PlatformViewID = int64_t; + struct ViewIdentifier { + std::optional platform_view_id; + + ViewIdentifier() {} + + ViewIdentifier(PlatformViewID view_id) : platform_view_id(view_id) {} + + struct Hash { + constexpr std::size_t operator()(const ViewIdentifier& desc) const { + if (!desc.platform_view_id.has_value()) { + return fml::HashCombine(); + } + + return fml::HashCombine(desc.platform_view_id.value()); + } + }; + + struct Equal { + constexpr bool operator()(const ViewIdentifier& lhs, + const ViewIdentifier& rhs) const { + return lhs.platform_view_id == rhs.platform_view_id; + } + }; + }; + + struct RenderTargetDescriptor { + SkISize surface_size = SkISize::MakeEmpty(); + + struct Hash { + constexpr std::size_t operator()( + const RenderTargetDescriptor& desc) const { + return fml::HashCombine(desc.surface_size.width(), + desc.surface_size.height()); + } + }; + + struct Equal { + bool operator()(const RenderTargetDescriptor& lhs, + const RenderTargetDescriptor& rhs) const { + return lhs.surface_size == rhs.surface_size; + } + }; + }; + + using ViewIdentifierSet = std::unordered_set; + + using PendingViews = std::unordered_map, + ViewIdentifier::Hash, + ViewIdentifier::Equal>; + + EmbedderExternalView(const SkISize& frame_size, + const SkMatrix& surface_transformation); + + EmbedderExternalView(const SkISize& frame_size, + const SkMatrix& surface_transformation, + ViewIdentifier view_identifier, + std::unique_ptr params); + + ~EmbedderExternalView(); + + bool IsRootView() const; + + bool HasPlatformView() const; + + bool HasEngineRenderedContents() const; + + ViewIdentifier GetViewIdentifier() const; + + const EmbeddedViewParams* GetEmbeddedViewParams() const; + + RenderTargetDescriptor CreateRenderTargetDescriptor() const; + + SkCanvas* GetCanvas() const; + + SkISize GetRenderSurfaceSize() const; + + bool Render(const EmbedderRenderTarget& render_target); + + private: + const SkISize render_surface_size_; + const SkMatrix surface_transformation_; + ViewIdentifier view_identifier_; + std::unique_ptr embedded_view_params_; + std::unique_ptr recorder_; + std::unique_ptr canvas_spy_; + + FML_DISALLOW_COPY_AND_ASSIGN(EmbedderExternalView); +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_EMBEDDER_EMBEDDER_EXTERNAL_VIEW_H_ diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index fae78b67d7dc7..db8aec7246614 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -8,6 +8,7 @@ #include "flutter/shell/platform/embedder/embedder_layers.h" #include "flutter/shell/platform/embedder/embedder_render_target.h" +#include "third_party/skia/include/gpu/GrContext.h" namespace flutter { @@ -36,9 +37,7 @@ SkMatrix EmbedderExternalViewEmbedder::GetSurfaceTransformation() const { } void EmbedderExternalViewEmbedder::Reset() { - pending_recorders_.clear(); - pending_canvas_spies_.clear(); - pending_params_.clear(); + pending_views_.clear(); composition_order_.clear(); } @@ -47,25 +46,6 @@ void EmbedderExternalViewEmbedder::CancelFrame() { Reset(); } -static FlutterBackingStoreConfig MakeBackingStoreConfig( - const SkISize& backing_store_size) { - FlutterBackingStoreConfig config = {}; - - config.struct_size = sizeof(config); - - config.size.width = backing_store_size.width(); - config.size.height = backing_store_size.height(); - - return config; -} - -static SkISize TransformedSurfaceSize(const SkISize& size, - const SkMatrix& transformation) { - const auto source_rect = SkRect::MakeWH(size.width(), size.height()); - const auto transformed_rect = transformation.mapRect(source_rect); - return SkISize::Make(transformed_rect.width(), transformed_rect.height()); -} - // |ExternalViewEmbedder| void EmbedderExternalViewEmbedder::BeginFrame(SkISize frame_size, GrContext* context, @@ -76,212 +56,203 @@ void EmbedderExternalViewEmbedder::BeginFrame(SkISize frame_size, pending_device_pixel_ratio_ = device_pixel_ratio; pending_surface_transformation_ = GetSurfaceTransformation(); - const auto surface_size = TransformedSurfaceSize( - pending_frame_size_, pending_surface_transformation_); - - // Decide if we want to discard the previous root render target. - if (root_render_target_) { - auto surface = root_render_target_->GetRenderSurface(); - // This is unlikely to happen but the embedder could have given the - // rasterizer a render target the previous frame that Skia could not - // materialize into a renderable surface. Discard the target and try again. - if (!surface) { - root_render_target_ = nullptr; - } else { - auto last_surface_size = - SkISize::Make(surface->width(), surface->height()); - if (surface_size != last_surface_size) { - root_render_target_ = nullptr; - } - } - } + static const auto kRootViewIdentifier = + EmbedderExternalView::ViewIdentifier{}; - // If there is no root render target, create one now. - // TODO(43778): This should now be moved to be later in the submit call. - if (!root_render_target_) { - root_render_target_ = create_render_target_callback_( - context, MakeBackingStoreConfig(surface_size)); - } - - root_picture_recorder_ = std::make_unique(); - root_picture_recorder_->beginRecording(pending_frame_size_.width(), - pending_frame_size_.height()); + pending_views_[kRootViewIdentifier] = std::make_unique( + pending_frame_size_, pending_surface_transformation_); + composition_order_.push_back(kRootViewIdentifier); } // |ExternalViewEmbedder| void EmbedderExternalViewEmbedder::PrerollCompositeEmbeddedView( int view_id, std::unique_ptr params) { - FML_DCHECK(pending_recorders_.count(view_id) == 0); - FML_DCHECK(pending_canvas_spies_.count(view_id) == 0); - FML_DCHECK(pending_params_.count(view_id) == 0); - FML_DCHECK(std::find(composition_order_.begin(), composition_order_.end(), - view_id) == composition_order_.end()); - - pending_recorders_[view_id] = std::make_unique(); - SkCanvas* recording_canvas = pending_recorders_[view_id]->beginRecording( - pending_frame_size_.width(), pending_frame_size_.height()); - pending_canvas_spies_[view_id] = - std::make_unique(recording_canvas); - pending_params_[view_id] = *params; + FML_DCHECK(pending_views_.count(view_id) == 0); + + pending_views_[view_id] = std::make_unique( + pending_frame_size_, // frame size + pending_surface_transformation_, // surface xformation + view_id, // view identifier + std::move(params) // embedded view params + ); composition_order_.push_back(view_id); } +// |ExternalViewEmbedder| +SkCanvas* EmbedderExternalViewEmbedder::GetRootCanvas() { + auto found = pending_views_.find(EmbedderExternalView::ViewIdentifier{}); + if (found == pending_views_.end()) { + FML_DLOG(WARNING) + << "No root canvas could be found. This is extremely unlikely and " + "indicates that the external view embedder did not receive the " + "notification to being the frame."; + return nullptr; + } + return found->second->GetCanvas(); +} + // |ExternalViewEmbedder| std::vector EmbedderExternalViewEmbedder::GetCurrentCanvases() { std::vector canvases; - for (const auto& spy : pending_canvas_spies_) { - canvases.push_back(spy.second->GetSpyingCanvas()); + for (const auto& view : pending_views_) { + const auto& external_view = view.second; + // This method (for legacy reasons) expects non-root current canvases. + if (!external_view->IsRootView()) { + canvases.push_back(external_view->GetCanvas()); + } } return canvases; } // |ExternalViewEmbedder| SkCanvas* EmbedderExternalViewEmbedder::CompositeEmbeddedView(int view_id) { - auto found = pending_canvas_spies_.find(view_id); - if (found == pending_canvas_spies_.end()) { + auto found = pending_views_.find(view_id); + if (found == pending_views_.end()) { FML_DCHECK(false) << "Attempted to composite a view that was not " "pre-rolled."; return nullptr; } - return found->second->GetSpyingCanvas(); + return found->second->GetCanvas(); } -bool EmbedderExternalViewEmbedder::RenderPictureToRenderTarget( - sk_sp picture, - const EmbedderRenderTarget* render_target) const { - if (!picture || render_target == nullptr) { - return false; - } - - auto render_surface = render_target->GetRenderSurface(); - - if (!render_surface) { - return false; - } - - auto render_canvas = render_surface->getCanvas(); +static FlutterBackingStoreConfig MakeBackingStoreConfig( + const SkISize& backing_store_size) { + FlutterBackingStoreConfig config = {}; - if (render_canvas == nullptr) { - return false; - } + config.struct_size = sizeof(config); - render_canvas->setMatrix(pending_surface_transformation_); - render_canvas->clear(SK_ColorTRANSPARENT); - render_canvas->drawPicture(picture); - render_canvas->flush(); + config.size.width = backing_store_size.width(); + config.size.height = backing_store_size.height(); - return true; + return config; } // |ExternalViewEmbedder| bool EmbedderExternalViewEmbedder::SubmitFrame(GrContext* context) { - Registry render_targets_used; - EmbedderLayers presented_layers(pending_frame_size_, - pending_device_pixel_ratio_, - pending_surface_transformation_); - - if (!root_render_target_) { - FML_LOG(ERROR) - << "Could not acquire the root render target from the embedder."; - return false; - } - - // Copy the contents of the root picture recorder onto the root surface. - if (!RenderPictureToRenderTarget( - root_picture_recorder_->finishRecordingAsPicture(), - root_render_target_.get())) { - FML_LOG(ERROR) << "Could not render into the root render target."; - return false; - } - // The root picture recorder will be reset when a new frame begins. - root_picture_recorder_.reset(); - - { - // The root surface is expressed as a layer. - presented_layers.PushBackingStoreLayer( - root_render_target_->GetBackingStore()); - } - - const auto surface_size = TransformedSurfaceSize( - pending_frame_size_, pending_surface_transformation_); - - for (const auto& view_id : composition_order_) { - FML_DCHECK(pending_recorders_.count(view_id) == 1); - FML_DCHECK(pending_canvas_spies_.count(view_id) == 1); - FML_DCHECK(pending_params_.count(view_id) == 1); - - const auto& params = pending_params_.at(view_id); - auto& recorder = pending_recorders_.at(view_id); - - auto picture = recorder->finishRecordingAsPicture(); - if (!picture) { - FML_LOG(ERROR) << "Could not finish recording into the picture before " - "on-screen composition."; - return false; - } - - // Tell the embedder that a platform view layer is present at this point. - presented_layers.PushPlatformViewLayer(view_id, params); - - if (!pending_canvas_spies_.at(view_id)->DidDrawIntoCanvas()) { - // Nothing was drawn into the overlay canvas, we don't need to tell the - // embedder to composite it. + auto [matched_render_targets, pending_keys] = + render_target_cache_.GetExistingTargetsInCache(pending_views_); + + // This is where unused render targets will be collected. Control may flow to + // the embedder. Here, the embedder has the opportunity to trample on the + // OpenGL context. + // + // We tell the render target cache to clear its unused entries before + // allocating new ones. This collection step before allocating new render + // targets ameliorates peak memory usage within the frame. + // + // @warning: Embedder may trample on our OpenGL context here. + render_target_cache_.ClearAllRenderTargetsInCache(); + + for (const auto& pending_key : pending_keys) { + const auto& external_view = pending_views_.at(pending_key); + + // If the external view does not have engine rendered contents, it makes no + // sense to ask to embedder to create a render target for us as we don't + // intend to render into it and ask the embedder for presentation anyway. + // Save some memory. + if (!external_view->HasEngineRenderedContents()) { continue; } - const auto backing_store_config = MakeBackingStoreConfig(surface_size); - - RegistryKey registry_key(view_id, backing_store_config); - - auto found_render_target = registry_.find(registry_key); - - // Find a cached render target in the registry. If none exists, ask the - // embedder for a new one. - std::shared_ptr render_target; - if (found_render_target == registry_.end()) { - render_target = - create_render_target_callback_(context, backing_store_config); - } else { - render_target = found_render_target->second; - } + // This is the size of render surface we want the embedder to create for + // us. As or right now, this is going to always be equal to the frame size + // post transformation. But, in case optimizations are applied that make + // it so that embedder rendered into surfaces that aren't full screen, + // this assumption will break. So it's just best to ask view for its size + // directly. + const auto render_surface_size = external_view->GetRenderSurfaceSize(); + + const auto backing_store_config = + MakeBackingStoreConfig(render_surface_size); + + // This is where the embedder will create render targets for us. Control + // flow to the embedder makes the engine susceptible to having the embedder + // trample on the OpenGL context. Before any Skia operations are performed, + // the context must be reset. + // + // @warning: Embedder may trample on our OpenGL context here. + auto render_target = + create_render_target_callback_(context, backing_store_config); if (!render_target) { - FML_LOG(ERROR) << "Could not acquire external render target for " - "on-screen composition."; + FML_LOG(ERROR) << "Embedder did not return a valid render target."; return false; } + matched_render_targets[pending_key] = std::move(render_target); + } - render_targets_used[registry_key] = render_target; + // The OpenGL context could have been trampled by the embedder at this point + // as it attempted to collect old render targets and create new ones. Tell + // Skia to not rely on existing bindings. + if (context) { + context->resetContext(kAll_GrBackendState); + } - if (!RenderPictureToRenderTarget(picture, render_target.get())) { - FML_LOG(ERROR) << "Could not render into the render target for platform " - "view of identifier " - << view_id; + // Scribble embedder provide render targets. The order in which we scribble + // into the buffers is irrelevant to the presentation order. + for (const auto& render_target : matched_render_targets) { + if (!pending_views_.at(render_target.first) + ->Render(*render_target.second)) { + FML_LOG(ERROR) + << "Could not render into the embedder supplied render target."; return false; } + } - // Indicate a layer for the backing store containing contents rendered by - // Flutter. - presented_layers.PushBackingStoreLayer(render_target->GetBackingStore()); + // We are going to be transferring control back over to the embedder there the + // context may be trampled upon again. Flush all operations to the underlying + // rendering API. + // + // @warning: Embedder may trample on our OpenGL context here. + if (context) { + context->flush(); } - // Flush the layer description down to the embedder for presentation. - presented_layers.InvokePresentCallback(present_callback_); + // Submit the scribbled layer to the embedder for presentation. + // + // @warning: Embedder may trample on our OpenGL context here. + { + EmbedderLayers presented_layers(pending_frame_size_, + pending_device_pixel_ratio_, + pending_surface_transformation_); + // In composition order, submit backing stores and platform views to the + // embedder. + for (const auto& view_id : composition_order_) { + // If the external view has a platform view, ask the emebdder to place it + // before the Flutter rendered contents for that interleaving level. + const auto& external_view = pending_views_.at(view_id); + if (external_view->HasPlatformView()) { + presented_layers.PushPlatformViewLayer( + external_view->GetViewIdentifier() + .platform_view_id.value(), // view id + *external_view->GetEmbeddedViewParams() // view params + ); + } - // Keep the previously used render target around in case they are required - // next frame. - registry_ = std::move(render_targets_used); + // If the view has engine rendered contents, ask the embedder to place + // Flutter rendered contents for this interleaving level on top of a + // platform view. + if (external_view->HasEngineRenderedContents()) { + const auto& exteral_render_target = matched_render_targets.at(view_id); + presented_layers.PushBackingStoreLayer( + exteral_render_target->GetBackingStore()); + } + } - return true; -} + // Flush the layer description down to the embedder for presentation. + // + // @warning: Embedder may trample on our OpenGL context here. + presented_layers.InvokePresentCallback(present_callback_); + } -// |ExternalViewEmbedder| -SkCanvas* EmbedderExternalViewEmbedder::GetRootCanvas() { - if (!root_picture_recorder_) { - return nullptr; + // Hold all rendered layers in the render target cache for one frame to see if + // they may be reused next frame. + for (auto& render_target : matched_render_targets) { + render_target_cache_.CacheRenderTarget(std::move(render_target.second)); } - return root_picture_recorder_->getRecordingCanvas(); + + return true; } } // namespace flutter diff --git a/shell/platform/embedder/embedder_external_view_embedder.h b/shell/platform/embedder/embedder_external_view_embedder.h index 16f4a5893843e..7000d2cde04cd 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.h +++ b/shell/platform/embedder/embedder_external_view_embedder.h @@ -9,10 +9,10 @@ #include #include "flutter/flow/embedded_views.h" +#include "flutter/fml/hash_combine.h" #include "flutter/fml/macros.h" -#include "flutter/shell/common/canvas_spy.h" -#include "flutter/shell/platform/embedder/embedder_render_target.h" -#include "third_party/skia/include/core/SkPictureRecorder.h" +#include "flutter/shell/platform/embedder/embedder_external_view.h" +#include "flutter/shell/platform/embedder/embedder_render_target_cache.h" namespace flutter { @@ -95,59 +95,20 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { SkCanvas* GetRootCanvas() override; private: - using ViewIdentifier = int64_t; - struct RegistryKey { - ViewIdentifier view_identifier = 0; - SkISize size = SkISize::Make(0, 0); - - RegistryKey(ViewIdentifier view_identifier, - const FlutterBackingStoreConfig& config) - : view_identifier(view_identifier), - size(SkISize::Make(config.size.width, config.size.height)) {} - - struct Hash { - constexpr std::size_t operator()(RegistryKey const& key) const { - return key.view_identifier; - }; - }; - - struct Equal { - constexpr bool operator()(const RegistryKey& lhs, - const RegistryKey& rhs) const { - return lhs.view_identifier == rhs.view_identifier && - lhs.size == rhs.size; - } - }; - }; - const CreateRenderTargetCallback create_render_target_callback_; const PresentCallback present_callback_; SurfaceTransformationCallback surface_transformation_callback_; - using Registry = std::unordered_map, - RegistryKey::Hash, - RegistryKey::Equal>; - SkISize pending_frame_size_ = SkISize::Make(0, 0); double pending_device_pixel_ratio_ = 1.0; SkMatrix pending_surface_transformation_; - std::map> - pending_recorders_; - std::map> pending_canvas_spies_; - std::map pending_params_; - std::vector composition_order_; - std::shared_ptr root_render_target_; - std::unique_ptr root_picture_recorder_; - Registry registry_; + EmbedderExternalView::PendingViews pending_views_; + std::vector composition_order_; + EmbedderRenderTargetCache render_target_cache_; void Reset(); SkMatrix GetSurfaceTransformation() const; - bool RenderPictureToRenderTarget( - sk_sp picture, - const EmbedderRenderTarget* render_target) const; - FML_DISALLOW_COPY_AND_ASSIGN(EmbedderExternalViewEmbedder); }; diff --git a/shell/platform/embedder/embedder_include2.c b/shell/platform/embedder/embedder_include2.c new file mode 100644 index 0000000000000..f03500b4308c1 --- /dev/null +++ b/shell/platform/embedder/embedder_include2.c @@ -0,0 +1,9 @@ +// 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/embedder/embedder.h" + +// This file is the same as embedder_include.c and ensures that static methods +// don't end up in public API header. This will cause duplicate symbols when the +// header in imported in the multiple translation units in the embedder. diff --git a/shell/platform/embedder/embedder_layers.cc b/shell/platform/embedder/embedder_layers.cc index d9e86ca1e049c..3b634235f820a 100644 --- a/shell/platform/embedder/embedder_layers.cc +++ b/shell/platform/embedder/embedder_layers.cc @@ -195,9 +195,8 @@ void EmbedderLayers::PushPlatformViewLayer( layer.size.height = transformed_layer_bounds.height(); presented_layers_.push_back(layer); -} // namespace flutter +} -/// @note Procedure doesn't copy all closures. void EmbedderLayers::InvokePresentCallback( const PresentCallback& callback) const { std::vector presented_layers_pointers; diff --git a/shell/platform/embedder/embedder_render_target_cache.cc b/shell/platform/embedder/embedder_render_target_cache.cc new file mode 100644 index 0000000000000..2820dd5b9578f --- /dev/null +++ b/shell/platform/embedder/embedder_render_target_cache.cc @@ -0,0 +1,62 @@ +// 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/embedder/embedder_render_target_cache.h" + +namespace flutter { + +EmbedderRenderTargetCache::EmbedderRenderTargetCache() = default; + +EmbedderRenderTargetCache::~EmbedderRenderTargetCache() = default; + +std::pair +EmbedderRenderTargetCache::GetExistingTargetsInCache( + const EmbedderExternalView::PendingViews& pending_views) { + RenderTargets resolved_render_targets; + EmbedderExternalView::ViewIdentifierSet unmatched_identifiers; + + for (const auto& view : pending_views) { + const auto& external_view = view.second; + if (!external_view->HasEngineRenderedContents()) { + continue; + } + auto& compatible_targets = + cached_render_targets_[external_view->CreateRenderTargetDescriptor()]; + if (compatible_targets.size() == 0) { + unmatched_identifiers.insert(view.first); + } else { + std::unique_ptr target = + std::move(compatible_targets.top()); + compatible_targets.pop(); + resolved_render_targets[view.first] = std::move(target); + } + } + return {std::move(resolved_render_targets), std::move(unmatched_identifiers)}; +} + +void EmbedderRenderTargetCache::ClearAllRenderTargetsInCache() { + cached_render_targets_.clear(); +} + +void EmbedderRenderTargetCache::CacheRenderTarget( + std::unique_ptr target) { + if (target == nullptr) { + return; + } + auto surface = target->GetRenderSurface(); + EmbedderExternalView::RenderTargetDescriptor desc{ + SkISize::Make(surface->width(), surface->height())}; + cached_render_targets_[desc].push(std::move(target)); +} + +size_t EmbedderRenderTargetCache::GetCachedTargetsCount() const { + size_t count = 0; + for (const auto& targets : cached_render_targets_) { + count += targets.second.size(); + } + return count; +} + +} // namespace flutter diff --git a/shell/platform/embedder/embedder_render_target_cache.h b/shell/platform/embedder/embedder_render_target_cache.h new file mode 100644 index 0000000000000..52ca9d6b5c8aa --- /dev/null +++ b/shell/platform/embedder/embedder_render_target_cache.h @@ -0,0 +1,57 @@ +// 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_EMBEDDER_EMBEDDER_RENDER_TARGET_CACHE_H_ +#define FLUTTER_SHELL_PLATFORM_EMBEDDER_EMBEDDER_RENDER_TARGET_CACHE_H_ + +#include +#include +#include + +#include "flutter/fml/macros.h" +#include "flutter/shell/platform/embedder/embedder_external_view.h" + +namespace flutter { + +//------------------------------------------------------------------------------ +/// @brief A cache used to reference render targets that are owned by the +/// embedder but needed by th engine to render a frame. +/// +class EmbedderRenderTargetCache { + public: + EmbedderRenderTargetCache(); + + ~EmbedderRenderTargetCache(); + + using RenderTargets = + std::unordered_map, + EmbedderExternalView::ViewIdentifier::Hash, + EmbedderExternalView::ViewIdentifier::Equal>; + + std::pair + GetExistingTargetsInCache( + const EmbedderExternalView::PendingViews& pending_views); + + void ClearAllRenderTargetsInCache(); + + void CacheRenderTarget(std::unique_ptr target); + + size_t GetCachedTargetsCount() const; + + private: + using CachedRenderTargets = + std::unordered_map>, + EmbedderExternalView::RenderTargetDescriptor::Hash, + EmbedderExternalView::RenderTargetDescriptor::Equal>; + + CachedRenderTargets cached_render_targets_; + + FML_DISALLOW_COPY_AND_ASSIGN(EmbedderRenderTargetCache); +}; + +} // namespace flutter + +#endif // FLUTTER_SHELL_PLATFORM_EMBEDDER_EMBEDDER_RENDER_TARGET_CACHE_H_ diff --git a/shell/platform/embedder/fixtures/main.dart b/shell/platform/embedder/fixtures/main.dart index 6368c25afc913..95c25aaefb6c1 100644 --- a/shell/platform/embedder/fixtures/main.dart +++ b/shell/platform/embedder/fixtures/main.dart @@ -689,3 +689,43 @@ void objects_can_be_posted() { signalNativeCount(port.sendPort.nativePort); } +@pragma('vm:entry-point') +void empty_scene_posts_zero_layers_to_compositor() { + window.onBeginFrame = (Duration duration) { + SceneBuilder builder = SceneBuilder(); + // Should not render anything. + builder.pushClipRect(Rect.fromLTRB(0.0, 0.0, 300.0, 200.0)); + window.render(builder.build()); + }; + window.scheduleFrame(); +} + +@pragma('vm:entry-point') +void compositor_can_post_only_platform_views() { + window.onBeginFrame = (Duration duration) { + SceneBuilder builder = SceneBuilder(); + builder.addPlatformView(42, width: 300.0, height: 200.0); + builder.addPlatformView(24, width: 300.0, height: 200.0); + window.render(builder.build()); + }; + window.scheduleFrame(); +} + +@pragma('vm:entry-point') +void render_targets_are_recycled() { + int frame_count = 0; + window.onBeginFrame = (Duration duration) { + SceneBuilder builder = SceneBuilder(); + for (int i = 0; i < 10; i++) { + builder.addPicture(Offset(0.0, 0.0), CreateGradientBox(Size(30.0, 20.0))); + builder.addPlatformView(42 + i, width: 30.0, height: 20.0); + } + window.render(builder.build()); + window.scheduleFrame(); + frame_count++; + if (frame_count == 8) { + signalNativeTest(); + } + }; + window.scheduleFrame(); +} diff --git a/shell/platform/embedder/tests/embedder_test_compositor.cc b/shell/platform/embedder/tests/embedder_test_compositor.cc index 153e5df4d6f80..13c329626b76e 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor.cc +++ b/shell/platform/embedder/tests/embedder_test_compositor.cc @@ -24,6 +24,14 @@ void EmbedderTestCompositor::SetRenderTargetType(RenderTargetType type) { type_ = type; } +static void InvokeAllCallbacks(const std::vector& callbacks) { + for (const auto& callback : callbacks) { + if (callback) { + callback(); + } + } +} + bool EmbedderTestCompositor::CreateBackingStore( const FlutterBackingStoreConfig* config, FlutterBackingStore* backing_store_out) { @@ -43,7 +51,8 @@ bool EmbedderTestCompositor::CreateBackingStore( return false; } if (success) { - backing_stores_count_++; + backing_stores_created_++; + InvokeAllCallbacks(on_create_render_target_callbacks_); } return success; } @@ -54,7 +63,8 @@ bool EmbedderTestCompositor::CollectBackingStore( // stores. Our user_data is just the canvas from that backing store and does // not need to be explicitly collected. Embedders might have some other state // they want to collect though. - backing_stores_count_--; + backing_stores_collected_++; + InvokeAllCallbacks(on_collect_render_target_callbacks_); return true; } @@ -168,6 +178,7 @@ bool EmbedderTestCompositor::Present(const FlutterLayer** layers, callback(layers, layers_count); } + InvokeAllCallbacks(on_present_callbacks_); return true; } @@ -185,7 +196,6 @@ bool EmbedderTestCompositor::CreateFramebufferRenderSurface( kBottomLeft_GrSurfaceOrigin, // surface origin nullptr, // surface properties false // mipmaps - ); if (!surface) { @@ -322,8 +332,31 @@ void EmbedderTestCompositor::SetPlatformViewRendererCallback( platform_view_renderer_callback_ = callback; } -size_t EmbedderTestCompositor::GetBackingStoresCount() const { - return backing_stores_count_; +size_t EmbedderTestCompositor::GetPendingBackingStoresCount() const { + FML_CHECK(backing_stores_created_ >= backing_stores_collected_); + return backing_stores_created_ - backing_stores_collected_; +} + +size_t EmbedderTestCompositor::GetBackingStoresCreatedCount() const { + return backing_stores_created_; +} + +size_t EmbedderTestCompositor::GetBackingStoresCollectedCount() const { + return backing_stores_collected_; +} + +void EmbedderTestCompositor::AddOnCreateRenderTargetCallback( + fml::closure callback) { + on_create_render_target_callbacks_.push_back(callback); +} + +void EmbedderTestCompositor::AddOnCollectRenderTargetCallback( + fml::closure callback) { + on_collect_render_target_callbacks_.push_back(callback); +} + +void EmbedderTestCompositor::AddOnPresentCallback(fml::closure callback) { + on_present_callbacks_.push_back(callback); } } // namespace testing diff --git a/shell/platform/embedder/tests/embedder_test_compositor.h b/shell/platform/embedder/tests/embedder_test_compositor.h index 418b179e18e9e..25c1173913a5b 100644 --- a/shell/platform/embedder/tests/embedder_test_compositor.h +++ b/shell/platform/embedder/tests/embedder_test_compositor.h @@ -7,6 +7,7 @@ #include +#include "flutter/fml/closure.h" #include "flutter/fml/macros.h" #include "flutter/shell/platform/embedder/embedder.h" #include "third_party/skia/include/gpu/GrContext.h" @@ -57,7 +58,17 @@ class EmbedderTestCompositor { sk_sp GetLastComposition(); - size_t GetBackingStoresCount() const; + size_t GetPendingBackingStoresCount() const; + + size_t GetBackingStoresCreatedCount() const; + + size_t GetBackingStoresCollectedCount() const; + + void AddOnCreateRenderTargetCallback(fml::closure callback); + + void AddOnCollectRenderTargetCallback(fml::closure callback); + + void AddOnPresentCallback(fml::closure callback); private: const SkISize surface_size_; @@ -67,8 +78,11 @@ class EmbedderTestCompositor { PresentCallback next_present_callback_; NextSceneCallback next_scene_callback_; sk_sp last_composition_; - // The number of currently allocated backing stores (created - collected). - size_t backing_stores_count_ = 0; + size_t backing_stores_created_ = 0; + size_t backing_stores_collected_ = 0; + std::vector on_create_render_target_callbacks_; + std::vector on_collect_render_target_callbacks_; + std::vector on_present_callbacks_; bool UpdateOffscrenComposition(const FlutterLayer** layers, size_t layers_count); diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 3f128e2e56a88..44cf0f18f8741 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1793,7 +1793,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderWithPlatformLayerOnBottom) { ASSERT_TRUE(ImageMatchesFixture( "compositor_with_platform_layer_on_bottom.png", scene_image)); - ASSERT_EQ(context.GetCompositor().GetBackingStoresCount(), 1u); + ASSERT_EQ(context.GetCompositor().GetPendingBackingStoresCount(), 1u); } //------------------------------------------------------------------------------ @@ -2397,26 +2397,11 @@ TEST_F(EmbedderTest, VerifyB141980393) { context.GetCompositor().SetNextPresentCallback( [&](const FlutterLayer** layers, size_t layers_count) { - ASSERT_EQ(layers_count, 2u); + ASSERT_EQ(layers_count, 1u); // Layer Root { - FlutterLayer layer = {}; - FlutterBackingStore backing_store = *layers[0]->backing_store; - layer.backing_store = &backing_store; - layer.struct_size = sizeof(layer); - layer.type = kFlutterLayerContentTypeBackingStore; - - // Our root surface has been rotated. - layer.size = FlutterSizeMake(600.0, 800.0); - layer.offset = FlutterPointMake(0.0, 0.0); - - ASSERT_EQ(*layers[0], layer); - } - - // Layer 1 - { - FlutterPlatformView platform_view = *layers[1]->platform_view; + FlutterPlatformView platform_view = *layers[0]->platform_view; platform_view.struct_size = sizeof(platform_view); platform_view.identifier = 1337; @@ -2458,7 +2443,7 @@ TEST_F(EmbedderTest, VerifyB141980393) { layer.size = FlutterSizeMake(xformed_platform_view_rect.width(), xformed_platform_view_rect.height()); - ASSERT_EQ(*layers[1], layer); + ASSERT_EQ(*layers[0], layer); } latch.Signal(); @@ -3544,10 +3529,10 @@ TEST_F(EmbedderTest, ClipsAreCorrectlyCalculated) { fml::AutoResetWaitableEvent latch; context.GetCompositor().SetNextPresentCallback( [&](const FlutterLayer** layers, size_t layers_count) { - ASSERT_EQ(layers_count, 3u); + ASSERT_EQ(layers_count, 2u); { - FlutterPlatformView platform_view = *layers[1]->platform_view; + FlutterPlatformView platform_view = *layers[0]->platform_view; platform_view.struct_size = sizeof(platform_view); platform_view.identifier = 42; @@ -3558,16 +3543,16 @@ TEST_F(EmbedderTest, ClipsAreCorrectlyCalculated) { layer.size = FlutterSizeMake(300.0, 400.0); layer.offset = FlutterPointMake(0.0, 0.0); - ASSERT_EQ(*layers[1], layer); + ASSERT_EQ(*layers[0], layer); bool clip_assertions_checked = false; // The total transformation on the stack upto the platform view. const auto total_xformation = - GetTotalMutationTransformationMatrix(layers[1]->platform_view); + GetTotalMutationTransformationMatrix(layers[0]->platform_view); FilterMutationsByType( - layers[1]->platform_view, + layers[0]->platform_view, kFlutterPlatformViewMutationTypeClipRect, [&](const auto& mutation) { FlutterRect clip = mutation.clip_rect; @@ -3621,10 +3606,10 @@ TEST_F(EmbedderTest, ComplexClipsAreCorrectlyCalculated) { fml::AutoResetWaitableEvent latch; context.GetCompositor().SetNextPresentCallback( [&](const FlutterLayer** layers, size_t layers_count) { - ASSERT_EQ(layers_count, 3u); + ASSERT_EQ(layers_count, 2u); { - FlutterPlatformView platform_view = *layers[1]->platform_view; + FlutterPlatformView platform_view = *layers[0]->platform_view; platform_view.struct_size = sizeof(platform_view); platform_view.identifier = 42; @@ -3635,7 +3620,7 @@ TEST_F(EmbedderTest, ComplexClipsAreCorrectlyCalculated) { layer.size = FlutterSizeMake(600.0, 1024.0); layer.offset = FlutterPointMake(0.0, -256.0); - ASSERT_EQ(*layers[1], layer); + ASSERT_EQ(*layers[0], layer); const auto** mutations = platform_view.mutations; @@ -4006,5 +3991,146 @@ TEST_F(EmbedderTest, CanPostTaskToAllNativeThreads) { ASSERT_FALSE(engine.is_valid()); } +TEST_F(EmbedderTest, CompositorCanPostZeroLayersForPresentation) { + auto& context = GetEmbedderContext(); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(300, 200)); + builder.SetCompositor(); + builder.SetDartEntrypoint("empty_scene_posts_zero_layers_to_compositor"); + context.GetCompositor().SetRenderTargetType( + EmbedderTestCompositor::RenderTargetType::kOpenGLTexture); + + fml::AutoResetWaitableEvent latch; + + context.GetCompositor().SetNextPresentCallback( + [&](const FlutterLayer** layers, size_t layers_count) { + ASSERT_EQ(layers_count, 0u); + latch.Signal(); + }); + + auto engine = builder.LaunchEngine(); + + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = 300; + event.height = 200; + event.pixel_ratio = 1.0; + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); + ASSERT_TRUE(engine.is_valid()); + latch.Wait(); + + ASSERT_EQ(context.GetCompositor().GetPendingBackingStoresCount(), 0u); +} + +TEST_F(EmbedderTest, CompositorCanPostOnlyPlatformViews) { + auto& context = GetEmbedderContext(); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(300, 200)); + builder.SetCompositor(); + builder.SetDartEntrypoint("compositor_can_post_only_platform_views"); + context.GetCompositor().SetRenderTargetType( + EmbedderTestCompositor::RenderTargetType::kOpenGLTexture); + + fml::AutoResetWaitableEvent latch; + + context.GetCompositor().SetNextPresentCallback( + [&](const FlutterLayer** layers, size_t layers_count) { + ASSERT_EQ(layers_count, 2u); + + // Layer 0 + { + FlutterPlatformView platform_view = *layers[0]->platform_view; + platform_view.struct_size = sizeof(platform_view); + platform_view.identifier = 42; + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypePlatformView; + layer.platform_view = &platform_view; + layer.size = FlutterSizeMake(300.0, 200.0); + layer.offset = FlutterPointMake(0.0, 0.0); + + ASSERT_EQ(*layers[0], layer); + } + + // Layer 1 + { + FlutterPlatformView platform_view = *layers[1]->platform_view; + platform_view.struct_size = sizeof(platform_view); + platform_view.identifier = 24; + FlutterLayer layer = {}; + layer.struct_size = sizeof(layer); + layer.type = kFlutterLayerContentTypePlatformView; + layer.platform_view = &platform_view; + layer.size = FlutterSizeMake(300.0, 200.0); + layer.offset = FlutterPointMake(0.0, 0.0); + + ASSERT_EQ(*layers[1], layer); + } + latch.Signal(); + }); + + auto engine = builder.LaunchEngine(); + + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = 300; + event.height = 200; + event.pixel_ratio = 1.0; + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); + ASSERT_TRUE(engine.is_valid()); + latch.Wait(); + + ASSERT_EQ(context.GetCompositor().GetPendingBackingStoresCount(), 0u); +} + +TEST_F(EmbedderTest, CompositorRenderTargetsAreRecycled) { + auto& context = GetEmbedderContext(); + + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(300, 200)); + builder.SetCompositor(); + builder.SetDartEntrypoint("render_targets_are_recycled"); + context.GetCompositor().SetRenderTargetType( + EmbedderTestCompositor::RenderTargetType::kOpenGLTexture); + + fml::CountDownLatch latch(2); + + context.AddNativeCallback("SignalNativeTest", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + latch.CountDown(); + })); + + context.GetCompositor().SetNextPresentCallback( + [&](const FlutterLayer** layers, size_t layers_count) { + ASSERT_EQ(layers_count, 20u); + latch.CountDown(); + }); + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + FlutterWindowMetricsEvent event = {}; + event.struct_size = sizeof(event); + event.width = 300; + event.height = 200; + event.pixel_ratio = 1.0; + ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), + kSuccess); + + latch.Wait(); + ASSERT_EQ(context.GetCompositor().GetPendingBackingStoresCount(), 10u); + ASSERT_EQ(context.GetCompositor().GetBackingStoresCreatedCount(), 10u); + ASSERT_EQ(context.GetCompositor().GetBackingStoresCollectedCount(), 0u); + // Killing the engine should immediately collect all pending render targets. + engine.reset(); + ASSERT_EQ(context.GetCompositor().GetPendingBackingStoresCount(), 0u); + ASSERT_EQ(context.GetCompositor().GetBackingStoresCreatedCount(), 10u); + ASSERT_EQ(context.GetCompositor().GetBackingStoresCollectedCount(), 10u); +} + } // namespace testing } // namespace flutter From d21bbf1d73c380b2cca96974e09e4f7a1aa00b1d Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Mon, 17 Feb 2020 15:27:23 -0800 Subject: [PATCH 2/3] ICYU optional --- shell/platform/embedder/embedder_external_view.h | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/embedder/embedder_external_view.h b/shell/platform/embedder/embedder_external_view.h index 508bbd9a75b6d..f842f3794bc00 100644 --- a/shell/platform/embedder/embedder_external_view.h +++ b/shell/platform/embedder/embedder_external_view.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_SHELL_PLATFORM_EMBEDDER_EMBEDDER_EXTERNAL_VIEW_H_ #define FLUTTER_SHELL_PLATFORM_EMBEDDER_EMBEDDER_EXTERNAL_VIEW_H_ +#include #include #include From 1b95bf24c482fc3358a6d643ee66fff7b581dfdd Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Tue, 18 Feb 2020 11:37:35 -0800 Subject: [PATCH 3/3] PR --- shell/platform/embedder/embedder_external_view.cc | 2 +- shell/platform/embedder/embedder_external_view_embedder.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/embedder_external_view.cc b/shell/platform/embedder/embedder_external_view.cc index 5119d550b0362..f940c058297bc 100644 --- a/shell/platform/embedder/embedder_external_view.cc +++ b/shell/platform/embedder/embedder_external_view.cc @@ -75,7 +75,7 @@ bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target) { TRACE_EVENT0("flutter", "EmbedderExternalView::Render"); FML_DCHECK(HasEngineRenderedContents()) - << "Unnecessary asked to render into a render target when there was " + << "Unnecessarily asked to render into a render target when there was " "nothing to render."; auto picture = recorder_->finishRecordingAsPicture(); diff --git a/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index db8aec7246614..e9ba4554697f0 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/shell/platform/embedder/embedder_external_view_embedder.cc @@ -86,7 +86,7 @@ SkCanvas* EmbedderExternalViewEmbedder::GetRootCanvas() { FML_DLOG(WARNING) << "No root canvas could be found. This is extremely unlikely and " "indicates that the external view embedder did not receive the " - "notification to being the frame."; + "notification to begin the frame."; return nullptr; } return found->second->GetCanvas();