From c87e428abb1ecf9537dab51007a2dba607ccd2f9 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 19 Nov 2019 20:02:22 -0800 Subject: [PATCH 01/11] Only use offscreen surface on iOS if BackdropFilter is present. --- flow/layers/layer_tree.cc | 3 ++- flow/layers/layer_tree.h | 5 +++++ lib/ui/compositing/scene.cc | 10 ++++++--- lib/ui/compositing/scene.h | 6 +++-- lib/ui/compositing/scene_builder.cc | 8 ++++--- lib/ui/compositing/scene_builder.h | 1 + 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 ++- 23 files changed, 80 insertions(+), 37 deletions(-) diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index b031ebf8cb8bd..ae84f8da7bda0 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -15,7 +15,8 @@ LayerTree::LayerTree() : frame_size_{}, rasterizer_tracing_threshold_(0), checkerboard_raster_cache_images_(false), - checkerboard_offscreen_layers_(false) {} + checkerboard_offscreen_layers_(false), + has_backdrop_filter_(false) {} LayerTree::~LayerTree() = default; diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index 124b8a85dea45..3e33464dbff24 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -71,6 +71,10 @@ class LayerTree { checkerboard_offscreen_layers_ = checkerboard; } + void set_has_backdrop_filter(bool present) { has_backdrop_filter_ = present; } + + bool has_backdrop_filter() { return has_backdrop_filter_; } + void set_device_pixel_ratio(double device_pixel_ratio) { device_pixel_ratio_ = device_pixel_ratio; } @@ -86,6 +90,7 @@ class LayerTree { uint32_t rasterizer_tracing_threshold_; bool checkerboard_raster_cache_images_; bool checkerboard_offscreen_layers_; + bool has_backdrop_filter_; FML_DISALLOW_COPY_AND_ASSIGN(LayerTree); }; diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index fd3e86c7e5176..849f2b86b7dd0 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -27,21 +27,25 @@ DART_BIND_ALL(Scene, FOR_EACH_BINDING) fml::RefPtr Scene::create(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers) { + bool checkerboardOffscreenLayers, + bool hasBackdropFilter) { return fml::MakeRefCounted( std::move(rootLayer), rasterizerTracingThreshold, - checkerboardRasterCacheImages, checkerboardOffscreenLayers); + checkerboardRasterCacheImages, checkerboardOffscreenLayers, + hasBackdropFilter); } Scene::Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers) + bool checkerboardOffscreenLayers, + bool hasBackdropFilter) : m_layerTree(new flutter::LayerTree()) { m_layerTree->set_root_layer(std::move(rootLayer)); m_layerTree->set_rasterizer_tracing_threshold(rasterizerTracingThreshold); m_layerTree->set_checkerboard_raster_cache_images( checkerboardRasterCacheImages); + m_layerTree->set_has_backdrop_filter(hasBackdropFilter); m_layerTree->set_checkerboard_offscreen_layers(checkerboardOffscreenLayers); } diff --git a/lib/ui/compositing/scene.h b/lib/ui/compositing/scene.h index 2d280a534db63..95d904e76775d 100644 --- a/lib/ui/compositing/scene.h +++ b/lib/ui/compositing/scene.h @@ -27,7 +27,8 @@ class Scene : public RefCountedDartWrappable { static fml::RefPtr create(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers); + bool checkerboardOffscreenLayers, + bool hasBackdropFilter); std::unique_ptr takeLayerTree(); @@ -43,7 +44,8 @@ class Scene : public RefCountedDartWrappable { explicit Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers); + bool checkerboardOffscreenLayers, + bool hasBackdropFilter); std::unique_ptr m_layerTree; }; diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 5b7148803c2c2..5167f9f45c707 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -152,6 +152,7 @@ fml::RefPtr SceneBuilder::pushColorFilter( fml::RefPtr SceneBuilder::pushBackdropFilter(ImageFilter* filter) { auto layer = std::make_shared(filter->filter()); PushLayer(layer); + has_backdrop_filter_ = true; return EngineLayer::MakeRetained(layer); } @@ -290,9 +291,10 @@ void SceneBuilder::setCheckerboardOffscreenLayers(bool checkerboard) { } fml::RefPtr SceneBuilder::build() { - fml::RefPtr scene = Scene::create( - std::move(root_layer_), rasterizer_tracing_threshold_, - checkerboard_raster_cache_images_, checkerboard_offscreen_layers_); + fml::RefPtr scene = + Scene::create(std::move(root_layer_), rasterizer_tracing_threshold_, + checkerboard_raster_cache_images_, + checkerboard_offscreen_layers_, has_backdrop_filter_); ClearDartWrapper(); return scene; } diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index d378876d960a2..83325ca0cd6bc 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -115,6 +115,7 @@ class SceneBuilder : public RefCountedDartWrappable { int rasterizer_tracing_threshold_ = 0; bool checkerboard_raster_cache_images_ = false; bool checkerboard_offscreen_layers_ = false; + bool has_backdrop_filter_ = false; void PushLayer(std::shared_ptr layer); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 9fbb6949d6dfb..d6e2c29d971c6 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -269,7 +269,8 @@ RasterStatus Rasterizer::DoDraw( RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) { FML_DCHECK(surface_); - auto frame = surface_->AcquireFrame(layer_tree.frame_size()); + auto frame = surface_->AcquireFrame(layer_tree.frame_size(), + layer_tree.has_backdrop_filter()); if (frame == nullptr) { return RasterStatus::kFailed; diff --git a/shell/common/surface.h b/shell/common/surface.h index 14f898fad3fe1..661f21f04cb05 100644 --- a/shell/common/surface.h +++ b/shell/common/surface.h @@ -50,7 +50,9 @@ class Surface { virtual bool IsValid() = 0; - virtual std::unique_ptr AcquireFrame(const SkISize& size) = 0; + virtual std::unique_ptr AcquireFrame( + const SkISize& size, + const bool needs_readback) = 0; virtual SkMatrix GetRootTransformation() const = 0; diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc index 5f30a48375d33..ac81687f84037 100644 --- a/shell/gpu/gpu_surface_gl.cc +++ b/shell/gpu/gpu_surface_gl.cc @@ -193,12 +193,17 @@ static sk_sp CreateOffscreenSurface(GrContext* context, &surface_props); } -bool GPUSurfaceGL::CreateOrUpdateSurfaces(const SkISize& size) { +bool GPUSurfaceGL::CreateOrUpdateSurfaces(const SkISize& size, + const bool needs_readback) { + bool needs_offscreen = delegate_->UseOffscreenSurface(needs_readback); if (onscreen_surface_ != nullptr && size == SkISize::Make(onscreen_surface_->width(), onscreen_surface_->height())) { // Surface size appears unchanged. So bail. - return true; + bool has_offscreen = offscreen_surface_ != nullptr; + if (needs_offscreen == has_offscreen) { + return true; + } } // We need to do some updates. @@ -228,7 +233,7 @@ bool GPUSurfaceGL::CreateOrUpdateSurfaces(const SkISize& size) { return false; } - if (delegate_->UseOffscreenSurface()) { + if (needs_offscreen) { offscreen_surface = CreateOffscreenSurface(context_.get(), size); if (offscreen_surface == nullptr) { FML_LOG(ERROR) << "Could not create offscreen surface."; @@ -248,7 +253,9 @@ SkMatrix GPUSurfaceGL::GetRootTransformation() const { } // |Surface| -std::unique_ptr GPUSurfaceGL::AcquireFrame(const SkISize& size) { +std::unique_ptr GPUSurfaceGL::AcquireFrame( + const SkISize& size, + const bool needs_readback) { if (delegate_ == nullptr) { return nullptr; } @@ -271,7 +278,7 @@ std::unique_ptr GPUSurfaceGL::AcquireFrame(const SkISize& size) { const auto root_surface_transformation = GetRootTransformation(); sk_sp surface = - AcquireRenderSurface(size, root_surface_transformation); + AcquireRenderSurface(size, root_surface_transformation, needs_readback); if (surface == nullptr) { return nullptr; @@ -335,14 +342,15 @@ bool GPUSurfaceGL::PresentSurface(SkCanvas* canvas) { sk_sp GPUSurfaceGL::AcquireRenderSurface( const SkISize& untransformed_size, - const SkMatrix& root_surface_transformation) { + const SkMatrix& root_surface_transformation, + const bool needs_readback) { 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)) { + if (!CreateOrUpdateSurfaces(transformed_size, needs_readback)) { return nullptr; } diff --git a/shell/gpu/gpu_surface_gl.h b/shell/gpu/gpu_surface_gl.h index 97325569bfd16..b7a26e1a057b7 100644 --- a/shell/gpu/gpu_surface_gl.h +++ b/shell/gpu/gpu_surface_gl.h @@ -32,7 +32,9 @@ class GPUSurfaceGL : public Surface { bool IsValid() override; // |Surface| - std::unique_ptr AcquireFrame(const SkISize& size) override; + std::unique_ptr AcquireFrame( + const SkISize& size, + const bool needs_readback) override; // |Surface| SkMatrix GetRootTransformation() const override; @@ -60,11 +62,12 @@ class GPUSurfaceGL : public Surface { bool valid_ = false; fml::WeakPtrFactory weak_factory_; - bool CreateOrUpdateSurfaces(const SkISize& size); + bool CreateOrUpdateSurfaces(const SkISize& size, const bool needs_readback); sk_sp AcquireRenderSurface( const SkISize& untransformed_size, - const SkMatrix& root_surface_transformation); + const SkMatrix& root_surface_transformation, + const bool needs_readback); bool PresentSurface(SkCanvas* canvas); diff --git a/shell/gpu/gpu_surface_gl_delegate.cc b/shell/gpu/gpu_surface_gl_delegate.cc index 1ef969fe8acc5..89c660688b3cc 100644 --- a/shell/gpu/gpu_surface_gl_delegate.cc +++ b/shell/gpu/gpu_surface_gl_delegate.cc @@ -12,7 +12,8 @@ bool GPUSurfaceGLDelegate::GLContextFBOResetAfterPresent() const { return false; } -bool GPUSurfaceGLDelegate::UseOffscreenSurface() const { +bool GPUSurfaceGLDelegate::UseOffscreenSurface( + const bool needs_readback) const { return false; } diff --git a/shell/gpu/gpu_surface_gl_delegate.h b/shell/gpu/gpu_surface_gl_delegate.h index dfe0ce7f468db..7a222c3567909 100644 --- a/shell/gpu/gpu_surface_gl_delegate.h +++ b/shell/gpu/gpu_surface_gl_delegate.h @@ -36,8 +36,10 @@ class GPUSurfaceGLDelegate : public GPUSurfaceDelegate { // subsequent frames. virtual bool GLContextFBOResetAfterPresent() const; - // Create an offscreen surface to render into before onscreen composition. - virtual bool UseOffscreenSurface() 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; // 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 fc6b0964766ce..30f8fe2fa35e1 100644 --- a/shell/gpu/gpu_surface_metal.h +++ b/shell/gpu/gpu_surface_metal.h @@ -37,7 +37,8 @@ class GPUSurfaceMetal : public Surface { bool IsValid() override; // |Surface| - std::unique_ptr AcquireFrame(const SkISize& size) override; + std::unique_ptr AcquireFrame(const SkISize& size, + const bool needs_readback) override; // |Surface| SkMatrix GetRootTransformation() const override; diff --git a/shell/gpu/gpu_surface_metal.mm b/shell/gpu/gpu_surface_metal.mm index 81abf740d48f6..bc71337dcb94f 100644 --- a/shell/gpu/gpu_surface_metal.mm +++ b/shell/gpu/gpu_surface_metal.mm @@ -83,7 +83,8 @@ } // |Surface| -std::unique_ptr GPUSurfaceMetal::AcquireFrame(const SkISize& size) { +std::unique_ptr GPUSurfaceMetal::AcquireFrame(const SkISize& size, + const bool needs_readback) { 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 346857ac47e6a..7bec87eed1948 100644 --- a/shell/gpu/gpu_surface_software.cc +++ b/shell/gpu/gpu_surface_software.cc @@ -24,7 +24,8 @@ bool GPUSurfaceSoftware::IsValid() { // |Surface| std::unique_ptr GPUSurfaceSoftware::AcquireFrame( - const SkISize& logical_size) { + const SkISize& logical_size, + const bool needs_readback) { // 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 af4d1cb3c107f..66b8b90500463 100644 --- a/shell/gpu/gpu_surface_software.h +++ b/shell/gpu/gpu_surface_software.h @@ -23,7 +23,9 @@ class GPUSurfaceSoftware : public Surface { bool IsValid() override; // |Surface| - std::unique_ptr AcquireFrame(const SkISize& size) override; + std::unique_ptr AcquireFrame( + const SkISize& size, + const bool needs_readback) override; // |Surface| SkMatrix GetRootTransformation() const override; diff --git a/shell/gpu/gpu_surface_vulkan.cc b/shell/gpu/gpu_surface_vulkan.cc index fbc9696b15d52..7a494849bdfcf 100644 --- a/shell/gpu/gpu_surface_vulkan.cc +++ b/shell/gpu/gpu_surface_vulkan.cc @@ -22,7 +22,8 @@ bool GPUSurfaceVulkan::IsValid() { // |Surface| std::unique_ptr GPUSurfaceVulkan::AcquireFrame( - const SkISize& size) { + const SkISize& size, + const bool needs_readback) { auto surface = window_.AcquireSurface(); if (surface == nullptr) { diff --git a/shell/gpu/gpu_surface_vulkan.h b/shell/gpu/gpu_surface_vulkan.h index 7c410dd526cf7..58c805936017e 100644 --- a/shell/gpu/gpu_surface_vulkan.h +++ b/shell/gpu/gpu_surface_vulkan.h @@ -26,7 +26,9 @@ class GPUSurfaceVulkan : public Surface { bool IsValid() override; // |Surface| - std::unique_ptr AcquireFrame(const SkISize& size) override; + std::unique_ptr AcquireFrame( + const SkISize& size, + const bool needs_readback) 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 33ca14d9fabea..4837ef22f4a42 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_); + auto frame = overlays_[view_id]->surface->AcquireFrame(frame_size_, true); 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 c1019bb442bb0..73148f34ce9ed 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 override; + bool UseOffscreenSurface(const bool needs_readback) 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 48e70e00a4e7a..1130375758831 100644 --- a/shell/platform/darwin/ios/ios_surface_gl.mm +++ b/shell/platform/darwin/ios/ios_surface_gl.mm @@ -49,11 +49,12 @@ return IsValid() ? render_target_->framebuffer() : GL_NONE; } -bool IOSSurfaceGL::UseOffscreenSurface() const { +bool IOSSurfaceGL::UseOffscreenSurface(const bool needs_readback) 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 - // always render to an offscreen surface, which will be much quicker to read/copy. - return true; + // render to an offscreen surface, which will be much quicker to read/copy, if + // they are present. + return needs_readback; } bool IOSSurfaceGL::GLContextMakeCurrent() { diff --git a/shell/platform/fuchsia/flutter/surface.cc b/shell/platform/fuchsia/flutter/surface.cc index 29fbbc7294cc1..1fda6a50f54d5 100644 --- a/shell/platform/fuchsia/flutter/surface.cc +++ b/shell/platform/fuchsia/flutter/surface.cc @@ -24,7 +24,8 @@ bool Surface::IsValid() { // |flutter::Surface| std::unique_ptr Surface::AcquireFrame( - const SkISize& size) { + const SkISize& size, + const bool needs_readback) { 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 66220f079b3e5..87d6d09320c08 100644 --- a/shell/platform/fuchsia/flutter/surface.h +++ b/shell/platform/fuchsia/flutter/surface.h @@ -29,7 +29,8 @@ class Surface final : public flutter::Surface { // |flutter::Surface| std::unique_ptr AcquireFrame( - const SkISize& size) override; + const SkISize& size, + const bool needs_readback) override; // |flutter::Surface| GrContext* GetContext() override; From 1710b338f2807a2f7df12ad73b29c45effcb1858 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 20 Nov 2019 17:56:29 -0800 Subject: [PATCH 02/11] Switch implementation to layer-based tracking of readbacks. --- flow/layers/backdrop_filter_layer.cc | 4 +++- flow/layers/container_layer.cc | 23 +++++++++++++++++++++-- flow/layers/container_layer.h | 7 +++++++ flow/layers/layer.cc | 20 +++++++++++++++++++- flow/layers/layer.h | 18 ++++++++++++++++++ flow/layers/layer_tree.cc | 3 +-- flow/layers/layer_tree.h | 9 ++++----- lib/ui/compositing/scene.cc | 10 +++------- lib/ui/compositing/scene.h | 6 ++---- lib/ui/compositing/scene_builder.cc | 8 +++----- lib/ui/compositing/scene_builder.h | 1 - shell/common/rasterizer.cc | 2 +- 12 files changed, 82 insertions(+), 29 deletions(-) diff --git a/flow/layers/backdrop_filter_layer.cc b/flow/layers/backdrop_filter_layer.cc index 573db97f191a2..cf6f5cf522b96 100644 --- a/flow/layers/backdrop_filter_layer.cc +++ b/flow/layers/backdrop_filter_layer.cc @@ -7,7 +7,9 @@ namespace flutter { BackdropFilterLayer::BackdropFilterLayer(sk_sp filter) - : filter_(std::move(filter)) {} + : filter_(std::move(filter)) { + set_layer_reads_surface(filter_.get() != nullptr); +} BackdropFilterLayer::~BackdropFilterLayer() = default; diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index d5c6a2a03a34a..7e77f84b4cbc3 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -6,13 +6,32 @@ namespace flutter { -ContainerLayer::ContainerLayer() {} +ContainerLayer::ContainerLayer() + : children_need_screen_readback_(0), renders_to_save_layer_(false) {} ContainerLayer::~ContainerLayer() = default; void ContainerLayer::Add(std::shared_ptr layer) { - layer->set_parent(this); + Layer* the_layer = layer.get(); + the_layer->set_parent(this); layers_.push_back(std::move(layer)); + if (the_layer->tree_reads_surface()) { + update_child_readback(the_layer); + } +} + +void ContainerLayer::update_child_readback(Layer* layer) { + if (layer->tree_reads_surface()) { + ++children_need_screen_readback_; + } else { + --children_need_screen_readback_; + } + update_screen_readback(); +} + +bool ContainerLayer::compute_tree_reads_surface() { + return layer_reads_surface() || + (!renders_to_save_layer_ && children_need_screen_readback_ > 0); } void ContainerLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index ef1c03328d1df..0dc832030095b 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -25,12 +25,16 @@ class ContainerLayer : public Layer { const std::vector>& layers() const { return layers_; } + void update_child_readback(Layer* layer); + protected: void PrerollChildren(PrerollContext* context, const SkMatrix& child_matrix, SkRect* child_paint_bounds); void PaintChildren(PaintContext& context) const; + virtual bool compute_tree_reads_surface() override; + #if defined(OS_FUCHSIA) void UpdateSceneChildren(SceneUpdateContext& context); #endif // defined(OS_FUCHSIA) @@ -41,6 +45,9 @@ class ContainerLayer : public Layer { private: std::vector> layers_; + int children_need_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 b729f582a0a9a..9689224e4a5be 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -3,6 +3,7 @@ // 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" @@ -13,7 +14,9 @@ Layer::Layer() : parent_(nullptr), needs_system_composite_(false), paint_bounds_(SkRect::MakeEmpty()), - unique_id_(NextUniqueID()) {} + unique_id_(NextUniqueID()), + tree_reads_surface_(false), + layer_reads_surface_(false) {} Layer::~Layer() = default; @@ -26,6 +29,21 @@ uint64_t Layer::NextUniqueID() { return id; } +bool Layer::compute_tree_reads_surface() { + return layer_reads_surface_; +} + +void Layer::update_screen_readback() { + bool new_tree_reads_surface = compute_tree_reads_surface(); + + if (tree_reads_surface_ != new_tree_reads_surface) { + tree_reads_surface_ = new_tree_reads_surface; + if (parent_ != nullptr) { + parent_->update_child_readback(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 66944376e8ce8..5df01adabc691 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -145,14 +145,32 @@ class Layer { bool needs_painting() const { return !paint_bounds_.isEmpty(); } + bool tree_reads_surface() { return tree_reads_surface_; } + uint64_t unique_id() const { return unique_id_; } + protected: + virtual bool compute_tree_reads_surface(); + void update_screen_readback(); + + bool layer_reads_surface() { return layer_reads_surface_; } + + void set_layer_reads_surface(bool reads) { + if (layer_reads_surface_ != reads) { + layer_reads_surface_ = reads; + update_screen_readback(); + } + } + 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.cc b/flow/layers/layer_tree.cc index ae84f8da7bda0..b031ebf8cb8bd 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -15,8 +15,7 @@ LayerTree::LayerTree() : frame_size_{}, rasterizer_tracing_threshold_(0), checkerboard_raster_cache_images_(false), - checkerboard_offscreen_layers_(false), - has_backdrop_filter_(false) {} + checkerboard_offscreen_layers_(false) {} LayerTree::~LayerTree() = default; diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index 3e33464dbff24..04be0a9d4ad64 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -43,6 +43,10 @@ class LayerTree { root_layer_ = std::move(root_layer); } + bool root_needs_screen_readback() { + 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; } @@ -71,10 +75,6 @@ class LayerTree { checkerboard_offscreen_layers_ = checkerboard; } - void set_has_backdrop_filter(bool present) { has_backdrop_filter_ = present; } - - bool has_backdrop_filter() { return has_backdrop_filter_; } - void set_device_pixel_ratio(double device_pixel_ratio) { device_pixel_ratio_ = device_pixel_ratio; } @@ -90,7 +90,6 @@ class LayerTree { uint32_t rasterizer_tracing_threshold_; bool checkerboard_raster_cache_images_; bool checkerboard_offscreen_layers_; - bool has_backdrop_filter_; FML_DISALLOW_COPY_AND_ASSIGN(LayerTree); }; diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index 849f2b86b7dd0..fd3e86c7e5176 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -27,25 +27,21 @@ DART_BIND_ALL(Scene, FOR_EACH_BINDING) fml::RefPtr Scene::create(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers, - bool hasBackdropFilter) { + bool checkerboardOffscreenLayers) { return fml::MakeRefCounted( std::move(rootLayer), rasterizerTracingThreshold, - checkerboardRasterCacheImages, checkerboardOffscreenLayers, - hasBackdropFilter); + checkerboardRasterCacheImages, checkerboardOffscreenLayers); } Scene::Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers, - bool hasBackdropFilter) + bool checkerboardOffscreenLayers) : m_layerTree(new flutter::LayerTree()) { m_layerTree->set_root_layer(std::move(rootLayer)); m_layerTree->set_rasterizer_tracing_threshold(rasterizerTracingThreshold); m_layerTree->set_checkerboard_raster_cache_images( checkerboardRasterCacheImages); - m_layerTree->set_has_backdrop_filter(hasBackdropFilter); m_layerTree->set_checkerboard_offscreen_layers(checkerboardOffscreenLayers); } diff --git a/lib/ui/compositing/scene.h b/lib/ui/compositing/scene.h index 95d904e76775d..2d280a534db63 100644 --- a/lib/ui/compositing/scene.h +++ b/lib/ui/compositing/scene.h @@ -27,8 +27,7 @@ class Scene : public RefCountedDartWrappable { static fml::RefPtr create(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers, - bool hasBackdropFilter); + bool checkerboardOffscreenLayers); std::unique_ptr takeLayerTree(); @@ -44,8 +43,7 @@ class Scene : public RefCountedDartWrappable { explicit Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers, - bool hasBackdropFilter); + bool checkerboardOffscreenLayers); std::unique_ptr m_layerTree; }; diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 5167f9f45c707..5b7148803c2c2 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -152,7 +152,6 @@ fml::RefPtr SceneBuilder::pushColorFilter( fml::RefPtr SceneBuilder::pushBackdropFilter(ImageFilter* filter) { auto layer = std::make_shared(filter->filter()); PushLayer(layer); - has_backdrop_filter_ = true; return EngineLayer::MakeRetained(layer); } @@ -291,10 +290,9 @@ void SceneBuilder::setCheckerboardOffscreenLayers(bool checkerboard) { } fml::RefPtr SceneBuilder::build() { - fml::RefPtr scene = - Scene::create(std::move(root_layer_), rasterizer_tracing_threshold_, - checkerboard_raster_cache_images_, - checkerboard_offscreen_layers_, has_backdrop_filter_); + fml::RefPtr scene = Scene::create( + std::move(root_layer_), rasterizer_tracing_threshold_, + checkerboard_raster_cache_images_, checkerboard_offscreen_layers_); ClearDartWrapper(); return scene; } diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 83325ca0cd6bc..d378876d960a2 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -115,7 +115,6 @@ class SceneBuilder : public RefCountedDartWrappable { int rasterizer_tracing_threshold_ = 0; bool checkerboard_raster_cache_images_ = false; bool checkerboard_offscreen_layers_ = false; - bool has_backdrop_filter_ = false; void PushLayer(std::shared_ptr layer); diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index d6e2c29d971c6..75b60577b888d 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -270,7 +270,7 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) { FML_DCHECK(surface_); auto frame = surface_->AcquireFrame(layer_tree.frame_size(), - layer_tree.has_backdrop_filter()); + layer_tree.root_needs_screen_readback()); if (frame == nullptr) { return RasterStatus::kFailed; From 9e2fbbd82c04f8a2f96fcde64590328e4a21f42b Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 20 Nov 2019 20:39:14 -0800 Subject: [PATCH 03/11] Fix Container::ClearChildren() and tag SaveLayer layers. --- 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.h | 15 ++++++++++++++- flow/layers/opacity_layer.cc | 8 +++++++- flow/layers/physical_shape_layer.cc | 1 + flow/layers/shader_mask_layer.cc | 4 +++- 8 files changed, 31 insertions(+), 4 deletions(-) diff --git a/flow/layers/clip_path_layer.cc b/flow/layers/clip_path_layer.cc index d08c19b34eeb9..0c63b2331d9f5 100644 --- a/flow/layers/clip_path_layer.cc +++ b/flow/layers/clip_path_layer.cc @@ -15,6 +15,7 @@ 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 de7590624e408..5ee6046b19a73 100644 --- a/flow/layers/clip_rect_layer.cc +++ b/flow/layers/clip_rect_layer.cc @@ -9,6 +9,7 @@ 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 9899a1658732d..037d91ddcb5ff 100644 --- a/flow/layers/clip_rrect_layer.cc +++ b/flow/layers/clip_rrect_layer.cc @@ -9,6 +9,7 @@ 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 f838b0612b2e5..6f355f4419ab4 100644 --- a/flow/layers/color_filter_layer.cc +++ b/flow/layers/color_filter_layer.cc @@ -7,7 +7,9 @@ namespace flutter { ColorFilterLayer::ColorFilterLayer(sk_sp filter) - : filter_(std::move(filter)) {} + : filter_(std::move(filter)) { + set_renders_to_save_layer(true); +} ColorFilterLayer::~ColorFilterLayer() = default; diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index 0dc832030095b..1cc64d75db115 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -39,8 +39,21 @@ class ContainerLayer : public Layer { void UpdateSceneChildren(SceneUpdateContext& context); #endif // defined(OS_FUCHSIA) + void set_renders_to_save_layer(bool protects) { + if (renders_to_save_layer_ != protects) { + renders_to_save_layer_ = protects; + update_screen_readback(); + } + } + // For OpacityLayer to restructure to have a single child. - void ClearChildren() { layers_.clear(); } + void ClearChildren() { + layers_.clear(); + if (children_need_screen_readback_ > 0) { + children_need_screen_readback_ = 0; + update_screen_readback(); + } + } private: std::vector> layers_; diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 6257700ffbddf..54f8e19d76eea 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -9,7 +9,13 @@ namespace flutter { OpacityLayer::OpacityLayer(int alpha, const SkPoint& offset) - : alpha_(alpha), offset_(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); +} OpacityLayer::~OpacityLayer() = default; diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 0a607a88c23b0..428e9fb8e4b2b 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -46,6 +46,7 @@ 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 36e7b7332aeae..064e0e1b2ad8b 100644 --- a/flow/layers/shader_mask_layer.cc +++ b/flow/layers/shader_mask_layer.cc @@ -9,7 +9,9 @@ namespace flutter { ShaderMaskLayer::ShaderMaskLayer(sk_sp shader, const SkRect& mask_rect, SkBlendMode blend_mode) - : shader_(shader), mask_rect_(mask_rect), blend_mode_(blend_mode) {} + : shader_(shader), mask_rect_(mask_rect), blend_mode_(blend_mode) { + set_renders_to_save_layer(true); +} ShaderMaskLayer::~ShaderMaskLayer() = default; From 3f27e321695a1f5ab79523772e6d90092d8fbd89 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 21 Nov 2019 23:12:57 -0800 Subject: [PATCH 04/11] Convert ContainerLayer child flag from counter to boolean. Document new fields and methods. --- flow/layers/container_layer.cc | 20 +++++++++++++++----- flow/layers/container_layer.h | 18 +++++++++++++++--- flow/layers/layer.h | 14 ++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 7e77f84b4cbc3..c8aa84fe9c6a9 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -7,7 +7,7 @@ namespace flutter { ContainerLayer::ContainerLayer() - : children_need_screen_readback_(0), renders_to_save_layer_(false) {} + : child_needs_screen_readback_(false), renders_to_save_layer_(false) {} ContainerLayer::~ContainerLayer() = default; @@ -21,17 +21,27 @@ void ContainerLayer::Add(std::shared_ptr layer) { } void ContainerLayer::update_child_readback(Layer* layer) { - if (layer->tree_reads_surface()) { - ++children_need_screen_readback_; + if (child_needs_screen_readback_ == layer->tree_reads_surface()) { + return; + } + if (child_needs_screen_readback_) { + // Transitioning to false, but only if there are no other children + // that read from the surface. + for (auto& child : layers_) { + if (child->tree_reads_surface()) { + return; + } + } + child_needs_screen_readback_ = false; } else { - --children_need_screen_readback_; + child_needs_screen_readback_ = true; } update_screen_readback(); } bool ContainerLayer::compute_tree_reads_surface() { return layer_reads_surface() || - (!renders_to_save_layer_ && children_need_screen_readback_ > 0); + (!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 1cc64d75db115..b6627dd475560 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -25,6 +25,8 @@ 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. void update_child_readback(Layer* layer); protected: @@ -39,6 +41,13 @@ class ContainerLayer : public Layer { 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 protects) { if (renders_to_save_layer_ != protects) { renders_to_save_layer_ = protects; @@ -49,8 +58,8 @@ class ContainerLayer : public Layer { // For OpacityLayer to restructure to have a single child. void ClearChildren() { layers_.clear(); - if (children_need_screen_readback_ > 0) { - children_need_screen_readback_ = 0; + if (child_needs_screen_readback_) { + child_needs_screen_readback_ = false; update_screen_readback(); } } @@ -58,7 +67,10 @@ class ContainerLayer : public Layer { private: std::vector> layers_; - int children_need_screen_readback_; + // 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.h b/flow/layers/layer.h index 5df01adabc691..ab44f419bede1 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -145,14 +145,28 @@ 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. This value has no setter as it is computed + // from other flags and properties on the layer. + // see Layer::update_screen_readback() + // see Layer::set_layer_reads_surface() + // see ContainerLayer::set_renders_to_save_layer() + // see ContainerLayer::update_child_readback() bool tree_reads_surface() { return tree_reads_surface_; } uint64_t unique_id() const { return unique_id_; } protected: virtual bool compute_tree_reads_surface(); + + // Compute a new value for tree_reads_surface_ and propagate it to + // ancestors if it changes. void update_screen_readback(); + // True iff the layer itself (not a child or other descendant) performs + // an operation which must read back from the surface on which it is + // rendered. bool layer_reads_surface() { return layer_reads_surface_; } void set_layer_reads_surface(bool reads) { From 88e90fd13ae738823c624289c16ae6122f1d8bb1 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 22 Nov 2019 11:49:17 -0800 Subject: [PATCH 05/11] Fix some function naming style issues. --- flow/layers/container_layer.cc | 8 ++++---- flow/layers/container_layer.h | 8 ++++---- flow/layers/layer.cc | 8 ++++---- flow/layers/layer.h | 22 +++++++++++++--------- 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index c8aa84fe9c6a9..ddbf68bba0af4 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -16,11 +16,11 @@ void ContainerLayer::Add(std::shared_ptr layer) { the_layer->set_parent(this); layers_.push_back(std::move(layer)); if (the_layer->tree_reads_surface()) { - update_child_readback(the_layer); + UpdateChildReadback(the_layer); } } -void ContainerLayer::update_child_readback(Layer* layer) { +void ContainerLayer::UpdateChildReadback(Layer* layer) { if (child_needs_screen_readback_ == layer->tree_reads_surface()) { return; } @@ -36,10 +36,10 @@ void ContainerLayer::update_child_readback(Layer* layer) { } else { child_needs_screen_readback_ = true; } - update_screen_readback(); + UpdateTreeReadsSurface(); } -bool ContainerLayer::compute_tree_reads_surface() { +bool ContainerLayer::ComputeTreeReadsSurface() { return layer_reads_surface() || (!renders_to_save_layer_ && child_needs_screen_readback_); } diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index b6627dd475560..12f60e33b82bd 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -27,7 +27,7 @@ class ContainerLayer : public Layer { // Called when the layer, which must be a child of this container, // changes its tree_reads_surface() result. - void update_child_readback(Layer* layer); + void UpdateChildReadback(Layer* layer); protected: void PrerollChildren(PrerollContext* context, @@ -35,7 +35,7 @@ class ContainerLayer : public Layer { SkRect* child_paint_bounds); void PaintChildren(PaintContext& context) const; - virtual bool compute_tree_reads_surface() override; + virtual bool ComputeTreeReadsSurface() override; #if defined(OS_FUCHSIA) void UpdateSceneChildren(SceneUpdateContext& context); @@ -51,7 +51,7 @@ class ContainerLayer : public Layer { void set_renders_to_save_layer(bool protects) { if (renders_to_save_layer_ != protects) { renders_to_save_layer_ = protects; - update_screen_readback(); + UpdateTreeReadsSurface(); } } @@ -60,7 +60,7 @@ class ContainerLayer : public Layer { layers_.clear(); if (child_needs_screen_readback_) { child_needs_screen_readback_ = false; - update_screen_readback(); + UpdateTreeReadsSurface(); } } diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index 9689224e4a5be..54562265cbef3 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -29,17 +29,17 @@ uint64_t Layer::NextUniqueID() { return id; } -bool Layer::compute_tree_reads_surface() { +bool Layer::ComputeTreeReadsSurface() { return layer_reads_surface_; } -void Layer::update_screen_readback() { - bool new_tree_reads_surface = compute_tree_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_->update_child_readback(this); + parent_->UpdateChildReadback(this); } } } diff --git a/flow/layers/layer.h b/flow/layers/layer.h index ab44f419bede1..9c667cd319704 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -147,22 +147,26 @@ class Layer { // 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. This value has no setter as it is computed - // from other flags and properties on the layer. - // see Layer::update_screen_readback() + // 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. + // + // see Layer::UpdateTreeReadsSurface() // see Layer::set_layer_reads_surface() // see ContainerLayer::set_renders_to_save_layer() - // see ContainerLayer::update_child_readback() + // see ContainerLayer::UpdateChildReadback() bool tree_reads_surface() { return tree_reads_surface_; } uint64_t unique_id() const { return unique_id_; } protected: - virtual bool compute_tree_reads_surface(); + // Compute a new value for tree_reads_surface_ from all of the various + // properties of this layer. + virtual bool ComputeTreeReadsSurface(); - // Compute a new value for tree_reads_surface_ and propagate it to - // ancestors if it changes. - void update_screen_readback(); + // Update the tree_reads_surface_ value and propagate changes to + // ancestors if needed. + void UpdateTreeReadsSurface(); // True iff the layer itself (not a child or other descendant) performs // an operation which must read back from the surface on which it is @@ -172,7 +176,7 @@ class Layer { void set_layer_reads_surface(bool reads) { if (layer_reads_surface_ != reads) { layer_reads_surface_ = reads; - update_screen_readback(); + UpdateTreeReadsSurface(); } } From eff72180c3312e290c01f8f2756142cbc994e3d9 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 25 Nov 2019 12:14:06 -0800 Subject: [PATCH 06/11] More aggressive out-of-lining. --- flow/layers/container_layer.cc | 19 +++++++++++++++++-- flow/layers/container_layer.h | 19 ++++--------------- flow/layers/layer.cc | 9 ++++++++- flow/layers/layer.h | 20 ++++++++------------ flow/layers/layer_tree.h | 2 +- 5 files changed, 38 insertions(+), 31 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index ddbf68bba0af4..c26acbd36ce41 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -20,7 +20,22 @@ void ContainerLayer::Add(std::shared_ptr layer) { } } -void ContainerLayer::UpdateChildReadback(Layer* 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 protects) { + if (renders_to_save_layer_ != protects) { + renders_to_save_layer_ = protects; + UpdateTreeReadsSurface(); + } +} + +void ContainerLayer::UpdateChildReadback(const Layer* layer) { if (child_needs_screen_readback_ == layer->tree_reads_surface()) { return; } @@ -39,7 +54,7 @@ void ContainerLayer::UpdateChildReadback(Layer* layer) { UpdateTreeReadsSurface(); } -bool ContainerLayer::ComputeTreeReadsSurface() { +bool ContainerLayer::ComputeTreeReadsSurface() const { return layer_reads_surface() || (!renders_to_save_layer_ && child_needs_screen_readback_); } diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index 12f60e33b82bd..62d749683c22b 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -27,7 +27,7 @@ class ContainerLayer : public Layer { // Called when the layer, which must be a child of this container, // changes its tree_reads_surface() result. - void UpdateChildReadback(Layer* layer); + void UpdateChildReadback(const Layer* layer); protected: void PrerollChildren(PrerollContext* context, @@ -35,7 +35,7 @@ class ContainerLayer : public Layer { SkRect* child_paint_bounds); void PaintChildren(PaintContext& context) const; - virtual bool ComputeTreeReadsSurface() override; + virtual bool ComputeTreeReadsSurface() const override; #if defined(OS_FUCHSIA) void UpdateSceneChildren(SceneUpdateContext& context); @@ -48,21 +48,10 @@ class ContainerLayer : public Layer { // 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 protects) { - if (renders_to_save_layer_ != protects) { - renders_to_save_layer_ = protects; - UpdateTreeReadsSurface(); - } - } + void set_renders_to_save_layer(bool protects); // For OpacityLayer to restructure to have a single child. - void ClearChildren() { - layers_.clear(); - if (child_needs_screen_readback_) { - child_needs_screen_readback_ = false; - UpdateTreeReadsSurface(); - } - } + void ClearChildren(); private: std::vector> layers_; diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index 54562265cbef3..4b6e4d93ddea7 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -29,7 +29,14 @@ uint64_t Layer::NextUniqueID() { return id; } -bool Layer::ComputeTreeReadsSurface() { +void Layer::set_layer_reads_surface(bool reads) { + if (layer_reads_surface_ != reads) { + layer_reads_surface_ = reads; + UpdateTreeReadsSurface(); + } +} + +bool Layer::ComputeTreeReadsSurface() const { return layer_reads_surface_; } diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 9c667cd319704..3d51cec2be62a 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -155,30 +155,26 @@ class Layer { // see Layer::set_layer_reads_surface() // see ContainerLayer::set_renders_to_save_layer() // see ContainerLayer::UpdateChildReadback() - bool tree_reads_surface() { return tree_reads_surface_; } + 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. - virtual bool ComputeTreeReadsSurface(); + // 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 must read back from the surface on which it is - // rendered. - bool layer_reads_surface() { return layer_reads_surface_; } - - void set_layer_reads_surface(bool reads) { - if (layer_reads_surface_ != reads) { - layer_reads_surface_ = reads; - UpdateTreeReadsSurface(); - } - } + // 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 reads); private: ContainerLayer* parent_; diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index 04be0a9d4ad64..99908740eccc3 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -43,7 +43,7 @@ class LayerTree { root_layer_ = std::move(root_layer); } - bool root_needs_screen_readback() { + bool root_needs_screen_readback() const { return root_layer_ && root_layer_->tree_reads_surface(); } From 2fd78cc113b42ad04abe0205e238f7a0c55adf50 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 25 Nov 2019 17:45:52 -0800 Subject: [PATCH 07/11] Unit tests for Layer readback flags. --- flow/BUILD.gn | 1 + flow/layers/layer_unittests.cc | 172 +++++++++++++++++++++++++++++++++ testing/run_tests.py | 6 +- 3 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 flow/layers/layer_unittests.cc diff --git a/flow/BUILD.gn b/flow/BUILD.gn index c2f0c98415a3e..c99f2177e772f 100644 --- a/flow/BUILD.gn +++ b/flow/BUILD.gn @@ -109,6 +109,7 @@ 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/layer_unittests.cc b/flow/layers/layer_unittests.cc new file mode 100644 index 0000000000000..e42dcd0332097 --- /dev/null +++ b/flow/layers/layer_unittests.cc @@ -0,0 +1,172 @@ +// Copyright 2019 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/container_layer.h" +#include "flutter/flow/layers/clip_rect_layer.h" +#include "flutter/flow/layers/clip_rrect_layer.h" +#include "flutter/flow/layers/clip_path_layer.h" +#include "flutter/flow/layers/color_filter_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 "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, ChildChangesReadback) { + 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_reads = ReadbackLayer::Make(true, false); + parent.Add(child_reads); + EXPECT_TRUE(parent.tree_reads_surface()); + child_reads->set_read(false); + EXPECT_FALSE(parent.tree_reads_surface()); + child_reads->set_read(true); + EXPECT_TRUE(parent.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()); +} + +} diff --git a/testing/run_tests.py b/testing/run_tests.py index f32e86398d247..0bb6a88d5b8d5 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -24,7 +24,11 @@ fml_unittests_filter = '--gtest_filter=-*TimeSensitiveTest*:*GpuThreadMerger*' def RunCmd(cmd, **kwargs): - print(subprocess.check_output(cmd, **kwargs)) + try: + print(subprocess.check_output(cmd, **kwargs)) + except subprocess.CalledProcessError as cpe: + print(cpe.output) + raise cpe def IsMac(): return sys.platform == 'darwin' From 794cab4358a7ca4d38c8df350ffdd445e7c72768 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 25 Nov 2019 17:55:38 -0800 Subject: [PATCH 08/11] Fix formatting and revert unintended change to run_tests.py --- flow/layers/layer_unittests.cc | 22 ++++++++++------------ testing/run_tests.py | 6 +----- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/flow/layers/layer_unittests.cc b/flow/layers/layer_unittests.cc index e42dcd0332097..651c9dbc6ae3a 100644 --- a/flow/layers/layer_unittests.cc +++ b/flow/layers/layer_unittests.cc @@ -2,11 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/flow/layers/container_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/clip_path_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" @@ -90,7 +90,7 @@ void TestClipRect(Clip clip_behavior, bool ret) { } TEST(Layer, ClipRectSaveLayer) { -// TestClipRect(Clip::none, true); // ClipRectLayer asserts !Clip::none + // TestClipRect(Clip::none, true); // ClipRectLayer asserts !Clip::none TestClipRect(Clip::hardEdge, true); TestClipRect(Clip::antiAlias, true); TestClipRect(Clip::antiAliasWithSaveLayer, false); @@ -104,7 +104,7 @@ void TestClipRRect(Clip clip_behavior, bool ret) { } TEST(Layer, ClipRRectSaveLayer) { -// TestClipRRect(Clip::none, true); // ClipRRectLayer asserts !Clip::none + // TestClipRRect(Clip::none, true); // ClipRRectLayer asserts !Clip::none TestClipRRect(Clip::hardEdge, true); TestClipRRect(Clip::antiAlias, true); TestClipRRect(Clip::antiAliasWithSaveLayer, false); @@ -122,7 +122,7 @@ void TestClipPath(Clip clip_behavior, bool ret) { } TEST(Layer, ClipPathSaveLayer) { -// TestClipPath(Clip::none, true); // ClipRRectLayer asserts !Clip::none + // TestClipPath(Clip::none, true); // ClipRRectLayer asserts !Clip::none TestClipPath(Clip::hardEdge, true); TestClipPath(Clip::antiAlias, true); TestClipPath(Clip::antiAliasWithSaveLayer, false); @@ -147,9 +147,8 @@ void TestPhysicalShapeLayer(Clip clip_behavior, bool ret) { 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); + 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); } @@ -162,11 +161,10 @@ TEST(Layer, PhysicalShapeSaveLayer) { } TEST(Layer, ShaderMaskSaveLayer) { - ShaderMaskLayer layer = ShaderMaskLayer(SkShaders::Empty(), - SkRect::MakeWH(5, 5), - SkBlendMode::kSrcOver); + 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/testing/run_tests.py b/testing/run_tests.py index 0bb6a88d5b8d5..f32e86398d247 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -24,11 +24,7 @@ fml_unittests_filter = '--gtest_filter=-*TimeSensitiveTest*:*GpuThreadMerger*' def RunCmd(cmd, **kwargs): - try: - print(subprocess.check_output(cmd, **kwargs)) - except subprocess.CalledProcessError as cpe: - print(cpe.output) - raise cpe + print(subprocess.check_output(cmd, **kwargs)) def IsMac(): return sys.platform == 'darwin' From 1f6de5d2de626d4b0ab6ebfb79c46fbfee9c2bd2 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 26 Nov 2019 15:48:24 -0800 Subject: [PATCH 09/11] Simplified handling of child flags per review feedback. Added a BackdropFilter flag test in the unit tests. --- flow/layers/container_layer.cc | 25 +++++++------------------ flow/layers/container_layer.h | 6 +++--- flow/layers/layer.cc | 8 ++++---- flow/layers/layer.h | 15 ++++++++------- flow/layers/layer_unittests.cc | 30 ++++++++++++++++++++++++------ 5 files changed, 46 insertions(+), 38 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index c26acbd36ce41..1fa7056c5454f 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -16,7 +16,7 @@ void ContainerLayer::Add(std::shared_ptr layer) { the_layer->set_parent(this); layers_.push_back(std::move(layer)); if (the_layer->tree_reads_surface()) { - UpdateChildReadback(the_layer); + NotifyChildReadback(the_layer); } } @@ -28,29 +28,18 @@ void ContainerLayer::ClearChildren() { } } -void ContainerLayer::set_renders_to_save_layer(bool protects) { - if (renders_to_save_layer_ != protects) { - renders_to_save_layer_ = protects; +void ContainerLayer::set_renders_to_save_layer(bool value) { + if (renders_to_save_layer_ != value) { + renders_to_save_layer_ = value; UpdateTreeReadsSurface(); } } -void ContainerLayer::UpdateChildReadback(const Layer* layer) { - if (child_needs_screen_readback_ == layer->tree_reads_surface()) { +void ContainerLayer::NotifyChildReadback(const Layer* layer) { + if (child_needs_screen_readback_ || !layer->tree_reads_surface()) { return; } - if (child_needs_screen_readback_) { - // Transitioning to false, but only if there are no other children - // that read from the surface. - for (auto& child : layers_) { - if (child->tree_reads_surface()) { - return; - } - } - child_needs_screen_readback_ = false; - } else { - child_needs_screen_readback_ = true; - } + child_needs_screen_readback_ = true; UpdateTreeReadsSurface(); } diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index 62d749683c22b..1e37f0164cf40 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -26,8 +26,8 @@ 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. - void UpdateChildReadback(const Layer* layer); + // changes its tree_reads_surface() result from false to true. + void NotifyChildReadback(const Layer* layer); protected: void PrerollChildren(PrerollContext* context, @@ -48,7 +48,7 @@ class ContainerLayer : public Layer { // 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 protects); + void set_renders_to_save_layer(bool value); // For OpacityLayer to restructure to have a single child. void ClearChildren(); diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index 4b6e4d93ddea7..052b70fac9827 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -29,9 +29,9 @@ uint64_t Layer::NextUniqueID() { return id; } -void Layer::set_layer_reads_surface(bool reads) { - if (layer_reads_surface_ != reads) { - layer_reads_surface_ = reads; +void Layer::set_layer_reads_surface(bool value) { + if (layer_reads_surface_ != value) { + layer_reads_surface_ = value; UpdateTreeReadsSurface(); } } @@ -46,7 +46,7 @@ void Layer::UpdateTreeReadsSurface() { if (tree_reads_surface_ != new_tree_reads_surface) { tree_reads_surface_ = new_tree_reads_surface; if (parent_ != nullptr) { - parent_->UpdateChildReadback(this); + parent_->NotifyChildReadback(this); } } } diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 3d51cec2be62a..ef114b4c381c3 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -150,11 +150,12 @@ class Layer { // 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 Layer::UpdateTreeReadsSurface() - // see Layer::set_layer_reads_surface() - // see ContainerLayer::set_renders_to_save_layer() - // see ContainerLayer::UpdateChildReadback() + // 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_; } @@ -162,19 +163,19 @@ class Layer { protected: // Compute a new value for tree_reads_surface_ from all of the various // properties of this layer. - // Used by UpdateTreeReadsSurface() + // Used by |UpdateTreeReadsSurface| virtual bool ComputeTreeReadsSurface() const; // Update the tree_reads_surface_ value and propagate changes to // ancestors if needed. - // Uses ComputeTreeReadsSurface() + // 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 reads); + void set_layer_reads_surface(bool value); private: ContainerLayer* parent_; diff --git a/flow/layers/layer_unittests.cc b/flow/layers/layer_unittests.cc index 651c9dbc6ae3a..360dba5acd5dc 100644 --- a/flow/layers/layer_unittests.cc +++ b/flow/layers/layer_unittests.cc @@ -1,7 +1,8 @@ -// Copyright 2019 The Flutter Authors. All rights reserved. +// 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" @@ -11,6 +12,8 @@ #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 { @@ -69,20 +72,35 @@ TEST(Layer, ReadbackButSaveLayer) { TestChildFlag(true, true, false); } -TEST(Layer, ChildChangesReadback) { +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()); - std::shared_ptr child_reads = ReadbackLayer::Make(true, false); - parent.Add(child_reads); + parent.Add(ReadbackLayer::Make(true, false)); EXPECT_TRUE(parent.tree_reads_surface()); - child_reads->set_read(false); +} + +TEST(Layer, ChildChangesToReadback) { + ReadbackLayer parent = ReadbackLayer(false, false); EXPECT_FALSE(parent.tree_reads_surface()); - child_reads->set_read(true); + 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)); From 8e86cbf57b014161f130079033e8d640a4baa81d Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 26 Nov 2019 16:05:53 -0800 Subject: [PATCH 10/11] Add license file suggested by Cirrus. --- ci/licenses_golden/licenses_flutter | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 6df05a1cc4085..6673a27d5672f 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -44,6 +44,7 @@ FILE: ../../../flutter/flow/layers/container_layer.cc FILE: ../../../flutter/flow/layers/container_layer.h FILE: ../../../flutter/flow/layers/layer.cc FILE: ../../../flutter/flow/layers/layer.h +FILE: ../../../flutter/flow/layers/layer_unittests.cc FILE: ../../../flutter/flow/layers/layer_tree.cc FILE: ../../../flutter/flow/layers/layer_tree.h FILE: ../../../flutter/flow/layers/opacity_layer.cc From da3939a9e6ff07b787e69b20fb9bb5d8100e99be Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 26 Nov 2019 16:44:56 -0800 Subject: [PATCH 11/11] Fix license file ordering. --- ci/licenses_golden/licenses_flutter | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 6673a27d5672f..335e525d9b195 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -44,9 +44,9 @@ FILE: ../../../flutter/flow/layers/container_layer.cc FILE: ../../../flutter/flow/layers/container_layer.h FILE: ../../../flutter/flow/layers/layer.cc FILE: ../../../flutter/flow/layers/layer.h -FILE: ../../../flutter/flow/layers/layer_unittests.cc 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