Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
afbb37a
Testing Preroll/Paint
JTKryptic Jun 16, 2022
5103d9d
Pushed opacity to platform view stack / Background Filter Proto 1
JTKryptic Jun 17, 2022
825fbc8
Create Backdrop Filter Mutator
JTKryptic Jun 23, 2022
4b5c705
Merge branch 'diff' into backdrop_filter_testing
JTKryptic Jun 23, 2022
24c077f
Removed Logs
JTKryptic Jun 23, 2022
e1a817d
Tests for PushFilter
JTKryptic Jun 27, 2022
d06155c
Backdrop Filter Mutator
JTKryptic Jun 27, 2022
452a0b7
Merge branch 'diff' into backdrop_filter_testing
JTKryptic Jun 27, 2022
6b74e7d
Formatting
JTKryptic Jun 27, 2022
455b87f
Merge branch 'backdrop_filter_testing'
JTKryptic Jun 27, 2022
bc1d19b
Format files
JTKryptic Jun 28, 2022
4c0f2a5
Merge branch 'main' into backdrop_filter_testing
JTKryptic Jun 28, 2022
55d328c
Delete backdrop_filter_diff
JTKryptic Jun 28, 2022
d080e8e
Code cleanup
JTKryptic Jun 29, 2022
93accab
Updating to DlImageFilter and code cleanup
JTKryptic Jun 29, 2022
a9cd101
Formatting
JTKryptic Jun 29, 2022
abc3a0e
bug fix
JTKryptic Jun 29, 2022
86a5913
Fix
JTKryptic Jul 7, 2022
3897724
Merge branch 'main' into backdrop_filter_testing
JTKryptic Jul 19, 2022
d60a016
Enum style guide fix
JTKryptic Jul 13, 2022
484957d
Cleanup
JTKryptic Jul 21, 2022
929be62
Cleanup and Tests
JTKryptic Jul 22, 2022
b280266
format
JTKryptic Jul 22, 2022
153df5d
format
JTKryptic Jul 22, 2022
70049ad
Memory leak failure fix
JTKryptic Jul 25, 2022
3a98c7d
format
JTKryptic Jul 25, 2022
1e6c271
Merge upstream_main into backdrop_filter_testing
JTKryptic Jul 25, 2022
5e94dee
Remove platform view flag
JTKryptic Jul 26, 2022
af66748
format
JTKryptic Jul 26, 2022
89cfd8b
nit
JTKryptic Jul 27, 2022
042c22e
Merge branch 'upstream_main' into backdrop_filter_testing
JTKryptic Jul 28, 2022
f9dd08b
test fix
JTKryptic Jul 28, 2022
9d00f75
format
JTKryptic Jul 28, 2022
f30e262
backdrop filter unit tests
JTKryptic Aug 2, 2022
8b3277f
shell cleaning
JTKryptic Aug 2, 2022
15fa54f
Merge branch 'upstream_main' into backdrop_filter_testing
JTKryptic Aug 2, 2022
5891b2e
formatting
JTKryptic Aug 2, 2022
7c05b74
Update ios_external_view_embedder.h
JTKryptic Aug 2, 2022
ca719f3
Update ios_external_view_embedder.mm
JTKryptic Aug 2, 2022
8d24f64
format
JTKryptic Aug 2, 2022
58b3ce3
PR comment changes
JTKryptic Aug 3, 2022
7b558e2
Update embedded_views.h
JTKryptic Aug 3, 2022
b92b17a
Merge branch 'upstream_main' into backdrop_filter_testing
JTKryptic Aug 3, 2022
0739448
Merge branch 'main' into backdrop_filter_testing
JTKryptic Aug 10, 2022
1d0bb6d
Update embedded_views.h
JTKryptic Aug 10, 2022
b2a2625
Update shell_test_external_view_embedder.cc
JTKryptic Aug 10, 2022
aef123c
Update shell_test_external_view_embedder.cc
JTKryptic Aug 10, 2022
85e0b4c
Update shell_test_external_view_embedder.cc
JTKryptic Aug 10, 2022
235a853
Update shell_test_external_view_embedder.h
JTKryptic Aug 10, 2022
e9d7cb1
test fix
JTKryptic Aug 10, 2022
a33caa4
fixes
JTKryptic Aug 10, 2022
3be1bf3
test fix
JTKryptic Aug 10, 2022
357ec82
test fix 3
JTKryptic Aug 10, 2022
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
17 changes: 17 additions & 0 deletions flow/embedded_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ class EmbeddedViewParams {
// Clippings are ignored.
const SkRect& finalBoundingRect() const { return final_bounding_rect_; }

// Pushes the stored DlImageFilter object to the mutators stack.
void PushImageFilter(std::shared_ptr<const DlImageFilter> filter) {
mutators_stack_.PushBackdropFilter(filter);
}

// Whether the embedder should construct DisplayList objects to hold the
// rendering commands for each between-view slice of the layer tree.
bool display_list_enabled() const { return display_list_enabled_; }
Expand Down Expand Up @@ -437,6 +442,18 @@ class ExternalViewEmbedder {
// 'EndFrame', otherwise returns false.
bool GetUsedThisFrame() const { return used_this_frame_; }

// Pushes the platform view id of a visited platform view to a list of
// visited platform views.
virtual void PushVisitedPlatformView(int64_t view_id) {}

// Pushes a DlImageFilter object to each platform view within a list of
// visited platform views.
//
// See also: |PushVisitedPlatformView| for pushing platform view ids to the
// visited platform views list.
virtual void PushFilterToVisitedPlatformViews(
std::shared_ptr<const DlImageFilter> filter) {}

private:
bool used_this_frame_ = false;

Expand Down
3 changes: 3 additions & 0 deletions flow/layers/backdrop_filter_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ void BackdropFilterLayer::Preroll(PrerollContext* context,
const SkMatrix& matrix) {
Layer::AutoPrerollSaveLayerState save =
Layer::AutoPrerollSaveLayerState::Create(context, true, bool(filter_));
if (context->view_embedder != nullptr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filter is being pushed but never cleared. The platform view gets an ever-increasing number of filters until app is unresponsive.

Copy link
Contributor

@cyanglaz cyanglaz Aug 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to where the filter isn't cleared? IIRC at each frame, the preroll context is re-created, meaning the mutator stack is re-created, so the filters in the stacks are cleared.

I agree with the other comment, the visitedPlatformView list is not cleared.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran with a print statement of the number of mutators in vector_ in MutatorsStack::PushBackdropFilter, it was continuously increasing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yeah, that's because the visitedPlatformView is increasing and multiple filters are pushed into the same platform view. This is the symptom of not clearing visitedPlatformView list, #34596 will fix it.

context->view_embedder->PushFilterToVisitedPlatformViews(filter_);
}
SkRect child_paint_bounds = SkRect::MakeEmpty();
PrerollChildren(context, matrix, &child_paint_bounds);
child_paint_bounds.join(context->cull_rect);
Expand Down
1 change: 1 addition & 0 deletions flow/layers/platform_view_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ void PlatformViewLayer::Preroll(PrerollContext* context,
context->display_list_enabled);
context->view_embedder->PrerollCompositeEmbeddedView(view_id_,
std::move(params));
context->view_embedder->PushVisitedPlatformView(view_id_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Platform view is pushed but nothing ever clears that list, so it will have the same issue as the filters, it slows down rendering until app is unresponsive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we are fixing it here: #34596

}

void PlatformViewLayer::Paint(PaintContext& context) const {
Expand Down
27 changes: 27 additions & 0 deletions flow/mutators_stack_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ TEST(MutatorsStack, Equality) {
stack.PushClipPath(path);
int alpha = 240;
stack.PushOpacity(alpha);
auto filter = std::make_shared<DlBlurImageFilter>(5, 5, DlTileMode::kClamp);
stack.PushBackdropFilter(filter);

MutatorsStack stackOther;
SkMatrix matrixOther = SkMatrix::Scale(1, 1);
Expand All @@ -175,6 +177,9 @@ TEST(MutatorsStack, Equality) {
stackOther.PushClipPath(otherPath);
int otherAlpha = 240;
stackOther.PushOpacity(otherAlpha);
auto otherFilter =
std::make_shared<DlBlurImageFilter>(5, 5, DlTileMode::kClamp);
stackOther.PushBackdropFilter(otherFilter);

ASSERT_TRUE(stack == stackOther);
}
Expand Down Expand Up @@ -204,6 +209,11 @@ TEST(Mutator, Initialization) {
int alpha = 240;
Mutator mutator5 = Mutator(alpha);
ASSERT_TRUE(mutator5.GetType() == MutatorType::kOpacity);

auto filter = std::make_shared<DlBlurImageFilter>(5, 5, DlTileMode::kClamp);
Mutator mutator6 = Mutator(filter);
ASSERT_TRUE(mutator6.GetType() == MutatorType::kBackdropFilter);
ASSERT_TRUE(mutator6.GetFilter() == *filter);
}

TEST(Mutator, CopyConstructor) {
Expand Down Expand Up @@ -232,6 +242,11 @@ TEST(Mutator, CopyConstructor) {
Mutator mutator5 = Mutator(alpha);
Mutator copy5 = Mutator(mutator5);
ASSERT_TRUE(mutator5 == copy5);

auto filter = std::make_shared<DlBlurImageFilter>(5, 5, DlTileMode::kClamp);
Mutator mutator6 = Mutator(filter);
Mutator copy6 = Mutator(mutator6);
ASSERT_TRUE(mutator6 == copy6);
}

TEST(Mutator, Equality) {
Expand Down Expand Up @@ -260,6 +275,11 @@ TEST(Mutator, Equality) {
Mutator mutator5 = Mutator(alpha);
Mutator otherMutator5 = Mutator(alpha);
ASSERT_TRUE(mutator5 == otherMutator5);

auto filter = std::make_shared<DlBlurImageFilter>(5, 5, DlTileMode::kClamp);
Mutator mutator6 = Mutator(filter);
Mutator otherMutator6 = Mutator(filter);
ASSERT_TRUE(mutator6 == otherMutator6);
}

TEST(Mutator, UnEquality) {
Expand All @@ -275,6 +295,13 @@ TEST(Mutator, UnEquality) {
Mutator mutator2 = Mutator(alpha);
Mutator otherMutator2 = Mutator(alpha2);
ASSERT_TRUE(mutator2 != otherMutator2);

auto filter = std::make_shared<DlBlurImageFilter>(5, 5, DlTileMode::kClamp);
auto filter2 =
std::make_shared<DlBlurImageFilter>(10, 10, DlTileMode::kClamp);
Mutator mutator3 = Mutator(filter);
Mutator otherMutator3 = Mutator(filter2);
ASSERT_TRUE(mutator3 != otherMutator3);
}

} // namespace testing
Expand Down
36 changes: 34 additions & 2 deletions shell/common/shell_test_external_view_embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ SkISize ShellTestExternalViewEmbedder::GetLastSubmittedFrameSize() {
return last_submitted_frame_size_;
}

std::vector<int64_t> ShellTestExternalViewEmbedder::GetVisitedPlatformViews() {
return visited_platform_views_;
}

MutatorsStack ShellTestExternalViewEmbedder::GetStack(int64_t view_id) {
return mutators_stacks_[view_id];
}

// |ExternalViewEmbedder|
void ShellTestExternalViewEmbedder::CancelFrame() {}

Expand All @@ -41,7 +49,16 @@ void ShellTestExternalViewEmbedder::BeginFrame(
// |ExternalViewEmbedder|
void ShellTestExternalViewEmbedder::PrerollCompositeEmbeddedView(
int view_id,
std::unique_ptr<EmbeddedViewParams> params) {}
std::unique_ptr<EmbeddedViewParams> params) {
SkRect view_bounds = SkRect::Make(frame_size_);
std::unique_ptr<EmbedderViewSlice> view;
if (params->display_list_enabled()) {
view = std::make_unique<DisplayListEmbedderViewSlice>(view_bounds);
} else {
view = std::make_unique<SkPictureEmbedderViewSlice>(view_bounds);
}
slices_.insert_or_assign(view_id, std::move(view));
}

// |ExternalViewEmbedder|
PostPrerollResult ShellTestExternalViewEmbedder::PostPrerollAction(
Expand All @@ -56,9 +73,24 @@ std::vector<SkCanvas*> ShellTestExternalViewEmbedder::GetCurrentCanvases() {
}

// |ExternalViewEmbedder|
void ShellTestExternalViewEmbedder::PushVisitedPlatformView(int64_t view_id) {
visited_platform_views_.push_back(view_id);
}

// |ExternalViewEmbedder|
void ShellTestExternalViewEmbedder::PushFilterToVisitedPlatformViews(
std::shared_ptr<const DlImageFilter> filter) {
for (int64_t id : visited_platform_views_) {
EmbeddedViewParams params = current_composition_params_[id];
params.PushImageFilter(filter);
current_composition_params_[id] = params;
mutators_stacks_[id] = params.mutatorsStack();
}
}

EmbedderPaintContext ShellTestExternalViewEmbedder::CompositeEmbeddedView(
int view_id) {
return {nullptr, nullptr};
return {slices_[view_id]->canvas(), slices_[view_id]->builder()};
}

// |ExternalViewEmbedder|
Expand Down
22 changes: 20 additions & 2 deletions shell/common/shell_test_external_view_embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "flutter/flow/embedded_views.h"
#include "flutter/fml/raster_thread_merger.h"
#include "third_party/skia/include/core/SkPictureRecorder.h"

namespace flutter {

Expand All @@ -32,9 +33,15 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder {
// the external view embedder.
int GetSubmittedFrameCount();

// Returns the size of last submitted frame surface
// Returns the size of last submitted frame surface.
SkISize GetLastSubmittedFrameSize();

// Returns the mutators stack for the given platform view.
MutatorsStack GetStack(int64_t);

// Returns the list of visited platform views.
std::vector<int64_t> GetVisitedPlatformViews();

private:
// |ExternalViewEmbedder|
void CancelFrame() override;
Expand All @@ -61,6 +68,13 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder {
// |ExternalViewEmbedder|
EmbedderPaintContext CompositeEmbeddedView(int view_id) override;

// |ExternalViewEmbedder|
void PushVisitedPlatformView(int64_t view_id) override;

// |ExternalViewEmbedder|
void PushFilterToVisitedPlatformViews(
std::shared_ptr<const DlImageFilter> filter) override;

// |ExternalViewEmbedder|
void SubmitFrame(GrDirectContext* context,
std::unique_ptr<SurfaceFrame> frame) override;
Expand All @@ -81,7 +95,11 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder {
PostPrerollResult post_preroll_result_;

bool support_thread_merging_;

SkISize frame_size_;
std::map<int64_t, std::unique_ptr<EmbedderViewSlice>> slices_;
std::map<int64_t, MutatorsStack> mutators_stacks_;
std::map<int64_t, EmbeddedViewParams> current_composition_params_;
std::vector<int64_t> visited_platform_views_;
std::atomic<int> submitted_frame_count_;
std::atomic<SkISize> last_submitted_frame_size_;

Expand Down
58 changes: 57 additions & 1 deletion shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@

#include "assets/directory_asset_bundle.h"
#include "common/graphics/persistent_cache.h"
#include "flutter/flow/layers/backdrop_filter_layer.h"
#include "flutter/flow/layers/display_list_layer.h"
#include "flutter/flow/layers/layer_raster_cache_item.h"
#include "flutter/flow/layers/platform_view_layer.h"
#include "flutter/flow/layers/transform_layer.h"
#include "flutter/fml/command_line.h"
#include "flutter/fml/dart/dart_converter.h"
Expand Down Expand Up @@ -765,12 +767,66 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) {

PumpOneFrame(shell.get(), 100, 100, builder);
end_frame_latch.Wait();

ASSERT_TRUE(end_frame_called);

DestroyShell(std::move(shell));
}

TEST_F(ShellTest, PushBackdropFilterToVisitedPlatformViews) {
auto settings = CreateSettingsForFixture();
fml::AutoResetWaitableEvent end_frame_latch;
bool end_frame_called = false;
auto end_frame_callback =
[&](bool should_resubmit_frame,
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
ASSERT_TRUE(raster_thread_merger.get() == nullptr);
ASSERT_FALSE(should_resubmit_frame);
end_frame_called = true;
end_frame_latch.Signal();
};
auto external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
end_frame_callback, PostPrerollResult::kResubmitFrame, false);
auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(),
false, external_view_embedder);

// Create the surface needed by rasterizer
PlatformViewNotifyCreated(shell.get());

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("emptyMain");

RunEngine(shell.get(), std::move(configuration));

LayerTreeBuilder builder = [&](std::shared_ptr<ContainerLayer> root) {
auto platform_view_layer = std::make_shared<PlatformViewLayer>(
SkPoint::Make(10, 10), SkSize::Make(10, 10), 50);
root->Add(platform_view_layer);
auto filter = std::make_shared<DlBlurImageFilter>(5, 5, DlTileMode::kClamp);
auto backdrop_filter_layer =
std::make_shared<BackdropFilterLayer>(filter, DlBlendMode::kSrcOver);
root->Add(backdrop_filter_layer);
auto platform_view_layer2 = std::make_shared<PlatformViewLayer>(
SkPoint::Make(10, 10), SkSize::Make(10, 10), 75);
backdrop_filter_layer->Add(platform_view_layer2);
};

PumpOneFrame(shell.get(), 100, 100, builder);
end_frame_latch.Wait();
ASSERT_EQ(external_view_embedder->GetVisitedPlatformViews().size(),
(const unsigned long)2);
ASSERT_EQ(external_view_embedder->GetVisitedPlatformViews()[0], 50);
ASSERT_EQ(external_view_embedder->GetVisitedPlatformViews()[1], 75);
ASSERT_TRUE(external_view_embedder->GetStack(75).is_empty());
ASSERT_FALSE(external_view_embedder->GetStack(50).is_empty());

auto filter = DlBlurImageFilter(5, 5, DlTileMode::kClamp);
auto mutator = *external_view_embedder->GetStack(50).Begin();
ASSERT_EQ(mutator->GetType(), MutatorType::kBackdropFilter);
ASSERT_EQ(mutator->GetFilter(), filter);

DestroyShell(std::move(shell));
}

// TODO(https://github.com/flutter/flutter/issues/59816): Enable on fuchsia.
TEST_F(ShellTest,
#if defined(OS_FUCHSIA)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,15 @@ - (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
}
}

void FlutterPlatformViewsController::PushFilterToVisitedPlatformViews(
std::shared_ptr<const DlImageFilter> filter) {
for (int64_t id : visited_platform_views_) {
EmbeddedViewParams params = current_composition_params_[id];
params.PushImageFilter(filter);
current_composition_params_[id] = params;
}
Comment on lines +323 to +327
Copy link
Contributor

@cyanglaz cyanglaz Aug 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add a test for this part. Maybe in the XCTests in #34596, instead of adding the filter directly to the stack, we can have a test use this method to add the filter.

So let's land #34596 first so we can easily add such tests.

CC @emilyabest

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to block this PR to land for now until #34596 is landed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offline discussion. We will land this PR first and add the final test in a follow up PR

}

void FlutterPlatformViewsController::PrerollCompositeEmbeddedView(
int view_id,
std::unique_ptr<EmbeddedViewParams> params) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ class FlutterPlatformViewsController {
// responder. Returns -1 if no such platform view is found.
long FindFirstResponderPlatformViewId();

// Pushes backdrop filter mutation to the mutator stack of each visited platform view.
void PushFilterToVisitedPlatformViews(std::shared_ptr<const DlImageFilter> filter);

// Pushes the view id of a visted platform view to the list of visied platform views.
void PushVisitedPlatformView(int64_t view_id) { visited_platform_views_.push_back(view_id); }

private:
static const size_t kMaxLayerAllocations = 2;

Expand Down Expand Up @@ -291,6 +297,9 @@ class FlutterPlatformViewsController {
// The last ID in this vector belond to the that is composited on top of all others.
std::vector<int64_t> composition_order_;

// A vector of visited platform view IDs.
std::vector<int64_t> visited_platform_views_;

// The latest composition order that was presented in Present().
std::vector<int64_t> active_composition_order_;

Expand Down
7 changes: 7 additions & 0 deletions shell/platform/darwin/ios/ios_external_view_embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ class IOSExternalViewEmbedder : public ExternalViewEmbedder {
// |ExternalViewEmbedder|
bool SupportsDynamicThreadMerging() override;

// |ExternalViewEmbedder|
void PushFilterToVisitedPlatformViews(
std::shared_ptr<const DlImageFilter> filter) override;

// |ExternalViewEmbedder|
void PushVisitedPlatformView(int64_t view_id) override;

FML_DISALLOW_COPY_AND_ASSIGN(IOSExternalViewEmbedder);
};

Expand Down
11 changes: 11 additions & 0 deletions shell/platform/darwin/ios/ios_external_view_embedder.mm
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,15 @@
return true;
}

// |ExternalViewEmbedder|
void IOSExternalViewEmbedder::PushFilterToVisitedPlatformViews(
std::shared_ptr<const DlImageFilter> filter) {
platform_views_controller_->PushFilterToVisitedPlatformViews(filter);
}

// |ExternalViewEmbedder|
void IOSExternalViewEmbedder::PushVisitedPlatformView(int64_t view_id) {
platform_views_controller_->PushVisitedPlatformView(view_id);
}

} // namespace flutter