From 1b7f1198e5dbb1f82f918335417997289d7b76f6 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 2 Dec 2019 13:37:40 -0800 Subject: [PATCH] Revert "Dynamically determine whether to use offscreen surface based on need (#13976)" This reverts commit a86ef946563b020108320bbfb974bf7343284fd3. --- ci/licenses_golden/licenses_flutter | 1 - flow/BUILD.gn | 1 - flow/layers/backdrop_filter_layer.cc | 4 +- flow/layers/clip_path_layer.cc | 1 - flow/layers/clip_rect_layer.cc | 1 - flow/layers/clip_rrect_layer.cc | 1 - flow/layers/color_filter_layer.cc | 4 +- flow/layers/container_layer.cc | 37 +--- flow/layers/container_layer.h | 23 +-- flow/layers/layer.cc | 27 +-- flow/layers/layer.h | 33 --- flow/layers/layer_tree.h | 4 - flow/layers/layer_unittests.cc | 188 ------------------ flow/layers/opacity_layer.cc | 8 +- flow/layers/physical_shape_layer.cc | 1 - flow/layers/shader_mask_layer.cc | 4 +- shell/common/rasterizer.cc | 3 +- shell/common/surface.h | 4 +- shell/gpu/gpu_surface_gl.cc | 22 +- shell/gpu/gpu_surface_gl.h | 9 +- shell/gpu/gpu_surface_gl_delegate.cc | 3 +- shell/gpu/gpu_surface_gl_delegate.h | 6 +- shell/gpu/gpu_surface_metal.h | 3 +- shell/gpu/gpu_surface_metal.mm | 3 +- shell/gpu/gpu_surface_software.cc | 3 +- shell/gpu/gpu_surface_software.h | 4 +- shell/gpu/gpu_surface_vulkan.cc | 3 +- shell/gpu/gpu_surface_vulkan.h | 4 +- .../framework/Source/FlutterPlatformViews.mm | 2 +- shell/platform/darwin/ios/ios_surface_gl.h | 2 +- shell/platform/darwin/ios/ios_surface_gl.mm | 7 +- shell/platform/fuchsia/flutter/surface.cc | 3 +- shell/platform/fuchsia/flutter/surface.h | 3 +- 33 files changed, 36 insertions(+), 386 deletions(-) delete mode 100644 flow/layers/layer_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 1e81f43602348..c296ebe416fbd 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -46,7 +46,6 @@ FILE: ../../../flutter/flow/layers/layer.cc FILE: ../../../flutter/flow/layers/layer.h FILE: ../../../flutter/flow/layers/layer_tree.cc FILE: ../../../flutter/flow/layers/layer_tree.h -FILE: ../../../flutter/flow/layers/layer_unittests.cc FILE: ../../../flutter/flow/layers/opacity_layer.cc FILE: ../../../flutter/flow/layers/opacity_layer.h FILE: ../../../flutter/flow/layers/performance_overlay_layer.cc diff --git a/flow/BUILD.gn b/flow/BUILD.gn index c99f2177e772f..c2f0c98415a3e 100644 --- a/flow/BUILD.gn +++ b/flow/BUILD.gn @@ -109,7 +109,6 @@ executable("flow_unittests") { "flow_run_all_unittests.cc", "flow_test_utils.cc", "flow_test_utils.h", - "layers/layer_unittests.cc", "layers/performance_overlay_layer_unittests.cc", "layers/physical_shape_layer_unittests.cc", "matrix_decomposition_unittests.cc", diff --git a/flow/layers/backdrop_filter_layer.cc b/flow/layers/backdrop_filter_layer.cc index cf6f5cf522b96..573db97f191a2 100644 --- a/flow/layers/backdrop_filter_layer.cc +++ b/flow/layers/backdrop_filter_layer.cc @@ -7,9 +7,7 @@ namespace flutter { BackdropFilterLayer::BackdropFilterLayer(sk_sp filter) - : filter_(std::move(filter)) { - set_layer_reads_surface(filter_.get() != nullptr); -} + : filter_(std::move(filter)) {} BackdropFilterLayer::~BackdropFilterLayer() = default; diff --git a/flow/layers/clip_path_layer.cc b/flow/layers/clip_path_layer.cc index 0c63b2331d9f5..d08c19b34eeb9 100644 --- a/flow/layers/clip_path_layer.cc +++ b/flow/layers/clip_path_layer.cc @@ -15,7 +15,6 @@ namespace flutter { ClipPathLayer::ClipPathLayer(const SkPath& clip_path, Clip clip_behavior) : clip_path_(clip_path), clip_behavior_(clip_behavior) { FML_DCHECK(clip_behavior != Clip::none); - set_renders_to_save_layer(clip_behavior == Clip::antiAliasWithSaveLayer); } ClipPathLayer::~ClipPathLayer() = default; diff --git a/flow/layers/clip_rect_layer.cc b/flow/layers/clip_rect_layer.cc index 5ee6046b19a73..de7590624e408 100644 --- a/flow/layers/clip_rect_layer.cc +++ b/flow/layers/clip_rect_layer.cc @@ -9,7 +9,6 @@ namespace flutter { ClipRectLayer::ClipRectLayer(const SkRect& clip_rect, Clip clip_behavior) : clip_rect_(clip_rect), clip_behavior_(clip_behavior) { FML_DCHECK(clip_behavior != Clip::none); - set_renders_to_save_layer(clip_behavior == Clip::antiAliasWithSaveLayer); } ClipRectLayer::~ClipRectLayer() = default; diff --git a/flow/layers/clip_rrect_layer.cc b/flow/layers/clip_rrect_layer.cc index 037d91ddcb5ff..9899a1658732d 100644 --- a/flow/layers/clip_rrect_layer.cc +++ b/flow/layers/clip_rrect_layer.cc @@ -9,7 +9,6 @@ namespace flutter { ClipRRectLayer::ClipRRectLayer(const SkRRect& clip_rrect, Clip clip_behavior) : clip_rrect_(clip_rrect), clip_behavior_(clip_behavior) { FML_DCHECK(clip_behavior != Clip::none); - set_renders_to_save_layer(clip_behavior == Clip::antiAliasWithSaveLayer); } ClipRRectLayer::~ClipRRectLayer() = default; diff --git a/flow/layers/color_filter_layer.cc b/flow/layers/color_filter_layer.cc index 6f355f4419ab4..f838b0612b2e5 100644 --- a/flow/layers/color_filter_layer.cc +++ b/flow/layers/color_filter_layer.cc @@ -7,9 +7,7 @@ namespace flutter { ColorFilterLayer::ColorFilterLayer(sk_sp filter) - : filter_(std::move(filter)) { - set_renders_to_save_layer(true); -} + : filter_(std::move(filter)) {} ColorFilterLayer::~ColorFilterLayer() = default; diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 1fa7056c5454f..d5c6a2a03a34a 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -6,46 +6,13 @@ namespace flutter { -ContainerLayer::ContainerLayer() - : child_needs_screen_readback_(false), renders_to_save_layer_(false) {} +ContainerLayer::ContainerLayer() {} ContainerLayer::~ContainerLayer() = default; void ContainerLayer::Add(std::shared_ptr layer) { - Layer* the_layer = layer.get(); - the_layer->set_parent(this); + layer->set_parent(this); layers_.push_back(std::move(layer)); - if (the_layer->tree_reads_surface()) { - NotifyChildReadback(the_layer); - } -} - -void ContainerLayer::ClearChildren() { - layers_.clear(); - if (child_needs_screen_readback_) { - child_needs_screen_readback_ = false; - UpdateTreeReadsSurface(); - } -} - -void ContainerLayer::set_renders_to_save_layer(bool value) { - if (renders_to_save_layer_ != value) { - renders_to_save_layer_ = value; - UpdateTreeReadsSurface(); - } -} - -void ContainerLayer::NotifyChildReadback(const Layer* layer) { - if (child_needs_screen_readback_ || !layer->tree_reads_surface()) { - return; - } - child_needs_screen_readback_ = true; - UpdateTreeReadsSurface(); -} - -bool ContainerLayer::ComputeTreeReadsSurface() const { - return layer_reads_surface() || - (!renders_to_save_layer_ && child_needs_screen_readback_); } void ContainerLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index 1e37f0164cf40..ef1c03328d1df 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -25,43 +25,22 @@ class ContainerLayer : public Layer { const std::vector>& layers() const { return layers_; } - // Called when the layer, which must be a child of this container, - // changes its tree_reads_surface() result from false to true. - void NotifyChildReadback(const Layer* layer); - protected: void PrerollChildren(PrerollContext* context, const SkMatrix& child_matrix, SkRect* child_paint_bounds); void PaintChildren(PaintContext& context) const; - virtual bool ComputeTreeReadsSurface() const override; - #if defined(OS_FUCHSIA) void UpdateSceneChildren(SceneUpdateContext& context); #endif // defined(OS_FUCHSIA) - // Specify whether or not the container has its children render - // to a SaveLayer which will prevent many rendering anomalies - // from propagating to the parent - such as if the children - // read back from the surface on which they render, or if the - // children perform non-associative rendering. Those children - // will now be performing those operations on the SaveLayer - // rather than the layer that this container renders onto. - void set_renders_to_save_layer(bool value); - // For OpacityLayer to restructure to have a single child. - void ClearChildren(); + void ClearChildren() { layers_.clear(); } private: std::vector> layers_; - // child_needs_screen_readback_ is maintained even if the - // renders_to_save_layer_ property is set in case both - // parameters are dynamically and independently determined. - bool child_needs_screen_readback_; - bool renders_to_save_layer_; - FML_DISALLOW_COPY_AND_ASSIGN(ContainerLayer); }; diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index 052b70fac9827..b729f582a0a9a 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "flutter/flow/layers/layer.h" -#include "flutter/flow/layers/container_layer.h" #include "flutter/flow/paint_utils.h" #include "third_party/skia/include/core/SkColorFilter.h" @@ -14,9 +13,7 @@ Layer::Layer() : parent_(nullptr), needs_system_composite_(false), paint_bounds_(SkRect::MakeEmpty()), - unique_id_(NextUniqueID()), - tree_reads_surface_(false), - layer_reads_surface_(false) {} + unique_id_(NextUniqueID()) {} Layer::~Layer() = default; @@ -29,28 +26,6 @@ uint64_t Layer::NextUniqueID() { return id; } -void Layer::set_layer_reads_surface(bool value) { - if (layer_reads_surface_ != value) { - layer_reads_surface_ = value; - UpdateTreeReadsSurface(); - } -} - -bool Layer::ComputeTreeReadsSurface() const { - return layer_reads_surface_; -} - -void Layer::UpdateTreeReadsSurface() { - bool new_tree_reads_surface = ComputeTreeReadsSurface(); - - if (tree_reads_surface_ != new_tree_reads_surface) { - tree_reads_surface_ = new_tree_reads_surface; - if (parent_ != nullptr) { - parent_->NotifyChildReadback(this); - } - } -} - void Layer::Preroll(PrerollContext* context, const SkMatrix& matrix) {} #if defined(OS_FUCHSIA) diff --git a/flow/layers/layer.h b/flow/layers/layer.h index ef114b4c381c3..66944376e8ce8 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -145,47 +145,14 @@ class Layer { bool needs_painting() const { return !paint_bounds_.isEmpty(); } - // True iff the layer, or some descendant of the layer, performs an - // operation which depends on (i.e. must read back from) the surface - // on which it is rendered in any way beyond the functionality of - // BlendMode. This value has no setter as it is computed from other - // flags and properties on the layer. - // For an example see |BackdropFilterLayer|. - // - // See |UpdateTreeReadsSurface| - // See |set_layer_reads_surface| - // See |ContainerLayer::set_renders_to_save_layer| - // See |ContainerLayer::NotifyChildReadback| - bool tree_reads_surface() const { return tree_reads_surface_; } - uint64_t unique_id() const { return unique_id_; } - protected: - // Compute a new value for tree_reads_surface_ from all of the various - // properties of this layer. - // Used by |UpdateTreeReadsSurface| - virtual bool ComputeTreeReadsSurface() const; - - // Update the tree_reads_surface_ value and propagate changes to - // ancestors if needed. - // Uses |ComputeTreeReadsSurface| - void UpdateTreeReadsSurface(); - - // True iff the layer itself (not a child or other descendant) performs - // an operation which reads from the surface on which it is rendered. - bool layer_reads_surface() const { return layer_reads_surface_; } - - void set_layer_reads_surface(bool value); - private: ContainerLayer* parent_; bool needs_system_composite_; SkRect paint_bounds_; uint64_t unique_id_; - bool tree_reads_surface_; - bool layer_reads_surface_; - static uint64_t NextUniqueID(); FML_DISALLOW_COPY_AND_ASSIGN(Layer); diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index 99908740eccc3..124b8a85dea45 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -43,10 +43,6 @@ class LayerTree { root_layer_ = std::move(root_layer); } - bool root_needs_screen_readback() const { - return root_layer_ && root_layer_->tree_reads_surface(); - } - const SkISize& frame_size() const { return frame_size_; } void set_frame_size(const SkISize& frame_size) { frame_size_ = frame_size; } diff --git a/flow/layers/layer_unittests.cc b/flow/layers/layer_unittests.cc deleted file mode 100644 index 360dba5acd5dc..0000000000000 --- a/flow/layers/layer_unittests.cc +++ /dev/null @@ -1,188 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "flutter/flow/layers/backdrop_filter_layer.h" -#include "flutter/flow/layers/clip_path_layer.h" -#include "flutter/flow/layers/clip_rect_layer.h" -#include "flutter/flow/layers/clip_rrect_layer.h" -#include "flutter/flow/layers/color_filter_layer.h" -#include "flutter/flow/layers/container_layer.h" -#include "flutter/flow/layers/opacity_layer.h" -#include "flutter/flow/layers/physical_shape_layer.h" -#include "flutter/flow/layers/shader_mask_layer.h" - -#include "third_party/skia/include/effects/SkBlurImageFilter.h" - -#include "gtest/gtest.h" - -namespace flutter { - -class ReadbackLayer : public ContainerLayer { - public: - ReadbackLayer(const bool reads, const bool saves) { - set_layer_reads_surface(reads); - set_renders_to_save_layer(saves); - } - ~ReadbackLayer() override = default; - - static std::shared_ptr Make(const bool reads, - const bool saves) { - return std::make_shared(reads, saves); - } - - void set_read(const bool reads) { set_layer_reads_surface(reads); } - - void Paint(PaintContext& context) const override {} -}; - -void TestLayerFlag(bool reads, bool saves) { - EXPECT_EQ(ReadbackLayer(reads, saves).tree_reads_surface(), reads); -} - -void TestChildFlag(bool child_reads, bool uses_save_layer, bool ret) { - ReadbackLayer parent = ReadbackLayer(false, uses_save_layer); - parent.Add(ReadbackLayer::Make(child_reads, false)); - EXPECT_EQ(parent.tree_reads_surface(), ret); -} - -TEST(Layer, ReadbackFalse) { - TestLayerFlag(false, false); - TestLayerFlag(false, true); -} - -TEST(Layer, ReadbackTrue) { - TestLayerFlag(true, false); - TestLayerFlag(true, true); -} - -TEST(Layer, NoReadbackNoSaveLayer) { - TestChildFlag(false, false, false); -} - -TEST(Layer, NoReadbackButSaveLayer) { - TestChildFlag(false, true, false); -} - -TEST(Layer, ReadbackNoSaveLayer) { - TestChildFlag(true, false, true); -} - -TEST(Layer, ReadbackButSaveLayer) { - TestChildFlag(true, true, false); -} - -TEST(Layer, AddedChildReadback) { - ReadbackLayer parent = ReadbackLayer(false, false); - EXPECT_FALSE(parent.tree_reads_surface()); - parent.Add(ReadbackLayer::Make(false, false)); - EXPECT_FALSE(parent.tree_reads_surface()); - parent.Add(ReadbackLayer::Make(true, false)); - EXPECT_TRUE(parent.tree_reads_surface()); -} - -TEST(Layer, ChildChangesToReadback) { - ReadbackLayer parent = ReadbackLayer(false, false); - EXPECT_FALSE(parent.tree_reads_surface()); - parent.Add(ReadbackLayer::Make(false, false)); - EXPECT_FALSE(parent.tree_reads_surface()); - std::shared_ptr child = ReadbackLayer::Make(false, false); - parent.Add(child); - EXPECT_FALSE(parent.tree_reads_surface()); - child->set_read(true); - EXPECT_TRUE(parent.tree_reads_surface()); -} - -TEST(Layer, BackdropFilterLayer) { - sk_sp filter = SkBlurImageFilter::Make( - 5.0f, 5.0f, nullptr, nullptr, SkBlurImageFilter::kClamp_TileMode); - EXPECT_TRUE(BackdropFilterLayer(filter).tree_reads_surface()); - filter.reset(); - EXPECT_FALSE(BackdropFilterLayer(filter).tree_reads_surface()); -} - -void TestClipRect(Clip clip_behavior, bool ret) { - ClipRectLayer layer = ClipRectLayer(SkRect::MakeWH(5, 5), clip_behavior); - layer.Add(ReadbackLayer::Make(true, false)); - EXPECT_EQ(layer.tree_reads_surface(), ret); -} - -TEST(Layer, ClipRectSaveLayer) { - // TestClipRect(Clip::none, true); // ClipRectLayer asserts !Clip::none - TestClipRect(Clip::hardEdge, true); - TestClipRect(Clip::antiAlias, true); - TestClipRect(Clip::antiAliasWithSaveLayer, false); -} - -void TestClipRRect(Clip clip_behavior, bool ret) { - SkRRect r_rect = SkRRect::MakeRect(SkRect::MakeWH(5, 5)); - ClipRRectLayer layer = ClipRRectLayer(r_rect, clip_behavior); - layer.Add(ReadbackLayer::Make(true, false)); - EXPECT_EQ(layer.tree_reads_surface(), ret); -} - -TEST(Layer, ClipRRectSaveLayer) { - // TestClipRRect(Clip::none, true); // ClipRRectLayer asserts !Clip::none - TestClipRRect(Clip::hardEdge, true); - TestClipRRect(Clip::antiAlias, true); - TestClipRRect(Clip::antiAliasWithSaveLayer, false); -} - -void TestClipPath(Clip clip_behavior, bool ret) { - SkPath path = SkPath(); - path.moveTo(0, 0); - path.lineTo(5, 0); - path.lineTo(0, 5); - path.close(); - ClipPathLayer layer = ClipPathLayer(path, clip_behavior); - layer.Add(ReadbackLayer::Make(true, false)); - EXPECT_EQ(layer.tree_reads_surface(), ret); -} - -TEST(Layer, ClipPathSaveLayer) { - // TestClipPath(Clip::none, true); // ClipRRectLayer asserts !Clip::none - TestClipPath(Clip::hardEdge, true); - TestClipPath(Clip::antiAlias, true); - TestClipPath(Clip::antiAliasWithSaveLayer, false); -} - -TEST(Layer, ColorFilterSaveLayer) { - sk_sp filter = SkColorFilters::LinearToSRGBGamma(); - ColorFilterLayer layer = ColorFilterLayer(filter); - layer.Add(ReadbackLayer::Make(true, false)); - EXPECT_FALSE(layer.tree_reads_surface()); -} - -TEST(Layer, OpacitySaveLayer) { - OpacityLayer layer = OpacityLayer(10, SkPoint::Make(0, 0)); - layer.Add(ReadbackLayer::Make(true, false)); - EXPECT_FALSE(layer.tree_reads_surface()); -} - -void TestPhysicalShapeLayer(Clip clip_behavior, bool ret) { - SkPath path = SkPath(); - path.moveTo(0, 0); - path.lineTo(5, 0); - path.lineTo(0, 5); - path.close(); - PhysicalShapeLayer layer = PhysicalShapeLayer( - SK_ColorRED, SK_ColorBLUE, 1.0f, 100.0f, 10.0f, path, clip_behavior); - layer.Add(ReadbackLayer::Make(true, false)); - EXPECT_EQ(layer.tree_reads_surface(), ret); -} - -TEST(Layer, PhysicalShapeSaveLayer) { - TestPhysicalShapeLayer(Clip::none, true); - TestPhysicalShapeLayer(Clip::hardEdge, true); - TestPhysicalShapeLayer(Clip::antiAlias, true); - TestPhysicalShapeLayer(Clip::antiAliasWithSaveLayer, false); -} - -TEST(Layer, ShaderMaskSaveLayer) { - ShaderMaskLayer layer = ShaderMaskLayer( - SkShaders::Empty(), SkRect::MakeWH(5, 5), SkBlendMode::kSrcOver); - layer.Add(ReadbackLayer::Make(true, false)); - EXPECT_FALSE(layer.tree_reads_surface()); -} - -} // namespace flutter diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 54f8e19d76eea..6257700ffbddf 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -9,13 +9,7 @@ namespace flutter { OpacityLayer::OpacityLayer(int alpha, const SkPoint& offset) - : alpha_(alpha), offset_(offset) { - // This type of layer either renders via a SaveLayer or it renders - // from a raster cache image. In either case, it does not pass - // any rendering from a child through to the surface so it is - // effectively "renders to save layer" in all cases. - set_renders_to_save_layer(true); -} + : alpha_(alpha), offset_(offset) {} OpacityLayer::~OpacityLayer() = default; diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 428e9fb8e4b2b..0a607a88c23b0 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -46,7 +46,6 @@ PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, // an SkPath. frameRRect_ = SkRRect::MakeRect(path.getBounds()); } - set_renders_to_save_layer(clip_behavior == Clip::antiAliasWithSaveLayer); } PhysicalShapeLayer::~PhysicalShapeLayer() = default; diff --git a/flow/layers/shader_mask_layer.cc b/flow/layers/shader_mask_layer.cc index 064e0e1b2ad8b..36e7b7332aeae 100644 --- a/flow/layers/shader_mask_layer.cc +++ b/flow/layers/shader_mask_layer.cc @@ -9,9 +9,7 @@ namespace flutter { ShaderMaskLayer::ShaderMaskLayer(sk_sp shader, const SkRect& mask_rect, SkBlendMode blend_mode) - : shader_(shader), mask_rect_(mask_rect), blend_mode_(blend_mode) { - set_renders_to_save_layer(true); -} + : shader_(shader), mask_rect_(mask_rect), blend_mode_(blend_mode) {} ShaderMaskLayer::~ShaderMaskLayer() = default; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index fa25b0d7933cf..11fdd3f6ee6ad 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -297,8 +297,7 @@ RasterStatus Rasterizer::DoDraw( RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) { FML_DCHECK(surface_); - auto frame = surface_->AcquireFrame(layer_tree.frame_size(), - layer_tree.root_needs_screen_readback()); + auto frame = surface_->AcquireFrame(layer_tree.frame_size()); if (frame == nullptr) { return RasterStatus::kFailed; diff --git a/shell/common/surface.h b/shell/common/surface.h index 661f21f04cb05..14f898fad3fe1 100644 --- a/shell/common/surface.h +++ b/shell/common/surface.h @@ -50,9 +50,7 @@ class Surface { virtual bool IsValid() = 0; - virtual std::unique_ptr AcquireFrame( - const SkISize& size, - const bool needs_readback) = 0; + virtual std::unique_ptr AcquireFrame(const SkISize& size) = 0; virtual SkMatrix GetRootTransformation() const = 0; diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc index ac81687f84037..5f30a48375d33 100644 --- a/shell/gpu/gpu_surface_gl.cc +++ b/shell/gpu/gpu_surface_gl.cc @@ -193,17 +193,12 @@ static sk_sp CreateOffscreenSurface(GrContext* context, &surface_props); } -bool GPUSurfaceGL::CreateOrUpdateSurfaces(const SkISize& size, - const bool needs_readback) { - bool needs_offscreen = delegate_->UseOffscreenSurface(needs_readback); +bool GPUSurfaceGL::CreateOrUpdateSurfaces(const SkISize& size) { if (onscreen_surface_ != nullptr && size == SkISize::Make(onscreen_surface_->width(), onscreen_surface_->height())) { // Surface size appears unchanged. So bail. - bool has_offscreen = offscreen_surface_ != nullptr; - if (needs_offscreen == has_offscreen) { - return true; - } + return true; } // We need to do some updates. @@ -233,7 +228,7 @@ bool GPUSurfaceGL::CreateOrUpdateSurfaces(const SkISize& size, return false; } - if (needs_offscreen) { + if (delegate_->UseOffscreenSurface()) { offscreen_surface = CreateOffscreenSurface(context_.get(), size); if (offscreen_surface == nullptr) { FML_LOG(ERROR) << "Could not create offscreen surface."; @@ -253,9 +248,7 @@ SkMatrix GPUSurfaceGL::GetRootTransformation() const { } // |Surface| -std::unique_ptr GPUSurfaceGL::AcquireFrame( - const SkISize& size, - const bool needs_readback) { +std::unique_ptr GPUSurfaceGL::AcquireFrame(const SkISize& size) { if (delegate_ == nullptr) { return nullptr; } @@ -278,7 +271,7 @@ std::unique_ptr GPUSurfaceGL::AcquireFrame( const auto root_surface_transformation = GetRootTransformation(); sk_sp surface = - AcquireRenderSurface(size, root_surface_transformation, needs_readback); + AcquireRenderSurface(size, root_surface_transformation); if (surface == nullptr) { return nullptr; @@ -342,15 +335,14 @@ bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) { sk_sp GPUSurfaceGL::AcquireRenderSurface( const SkISize& untransformed_size, - const SkMatrix& root_surface_transformation, - const bool needs_readback) { + const SkMatrix& root_surface_transformation) { const auto transformed_rect = root_surface_transformation.mapRect( SkRect::MakeWH(untransformed_size.width(), untransformed_size.height())); const auto transformed_size = SkISize::Make(transformed_rect.width(), transformed_rect.height()); - if (!CreateOrUpdateSurfaces(transformed_size, needs_readback)) { + if (!CreateOrUpdateSurfaces(transformed_size)) { return nullptr; } diff --git a/shell/gpu/gpu_surface_gl.h b/shell/gpu/gpu_surface_gl.h index b7a26e1a057b7..97325569bfd16 100644 --- a/shell/gpu/gpu_surface_gl.h +++ b/shell/gpu/gpu_surface_gl.h @@ -32,9 +32,7 @@ class GPUSurfaceGL : public Surface { bool IsValid() override; // |Surface| - std::unique_ptr AcquireFrame( - const SkISize& size, - const bool needs_readback) override; + std::unique_ptr AcquireFrame(const SkISize& size) override; // |Surface| SkMatrix GetRootTransformation() const override; @@ -62,12 +60,11 @@ class GPUSurfaceGL : public Surface { bool valid_ = false; fml::WeakPtrFactory weak_factory_; - bool CreateOrUpdateSurfaces(const SkISize& size, const bool needs_readback); + bool CreateOrUpdateSurfaces(const SkISize& size); sk_sp AcquireRenderSurface( const SkISize& untransformed_size, - const SkMatrix& root_surface_transformation, - const bool needs_readback); + const SkMatrix& root_surface_transformation); bool PresentSurface(SkCanvas* canvas); diff --git a/shell/gpu/gpu_surface_gl_delegate.cc b/shell/gpu/gpu_surface_gl_delegate.cc index 89c660688b3cc..1ef969fe8acc5 100644 --- a/shell/gpu/gpu_surface_gl_delegate.cc +++ b/shell/gpu/gpu_surface_gl_delegate.cc @@ -12,8 +12,7 @@ bool GPUSurfaceGLDelegate::GLContextFBOResetAfterPresent() const { return false; } -bool GPUSurfaceGLDelegate::UseOffscreenSurface( - const bool needs_readback) const { +bool GPUSurfaceGLDelegate::UseOffscreenSurface() const { return false; } diff --git a/shell/gpu/gpu_surface_gl_delegate.h b/shell/gpu/gpu_surface_gl_delegate.h index 7a222c3567909..dfe0ce7f468db 100644 --- a/shell/gpu/gpu_surface_gl_delegate.h +++ b/shell/gpu/gpu_surface_gl_delegate.h @@ -36,10 +36,8 @@ class GPUSurfaceGLDelegate : public GPUSurfaceDelegate { // subsequent frames. virtual bool GLContextFBOResetAfterPresent() const; - // Create an offscreen surface to render into before onscreen composition - // based on whether or not the frame will perform any operations that will - // require readback from the rendering target. - virtual bool UseOffscreenSurface(const bool needs_readback) const; + // Create an offscreen surface to render into before onscreen composition. + virtual bool UseOffscreenSurface() const; // A transformation applied to the onscreen surface before the canvas is // flushed. diff --git a/shell/gpu/gpu_surface_metal.h b/shell/gpu/gpu_surface_metal.h index 30f8fe2fa35e1..fc6b0964766ce 100644 --- a/shell/gpu/gpu_surface_metal.h +++ b/shell/gpu/gpu_surface_metal.h @@ -37,8 +37,7 @@ class GPUSurfaceMetal : public Surface { bool IsValid() override; // |Surface| - std::unique_ptr AcquireFrame(const SkISize& size, - const bool needs_readback) override; + std::unique_ptr AcquireFrame(const SkISize& size) override; // |Surface| SkMatrix GetRootTransformation() const override; diff --git a/shell/gpu/gpu_surface_metal.mm b/shell/gpu/gpu_surface_metal.mm index bc71337dcb94f..81abf740d48f6 100644 --- a/shell/gpu/gpu_surface_metal.mm +++ b/shell/gpu/gpu_surface_metal.mm @@ -83,8 +83,7 @@ } // |Surface| -std::unique_ptr GPUSurfaceMetal::AcquireFrame(const SkISize& size, - const bool needs_readback) { +std::unique_ptr GPUSurfaceMetal::AcquireFrame(const SkISize& size) { if (!IsValid()) { FML_LOG(ERROR) << "Metal surface was invalid."; return nullptr; diff --git a/shell/gpu/gpu_surface_software.cc b/shell/gpu/gpu_surface_software.cc index 7bec87eed1948..346857ac47e6a 100644 --- a/shell/gpu/gpu_surface_software.cc +++ b/shell/gpu/gpu_surface_software.cc @@ -24,8 +24,7 @@ bool GPUSurfaceSoftware::IsValid() { // |Surface| std::unique_ptr GPUSurfaceSoftware::AcquireFrame( - const SkISize& logical_size, - const bool needs_readback) { + const SkISize& logical_size) { // TODO(38466): Refactor GPU surface APIs take into account the fact that an // external view embedder may want to render to the root surface. if (!render_to_surface_) { diff --git a/shell/gpu/gpu_surface_software.h b/shell/gpu/gpu_surface_software.h index 66b8b90500463..af4d1cb3c107f 100644 --- a/shell/gpu/gpu_surface_software.h +++ b/shell/gpu/gpu_surface_software.h @@ -23,9 +23,7 @@ class GPUSurfaceSoftware : public Surface { bool IsValid() override; // |Surface| - std::unique_ptr AcquireFrame( - const SkISize& size, - const bool needs_readback) override; + std::unique_ptr AcquireFrame(const SkISize& size) override; // |Surface| SkMatrix GetRootTransformation() const override; diff --git a/shell/gpu/gpu_surface_vulkan.cc b/shell/gpu/gpu_surface_vulkan.cc index 7a494849bdfcf..fbc9696b15d52 100644 --- a/shell/gpu/gpu_surface_vulkan.cc +++ b/shell/gpu/gpu_surface_vulkan.cc @@ -22,8 +22,7 @@ bool GPUSurfaceVulkan::IsValid() { // |Surface| std::unique_ptr GPUSurfaceVulkan::AcquireFrame( - const SkISize& size, - const bool needs_readback) { + const SkISize& size) { auto surface = window_.AcquireSurface(); if (surface == nullptr) { diff --git a/shell/gpu/gpu_surface_vulkan.h b/shell/gpu/gpu_surface_vulkan.h index 58c805936017e..7c410dd526cf7 100644 --- a/shell/gpu/gpu_surface_vulkan.h +++ b/shell/gpu/gpu_surface_vulkan.h @@ -26,9 +26,7 @@ class GPUSurfaceVulkan : public Surface { bool IsValid() override; // |Surface| - std::unique_ptr AcquireFrame( - const SkISize& size, - const bool needs_readback) override; + std::unique_ptr AcquireFrame(const SkISize& size) override; // |Surface| SkMatrix GetRootTransformation() const override; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 4837ef22f4a42..33ca14d9fabea 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -369,7 +369,7 @@ bool did_submit = true; for (int64_t view_id : composition_order_) { EnsureOverlayInitialized(view_id, gl_context, gr_context); - auto frame = overlays_[view_id]->surface->AcquireFrame(frame_size_, true); + auto frame = overlays_[view_id]->surface->AcquireFrame(frame_size_); SkCanvas* canvas = frame->SkiaCanvas(); canvas->drawPicture(picture_recorders_[view_id]->finishRecordingAsPicture()); canvas->flush(); diff --git a/shell/platform/darwin/ios/ios_surface_gl.h b/shell/platform/darwin/ios/ios_surface_gl.h index 73148f34ce9ed..c1019bb442bb0 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.h +++ b/shell/platform/darwin/ios/ios_surface_gl.h @@ -48,7 +48,7 @@ class IOSSurfaceGL final : public IOSSurface, intptr_t GLContextFBO() const override; - bool UseOffscreenSurface(const bool needs_readback) const override; + bool UseOffscreenSurface() const override; // |GPUSurfaceGLDelegate| ExternalViewEmbedder* GetExternalViewEmbedder() override; diff --git a/shell/platform/darwin/ios/ios_surface_gl.mm b/shell/platform/darwin/ios/ios_surface_gl.mm index 1130375758831..48e70e00a4e7a 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -49,12 +49,11 @@ return IsValid() ? render_target_->framebuffer() : GL_NONE; } -bool IOSSurfaceGL::UseOffscreenSurface(const bool needs_readback) const { +bool IOSSurfaceGL::UseOffscreenSurface() const { // The onscreen surface wraps a GL renderbuffer, which is extremely slow to read. // Certain filter effects require making a copy of the current destination, so we - // render to an offscreen surface, which will be much quicker to read/copy, if - // they are present. - return needs_readback; + // always render to an offscreen surface, which will be much quicker to read/copy. + return true; } bool IOSSurfaceGL::GLContextMakeCurrent() { diff --git a/shell/platform/fuchsia/flutter/surface.cc b/shell/platform/fuchsia/flutter/surface.cc index 1fda6a50f54d5..29fbbc7294cc1 100644 --- a/shell/platform/fuchsia/flutter/surface.cc +++ b/shell/platform/fuchsia/flutter/surface.cc @@ -24,8 +24,7 @@ bool Surface::IsValid() { // |flutter::Surface| std::unique_ptr Surface::AcquireFrame( - const SkISize& size, - const bool needs_readback) { + const SkISize& size) { return std::make_unique( nullptr, [](const flutter::SurfaceFrame& surface_frame, SkCanvas* canvas) { return true; }); diff --git a/shell/platform/fuchsia/flutter/surface.h b/shell/platform/fuchsia/flutter/surface.h index 87d6d09320c08..66220f079b3e5 100644 --- a/shell/platform/fuchsia/flutter/surface.h +++ b/shell/platform/fuchsia/flutter/surface.h @@ -29,8 +29,7 @@ class Surface final : public flutter::Surface { // |flutter::Surface| std::unique_ptr AcquireFrame( - const SkISize& size, - const bool needs_readback) override; + const SkISize& size) override; // |flutter::Surface| GrContext* GetContext() override;