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

Revert layer state stack #37090

Merged
merged 2 commits into from
Oct 27, 2022
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
3 changes: 0 additions & 3 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -777,9 +777,6 @@ FILE: ../../../flutter/flow/layers/layer.cc
FILE: ../../../flutter/flow/layers/layer.h
FILE: ../../../flutter/flow/layers/layer_raster_cache_item.cc
FILE: ../../../flutter/flow/layers/layer_raster_cache_item.h
FILE: ../../../flutter/flow/layers/layer_state_stack.cc
FILE: ../../../flutter/flow/layers/layer_state_stack.h
FILE: ../../../flutter/flow/layers/layer_state_stack_unittests.cc
FILE: ../../../flutter/flow/layers/layer_tree.cc
FILE: ../../../flutter/flow/layers/layer_tree.h
FILE: ../../../flutter/flow/layers/layer_tree_unittests.cc
Expand Down
15 changes: 0 additions & 15 deletions display_list/display_list_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -709,21 +709,6 @@ SkRect DisplayListBuilder::getLocalClipBounds() {
return kMaxCullRect_;
}

bool DisplayListBuilder::quickReject(const SkRect& bounds) const {
if (bounds.isEmpty()) {
return true;
}
SkMatrix matrix = getTransform();
// We don't need the inverse, but this method tells us if the matrix
// is singular in which case we can reject all rendering.
if (!matrix.invert(nullptr)) {
return true;
}
SkRect dev_bounds;
matrix.mapRect(bounds).roundOut(&dev_bounds);
return !current_layer_->clip_bounds.intersects(dev_bounds);
}

void DisplayListBuilder::drawPaint() {
Push<DrawPaintOp>(0, 1);
CheckLayerOpacityCompatibility();
Expand Down
7 changes: 1 addition & 6 deletions display_list/display_list_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
/// Returns the 3x3 partial perspective transform representing all transform
/// operations executed so far in this DisplayList within the enclosing
/// save stack.
SkMatrix getTransform() const { return current_layer_->matrix.asM33(); }
SkMatrix getTransform() { return current_layer_->matrix.asM33(); }

void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override;
void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override;
Expand All @@ -221,11 +221,6 @@ class DisplayListBuilder final : public virtual Dispatcher,
/// recorded rendering operations are interpreted.
SkRect getLocalClipBounds();

/// Return true iff the supplied bounds are easily shown to be outside
/// of the current clip bounds. This method may conservatively return
/// false if it cannot make the determination.
bool quickReject(const SkRect& bounds) const;

void drawPaint() override;
void drawPaint(const DlPaint& paint);
void drawColor(DlColor color, DlBlendMode mode) override;
Expand Down
174 changes: 0 additions & 174 deletions display_list/display_list_canvas_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2040,9 +2040,6 @@ class CanvasCompareTester {
const uint32_t* test_row = test_pixels->addr32(0, y);
for (int x = 0; x < test_pixels->width(); x++) {
if (ref_row[x] != test_row[x]) {
if (should_match) {
FML_LOG(ERROR) << std::hex << ref_row[x] << " != " << test_row[x];
}
pixels_different++;
}
}
Expand Down Expand Up @@ -3241,176 +3238,5 @@ TEST_F(DisplayListCanvas, DrawShadowDpr) {
CanvasCompareTester::DefaultTolerance.addBoundsPadding(3, 3));
}

TEST_F(DisplayListCanvas, SaveLayerConsolidation) {
float commutable_color_matrix[]{
// clang-format off
0, 1, 0, 0, 0,
0, 0, 1, 0, 0,
1, 0, 0, 0, 0,
0, 0, 0, 1, 0,
// clang-format on
};
float non_commutable_color_matrix[]{
// clang-format off
0, 1, 0, .1, 0,
0, 0, 1, .1, 0,
1, 0, 0, .1, 0,
0, 0, 0, .7, 0,
// clang-format on
};
SkMatrix contract_matrix;
contract_matrix.setScale(0.9f, 0.9f, kRenderCenterX, kRenderCenterY);

std::vector<SkScalar> opacities = {
0,
0.5f,
SK_Scalar1,
};
std::vector<std::shared_ptr<DlColorFilter>> color_filters = {
std::make_shared<DlBlendColorFilter>(DlColor::kCyan(),
DlBlendMode::kSrcATop),
std::make_shared<DlMatrixColorFilter>(commutable_color_matrix),
std::make_shared<DlMatrixColorFilter>(non_commutable_color_matrix),
DlSrgbToLinearGammaColorFilter::instance,
DlLinearToSrgbGammaColorFilter::instance,
};
std::vector<std::shared_ptr<DlImageFilter>> image_filters = {
std::make_shared<DlBlurImageFilter>(5.0f, 5.0f, DlTileMode::kDecal),
std::make_shared<DlDilateImageFilter>(5.0f, 5.0f),
std::make_shared<DlErodeImageFilter>(5.0f, 5.0f),
std::make_shared<DlMatrixImageFilter>(contract_matrix,
DlImageSampling::kLinear),
};
RenderEnvironment env = RenderEnvironment::MakeN32();

auto render_content = [](DisplayListBuilder& builder) {
builder.drawRect(
SkRect{kRenderLeft, kRenderTop, kRenderCenterX, kRenderCenterY},
DlPaint().setColor(DlColor::kYellow()));
builder.drawRect(
SkRect{kRenderCenterX, kRenderTop, kRenderRight, kRenderCenterY},
DlPaint().setColor(DlColor::kRed()));
builder.drawRect(
SkRect{kRenderLeft, kRenderCenterY, kRenderCenterX, kRenderBottom},
DlPaint().setColor(DlColor::kBlue()));
builder.drawRect(
SkRect{kRenderCenterX, kRenderCenterY, kRenderRight, kRenderBottom},
DlPaint().setColor(DlColor::kRed().modulateOpacity(0.5f)));
};

// clang-format off
auto test_attributes = [&env, render_content]
(DlPaint& paint1, DlPaint& paint2, const DlPaint& paint_both,
bool same, bool rev_same, const std::string& desc1,
const std::string& desc2) {
// clang-format on
DisplayListBuilder nested_builder;
nested_builder.saveLayer(&kTestBounds, &paint1);
nested_builder.saveLayer(&kTestBounds, &paint2);
render_content(nested_builder);
auto nested_surface = env.MakeSurface();
nested_builder.Build()->RenderTo(nested_surface->canvas());

DisplayListBuilder reverse_builder;
reverse_builder.saveLayer(&kTestBounds, &paint2);
reverse_builder.saveLayer(&kTestBounds, &paint1);
render_content(reverse_builder);
auto reverse_surface = env.MakeSurface();
reverse_builder.Build()->RenderTo(reverse_surface->canvas());

DisplayListBuilder combined_builder;
combined_builder.saveLayer(&kTestBounds, &paint_both);
render_content(combined_builder);
auto combined_surface = env.MakeSurface();
combined_builder.Build()->RenderTo(combined_surface->canvas());

// Set this boolean to true to test if combinations that are marked
// as incompatible actually are compatible despite our predictions.
// Some of the combinations that we treat as incompatible actually
// are compatible with swapping the order of the operations, but
// it would take a bit of new infrastructure to really identify
// those combinations. The only hard constraint to test here is
// when we claim that they are compatible and they aren't.
const bool always = false;

if (always || same) {
CanvasCompareTester::quickCompareToReference(
nested_surface->pixmap(), combined_surface->pixmap(), same,
"nested " + desc1 + " then " + desc2);
}
if (always || rev_same) {
CanvasCompareTester::quickCompareToReference(
reverse_surface->pixmap(), combined_surface->pixmap(), rev_same,
"nested " + desc2 + " then " + desc1);
}
};

// CF then Opacity should always work.
// The reverse sometimes works.
for (size_t cfi = 0; cfi < color_filters.size(); cfi++) {
auto color_filter = color_filters[cfi];
std::string cf_desc = "color filter #" + std::to_string(cfi + 1);
DlPaint nested_paint1 = DlPaint().setColorFilter(color_filter);

for (size_t oi = 0; oi < opacities.size(); oi++) {
SkScalar opacity = opacities[oi];
std::string op_desc = "opacity " + std::to_string(opacity);
DlPaint nested_paint2 = DlPaint().setOpacity(opacity);

DlPaint combined_paint = nested_paint1;
combined_paint.setOpacity(opacity);

bool op_then_cf_works = opacity <= 0.0 || opacity >= 1.0 ||
color_filter->can_commute_with_opacity();

test_attributes(nested_paint1, nested_paint2, combined_paint, true,
op_then_cf_works, cf_desc, op_desc);
}
}

// Opacity then IF should always work.
// The reverse can also work for some values of opacity.
// The reverse should also theoretically work for some IFs, but we
// get some rounding errors that are more than just trivial.
for (size_t oi = 0; oi < opacities.size(); oi++) {
SkScalar opacity = opacities[oi];
std::string op_desc = "opacity " + std::to_string(opacity);
DlPaint nested_paint1 = DlPaint().setOpacity(opacity);

for (size_t ifi = 0; ifi < image_filters.size(); ifi++) {
auto image_filter = image_filters[ifi];
std::string if_desc = "image filter #" + std::to_string(ifi + 1);
DlPaint nested_paint2 = DlPaint().setImageFilter(image_filter);

DlPaint combined_paint = nested_paint1;
combined_paint.setImageFilter(image_filter);

bool if_then_op_works = opacity <= 0.0 || opacity >= 1.0;
test_attributes(nested_paint1, nested_paint2, combined_paint, true,
if_then_op_works, op_desc, if_desc);
}
}

// CF then IF should always work.
// The reverse might work, but we lack the infrastructure to check it.
for (size_t cfi = 0; cfi < color_filters.size(); cfi++) {
auto color_filter = color_filters[cfi];
std::string cf_desc = "color filter #" + std::to_string(cfi + 1);
DlPaint nested_paint1 = DlPaint().setColorFilter(color_filter);

for (size_t ifi = 0; ifi < image_filters.size(); ifi++) {
auto image_filter = image_filters[ifi];
std::string if_desc = "image filter #" + std::to_string(ifi + 1);
DlPaint nested_paint2 = DlPaint().setImageFilter(image_filter);

DlPaint combined_paint = nested_paint1;
combined_paint.setImageFilter(image_filter);

test_attributes(nested_paint1, nested_paint2, combined_paint, true, false,
cf_desc, if_desc);
}
}
}

} // namespace testing
} // namespace flutter
3 changes: 0 additions & 3 deletions display_list/display_list_color.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ struct DlColor {
constexpr DlColor() : argb(0xFF000000) {}
constexpr DlColor(uint32_t argb) : argb(argb) {}

static constexpr uint8_t toAlpha(SkScalar opacity) { return toC(opacity); }
static constexpr SkScalar toOpacity(uint8_t alpha) { return toF(alpha); }

// clang-format off
static constexpr DlColor kTransparent() {return 0x00000000;};
static constexpr DlColor kBlack() {return 0xFF000000;};
Expand Down
13 changes: 0 additions & 13 deletions display_list/display_list_color_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ class DlColorFilter
// pixels non-transparent and therefore expand the bounds.
virtual bool modifies_transparent_black() const = 0;

// Return a boolean indicating whether the color filtering operation can
// be applied either before or after modulating the pixels with an opacity
// value without changing the operation.
virtual bool can_commute_with_opacity() const { return false; }

// Return a DlBlendColorFilter pointer to this object iff it is a Blend
// type of ColorFilter, otherwise return nullptr.
virtual const DlBlendColorFilter* asBlend() const { return nullptr; }
Expand Down Expand Up @@ -155,12 +150,6 @@ class DlMatrixColorFilter final : public DlColorFilter {
sk_filter->filterColor(SK_ColorTRANSPARENT) != SK_ColorTRANSPARENT;
}

bool can_commute_with_opacity() const override {
return matrix_[3] == 0 && matrix_[8] == 0 && matrix_[13] == 0 &&
matrix_[15] == 0 && matrix_[16] == 0 && matrix_[17] == 0 &&
(matrix_[18] >= 0.0 && matrix_[18] <= 1.0) && matrix_[19] == 0;
}

std::shared_ptr<DlColorFilter> shared() const override {
return std::make_shared<DlMatrixColorFilter>(this);
}
Expand Down Expand Up @@ -204,7 +193,6 @@ class DlSrgbToLinearGammaColorFilter final : public DlColorFilter {
}
size_t size() const override { return sizeof(*this); }
bool modifies_transparent_black() const override { return false; }
bool can_commute_with_opacity() const override { return true; }

std::shared_ptr<DlColorFilter> shared() const override { return instance; }
sk_sp<SkColorFilter> skia_object() const override { return sk_filter_; }
Expand Down Expand Up @@ -237,7 +225,6 @@ class DlLinearToSrgbGammaColorFilter final : public DlColorFilter {
}
size_t size() const override { return sizeof(*this); }
bool modifies_transparent_black() const override { return false; }
bool can_commute_with_opacity() const override { return true; }

std::shared_ptr<DlColorFilter> shared() const override { return instance; }
sk_sp<SkColorFilter> skia_object() const override { return sk_filter_; }
Expand Down
8 changes: 2 additions & 6 deletions display_list/display_list_paint.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ class DlPaint {
color_.argb = alpha << 24 | (color_.argb & 0x00FFFFFF);
return *this;
}
DlPaint& setOpacity(SkScalar opacity) {
setAlpha(SkScalarRoundToInt(opacity * 0xff));
return *this;
}

DlBlendMode getBlendMode() const {
return static_cast<DlBlendMode>(blendMode_);
Expand Down Expand Up @@ -170,7 +166,7 @@ class DlPaint {
return colorFilter_;
}
const DlColorFilter* getColorFilterPtr() const { return colorFilter_.get(); }
DlPaint& setColorFilter(const std::shared_ptr<const DlColorFilter> filter) {
DlPaint& setColorFilter(std::shared_ptr<const DlColorFilter> filter) {
colorFilter_ = filter ? filter->shared() : nullptr;
return *this;
}
Expand All @@ -183,7 +179,7 @@ class DlPaint {
return imageFilter_;
}
const DlImageFilter* getImageFilterPtr() const { return imageFilter_.get(); }
DlPaint& setImageFilter(const std::shared_ptr<const DlImageFilter> filter) {
DlPaint& setImageFilter(std::shared_ptr<const DlImageFilter> filter) {
imageFilter_ = filter;
return *this;
}
Expand Down
4 changes: 1 addition & 3 deletions flow/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ source_set("flow") {
"layers/layer.h",
"layers/layer_raster_cache_item.cc",
"layers/layer_raster_cache_item.h",
"layers/layer_state_stack.cc",
"layers/layer_state_stack.h",
"layers/layer_tree.cc",
"layers/layer_tree.h",
"layers/offscreen_surface.cc",
Expand Down Expand Up @@ -158,7 +156,6 @@ if (enable_unittests) {
"layers/container_layer_unittests.cc",
"layers/display_list_layer_unittests.cc",
"layers/image_filter_layer_unittests.cc",
"layers/layer_state_stack_unittests.cc",
"layers/layer_tree_unittests.cc",
"layers/offscreen_surface_unittests.cc",
"layers/opacity_layer_unittests.cc",
Expand All @@ -173,6 +170,7 @@ if (enable_unittests) {
"rtree_unittests.cc",
"skia_gpu_object_unittests.cc",
"surface_frame_unittests.cc",
"testing/auto_save_layer_unittests.cc",
"testing/mock_layer_unittests.cc",
"testing/mock_texture_unittests.cc",
"texture_unittests.cc",
Expand Down
1 change: 1 addition & 0 deletions flow/embedded_view_params_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithRotation90) {
EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack);
const SkRect& rect = params.finalBoundingRect();

FML_DLOG(ERROR) << rect.x();
ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), -1));
ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0));
ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 1));
Expand Down
6 changes: 0 additions & 6 deletions flow/embedded_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,6 @@ void MutatorsStack::Pop() {
vector_.pop_back();
};

void MutatorsStack::PopTo(size_t stack_count) {
while (vector_.size() > stack_count) {
Pop();
}
}

const std::vector<std::shared_ptr<Mutator>>::const_reverse_iterator
MutatorsStack::Top() const {
return vector_.rend();
Expand Down
3 changes: 0 additions & 3 deletions flow/embedded_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,6 @@ class MutatorsStack {
// and destroys it.
void Pop();

void PopTo(size_t stack_count);

// Returns a reverse iterator pointing to the top of the stack, which is the
// mutator that is furtherest from the leaf node.
const std::vector<std::shared_ptr<Mutator>>::const_reverse_iterator Top()
Expand All @@ -180,7 +178,6 @@ class MutatorsStack {
const std::vector<std::shared_ptr<Mutator>>::const_iterator End() const;

bool is_empty() const { return vector_.empty(); }
size_t stack_count() const { return vector_.size(); }

bool operator==(const MutatorsStack& other) const {
if (vector_.size() != other.vector_.size()) {
Expand Down
Loading