diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc index ddb85acd2b0c7..b0d17870a707b 100644 --- a/flow/embedded_views.cc +++ b/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/flow/embedded_views.h b/flow/embedded_views.h index 89a2aa7e5eddf..22b2c1e2db967 100644 --- a/flow/embedded_views.h +++ b/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/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 060d928f10bca..1c2cfd2e6a4bf 100644 --- a/shell/common/rasterizer.cc +++ b/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/shell/common/rasterizer.h b/shell/common/rasterizer.h index 04620957ed383..949402257f4a1 100644 --- a/shell/common/rasterizer.h +++ b/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/shell/platform/embedder/embedder.h b/shell/platform/embedder/embedder.h index 86a3a22ee0ad7..702b1d4923043 100644 --- a/shell/platform/embedder/embedder.h +++ b/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/shell/platform/embedder/embedder_external_view_embedder.cc b/shell/platform/embedder/embedder_external_view_embedder.cc index 1e9ddf90c4390..75631cb3513d4 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.cc +++ b/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/shell/platform/embedder/embedder_external_view_embedder.h b/shell/platform/embedder/embedder_external_view_embedder.h index ef51cc1b245ab..63f383f8bb4fd 100644 --- a/shell/platform/embedder/embedder_external_view_embedder.h +++ b/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/shell/platform/embedder/embedder_render_target_cache.h b/shell/platform/embedder/embedder_render_target_cache.h index fb3251a5b9fdf..04dd9de727871 100644 --- a/shell/platform/embedder/embedder_render_target_cache.h +++ b/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/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 17110a8adf67f..a0701574ab0cf 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/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);