From 28cc748c00687d1a18b731c753c7d56c95e38dda Mon Sep 17 00:00:00 2001 From: Yuqian Li Date: Thu, 4 Oct 2018 09:20:31 -0700 Subject: [PATCH 1/2] Revert "Revert "Share engine layers with the framework" (#6412)" This reverts commit 74662ab695238af0e7402f41c4bf9ad862bd37d2. --- ci/licenses_golden/licenses_flutter | 66 ++++++++++++++++++++++++++++- flow/layers/container_layer.cc | 2 +- flow/layers/container_layer.h | 6 +-- flow/layers/layer_tree.h | 4 +- lib/ui/BUILD.gn | 2 + lib/ui/compositing.dart | 16 +++++-- lib/ui/compositing/scene.cc | 4 +- lib/ui/compositing/scene.h | 4 +- lib/ui/compositing/scene_builder.cc | 32 +++++++++----- lib/ui/compositing/scene_builder.h | 19 +++++---- lib/ui/dart_ui.cc | 2 + lib/ui/painting.dart | 7 +++ lib/ui/painting/engine_layer.cc | 26 ++++++++++++ lib/ui/painting/engine_layer.h | 44 +++++++++++++++++++ 14 files changed, 200 insertions(+), 34 deletions(-) create mode 100644 lib/ui/painting/engine_layer.cc create mode 100644 lib/ui/painting/engine_layer.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 09f307de62a57..a75501cd4db90 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -470,7 +470,71 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ==================================================================================================== LIBRARY: engine -ORIGIN: ../../../flutter/benchmarking/benchmarking.cc + ../../../LICENSE +ORIGIN: ../../../flutter/flutter_kernel_transformers/lib/track_widget_constructor_locations.dart + ../../../LICENSE +TYPE: LicenseType.bsd +FILE: ../../../flutter/flutter_kernel_transformers/lib/track_widget_constructor_locations.dart +FILE: ../../../flutter/fml/paths_unittests.cc +FILE: ../../../flutter/lib/ui/isolate_name_server.dart +FILE: ../../../flutter/lib/ui/painting/engine_layer.cc +FILE: ../../../flutter/lib/ui/painting/engine_layer.h +FILE: ../../../flutter/lib/ui/painting/image_encoding.cc +FILE: ../../../flutter/lib/ui/painting/image_encoding.h +FILE: ../../../flutter/lib/ui/plugins.dart +FILE: ../../../flutter/lib/ui/semantics/custom_accessibility_action.cc +FILE: ../../../flutter/lib/ui/semantics/custom_accessibility_action.h +FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.cc +FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.h +FILE: ../../../flutter/shell/platform/android/apk_asset_provider.h +FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformView.java +FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewFactory.java +FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewRegistry.java +FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewRegistryImpl.java +FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java +FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/SingleViewPresentation.java +FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/VirtualDisplayController.java +FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterCallbackInformation.java +FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterRunArguments.java +FILE: ../../../flutter/shell/platform/darwin/ios/framework/Headers/FlutterCallbackCache.h +FILE: ../../../flutter/shell/platform/darwin/ios/framework/Headers/FlutterPluginAppLifeCycleDelegate.h +FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterCallbackCache.mm +FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterCallbackCache_Internal.h +FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPluginAppLifeCycleDelegate.mm +FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.h +FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.mm +FILE: ../../../flutter/shell/platform/darwin/ios/headless_platform_view_ios.h +---------------------------------------------------------------------------------------------------- +Copyright 2018 The Chromium Authors. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Google Inc. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +==================================================================================================== + +==================================================================================================== +LIBRARY: engine +ORIGIN: ../../../flutter/fml/platform/android/paths_android.h + ../../../LICENSE TYPE: LicenseType.bsd FILE: ../../../flutter/benchmarking/benchmarking.cc FILE: ../../../flutter/benchmarking/benchmarking.h diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index f398c96ad1afe..01ced5fd6a8aa 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -10,7 +10,7 @@ ContainerLayer::ContainerLayer() {} ContainerLayer::~ContainerLayer() = default; -void ContainerLayer::Add(std::unique_ptr layer) { +void ContainerLayer::Add(std::shared_ptr layer) { layer->set_parent(this); layers_.push_back(std::move(layer)); } diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index 0da0a7185963e..aa73bb9d59c4a 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -15,7 +15,7 @@ class ContainerLayer : public Layer { ContainerLayer(); ~ContainerLayer() override; - void Add(std::unique_ptr layer); + void Add(std::shared_ptr layer); void Preroll(PrerollContext* context, const SkMatrix& matrix) override; @@ -23,7 +23,7 @@ class ContainerLayer : public Layer { void UpdateScene(SceneUpdateContext& context) override; #endif // defined(OS_FUCHSIA) - const std::vector>& layers() const { return layers_; } + const std::vector>& layers() const { return layers_; } protected: void PrerollChildren(PrerollContext* context, @@ -36,7 +36,7 @@ class ContainerLayer : public Layer { #endif // defined(OS_FUCHSIA) private: - std::vector> layers_; + std::vector> layers_; FML_DISALLOW_COPY_AND_ASSIGN(ContainerLayer); }; diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index bae883ca207be..93d4e27cde2d5 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -38,7 +38,7 @@ class LayerTree { Layer* root_layer() const { return root_layer_.get(); } - void set_root_layer(std::unique_ptr root_layer) { + void set_root_layer(std::shared_ptr root_layer) { root_layer_ = std::move(root_layer); } @@ -73,7 +73,7 @@ class LayerTree { private: SkISize frame_size_; // Physical pixels. - std::unique_ptr root_layer_; + std::shared_ptr root_layer_; fml::TimeDelta construction_time_; uint32_t rasterizer_tracing_threshold_; bool checkerboard_raster_cache_images_; diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index cfb8461b19615..28791a35af619 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -23,6 +23,8 @@ source_set("ui") { "painting/canvas.h", "painting/codec.cc", "painting/codec.h", + "painting/engine_layer.h", + "painting/engine_layer.cc", "painting/frame_info.cc", "painting/frame_info.h", "painting/gradient.cc", diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 1e2af68025d6e..a48b33447e023 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -69,7 +69,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// This is equivalent to [pushTransform] with a matrix with only translation. /// /// See [pop] for details about the operation stack. - void pushOffset(double dx, double dy) native 'SceneBuilder_pushOffset'; + EngineLayer pushOffset(double dx, double dy) native 'SceneBuilder_pushOffset'; /// Pushes a rectangular clip operation onto the operation stack. /// @@ -177,10 +177,10 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. // ignore: deprecated_member_use - void pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = defaultClipBehavior}) { - _pushPhysicalShape(path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index); + EngineLayer pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = defaultClipBehavior}) { + return _pushPhysicalShape(path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index); } - void _pushPhysicalShape(Path path, double elevation, int color, int shadowColor, int clipBehavior) native + EngineLayer _pushPhysicalShape(Path path, double elevation, int color, int shadowColor, int clipBehavior) native 'SceneBuilder_pushPhysicalShape'; /// Ends the effect of the most recently pushed operation. @@ -191,6 +191,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// stack. void pop() native 'SceneBuilder_pop'; + /// Add a retained engine layer subtree from previous frames. + /// + /// All the engine layers that are in the subtree of the retained layer will + /// be automatically appended to the current engine layer tree. Therefore, + /// once this is called, there's no need to call [addToScene] for its children + /// layers. + EngineLayer addRetained(EngineLayer retainedLayer) native 'SceneBuilder_addRetained'; + /// Adds an object to the scene that displays performance statistics. /// /// Useful during development to assess the performance of the application. diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index 2863f5307ad40..735ce44197921 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -26,7 +26,7 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Scene); DART_BIND_ALL(Scene, FOR_EACH_BINDING) -fml::RefPtr Scene::create(std::unique_ptr rootLayer, +fml::RefPtr Scene::create(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, bool checkerboardOffscreenLayers) { @@ -35,7 +35,7 @@ fml::RefPtr Scene::create(std::unique_ptr rootLayer, checkerboardRasterCacheImages, checkerboardOffscreenLayers); } -Scene::Scene(std::unique_ptr rootLayer, +Scene::Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, bool checkerboardOffscreenLayers) diff --git a/lib/ui/compositing/scene.h b/lib/ui/compositing/scene.h index 09f6a3e0d8330..bf1f1b78d826e 100644 --- a/lib/ui/compositing/scene.h +++ b/lib/ui/compositing/scene.h @@ -24,7 +24,7 @@ class Scene : public RefCountedDartWrappable { public: ~Scene() override; - static fml::RefPtr create(std::unique_ptr rootLayer, + static fml::RefPtr create(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, bool checkerboardOffscreenLayers); @@ -40,7 +40,7 @@ class Scene : public RefCountedDartWrappable { static void RegisterNatives(tonic::DartLibraryNatives* natives); private: - explicit Scene(std::unique_ptr rootLayer, + explicit Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, bool checkerboardOffscreenLayers); diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 439821cf1491e..98888d77ad1fd 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -53,6 +53,7 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, SceneBuilder); V(SceneBuilder, pushShaderMask) \ V(SceneBuilder, pushPhysicalShape) \ V(SceneBuilder, pop) \ + V(SceneBuilder, addRetained) \ V(SceneBuilder, addPicture) \ V(SceneBuilder, addTexture) \ V(SceneBuilder, addChildScene) \ @@ -80,11 +81,12 @@ void SceneBuilder::pushTransform(const tonic::Float64List& matrix4) { PushLayer(std::move(layer)); } -void SceneBuilder::pushOffset(double dx, double dy) { +fml::RefPtr SceneBuilder::pushOffset(double dx, double dy) { SkMatrix sk_matrix = SkMatrix::MakeTrans(dx, dy); - auto layer = std::make_unique(); + auto layer = std::make_shared(); layer->set_transform(sk_matrix); - PushLayer(std::move(layer)); + PushLayer(layer); + return EngineLayer::MakeRetained(layer); } void SceneBuilder::pushClipRect(double left, @@ -148,21 +150,29 @@ void SceneBuilder::pushShaderMask(Shader* shader, PushLayer(std::move(layer)); } -void SceneBuilder::pushPhysicalShape(const CanvasPath* path, - double elevation, - int color, - int shadow_color, - int clipBehavior) { +fml::RefPtr SceneBuilder::pushPhysicalShape(const CanvasPath* path, + double elevation, + int color, + int shadow_color, + int clipBehavior) { const SkPath& sk_path = path->path(); flow::Clip clip_behavior = static_cast(clipBehavior); - auto layer = std::make_unique(clip_behavior); + auto layer = std::make_shared(clip_behavior); layer->set_path(sk_path); layer->set_elevation(elevation); layer->set_color(static_cast(color)); layer->set_shadow_color(static_cast(shadow_color)); layer->set_device_pixel_ratio( UIDartState::Current()->window()->viewport_metrics().device_pixel_ratio); - PushLayer(std::move(layer)); + PushLayer(layer); + return EngineLayer::MakeRetained(layer); +} + +void SceneBuilder::addRetained(fml::RefPtr retainedLayer) { + if (!current_layer_) { + return; + } + current_layer_->Add(retainedLayer->Layer()); } void SceneBuilder::pop() { @@ -260,7 +270,7 @@ fml::RefPtr SceneBuilder::build() { return scene; } -void SceneBuilder::PushLayer(std::unique_ptr layer) { +void SceneBuilder::PushLayer(std::shared_ptr layer) { FML_DCHECK(layer); if (!root_layer_) { diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 8e3ed13a411a3..e8638e6aee3bf 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -12,6 +12,7 @@ #include "flutter/lib/ui/compositing/scene.h" #include "flutter/lib/ui/compositing/scene_host.h" #include "flutter/lib/ui/dart_wrapper.h" +#include "flutter/lib/ui/painting/engine_layer.h" #include "flutter/lib/ui/painting/image_filter.h" #include "flutter/lib/ui/painting/path.h" #include "flutter/lib/ui/painting/picture.h" @@ -33,7 +34,7 @@ class SceneBuilder : public RefCountedDartWrappable { ~SceneBuilder() override; void pushTransform(const tonic::Float64List& matrix4); - void pushOffset(double dx, double dy); + fml::RefPtr pushOffset(double dx, double dy); void pushClipRect(double left, double right, double top, @@ -50,11 +51,13 @@ class SceneBuilder : public RefCountedDartWrappable { double maskRectTop, double maskRectBottom, int blendMode); - void pushPhysicalShape(const CanvasPath* path, - double elevation, - int color, - int shadowColor, - int clipBehavior); + fml::RefPtr pushPhysicalShape(const CanvasPath* path, + double elevation, + int color, + int shadowColor, + int clipBehavior); + + void addRetained(fml::RefPtr retainedLayer); void pop(); @@ -92,14 +95,14 @@ class SceneBuilder : public RefCountedDartWrappable { private: SceneBuilder(); - std::unique_ptr root_layer_; + std::shared_ptr root_layer_; flow::ContainerLayer* current_layer_ = nullptr; int rasterizer_tracing_threshold_ = 0; bool checkerboard_raster_cache_images_ = false; bool checkerboard_offscreen_layers_ = false; - void PushLayer(std::unique_ptr layer); + void PushLayer(std::shared_ptr layer); FML_DISALLOW_COPY_AND_ASSIGN(SceneBuilder); }; diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 0d1e18c645ba5..444c0b13f924b 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -11,6 +11,7 @@ #include "flutter/lib/ui/isolate_name_server/isolate_name_server_natives.h" #include "flutter/lib/ui/painting/canvas.h" #include "flutter/lib/ui/painting/codec.h" +#include "flutter/lib/ui/painting/engine_layer.h" #include "flutter/lib/ui/painting/frame_info.h" #include "flutter/lib/ui/painting/gradient.h" #include "flutter/lib/ui/painting/image.h" @@ -70,6 +71,7 @@ void DartUI::InitForGlobal() { CanvasPathMeasure::RegisterNatives(g_natives); Codec::RegisterNatives(g_natives); DartRuntimeHooks::RegisterNatives(g_natives); + EngineLayer::RegisterNatives(g_natives); FrameInfo::RegisterNatives(g_natives); ImageFilter::RegisterNatives(g_natives); ImageShader::RegisterNatives(g_natives); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index af408b4f75ad8..5a79abade2b4c 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1742,6 +1742,13 @@ enum PathOperation { reverseDifference, } +/// A handle for the framework to hold and retain an engine layer across frames. +class EngineLayer extends NativeFieldWrapperClass2 { + /// This class is created by the engine, and should not be instantiated + /// or extended directly. + EngineLayer._(); +} + /// A complex, one-dimensional subset of a plane. /// /// A path consists of a number of subpaths, and a _current point_. diff --git a/lib/ui/painting/engine_layer.cc b/lib/ui/painting/engine_layer.cc new file mode 100644 index 0000000000000..5ff6e96f61574 --- /dev/null +++ b/lib/ui/painting/engine_layer.cc @@ -0,0 +1,26 @@ +// Copyright 2018 The Chromium 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/lib/ui/painting/engine_layer.h" + +#include "flutter/flow/layers/container_layer.h" + +#include "third_party/tonic/converter/dart_converter.h" +#include "third_party/tonic/dart_args.h" +#include "third_party/tonic/dart_binding_macros.h" +#include "third_party/tonic/dart_library_natives.h" + +using tonic::ToDart; + +namespace blink { + +EngineLayer::~EngineLayer() = default; + +IMPLEMENT_WRAPPERTYPEINFO(ui, EngineLayer); + +#define FOR_EACH_BINDING(V) // nothing to bind + +DART_BIND_ALL(EngineLayer, FOR_EACH_BINDING) + +} // namespace blink diff --git a/lib/ui/painting/engine_layer.h b/lib/ui/painting/engine_layer.h new file mode 100644 index 0000000000000..e79b5f8e63967 --- /dev/null +++ b/lib/ui/painting/engine_layer.h @@ -0,0 +1,44 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_LIB_UI_PAINTING_ENGINE_LAYER_H_ +#define FLUTTER_LIB_UI_PAINTING_ENGINE_LAYER_H_ + +#include "flutter/lib/ui/dart_wrapper.h" + +#include "flutter/flow/layers/layer.h" + +namespace tonic { +class DartLibraryNatives; +} // namespace tonic + +namespace blink { + +class EngineLayer; + +class EngineLayer : public RefCountedDartWrappable { + DEFINE_WRAPPERTYPEINFO(); + + public: + ~EngineLayer() override; + static fml::RefPtr MakeRetained( + std::shared_ptr layer) { + return fml::MakeRefCounted(layer); + } + + static void RegisterNatives(tonic::DartLibraryNatives* natives); + + std::shared_ptr Layer() const { return layer_; } + + private: + explicit EngineLayer(std::shared_ptr layer) + : layer_(layer) {} + std::shared_ptr layer_; + + FML_FRIEND_MAKE_REF_COUNTED(EngineLayer); +}; + +} // namespace blink + +#endif // FLUTTER_LIB_UI_PAINTING_ENGINE_LAYER_H_ From 7031f7c1161bc18795e628adf29b6ad8f33e779e Mon Sep 17 00:00:00 2001 From: Yuqian Li Date: Thu, 4 Oct 2018 09:33:48 -0700 Subject: [PATCH 2/2] Do not share the SkImage inside RasterCacheResult --- flow/layers/picture_layer.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/flow/layers/picture_layer.cc b/flow/layers/picture_layer.cc index 3cbf15a609ad1..05d40d8bbd093 100644 --- a/flow/layers/picture_layer.cc +++ b/flow/layers/picture_layer.cc @@ -46,6 +46,14 @@ void PictureLayer::Paint(PaintContext& context) const { if (raster_cache_result_.is_valid()) { raster_cache_result_.draw(context.canvas); + + // We cannot share the RasterCacheResult with the framework (UI thread) + // because it includes an SkImage and its associated GrContext doesn't + // seem to be thread safe when we construct SkImage in one thread and + // destruct it in another thread. Hence set it to empty right after use + // so the retained engine layer that's sent to the framework doesn't hold + // an SkImage reference. + raster_cache_result_ = {}; } else { context.canvas.drawPicture(picture()); }