Fix partial repaint when TextureLayer is inside retained layer (flutter/engine#29482)

This commit is contained in:
Matej Knopp
2021-11-02 23:40:48 +01:00
committed by GitHub
parent 6a3b2c0194
commit 7e8fc6005e
6 changed files with 65 additions and 5 deletions

View File

@@ -21,6 +21,7 @@ void DiffContext::BeginSubtree() {
state_stack_.push_back(state_);
state_.rect_index_ = rects_->size();
state_.has_filter_bounds_adjustment = false;
state_.has_texture = false;
if (state_.transform_override) {
state_.transform = *state_.transform_override;
state_.transform_override = std::nullopt;
@@ -40,7 +41,8 @@ DiffContext::State::State()
: dirty(false),
cull_rect(kGiantRect),
rect_index_(0),
has_filter_bounds_adjustment(false) {}
has_filter_bounds_adjustment(false),
has_texture(false) {}
void DiffContext::PushTransform(const SkMatrix& transform) {
state_.transform.preConcat(transform);
@@ -140,6 +142,16 @@ void DiffContext::AddLayerBounds(const SkRect& rect) {
}
}
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_texture = true;
}
state_.has_texture = true;
}
void DiffContext::AddExistingPaintRegion(const PaintRegion& region) {
// Adding paint region for retained layer implies that current subtree is not
// dirty, so we know, for example, that the inherited transforms must match
@@ -162,7 +174,8 @@ PaintRegion DiffContext::CurrentSubtreeRegion() const {
bool has_readback = std::any_of(
readbacks_.begin(), readbacks_.end(),
[&](const Readback& r) { return r.position >= state_.rect_index_; });
return PaintRegion(rects_, state_.rect_index_, rects_->size(), has_readback);
return PaintRegion(rects_, state_.rect_index_, rects_->size(), has_readback,
state_.has_texture);
}
void DiffContext::AddDamage(const PaintRegion& damage) {

View File

@@ -104,6 +104,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();
// Add layer bounds to current paint region; rect is in "local" (layer)
// coordinates.
void AddLayerBounds(const SkRect& rect);
@@ -203,6 +207,9 @@ class DiffContext {
// Whether this subtree has filter bounds adjustment function. If so,
// it will need to be removed from stack when subtree is closed.
bool has_filter_bounds_adjustment;
// Whether there is a texture layer in this subtree.
bool has_texture;
};
std::shared_ptr<std::vector<SkRect>> rects_;

View File

@@ -77,7 +77,8 @@ void ContainerLayer::DiffChildren(DiffContext* context,
auto layer = layers_[i];
auto prev_layer = prev_layers[i_prev];
auto paint_region = context->GetOldLayerPaintRegion(prev_layer.get());
if (layer == prev_layer && !paint_region.has_readback()) {
if (layer == prev_layer && !paint_region.has_readback() &&
!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

View File

@@ -28,6 +28,13 @@ void TextureLayer::Diff(DiffContext* context, const Layer* old_layer) {
// dirty
context->MarkSubtreeDirty(context->GetOldLayerPaintRegion(prev));
}
// Make sure DiffContext knows there is a TextureLayer in this subtree.
// This prevents ContainerLayer from skipping TextureLayer diffing when
// TextureLayer is inside retained layer.
// See ContainerLayer::DiffChildren
// https://github.com/flutter/flutter/issues/92925
context->MarkSubtreeHasTextureLayer();
context->AddLayerBounds(SkRect::MakeXYWH(offset_.x(), offset_.y(),
size_.width(), size_.height()));
context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion());

View File

@@ -4,6 +4,7 @@
#include "flutter/flow/layers/texture_layer.h"
#include "flutter/flow/testing/diff_context_test.h"
#include "flutter/flow/testing/layer_test.h"
#include "flutter/flow/testing/mock_layer.h"
#include "flutter/flow/testing/mock_texture.h"
@@ -94,5 +95,26 @@ TEST_F(TextureLayerTest, PaintingWithLinearSampling) {
EXPECT_EQ(mock_canvas().draw_calls(), std::vector<MockCanvas::DrawCall>());
}
using TextureLayerDiffTest = DiffContextTest;
TEST_F(TextureLayerDiffTest, TextureInRetainedLayer) {
MockLayerTree tree1;
auto container = std::make_shared<ContainerLayer>();
tree1.root()->Add(container);
auto layer = std::make_shared<TextureLayer>(
SkPoint::Make(0, 0), SkSize::Make(100, 100), 0, false,
SkSamplingOptions(SkFilterMode::kLinear));
container->Add(layer);
MockLayerTree tree2;
tree2.root()->Add(container); // retained layer
auto damage = DiffLayerTree(tree1, MockLayerTree());
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 100, 100));
damage = DiffLayerTree(tree2, tree1);
EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 100, 100));
}
} // namespace testing
} // namespace flutter

View File

@@ -25,8 +25,13 @@ class PaintRegion {
PaintRegion(std::shared_ptr<std::vector<SkRect>> rects,
size_t from,
size_t to,
bool has_readback)
: rects_(rects), from_(from), to_(to), has_readback_(has_readback) {}
bool has_readback,
bool has_texture)
: rects_(rects),
from_(from),
to_(to),
has_readback_(has_readback),
has_texture_(has_texture) {}
std::vector<SkRect>::const_iterator begin() const {
FML_DCHECK(is_valid());
@@ -47,11 +52,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_; }
private:
std::shared_ptr<std::vector<SkRect>> rects_;
size_t from_ = 0;
size_t to_ = 0;
bool has_readback_ = false;
bool has_texture_ = false;
};
} // namespace flutter