From e1d9594b8713a57f70469d8d3f7a257a2833decb Mon Sep 17 00:00:00 2001 From: Zachary Anderson Date: Fri, 19 Aug 2022 10:01:01 -0700 Subject: [PATCH] Revert "Pushing BackdropFilter Mutator (#34355)" (#35543) This reverts commit 3ec9f21b22e3ce7fcc6cd32b1876f193c4a5bf3a. --- flow/embedded_views.h | 17 ------ flow/layers/backdrop_filter_layer.cc | 3 - flow/layers/platform_view_layer.cc | 1 - flow/mutators_stack_unittests.cc | 27 --------- .../shell_test_external_view_embedder.cc | 36 +----------- .../shell_test_external_view_embedder.h | 22 +------ shell/common/shell_unittests.cc | 58 +------------------ .../framework/Source/FlutterPlatformViews.mm | 9 --- .../Source/FlutterPlatformViews_Internal.h | 9 --- .../darwin/ios/ios_external_view_embedder.h | 7 --- .../darwin/ios/ios_external_view_embedder.mm | 11 ---- 11 files changed, 5 insertions(+), 195 deletions(-) diff --git a/flow/embedded_views.h b/flow/embedded_views.h index cc252ae527835..1839a8aa2e3f8 100644 --- a/flow/embedded_views.h +++ b/flow/embedded_views.h @@ -248,11 +248,6 @@ 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 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_; } @@ -444,18 +439,6 @@ 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 filter) {} - private: bool used_this_frame_ = false; diff --git a/flow/layers/backdrop_filter_layer.cc b/flow/layers/backdrop_filter_layer.cc index 9f67b746b1c9b..d52ac787c44c0 100644 --- a/flow/layers/backdrop_filter_layer.cc +++ b/flow/layers/backdrop_filter_layer.cc @@ -43,9 +43,6 @@ void BackdropFilterLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { Layer::AutoPrerollSaveLayerState save = Layer::AutoPrerollSaveLayerState::Create(context, true, bool(filter_)); - if (context->view_embedder != nullptr) { - context->view_embedder->PushFilterToVisitedPlatformViews(filter_); - } SkRect child_paint_bounds = SkRect::MakeEmpty(); PrerollChildren(context, matrix, &child_paint_bounds); child_paint_bounds.join(context->cull_rect); diff --git a/flow/layers/platform_view_layer.cc b/flow/layers/platform_view_layer.cc index a86886fb1b04f..8a46cbbce127c 100644 --- a/flow/layers/platform_view_layer.cc +++ b/flow/layers/platform_view_layer.cc @@ -29,7 +29,6 @@ void PlatformViewLayer::Preroll(PrerollContext* context, context->display_list_enabled); context->view_embedder->PrerollCompositeEmbeddedView(view_id_, std::move(params)); - context->view_embedder->PushVisitedPlatformView(view_id_); } void PlatformViewLayer::Paint(PaintContext& context) const { diff --git a/flow/mutators_stack_unittests.cc b/flow/mutators_stack_unittests.cc index c93838cfa68bf..a460125ef9627 100644 --- a/flow/mutators_stack_unittests.cc +++ b/flow/mutators_stack_unittests.cc @@ -163,8 +163,6 @@ TEST(MutatorsStack, Equality) { stack.PushClipPath(path); int alpha = 240; stack.PushOpacity(alpha); - auto filter = std::make_shared(5, 5, DlTileMode::kClamp); - stack.PushBackdropFilter(filter); MutatorsStack stackOther; SkMatrix matrixOther = SkMatrix::Scale(1, 1); @@ -177,9 +175,6 @@ TEST(MutatorsStack, Equality) { stackOther.PushClipPath(otherPath); int otherAlpha = 240; stackOther.PushOpacity(otherAlpha); - auto otherFilter = - std::make_shared(5, 5, DlTileMode::kClamp); - stackOther.PushBackdropFilter(otherFilter); ASSERT_TRUE(stack == stackOther); } @@ -209,11 +204,6 @@ TEST(Mutator, Initialization) { int alpha = 240; Mutator mutator5 = Mutator(alpha); ASSERT_TRUE(mutator5.GetType() == MutatorType::kOpacity); - - auto filter = std::make_shared(5, 5, DlTileMode::kClamp); - Mutator mutator6 = Mutator(filter); - ASSERT_TRUE(mutator6.GetType() == MutatorType::kBackdropFilter); - ASSERT_TRUE(mutator6.GetFilter() == *filter); } TEST(Mutator, CopyConstructor) { @@ -242,11 +232,6 @@ TEST(Mutator, CopyConstructor) { Mutator mutator5 = Mutator(alpha); Mutator copy5 = Mutator(mutator5); ASSERT_TRUE(mutator5 == copy5); - - auto filter = std::make_shared(5, 5, DlTileMode::kClamp); - Mutator mutator6 = Mutator(filter); - Mutator copy6 = Mutator(mutator6); - ASSERT_TRUE(mutator6 == copy6); } TEST(Mutator, Equality) { @@ -275,11 +260,6 @@ TEST(Mutator, Equality) { Mutator mutator5 = Mutator(alpha); Mutator otherMutator5 = Mutator(alpha); ASSERT_TRUE(mutator5 == otherMutator5); - - auto filter = std::make_shared(5, 5, DlTileMode::kClamp); - Mutator mutator6 = Mutator(filter); - Mutator otherMutator6 = Mutator(filter); - ASSERT_TRUE(mutator6 == otherMutator6); } TEST(Mutator, UnEquality) { @@ -295,13 +275,6 @@ TEST(Mutator, UnEquality) { Mutator mutator2 = Mutator(alpha); Mutator otherMutator2 = Mutator(alpha2); ASSERT_TRUE(mutator2 != otherMutator2); - - auto filter = std::make_shared(5, 5, DlTileMode::kClamp); - auto filter2 = - std::make_shared(10, 10, DlTileMode::kClamp); - Mutator mutator3 = Mutator(filter); - Mutator otherMutator3 = Mutator(filter2); - ASSERT_TRUE(mutator3 != otherMutator3); } } // namespace testing diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index 579f682089517..a366e6f99075c 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/shell/common/shell_test_external_view_embedder.cc @@ -28,14 +28,6 @@ SkISize ShellTestExternalViewEmbedder::GetLastSubmittedFrameSize() { return last_submitted_frame_size_; } -std::vector ShellTestExternalViewEmbedder::GetVisitedPlatformViews() { - return visited_platform_views_; -} - -MutatorsStack ShellTestExternalViewEmbedder::GetStack(int64_t view_id) { - return mutators_stacks_[view_id]; -} - // |ExternalViewEmbedder| void ShellTestExternalViewEmbedder::CancelFrame() {} @@ -49,16 +41,7 @@ void ShellTestExternalViewEmbedder::BeginFrame( // |ExternalViewEmbedder| void ShellTestExternalViewEmbedder::PrerollCompositeEmbeddedView( int view_id, - std::unique_ptr params) { - SkRect view_bounds = SkRect::Make(frame_size_); - std::unique_ptr view; - if (params->display_list_enabled()) { - view = std::make_unique(view_bounds); - } else { - view = std::make_unique(view_bounds); - } - slices_.insert_or_assign(view_id, std::move(view)); -} + std::unique_ptr params) {} // |ExternalViewEmbedder| PostPrerollResult ShellTestExternalViewEmbedder::PostPrerollAction( @@ -79,24 +62,9 @@ ShellTestExternalViewEmbedder::GetCurrentBuilders() { } // |ExternalViewEmbedder| -void ShellTestExternalViewEmbedder::PushVisitedPlatformView(int64_t view_id) { - visited_platform_views_.push_back(view_id); -} - -// |ExternalViewEmbedder| -void ShellTestExternalViewEmbedder::PushFilterToVisitedPlatformViews( - std::shared_ptr 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 {slices_[view_id]->canvas(), slices_[view_id]->builder()}; + return {nullptr, nullptr}; } // |ExternalViewEmbedder| diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 583a09182e5fc..1bb70fd131e07 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/shell/common/shell_test_external_view_embedder.h @@ -7,7 +7,6 @@ #include "flutter/flow/embedded_views.h" #include "flutter/fml/raster_thread_merger.h" -#include "third_party/skia/include/core/SkPictureRecorder.h" namespace flutter { @@ -33,15 +32,9 @@ 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 GetVisitedPlatformViews(); - private: // |ExternalViewEmbedder| void CancelFrame() override; @@ -71,13 +64,6 @@ 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 filter) override; - // |ExternalViewEmbedder| void SubmitFrame(GrDirectContext* context, std::unique_ptr frame) override; @@ -98,11 +84,7 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder { PostPrerollResult post_preroll_result_; bool support_thread_merging_; - SkISize frame_size_; - std::map> slices_; - std::map mutators_stacks_; - std::map current_composition_params_; - std::vector visited_platform_views_; + std::atomic submitted_frame_count_; std::atomic last_submitted_frame_size_; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 124dbf5d45a70..7bd244d98aba2 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -13,10 +13,8 @@ #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" @@ -767,62 +765,8 @@ 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 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( - 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 root) { - auto platform_view_layer = std::make_shared( - SkPoint::Make(10, 10), SkSize::Make(10, 10), 50); - root->Add(platform_view_layer); - auto filter = std::make_shared(5, 5, DlTileMode::kClamp); - auto backdrop_filter_layer = - std::make_shared(filter, DlBlendMode::kSrcOver); - root->Add(backdrop_filter_layer); - auto platform_view_layer2 = std::make_shared( - 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); + ASSERT_TRUE(end_frame_called); DestroyShell(std::move(shell)); } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 9987fe6dd8265..bea122558e458 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -318,15 +318,6 @@ - (BOOL)flt_hasFirstResponderInViewHierarchySubtree { } } -void FlutterPlatformViewsController::PushFilterToVisitedPlatformViews( - std::shared_ptr filter) { - for (int64_t id : visited_platform_views_) { - EmbeddedViewParams params = current_composition_params_[id]; - params.PushImageFilter(filter); - current_composition_params_[id] = params; - } -} - void FlutterPlatformViewsController::PrerollCompositeEmbeddedView( int view_id, std::unique_ptr params) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index c7877dc7601ed..0a2e134f876dd 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -186,12 +186,6 @@ 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 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; @@ -299,9 +293,6 @@ class FlutterPlatformViewsController { // The last ID in this vector belond to the that is composited on top of all others. std::vector composition_order_; - // A vector of visited platform view IDs. - std::vector visited_platform_views_; - // The latest composition order that was presented in Present(). std::vector active_composition_order_; diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.h b/shell/platform/darwin/ios/ios_external_view_embedder.h index 03cec910bae39..3a5a0b3c17613 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.h +++ b/shell/platform/darwin/ios/ios_external_view_embedder.h @@ -67,13 +67,6 @@ class IOSExternalViewEmbedder : public ExternalViewEmbedder { // |ExternalViewEmbedder| bool SupportsDynamicThreadMerging() override; - // |ExternalViewEmbedder| - void PushFilterToVisitedPlatformViews( - std::shared_ptr filter) override; - - // |ExternalViewEmbedder| - void PushVisitedPlatformView(int64_t view_id) override; - FML_DISALLOW_COPY_AND_ASSIGN(IOSExternalViewEmbedder); }; diff --git a/shell/platform/darwin/ios/ios_external_view_embedder.mm b/shell/platform/darwin/ios/ios_external_view_embedder.mm index 76995be2c56ad..e67983321c770 100644 --- a/shell/platform/darwin/ios/ios_external_view_embedder.mm +++ b/shell/platform/darwin/ios/ios_external_view_embedder.mm @@ -98,15 +98,4 @@ return true; } -// |ExternalViewEmbedder| -void IOSExternalViewEmbedder::PushFilterToVisitedPlatformViews( - std::shared_ptr filter) { - platform_views_controller_->PushFilterToVisitedPlatformViews(filter); -} - -// |ExternalViewEmbedder| -void IOSExternalViewEmbedder::PushVisitedPlatformView(int64_t view_id) { - platform_views_controller_->PushVisitedPlatformView(view_id); -} - } // namespace flutter