From 7553282a55d3be88c04d0bac98e00751ef74f4d3 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 4 Apr 2024 13:47:30 -0700 Subject: [PATCH] Multiview backing store (flutter/engine#51722) With this PR, backing stores are labeled with view IDs. When the engine requests the embedder to create a backing store, the engine will promise that it will only be used for a specific view. This follows the design doc http://flutter.dev/go/backing-stores-for-multi-view-partial-repaint, so that backing stores can be used as a front surface that retains its content last frame. The engine will create a render target cache for each view to cache backing stores separately. ### Alternative design The separate render target cache for each view is not needed to implement the design doc, since all usages described in the design doc avoids the engine cache. Instead, we can make the engine still only manage one render target cache for all views, and backing stores in it are interchangeable across views. We might describe the behavior in this way: * In general, the view ID is just provided to the creating callback as information. (This is how it is seen when the engine cache is avoided.) * If the engine cache is used, then the created backing store might not be immediately collected, and might be reused for different views. But it's really hard to explain the mechanism and the result is really confusing (as can be seen from my attempt). Why is there a view ID but it's not used, and if you enable the engine cache it's not even followed? That's why I chose the current approach. Feel free to suggest otherwise for this. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- engine/src/flutter/flow/embedded_views.cc | 2 + engine/src/flutter/flow/embedded_views.h | 9 + engine/src/flutter/shell/common/rasterizer.cc | 3 + engine/src/flutter/shell/common/rasterizer.h | 9 +- .../shell/platform/embedder/embedder.h | 11 ++ .../embedder_external_view_embedder.cc | 42 +++-- .../embedder_external_view_embedder.h | 6 +- .../embedder/embedder_render_target_cache.h | 4 + .../embedder/tests/embedder_unittests.cc | 171 ++++++++++++++++++ 9 files changed, 236 insertions(+), 21 deletions(-) diff --git a/engine/src/flutter/flow/embedded_views.cc b/engine/src/flutter/flow/embedded_views.cc index ddb85acd2b0..b0d17870a70 100644 --- a/engine/src/flutter/flow/embedded_views.cc +++ b/engine/src/flutter/flow/embedded_views.cc @@ -42,6 +42,8 @@ bool DisplayListEmbedderViewSlice::recording_ended() { return builder_ == nullptr; } +void ExternalViewEmbedder::CollectView(int64_t view_id) {} + void ExternalViewEmbedder::SubmitFlutterView( int64_t flutter_view_id, GrDirectContext* context, diff --git a/engine/src/flutter/flow/embedded_views.h b/engine/src/flutter/flow/embedded_views.h index 89a2aa7e5ed..22b2c1e2db9 100644 --- a/engine/src/flutter/flow/embedded_views.h +++ b/engine/src/flutter/flow/embedded_views.h @@ -400,6 +400,15 @@ class ExternalViewEmbedder { virtual ~ExternalViewEmbedder() = default; + // Deallocate the resources for displaying a view. + // + // This method must be called when a view is removed from the engine. + // + // When the ExternalViewEmbedder is requested to draw an unrecognized view, it + // implicitly allocates necessary resources. These resources must be + // explicitly deallocated. + virtual void CollectView(int64_t view_id); + // Usually, the root canvas is not owned by the view embedder. However, if // the view embedder wants to provide a canvas to the rasterizer, it may // return one here. This canvas takes priority over the canvas materialized diff --git a/engine/src/flutter/shell/common/rasterizer.cc b/engine/src/flutter/shell/common/rasterizer.cc index 060d928f10b..1c2cfd2e6a4 100644 --- a/engine/src/flutter/shell/common/rasterizer.cc +++ b/engine/src/flutter/shell/common/rasterizer.cc @@ -184,6 +184,9 @@ void Rasterizer::NotifyLowMemoryWarning() const { } void Rasterizer::CollectView(int64_t view_id) { + if (external_view_embedder_) { + external_view_embedder_->CollectView(view_id); + } view_records_.erase(view_id); } diff --git a/engine/src/flutter/shell/common/rasterizer.h b/engine/src/flutter/shell/common/rasterizer.h index 04620957ed3..949402257f4 100644 --- a/engine/src/flutter/shell/common/rasterizer.h +++ b/engine/src/flutter/shell/common/rasterizer.h @@ -261,11 +261,12 @@ class Rasterizer final : public SnapshotDelegate, //---------------------------------------------------------------------------- /// @brief Deallocate the resources for displaying a view. /// - /// This method must be called when a view is removed. + /// This method must be called on the raster task runner when a + /// view is removed from the engine. /// - /// The rasterizer don't need views to be registered. Last-frame - /// states for views are recorded when layer trees are rasterized - /// to the view and used during `Rasterizer::DrawLastLayerTrees`. + /// When the rasterizer is requested to draw an unrecognized view, + /// it implicitly allocates necessary resources. These resources + /// must be explicitly deallocated. /// /// @param[in] view_id The ID of the view. /// diff --git a/engine/src/flutter/shell/platform/embedder/embedder.h b/engine/src/flutter/shell/platform/embedder/embedder.h index 86a3a22ee0a..702b1d49230 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder.h +++ b/engine/src/flutter/shell/platform/embedder/embedder.h @@ -1780,6 +1780,9 @@ typedef struct { size_t struct_size; /// The size of the render target the engine expects to render into. FlutterSize size; + /// The identifier for the view that the engine will use this backing store to + /// render into. + FlutterViewId view_id; } FlutterBackingStoreConfig; typedef enum { @@ -1893,9 +1896,13 @@ typedef struct { /// `FlutterBackingStore::struct_size` when specifying a new backing store to /// the engine. This only matters if the embedder expects to be used with /// engines older than the version whose headers it used during compilation. + /// + /// The callback should return true if the operation was successful. FlutterBackingStoreCreateCallback create_backing_store_callback; /// A callback invoked by the engine to release the backing store. The /// embedder may collect any resources associated with the backing store. + /// + /// The callback should return true if the operation was successful. FlutterBackingStoreCollectCallback collect_backing_store_callback; /// Callback invoked by the engine to composite the contents of each layer /// onto the implicit view. @@ -1907,6 +1914,8 @@ typedef struct { /// Only one of `present_layers_callback` and `present_view_callback` may be /// provided. Providing both is an error and engine initialization will /// terminate. + /// + /// The callback should return true if the operation was successful. FlutterLayersPresentCallback present_layers_callback; /// Avoid caching backing stores provided by this compositor. bool avoid_backing_store_cache; @@ -1916,6 +1925,8 @@ typedef struct { /// Only one of `present_layers_callback` and `present_view_callback` may be /// provided. Providing both is an error and engine initialization will /// terminate. + /// + /// The callback should return true if the operation was successful. FlutterPresentViewCallback present_view_callback; } FlutterCompositor; diff --git a/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.cc b/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.cc index 1e9ddf90c43..75631cb3513 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.cc @@ -29,6 +29,10 @@ EmbedderExternalViewEmbedder::EmbedderExternalViewEmbedder( EmbedderExternalViewEmbedder::~EmbedderExternalViewEmbedder() = default; +void EmbedderExternalViewEmbedder::CollectView(int64_t view_id) { + render_target_caches_.erase(view_id); +} + void EmbedderExternalViewEmbedder::SetSurfaceTransformationCallback( SurfaceTransformationCallback surface_transformation_callback) { surface_transformation_callback_ = std::move(surface_transformation_callback); @@ -114,6 +118,7 @@ DlCanvas* EmbedderExternalViewEmbedder::CompositeEmbeddedView(int64_t view_id) { } static FlutterBackingStoreConfig MakeBackingStoreConfig( + int64_t view_id, const SkISize& backing_store_size) { FlutterBackingStoreConfig config = {}; @@ -121,6 +126,7 @@ static FlutterBackingStoreConfig MakeBackingStoreConfig( config.size.width = backing_store_size.width(); config.size.height = backing_store_size.height(); + config.view_id = view_id; return config; } @@ -284,6 +290,10 @@ class Layer { /// Implements https://flutter.dev/go/optimized-platform-view-layers class LayerBuilder { public: + using RenderTargetProvider = + std::function( + const SkISize& frame_size)>; + explicit LayerBuilder(SkISize frame_size) : frame_size_(frame_size) { layers_.push_back(Layer()); } @@ -304,13 +314,10 @@ class LayerBuilder { } /// Prepares the render targets for all layers that have Flutter contents. - void PrepareBackingStore( - const std::function( - FlutterBackingStoreConfig)>& target_provider) { - auto config = MakeBackingStoreConfig(frame_size_); + void PrepareBackingStore(const RenderTargetProvider& target_provider) { for (auto& layer : layers_) { if (layer.has_flutter_contents()) { - layer.SetRenderTarget(target_provider(config)); + layer.SetRenderTarget(target_provider(frame_size_)); } } } @@ -416,6 +423,10 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( GrDirectContext* context, const std::shared_ptr& aiks_context, std::unique_ptr frame) { + // The unordered_map render_target_cache creates a new entry if the view ID is + // unrecognized. + EmbedderRenderTargetCache& render_target_cache = + render_target_caches_[flutter_view_id]; SkRect _rect = SkRect::MakeIWH(pending_frame_size_.width(), pending_frame_size_.height()); pending_surface_transformation_.mapRect(&_rect); @@ -427,17 +438,16 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( builder.AddExternalView(view.get()); } - builder.PrepareBackingStore([&](FlutterBackingStoreConfig config) { - std::unique_ptr target; + builder.PrepareBackingStore([&](const SkISize& frame_size) { if (!avoid_backing_store_cache_) { - target = render_target_cache_.GetRenderTarget( - EmbedderExternalView::RenderTargetDescriptor( - SkISize{static_cast(config.size.width), - static_cast(config.size.height)})); - } - if (target != nullptr) { - return target; + std::unique_ptr target = + render_target_cache.GetRenderTarget( + EmbedderExternalView::RenderTargetDescriptor(frame_size)); + if (target != nullptr) { + return target; + } } + auto config = MakeBackingStoreConfig(flutter_view_id, frame_size); return create_render_target_callback_(context, aiks_context, config); }); @@ -454,7 +464,7 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( // // @warning: Embedder may trample on our OpenGL context here. auto deferred_cleanup_render_targets = - render_target_cache_.ClearAllRenderTargetsInCache(); + render_target_cache.ClearAllRenderTargetsInCache(); // 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 @@ -502,7 +512,7 @@ void EmbedderExternalViewEmbedder::SubmitFlutterView( auto render_targets = builder.ClearAndCollectRenderTargets(); for (auto& render_target : render_targets) { if (!avoid_backing_store_cache_) { - render_target_cache_.CacheRenderTarget(std::move(render_target)); + render_target_cache.CacheRenderTarget(std::move(render_target)); } } diff --git a/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.h b/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.h index ef51cc1b245..63f383f8bb4 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.h +++ b/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.h @@ -66,6 +66,9 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { /// ~EmbedderExternalViewEmbedder() override; + // |ExternalViewEmbedder| + void CollectView(int64_t view_id) override; + //---------------------------------------------------------------------------- /// @brief Sets the surface transformation callback used by the external /// view embedder to ask the platform for the per frame root @@ -118,7 +121,8 @@ class EmbedderExternalViewEmbedder final : public ExternalViewEmbedder { SkMatrix pending_surface_transformation_; EmbedderExternalView::PendingViews pending_views_; std::vector composition_order_; - EmbedderRenderTargetCache render_target_cache_; + // The render target caches for views. Each key is a view ID. + std::unordered_map render_target_caches_; void Reset(); diff --git a/engine/src/flutter/shell/platform/embedder/embedder_render_target_cache.h b/engine/src/flutter/shell/platform/embedder/embedder_render_target_cache.h index fb3251a5b9f..04dd9de7278 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder_render_target_cache.h +++ b/engine/src/flutter/shell/platform/embedder/embedder_render_target_cache.h @@ -19,6 +19,10 @@ 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. /// +/// A map of class is managed by EmbedderExternalViewEmbedder. Each +/// instance of this class manages the cached render targets for a +/// view. +/// class EmbedderRenderTargetCache { public: EmbedderRenderTargetCache(); diff --git a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc index 17110a8adf6..a0701574ab0 100644 --- a/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc +++ b/engine/src/flutter/shell/platform/embedder/tests/embedder_unittests.cc @@ -1814,6 +1814,177 @@ TEST_F(EmbedderTest, CanRenderMultipleViews) { latch123.Wait(); } +//------------------------------------------------------------------------------ +/// Test that the backing store is created with the correct view ID, is used +/// for the correct view, and is cached according to their views. +/// +/// The test involves two frames: +/// 1. The first frame renders the implicit view and the second view. +/// 2. The second frame renders the implicit view and the third view. +/// +/// The test verifies that: +/// - Each backing store is created with a valid view ID. +/// - Each backing store is presented for the view that it was created for. +/// - Both frames render the expected sets of views. +/// - By the end of frame 1, only 2 backing stores were created. +/// - By the end of frame 2, only 3 backing stores were created. This ensures +/// that the backing store for the 2nd view is not reused for the 3rd view. +TEST_F(EmbedderTest, BackingStoresCorrespondToTheirViews) { + constexpr FlutterViewId kSecondViewId = 123; + constexpr FlutterViewId kThirdViewId = 456; + auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); + + EmbedderConfigBuilder builder(context); + builder.SetDartEntrypoint("render_all_views"); + builder.SetSoftwareRendererConfig(SkISize::Make(800, 600)); + builder.SetCompositor(); + + EmbedderTestBackingStoreProducer producer( + context.GetCompositor().GetGrContext(), + EmbedderTestBackingStoreProducer::RenderTargetType::kSoftwareBuffer); + + // The variables needed by the callbacks of the compositor. + struct CompositorUserData { + EmbedderTestBackingStoreProducer* producer; + // Each latch is signaled when its corresponding view is presented. + fml::AutoResetWaitableEvent latch_implicit; + fml::AutoResetWaitableEvent latch_second; + fml::AutoResetWaitableEvent latch_third; + // Whether the respective view should be rendered in the frame. + bool second_expected; + bool third_expected; + // The total number of backing stores created to verify caching. + int backing_stores_created; + }; + CompositorUserData compositor_user_data{ + .producer = &producer, + .backing_stores_created = 0, + }; + + builder.GetCompositor() = FlutterCompositor{ + .struct_size = sizeof(FlutterCompositor), + .user_data = reinterpret_cast(&compositor_user_data), + .create_backing_store_callback = + [](const FlutterBackingStoreConfig* config, + FlutterBackingStore* backing_store_out, void* user_data) { + // Verify that the backing store comes with the correct view ID. + EXPECT_TRUE(config->view_id == 0 || + config->view_id == kSecondViewId || + config->view_id == kThirdViewId); + auto compositor_user_data = + reinterpret_cast(user_data); + compositor_user_data->backing_stores_created += 1; + bool result = compositor_user_data->producer->Create( + config, backing_store_out); + // The created backing store has a user_data that records the view + // that the store is created for. + backing_store_out->user_data = + reinterpret_cast(config->view_id); + return result; + }, + .collect_backing_store_callback = [](const FlutterBackingStore* renderer, + void* user_data) { return true; }, + .present_layers_callback = nullptr, + .avoid_backing_store_cache = false, + .present_view_callback = + [](const FlutterPresentViewInfo* info) { + EXPECT_EQ(info->layers_count, 1u); + // Verify that the given layer's backing store has the same view ID + // as the target view. + int64_t store_view_id = reinterpret_cast( + info->layers[0]->backing_store->user_data); + EXPECT_EQ(store_view_id, info->view_id); + auto compositor_user_data = + reinterpret_cast(info->user_data); + // Verify that the respective views are rendered. + switch (info->view_id) { + case 0: + compositor_user_data->latch_implicit.Signal(); + break; + case kSecondViewId: + EXPECT_TRUE(compositor_user_data->second_expected); + compositor_user_data->latch_second.Signal(); + break; + case kThirdViewId: + EXPECT_TRUE(compositor_user_data->third_expected); + compositor_user_data->latch_third.Signal(); + break; + default: + FML_UNREACHABLE(); + } + return true; + }, + }; + + compositor_user_data.second_expected = true; + compositor_user_data.third_expected = false; + + /*=== First frame ===*/ + + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + + // Give the implicit view a non-zero size so that it renders something. + FlutterWindowMetricsEvent metrics_implicit = { + .struct_size = sizeof(FlutterWindowMetricsEvent), + .width = 800, + .height = 600, + .pixel_ratio = 1.0, + .view_id = 0, + }; + ASSERT_EQ( + FlutterEngineSendWindowMetricsEvent(engine.get(), &metrics_implicit), + kSuccess); + + // Add the second view. + FlutterWindowMetricsEvent metrics_add = { + .struct_size = sizeof(FlutterWindowMetricsEvent), + .width = 800, + .height = 600, + .pixel_ratio = 1.0, + .view_id = kSecondViewId, + }; + + FlutterAddViewInfo add_view_info = {}; + add_view_info.struct_size = sizeof(FlutterAddViewInfo); + add_view_info.view_id = kSecondViewId; + add_view_info.view_metrics = &metrics_add; + add_view_info.add_view_callback = [](const FlutterAddViewResult* result) { + ASSERT_TRUE(result->added); + }; + + ASSERT_EQ(FlutterEngineAddView(engine.get(), &add_view_info), kSuccess); + + compositor_user_data.latch_implicit.Wait(); + compositor_user_data.latch_second.Wait(); + + /*=== Second frame ===*/ + + compositor_user_data.second_expected = false; + compositor_user_data.third_expected = true; + EXPECT_EQ(compositor_user_data.backing_stores_created, 2); + + // Remove the second view + FlutterRemoveViewInfo remove_view_info = {}; + remove_view_info.struct_size = sizeof(FlutterRemoveViewInfo); + remove_view_info.view_id = kSecondViewId; + remove_view_info.remove_view_callback = + [](const FlutterRemoveViewResult* result) { + ASSERT_TRUE(result->removed); + }; + ASSERT_EQ(FlutterEngineRemoveView(engine.get(), &remove_view_info), kSuccess); + + // Add the third view. + add_view_info.view_id = kThirdViewId; + metrics_add.view_id = kThirdViewId; + ASSERT_EQ(FlutterEngineAddView(engine.get(), &add_view_info), kSuccess); + // Adding the view should have scheduled a frame. + + compositor_user_data.latch_implicit.Wait(); + compositor_user_data.latch_third.Wait(); + EXPECT_EQ(compositor_user_data.backing_stores_created, 3); +} + TEST_F(EmbedderTest, CanUpdateLocales) { auto& context = GetEmbedderContext(EmbedderTestContextType::kSoftwareContext); EmbedderConfigBuilder builder(context);