From f8525f5f6b71e4ba5a4b63a92df9aa7b4445b02a Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Tue, 30 Jul 2024 17:47:26 +0200 Subject: [PATCH] Do not disable partial repaint based on thread merging state --- flow/diff_context.cc | 14 ++++++++----- flow/diff_context.h | 12 +++++++---- flow/layers/container_layer.cc | 2 +- flow/layers/platform_view_layer.cc | 11 ++++++++++ flow/layers/platform_view_layer.h | 1 + flow/layers/platform_view_layer_unittests.cc | 21 ++++++++++++++++++++ flow/layers/texture_layer.cc | 2 +- flow/paint_region.h | 12 +++++------ shell/common/rasterizer.cc | 10 +--------- 9 files changed, 59 insertions(+), 26 deletions(-) diff --git a/flow/diff_context.cc b/flow/diff_context.cc index 641b06588ba68..f505c0f863a3c 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_texture = false; + state_.has_volatile_layer = false; state_.integral_transform = false; if (had_integral_transform) { @@ -162,6 +162,10 @@ bool DiffContext::PushCullRect(const SkRect& clip) { return !state_.matrix_clip.device_cull_rect().isEmpty(); } +void DiffContext::ForceFullRepaint() { + damage_ = SkRect::MakeIWH(frame_size_.width(), frame_size_.height()); +} + SkMatrix DiffContext::GetTransform3x3() const { return state_.matrix_clip.matrix_3x3(); } @@ -203,14 +207,14 @@ void DiffContext::AddLayerBounds(const SkRect& rect) { } } -void DiffContext::MarkSubtreeHasTextureLayer() { +void DiffContext::MarkSubtreeHasVolatileLayer() { // 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_texture = true; + state.has_volatile_layer = true; } - state_.has_texture = true; + state_.has_volatile_layer = true; } void DiffContext::AddExistingPaintRegion(const PaintRegion& region) { @@ -238,7 +242,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_texture); + state_.has_volatile_layer); } void DiffContext::AddDamage(const PaintRegion& damage) { diff --git a/flow/diff_context.h b/flow/diff_context.h index 0eb536c49cbe4..1e0f34ae14db5 100644 --- a/flow/diff_context.h +++ b/flow/diff_context.h @@ -77,6 +77,9 @@ class DiffContext { // Pushes cull rect for current subtree bool PushCullRect(const SkRect& clip); + // Marks entire frame as dirty. + void ForceFullRepaint(); + // Function that adjusts layer bounds (in device coordinates) depending // on filter. using FilterBoundsAdjustment = std::function; @@ -107,9 +110,10 @@ class DiffContext { bool IsSubtreeDirty() const { return state_.dirty; } - // 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(); + // 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(); // Add layer bounds to current paint region; rect is in "local" (layer) // coordinates. @@ -231,7 +235,7 @@ class DiffContext { bool has_filter_bounds_adjustment = false; // Whether there is a texture layer in this subtree. - bool has_texture = false; + bool has_volatile_layer = false; }; void MakeTransformIntegral(DisplayListMatrixClipState& matrix_clip); diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 74829417f2432..384559c5c2ab2 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_texture()) { + !paint_region.has_volatile_layer()) { // 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 d641a12665d38..376981ffde94f 100644 --- a/flow/layers/platform_view_layer.cc +++ b/flow/layers/platform_view_layer.cc @@ -13,6 +13,17 @@ 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(); + // It is not necessary to track the actual paint region for the layer due to + // forced full repaint below, but the paint region carries the volatile layer + // flag. + context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion()); + context->ForceFullRepaint(); +} + 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 ac5792af55872..360a0d01f9117 100644 --- a/flow/layers/platform_view_layer.h +++ b/flow/layers/platform_view_layer.h @@ -16,6 +16,7 @@ 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 b89447f3a589e..a732740bfd076 100644 --- a/flow/layers/platform_view_layer_unittests.cc +++ b/flow/layers/platform_view_layer_unittests.cc @@ -6,6 +6,7 @@ #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" @@ -139,5 +140,25 @@ 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)); +} + } // namespace testing } // namespace flutter diff --git a/flow/layers/texture_layer.cc b/flow/layers/texture_layer.cc index c36a5eb6274d4..6fa71a9f7ed52 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->MarkSubtreeHasTextureLayer(); + context->MarkSubtreeHasVolatileLayer(); 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 52f29762f67ad..e034b45821104 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_texture) + bool has_volatile_layer) : rects_(std::move(rects)), from_(from), to_(to), has_readback_(has_readback), - has_texture_(has_texture) {} + has_volatile_layer_(has_volatile_layer) {} 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 in subtree represented by this - // region. - bool has_texture() const { return has_texture_; } + // Returns whether there is a TextureLayer or PlatformViewLayer in subtree + // represented by this region. + bool has_volatile_layer() const { return has_volatile_layer_; } private: std::shared_ptr> rects_; size_t from_ = 0; size_t to_ = 0; bool has_readback_ = false; - bool has_texture_ = false; + bool has_volatile_layer_ = false; }; } // namespace flutter diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 6239e22192a79..de0a285a27ae9 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -743,17 +743,9 @@ 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() && !force_full_repaint) { + if (existing_damage.has_value()) { damage->SetPreviousLayerTree(GetLastLayerTree(view_id)); damage->AddAdditionalDamage(existing_damage.value()); damage->SetClipAlignment(