Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Revert "Reland: Partial repaint platform views" #54537

Merged
merged 1 commit into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions flow/diff_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 4 additions & 8 deletions flow/diff_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion flow/layers/container_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions flow/layers/platform_view_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
1 change: 0 additions & 1 deletion flow/layers/platform_view_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
41 changes: 0 additions & 41 deletions flow/layers/platform_view_layer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<ContainerLayer>();
tree1.root()->Add(container);
auto layer = std::make_shared<PlatformViewLayer>(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<ContainerLayer>();
tree1.root()->Add(container);
auto layer = std::make_shared<PlatformViewLayer>(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<ContainerLayer>();
tree2.root()->Add(container2);

damage = DiffLayerTree(tree2, tree1);
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 800, 600));
}

} // namespace testing
} // namespace flutter
2 changes: 1 addition & 1 deletion flow/layers/texture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
12 changes: 6 additions & 6 deletions flow/paint_region.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<SkRect>::const_iterator begin() const {
FML_DCHECK(is_valid());
Expand All @@ -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<std::vector<SkRect>> rects_;
size_t from_ = 0;
size_t to_ = 0;
bool has_readback_ = false;
bool has_volatile_layer_ = false;
bool has_texture_ = false;
};

} // namespace flutter
Expand Down
10 changes: 9 additions & 1 deletion shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<FrameDamage>();
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(
Expand Down