diff --git a/flow/diff_context.cc b/flow/diff_context.cc index 18a14cd17675e..641b06588ba68 100644 --- a/flow/diff_context.cc +++ b/flow/diff_context.cc @@ -27,7 +27,7 @@ void DiffContext::BeginSubtree() { bool had_integral_transform = state_.integral_transform; state_.rect_index = rects_->size(); state_.has_filter_bounds_adjustment = false; - state_.has_volatile_layer = false; + state_.has_texture = false; state_.integral_transform = false; if (had_integral_transform) { @@ -203,20 +203,14 @@ void DiffContext::AddLayerBounds(const SkRect& rect) { } } -void DiffContext::RepaintEntireFrame() { - auto frame_rect = SkRect::MakeIWH(frame_size_.width(), frame_size_.height()); - rects_->push_back(frame_rect); - AddDamage(frame_rect); -} - -void DiffContext::MarkSubtreeHasVolatileLayer() { +void DiffContext::MarkSubtreeHasTextureLayer() { // Set the has_texture flag on current state and all parent states. That // way we'll know that we can't skip diff for retained layers because // they contain a TextureLayer. for (auto& state : state_stack_) { - state.has_volatile_layer = true; + state.has_texture = true; } - state_.has_volatile_layer = true; + state_.has_texture = true; } void DiffContext::AddExistingPaintRegion(const PaintRegion& region) { @@ -244,7 +238,7 @@ PaintRegion DiffContext::CurrentSubtreeRegion() const { readbacks_.begin(), readbacks_.end(), [&](const Readback& r) { return r.position >= state_.rect_index; }); return PaintRegion(rects_, state_.rect_index, rects_->size(), has_readback, - state_.has_volatile_layer); + state_.has_texture); } void DiffContext::AddDamage(const PaintRegion& damage) { diff --git a/flow/diff_context.h b/flow/diff_context.h index 539d5f73cec70..0eb536c49cbe4 100644 --- a/flow/diff_context.h +++ b/flow/diff_context.h @@ -107,18 +107,14 @@ class DiffContext { bool IsSubtreeDirty() const { return state_.dirty; } - // Marks that current subtree contains a volatile layer. A volatile layer will - // do diffing even if it is a retained subtree. Necessary for TextureLayer - // and PlatformViewLayer. - void MarkSubtreeHasVolatileLayer(); + // Marks that current subtree contains a TextureLayer. This is needed to + // ensure that we'll Diff the TextureLayer even if inside retained layer. + void MarkSubtreeHasTextureLayer(); // Add layer bounds to current paint region; rect is in "local" (layer) // coordinates. void AddLayerBounds(const SkRect& rect); - // Marks entire frame as dirty. - void RepaintEntireFrame(); - // Add entire paint region of retained layer for current subtree. This can // only be used in subtrees that are not dirty, otherwise ancestor transforms // or clips may result in different paint region. @@ -235,7 +231,7 @@ class DiffContext { bool has_filter_bounds_adjustment = false; // Whether there is a texture layer in this subtree. - bool has_volatile_layer = false; + bool has_texture = false; }; void MakeTransformIntegral(DisplayListMatrixClipState& matrix_clip); diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 384559c5c2ab2..74829417f2432 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -78,7 +78,7 @@ void ContainerLayer::DiffChildren(DiffContext* context, auto prev_layer = prev_layers[i_prev]; auto paint_region = context->GetOldLayerPaintRegion(prev_layer.get()); if (layer == prev_layer && !paint_region.has_readback() && - !paint_region.has_volatile_layer()) { + !paint_region.has_texture()) { // for retained layers, stop processing the subtree and add existing // region; We know current subtree is not dirty (every ancestor up to // here matches) so the retained subtree will render identically to diff --git a/flow/layers/platform_view_layer.cc b/flow/layers/platform_view_layer.cc index 38b816d31660c..d641a12665d38 100644 --- a/flow/layers/platform_view_layer.cc +++ b/flow/layers/platform_view_layer.cc @@ -13,17 +13,6 @@ PlatformViewLayer::PlatformViewLayer(const SkPoint& offset, int64_t view_id) : offset_(offset), size_(size), view_id_(view_id) {} -void PlatformViewLayer::Diff(DiffContext* context, const Layer* old_layer) { - DiffContext::AutoSubtreeRestore subtree(context); - // Ensure that Diff is called again even if this layer is in retained subtree. - context->MarkSubtreeHasVolatileLayer(); - // Partial repaint is disabled when platform view is present. This will also - // set whole frame as the layer paint region, which will ensure full repaint - // when the layer is removed. - context->RepaintEntireFrame(); - context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion()); -} - void PlatformViewLayer::Preroll(PrerollContext* context) { set_paint_bounds(SkRect::MakeXYWH(offset_.x(), offset_.y(), size_.width(), size_.height())); diff --git a/flow/layers/platform_view_layer.h b/flow/layers/platform_view_layer.h index 360a0d01f9117..ac5792af55872 100644 --- a/flow/layers/platform_view_layer.h +++ b/flow/layers/platform_view_layer.h @@ -16,7 +16,6 @@ class PlatformViewLayer : public Layer { public: PlatformViewLayer(const SkPoint& offset, const SkSize& size, int64_t view_id); - void Diff(DiffContext* context, const Layer* old_layer) override; void Preroll(PrerollContext* context) override; void Paint(PaintContext& context) const override; diff --git a/flow/layers/platform_view_layer_unittests.cc b/flow/layers/platform_view_layer_unittests.cc index 97e1ee9ce7383..b89447f3a589e 100644 --- a/flow/layers/platform_view_layer_unittests.cc +++ b/flow/layers/platform_view_layer_unittests.cc @@ -6,7 +6,6 @@ #include "flutter/flow/layers/platform_view_layer.h" #include "flutter/flow/layers/transform_layer.h" -#include "flutter/flow/testing/diff_context_test.h" #include "flutter/flow/testing/layer_test.h" #include "flutter/flow/testing/mock_embedder.h" #include "flutter/flow/testing/mock_layer.h" @@ -140,45 +139,5 @@ TEST_F(PlatformViewLayerTest, StateTransfer) { transform_layer1->Paint(paint_ctx); } -using PlatformViewLayerDiffTest = DiffContextTest; - -TEST_F(PlatformViewLayerDiffTest, PlatformViewRetainedLayer) { - MockLayerTree tree1(SkISize::Make(800, 600)); - auto container = std::make_shared(); - tree1.root()->Add(container); - auto layer = std::make_shared(SkPoint::Make(100, 100), - SkSize::Make(100, 100), 0); - container->Add(layer); - - MockLayerTree tree2(SkISize::Make(800, 600)); - tree2.root()->Add(container); // retained layer - - auto damage = DiffLayerTree(tree1, MockLayerTree(SkISize::Make(800, 600))); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600)); - - damage = DiffLayerTree(tree2, tree1); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600)); -} - -TEST_F(PlatformViewLayerDiffTest, FullRepaintAfterRemovingLayer) { - MockLayerTree tree1(SkISize::Make(800, 600)); - auto container = std::make_shared(); - tree1.root()->Add(container); - auto layer = std::make_shared(SkPoint::Make(100, 100), - SkSize::Make(100, 100), 0); - container->Add(layer); - - auto damage = DiffLayerTree(tree1, MockLayerTree(SkISize::Make(800, 600))); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600)); - - // Second layer tree with the PlatformViewLayer removed. - MockLayerTree tree2(SkISize::Make(800, 600)); - auto container2 = std::make_shared(); - tree2.root()->Add(container2); - - damage = DiffLayerTree(tree2, tree1); - EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600)); -} - } // namespace testing } // namespace flutter diff --git a/flow/layers/texture_layer.cc b/flow/layers/texture_layer.cc index 6fa71a9f7ed52..c36a5eb6274d4 100644 --- a/flow/layers/texture_layer.cc +++ b/flow/layers/texture_layer.cc @@ -34,7 +34,7 @@ void TextureLayer::Diff(DiffContext* context, const Layer* old_layer) { // TextureLayer is inside retained layer. // See ContainerLayer::DiffChildren // https://github.com/flutter/flutter/issues/92925 - context->MarkSubtreeHasVolatileLayer(); + context->MarkSubtreeHasTextureLayer(); context->AddLayerBounds(SkRect::MakeXYWH(offset_.x(), offset_.y(), size_.width(), size_.height())); context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion()); diff --git a/flow/paint_region.h b/flow/paint_region.h index e034b45821104..52f29762f67ad 100644 --- a/flow/paint_region.h +++ b/flow/paint_region.h @@ -30,12 +30,12 @@ class PaintRegion { size_t from, size_t to, bool has_readback, - bool has_volatile_layer) + bool has_texture) : rects_(std::move(rects)), from_(from), to_(to), has_readback_(has_readback), - has_volatile_layer_(has_volatile_layer) {} + has_texture_(has_texture) {} std::vector::const_iterator begin() const { FML_DCHECK(is_valid()); @@ -56,16 +56,16 @@ class PaintRegion { // that performs readback bool has_readback() const { return has_readback_; } - // Returns whether there is a TextureLayer or PlatformViewLayer in subtree - // represented by this region. - bool has_volatile_layer() const { return has_volatile_layer_; } + // Returns whether there is a TextureLayer in subtree represented by this + // region. + bool has_texture() const { return has_texture_; } private: std::shared_ptr> rects_; size_t from_ = 0; size_t to_ = 0; bool has_readback_ = false; - bool has_volatile_layer_ = false; + bool has_texture_ = false; }; } // namespace flutter diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index de0a285a27ae9..6239e22192a79 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -743,9 +743,17 @@ DrawSurfaceStatus Rasterizer::DrawToSurfaceUnsafe( // when leaf layer tracing is enabled we wish to repaint the whole frame // for accurate performance metrics. if (frame->framebuffer_info().supports_partial_repaint) { + // Disable partial repaint if external_view_embedder_ SubmitFlutterView is + // involved - ExternalViewEmbedder unconditionally clears the entire + // surface and also partial repaint with platform view present is + // something that still need to be figured out. + bool force_full_repaint = + external_view_embedder_ && + (!raster_thread_merger_ || raster_thread_merger_->IsMerged()); + damage = std::make_unique(); auto existing_damage = frame->framebuffer_info().existing_damage; - if (existing_damage.has_value()) { + if (existing_damage.has_value() && !force_full_repaint) { damage->SetPreviousLayerTree(GetLastLayerTree(view_id)); damage->AddAdditionalDamage(existing_damage.value()); damage->SetClipAlignment( diff --git a/shell/platform/darwin/ios/framework/Source/platform_views_controller.h b/shell/platform/darwin/ios/framework/Source/platform_views_controller.h index 551792ef8d47b..1079b6d767c3a 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_views_controller.h +++ b/shell/platform/darwin/ios/framework/Source/platform_views_controller.h @@ -84,14 +84,16 @@ class PlatformViewsController { /// /// Called from the raster thread. PostPrerollResult PostPrerollAction( - const fml::RefPtr& raster_thread_merger); + const fml::RefPtr& raster_thread_merger, + bool impeller_enabled); /// @brief Mark the end of a compositor frame. /// /// May determine changes are required to the thread merging state. /// Called from the raster thread. void EndFrame(bool should_resubmit_frame, - const fml::RefPtr& raster_thread_merger); + const fml::RefPtr& raster_thread_merger, + bool impeller_enabled); /// @brief Returns the Canvas for the overlay slice for the given platform view. /// diff --git a/shell/platform/darwin/ios/framework/Source/platform_views_controller.mm b/shell/platform/darwin/ios/framework/Source/platform_views_controller.mm index 2627268961997..806d9510011d9 100644 --- a/shell/platform/darwin/ios/framework/Source/platform_views_controller.mm +++ b/shell/platform/darwin/ios/framework/Source/platform_views_controller.mm @@ -15,13 +15,11 @@ namespace { -#ifdef FML_OS_IOS_SIMULATOR // The number of frames the rasterizer task runner will continue // to run on the platform thread after no platform view is rendered. // // Note: this is an arbitrary number. static const int kDefaultMergedLeaseDuration = 10; -#endif // FML_OS_IOS_SIMULATOR static constexpr NSUInteger kFlutterClippingMaskViewPoolCapacity = 5; @@ -291,43 +289,52 @@ bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, } PostPrerollResult PlatformViewsController::PostPrerollAction( - const fml::RefPtr& raster_thread_merger) { + const fml::RefPtr& raster_thread_merger, + bool impeller_enabled) { // TODO(jonahwilliams): remove this once Software backend is removed for iOS Sim. #ifdef FML_OS_IOS_SIMULATOR - if (composition_order_.empty()) { - return PostPrerollResult::kSuccess; - } - if (!raster_thread_merger->IsMerged()) { - // The raster thread merger may be disabled if the rasterizer is being - // created or teared down. - // - // In such cases, the current frame is dropped, and a new frame is attempted - // with the same layer tree. - // - // Eventually, the frame is submitted once this method returns `kSuccess`. - // At that point, the raster tasks are handled on the platform thread. - CancelFrame(); - return PostPrerollResult::kSkipAndRetryFrame; - } - // If the post preroll action is successful, we will display platform views in the current frame. - // In order to sync the rendering of the platform views (quartz) with skia's rendering, - // We need to begin an explicit CATransaction. This transaction needs to be submitted - // after the current frame is submitted. - raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); - return PostPrerollResult::kSuccess; + const bool merge_threads = true; #else - return PostPrerollResult::kSuccess; + const bool merge_threads = !impeller_enabled; #endif // FML_OS_IOS_SIMULATOR + + if (merge_threads) { + if (composition_order_.empty()) { + return PostPrerollResult::kSuccess; + } + if (!raster_thread_merger->IsMerged()) { + // The raster thread merger may be disabled if the rasterizer is being + // created or teared down. + // + // In such cases, the current frame is dropped, and a new frame is attempted + // with the same layer tree. + // + // Eventually, the frame is submitted once this method returns `kSuccess`. + // At that point, the raster tasks are handled on the platform thread. + CancelFrame(); + return PostPrerollResult::kSkipAndRetryFrame; + } + // If the post preroll action is successful, we will display platform views in the current + // frame. In order to sync the rendering of the platform views (quartz) with skia's rendering, + // We need to begin an explicit CATransaction. This transaction needs to be submitted + // after the current frame is submitted. + raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); + } + return PostPrerollResult::kSuccess; } void PlatformViewsController::EndFrame( bool should_resubmit_frame, - const fml::RefPtr& raster_thread_merger) { + const fml::RefPtr& raster_thread_merger, + bool impeller_enabled) { #if FML_OS_IOS_SIMULATOR - if (should_resubmit_frame) { + bool run_check = true; +#else + bool run_check = !impeller_enabled; +#endif // FML_OS_IOS_SIMULATOR + if (run_check && should_resubmit_frame) { raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); } -#endif // FML_OS_IOS_SIMULATOR } void PlatformViewsController::PushFilterToVisitedPlatformViews( diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.mm b/shell/platform/darwin/ios/ios_external_view_embedder.mm index b7c1333d91bf5..f8c2f47c7aefc 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.mm +++ b/shell/platform/darwin/ios/ios_external_view_embedder.mm @@ -59,7 +59,8 @@ const fml::RefPtr& raster_thread_merger) { TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::PostPrerollAction"); FML_CHECK(platform_views_controller_); - PostPrerollResult result = platform_views_controller_->PostPrerollAction(raster_thread_merger); + PostPrerollResult result = platform_views_controller_->PostPrerollAction( + raster_thread_merger, ios_context_->GetBackend() != IOSRenderingBackend::kSkia); return result; } @@ -91,7 +92,8 @@ bool should_resubmit_frame, const fml::RefPtr& raster_thread_merger) { TRACE_EVENT0("flutter", "IOSExternalViewEmbedder::EndFrame"); - platform_views_controller_->EndFrame(should_resubmit_frame, raster_thread_merger); + platform_views_controller_->EndFrame(should_resubmit_frame, raster_thread_merger, + ios_context_->GetBackend() != IOSRenderingBackend::kSkia); } // |ExternalViewEmbedder| @@ -100,7 +102,7 @@ #if FML_OS_IOS_SIMULATOR return true; #else - return false; + return ios_context_->GetBackend() == IOSRenderingBackend::kSkia; #endif // FML_OS_IOS_SIMULATOR }