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(