From a46a98c526222b386f46a9a3e1602d6ac749fe44 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Tue, 26 Nov 2019 12:47:01 -0800 Subject: [PATCH 1/6] Remove erroneous ChildView opacity --- flow/view_holder.cc | 20 +++++--------------- flow/view_holder.h | 4 ---- lib/ui/compositing.dart | 11 ----------- lib/ui/compositing/scene_host.cc | 16 +++------------- lib/ui/compositing/scene_host.h | 11 ++++++----- lib/web_ui/lib/src/ui/compositing.dart | 12 ------------ 6 files changed, 14 insertions(+), 60 deletions(-) diff --git a/flow/view_holder.cc b/flow/view_holder.cc index b49ca430da40d..7f8929d933705 100644 --- a/flow/view_holder.cc +++ b/flow/view_holder.cc @@ -104,14 +104,11 @@ void ViewHolder::UpdateScene(SceneUpdateContext& context, const SkSize& size, bool hit_testable) { if (pending_view_holder_token_.value) { - opacity_node_ = - std::make_unique(context.session()); entity_node_ = std::make_unique(context.session()); view_holder_ = std::make_unique( context.session(), std::move(pending_view_holder_token_), "Flutter SceneHost"); - opacity_node_->AddChild(*entity_node_); entity_node_->Attach(*view_holder_); ui_task_runner_->PostTask( [bind_callback = std::move(pending_bind_callback_), @@ -119,20 +116,18 @@ void ViewHolder::UpdateScene(SceneUpdateContext& context, bind_callback(view_holder_id); }); } - FML_DCHECK(opacity_node_); + FML_DCHECK(entity_node_); FML_DCHECK(view_holder_); - context.top_entity()->entity_node().AddChild(*opacity_node_); + context.top_entity()->embedder_node().AddChild(*entity_node_); entity_node_->SetTranslation(offset.x(), offset.y(), -0.1f); entity_node_->SetHitTestBehavior( hit_testable ? fuchsia::ui::gfx::HitTestBehavior::kDefault : fuchsia::ui::gfx::HitTestBehavior::kSuppress); - if (has_pending_opacity_) { - opacity_node_->SetOpacity(pending_opacity_); - - has_pending_opacity_ = false; - } if (has_pending_properties_) { + // TODO(dworsham): This should be derived from size and elevation. We + // should be able to Z-limit the view's box but otherwise it uses all of the + // available airspace. view_holder_->SetViewProperties(std::move(pending_properties_)); has_pending_properties_ = false; @@ -151,9 +146,4 @@ void ViewHolder::SetProperties(double width, has_pending_properties_ = true; } -void ViewHolder::SetOpacity(double opacity) { - pending_opacity_ = std::clamp(opacity, 0.0, 1.0); - has_pending_opacity_ = true; -} - } // namespace flutter diff --git a/flow/view_holder.h b/flow/view_holder.h index 8b49e216c2800..82d43eba826d4 100644 --- a/flow/view_holder.h +++ b/flow/view_holder.h @@ -51,7 +51,6 @@ class ViewHolder { double insetBottom, double insetLeft, bool focusable); - void SetOpacity(double opacity); // Creates or updates the contained ViewHolder resource using the specified // |SceneUpdateContext|. @@ -63,7 +62,6 @@ class ViewHolder { private: fml::RefPtr ui_task_runner_; - std::unique_ptr opacity_node_; std::unique_ptr entity_node_; std::unique_ptr view_holder_; @@ -71,9 +69,7 @@ class ViewHolder { BindCallback pending_bind_callback_; fuchsia::ui::gfx::ViewProperties pending_properties_; - double pending_opacity_; bool has_pending_properties_ = false; - bool has_pending_opacity_ = false; FML_DISALLOW_COPY_AND_ASSIGN(ViewHolder); }; diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 3eb13535e44cb..fc1b368bf4f82 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -694,13 +694,6 @@ class SceneHost extends NativeFieldWrapperClass2 { void Function(bool) viewStateChangedCallback) { _constructor(viewHolderToken, viewConnectedCallback, viewDisconnectedCallback, viewStateChangedCallback); } - SceneHost.fromViewHolderToken( - dynamic viewHolderToken, - void Function() viewConnectedCallback, - void Function() viewDisconnectedCallback, - void Function(bool) viewStateChangedCallback) { - _constructor(viewHolderToken, viewConnectedCallback, viewDisconnectedCallback, viewStateChangedCallback); - } void _constructor(dynamic viewHolderToken, void Function() viewConnectedCallback, void Function() viewDisconnectedCallback, void Function(bool) viewStateChangedCallback) native 'SceneHost_constructor'; @@ -720,8 +713,4 @@ class SceneHost extends NativeFieldWrapperClass2 { double insetBottom, double insetLeft, bool focusable) native 'SceneHost_setProperties'; - - /// Set the opacity of the linked scene. This opacity value is applied only - /// once, when the child scene is composited into our own. - void setOpacity(double opacity) native 'SceneHost_setOpacity'; } diff --git a/lib/ui/compositing/scene_host.cc b/lib/ui/compositing/scene_host.cc index 889f5cb174351..e4f83cc3ff0e8 100644 --- a/lib/ui/compositing/scene_host.cc +++ b/lib/ui/compositing/scene_host.cc @@ -85,10 +85,9 @@ namespace flutter { IMPLEMENT_WRAPPERTYPEINFO(ui, SceneHost); -#define FOR_EACH_BINDING(V) \ - V(SceneHost, dispose) \ - V(SceneHost, setProperties) \ - V(SceneHost, setOpacity) +#define FOR_EACH_BINDING(V) \ + V(SceneHost, dispose) \ + V(SceneHost, setProperties) FOR_EACH_BINDING(DART_NATIVE_CALLBACK) @@ -205,13 +204,4 @@ void SceneHost::setProperties(double width, }); } -void SceneHost::setOpacity(double opacity) { - gpu_task_runner_->PostTask([id = koid_, opacity]() { - auto* view_holder = flutter::ViewHolder::FromId(id); - FML_DCHECK(view_holder); - - view_holder->SetOpacity(opacity); - }); -} - } // namespace flutter diff --git a/lib/ui/compositing/scene_host.h b/lib/ui/compositing/scene_host.h index 9e47cf49a3645..05c36e3c4f0cf 100644 --- a/lib/ui/compositing/scene_host.h +++ b/lib/ui/compositing/scene_host.h @@ -33,14 +33,11 @@ class SceneHost : public RefCountedDartWrappable { static void OnViewDisconnected(scenic::ResourceId id); static void OnViewStateChanged(scenic::ResourceId id, bool state); - SceneHost(fml::RefPtr viewHolderToken, - Dart_Handle viewConnectedCallback, - Dart_Handle viewDisconnectedCallback, - Dart_Handle viewStateChangedCallback); ~SceneHost() override; zx_koid_t id() const { return koid_; } + // These are visible to Dart. void dispose(); void setProperties(double width, double height, @@ -49,9 +46,13 @@ class SceneHost : public RefCountedDartWrappable { double insetBottom, double insetLeft, bool focusable); - void setOpacity(double opacity); private: + SceneHost(fml::RefPtr viewHolderToken, + Dart_Handle viewConnectedCallback, + Dart_Handle viewDisconnectedCallback, + Dart_Handle viewStateChangedCallback); + fml::RefPtr gpu_task_runner_; tonic::DartPersistentValue view_connected_callback_; tonic::DartPersistentValue view_disconnected_callback_; diff --git a/lib/web_ui/lib/src/ui/compositing.dart b/lib/web_ui/lib/src/ui/compositing.dart index d33ccc6c216a9..96d83d52fd906 100644 --- a/lib/web_ui/lib/src/ui/compositing.dart +++ b/lib/web_ui/lib/src/ui/compositing.dart @@ -383,12 +383,6 @@ class SceneHost { void Function() viewDisconnectedCallback, void Function(bool) viewStateChangedCallback); - SceneHost.fromViewHolderToken( - dynamic viewHolderToken, - void Function() viewConnectedCallback, - void Function() viewDisconnectedCallback, - void Function(bool) viewStateChangedCallback); - /// Releases the resources associated with the SceneHost. /// /// After calling this function, the SceneHost cannot be used further. @@ -400,10 +394,4 @@ class SceneHost { double insetRight, double insetBottom, double insetLeft, bool focusable) { throw UnimplementedError(); } - - /// Set the opacity of the linked scene. This opacity value is applied only - /// once, when the child scene is composited into our own. - void setOpacity(double opacity) { - throw UnimplementedError(); - } } From 1423191f3302f8f10195df258aa43c4fb21d4611 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Tue, 26 Nov 2019 12:47:46 -0800 Subject: [PATCH 2/6] Wire frame metrics through contexts --- flow/layers/layer.h | 13 +++- flow/layers/layer_tree.cc | 60 +++++++++++++------ flow/layers/layer_tree.h | 20 +++---- flow/layers/layer_tree_unittests.cc | 17 +++--- flow/layers/physical_shape_layer.cc | 12 +--- flow/layers/physical_shape_layer.h | 6 -- flow/layers/physical_shape_layer_unittests.cc | 23 +------ flow/raster_cache.cc | 4 +- flow/scene_update_context.cc | 6 +- flow/scene_update_context.h | 16 ++++- flow/testing/layer_test.h | 9 ++- lib/ui/compositing/scene.cc | 26 +++++--- lib/ui/compositing/scene.h | 2 +- lib/ui/compositing/scene_builder.cc | 8 --- lib/ui/window/viewport_metrics.cc | 16 +++-- lib/ui/window/viewport_metrics.h | 5 +- shell/common/engine.cc | 9 ++- shell/common/persistent_cache_unittests.cc | 2 +- shell/common/shell_test.cc | 22 +++++-- shell/common/shell_test.h | 4 +- shell/common/shell_unittests.cc | 26 ++++++++ .../embedder/tests/embedder_unittests.cc | 1 - 22 files changed, 190 insertions(+), 117 deletions(-) diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 66944376e8ce8..ba91353140889 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -52,11 +52,18 @@ struct PrerollContext { SkColorSpace* dst_color_space; SkRect cull_rect; - // The following allows us to paint in the end of subtree preroll + // These allow us to paint in the end of subtree Preroll. const Stopwatch& raster_time; const Stopwatch& ui_time; TextureRegistry& texture_registry; const bool checkerboard_offscreen_layers; + + // These allow us to make use of the scene metrics during Preroll. + float frame_physical_depth; + float frame_device_pixel_ratio; + + // These allow us to track properties like elevation and opacity which stack + // with each other during Preroll. float total_elevation = 0.0f; bool has_platform_view = false; }; @@ -90,6 +97,10 @@ class Layer { TextureRegistry& texture_registry; const RasterCache* raster_cache; const bool checkerboard_offscreen_layers; + + // These allow us to make use of the scene metrics during Paint. + float frame_physical_depth; + float frame_device_pixel_ratio; }; // Calls SkCanvas::saveLayer and restores the layer upon destruction. Also diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index caa5531a79c06..f3fb164136913 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -11,8 +11,12 @@ namespace flutter { -LayerTree::LayerTree() - : frame_size_{}, +LayerTree::LayerTree(const SkISize& frame_size, + float frame_physical_depth, + float frame_device_pixel_ratio) + : frame_size_(frame_size), + frame_physical_depth_(frame_physical_depth), + frame_device_pixel_ratio_(frame_device_pixel_ratio), rasterizer_tracing_threshold_(0), checkerboard_raster_cache_images_(false), checkerboard_offscreen_layers_(false) {} @@ -46,7 +50,9 @@ void LayerTree::Preroll(CompositorContext::ScopedFrame& frame, frame.context().raster_time(), frame.context().ui_time(), frame.context().texture_registry(), - checkerboard_offscreen_layers_}; + checkerboard_offscreen_layers_, + frame_physical_depth_, + frame_device_pixel_ratio_}; root_layer_->Preroll(&context, frame.root_surface_transformation()); } @@ -55,12 +61,22 @@ void LayerTree::Preroll(CompositorContext::ScopedFrame& frame, void LayerTree::UpdateScene(SceneUpdateContext& context, scenic::ContainerNode& container) { TRACE_EVENT0("flutter", "LayerTree::UpdateScene"); + + // Ensure the context is aware of the view metrics. + context.set_dimensions(frame_size_, frame_physical_depth_, + frame_device_pixel_ratio_); + const auto& metrics = context.metrics(); + FML_DCHECK(metrics->scale_x > 0.0f); + FML_DCHECK(metrics->scale_y > 0.0f); + FML_DCHECK(metrics->scale_z > 0.0f); + SceneUpdateContext::Transform transform(context, // context 1.0f / metrics->scale_x, // X 1.0f / metrics->scale_y, // Y 1.0f / metrics->scale_z // Z ); + SceneUpdateContext::Frame frame( context, SkRRect::MakeRect( @@ -104,7 +120,9 @@ void LayerTree::Paint(CompositorContext::ScopedFrame& frame, frame.context().ui_time(), frame.context().texture_registry(), ignore_raster_cache ? nullptr : &frame.context().raster_cache(), - checkerboard_offscreen_layers_}; + checkerboard_offscreen_layers_, + frame_physical_depth_, + frame_device_pixel_ratio_}; if (root_layer_->needs_painting()) root_layer_->Paint(context); @@ -128,16 +146,18 @@ sk_sp LayerTree::Flatten(const SkRect& bounds) { root_surface_transformation.reset(); PrerollContext preroll_context{ - nullptr, // raster_cache (don't consult the cache) - nullptr, // gr_context (used for the raster cache) - nullptr, // external view embedder - unused_stack, // mutator stack - nullptr, // SkColorSpace* dst_color_space - kGiantRect, // SkRect cull_rect - unused_stopwatch, // frame time (dont care) - unused_stopwatch, // engine time (dont care) - unused_texture_registry, // texture registry (not supported) - false, // checkerboard_offscreen_layers + nullptr, // raster_cache (don't consult the cache) + nullptr, // gr_context (used for the raster cache) + nullptr, // external view embedder + unused_stack, // mutator stack + nullptr, // SkColorSpace* dst_color_space + kGiantRect, // SkRect cull_rect + unused_stopwatch, // frame time (dont care) + unused_stopwatch, // engine time (dont care) + unused_texture_registry, // texture registry (not supported) + false, // checkerboard_offscreen_layers + frame_physical_depth_, // maximum depth allowed for rendering + frame_device_pixel_ratio_ // ratio between logical and physical }; SkISize canvas_size = canvas->getBaseLayerSize(); @@ -149,11 +169,13 @@ sk_sp LayerTree::Flatten(const SkRect& bounds) { canvas, // canvas nullptr, nullptr, - unused_stopwatch, // frame time (dont care) - unused_stopwatch, // engine time (dont care) - unused_texture_registry, // texture registry (not supported) - nullptr, // raster cache - false // checkerboard offscreen layers + unused_stopwatch, // frame time (dont care) + unused_stopwatch, // engine time (dont care) + unused_texture_registry, // texture registry (not supported) + nullptr, // raster cache + false, // checkerboard offscreen layers + frame_physical_depth_, // maximum depth allowed for rendering + frame_device_pixel_ratio_ // ratio between logical and physical }; // Even if we don't have a root layer, we still need to create an empty diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index b9edeeceba2ed..10a10d6711ab1 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -14,13 +14,14 @@ #include "flutter/fml/macros.h" #include "flutter/fml/time/time_delta.h" #include "third_party/skia/include/core/SkPicture.h" -#include "third_party/skia/include/core/SkSize.h" namespace flutter { class LayerTree { public: - LayerTree(); + LayerTree(const SkISize& frame_size, + float frame_physical_depth, + float frame_device_pixel_ratio); void Preroll(CompositorContext::ScopedFrame& frame, bool ignore_raster_cache = false); @@ -42,8 +43,8 @@ class LayerTree { } const SkISize& frame_size() const { return frame_size_; } - - void set_frame_size(const SkISize& frame_size) { frame_size_ = frame_size; } + float frame_physical_depth() const { return frame_physical_depth_; } + float frame_device_pixel_ratio() const { return frame_device_pixel_ratio_; } void RecordBuildTime(fml::TimePoint begin_start); fml::TimePoint build_start() const { return build_start_; } @@ -69,18 +70,15 @@ class LayerTree { checkerboard_offscreen_layers_ = checkerboard; } - void set_device_pixel_ratio(double device_pixel_ratio) { - device_pixel_ratio_ = device_pixel_ratio; - } - - double device_pixel_ratio() const { return device_pixel_ratio_; } + double device_pixel_ratio() const { return frame_device_pixel_ratio_; } private: - SkISize frame_size_ = SkISize::MakeEmpty(); // Physical pixels. - double device_pixel_ratio_ = 1.0; std::shared_ptr root_layer_; fml::TimePoint build_start_; fml::TimePoint build_finish_; + SkISize frame_size_ = SkISize::MakeEmpty(); // Physical pixels. + float frame_physical_depth_; + float frame_device_pixel_ratio_ = 1.0f; // Logical / Physical pixels ratio. uint32_t rasterizer_tracing_threshold_; bool checkerboard_raster_cache_images_; bool checkerboard_offscreen_layers_; diff --git a/flow/layers/layer_tree_unittests.cc b/flow/layers/layer_tree_unittests.cc index e0961723e3c6e..7d4922ff3be48 100644 --- a/flow/layers/layer_tree_unittests.cc +++ b/flow/layers/layer_tree_unittests.cc @@ -17,13 +17,16 @@ namespace testing { class LayerTreeTest : public CanvasTest { public: - void SetUp() override { - root_transform_ = SkMatrix::MakeTrans(1.0f, 1.0f); - scoped_frame_ = compositor_context_.AcquireFrame( - nullptr, &mock_canvas(), nullptr, root_transform_, false, nullptr); - } - - void TearDown() override { scoped_frame_ = nullptr; } + LayerTreeTest() + : layer_tree_(SkISize::Make(64, 64), 100.0f, 1.0f), + compositor_context_(fml::kDefaultFrameBudget), + root_transform_(SkMatrix::MakeTrans(1.0f, 1.0f)), + scoped_frame_(compositor_context_.AcquireFrame(nullptr, + &mock_canvas(), + nullptr, + root_transform_, + false, + nullptr)) {} LayerTree& layer_tree() { return layer_tree_; } CompositorContext::ScopedFrame& frame() { return *scoped_frame_.get(); } diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 91b6c73071d17..c5012b8bb6489 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -14,17 +14,11 @@ const SkScalar kLightRadius = 800; PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, SkColor shadow_color, - SkScalar device_pixel_ratio, - float viewport_depth, float elevation, const SkPath& path, Clip clip_behavior) : color_(color), shadow_color_(shadow_color), - device_pixel_ratio_(device_pixel_ratio), -#if defined(OS_FUCHSIA) - viewport_depth_(viewport_depth), -#endif elevation_(elevation), path_(path), isRect_(false), @@ -70,7 +64,7 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, // bounds to leave space for the shadow. We fill this whole region and clip // children to it so we don't need to join the child paint bounds. set_paint_bounds(ComputeShadowBounds(path_.getBounds(), elevation_, - device_pixel_ratio_)); + context->frame_device_pixel_ratio)); #endif // defined(OS_FUCHSIA) } } @@ -97,7 +91,7 @@ void PhysicalShapeLayer::UpdateScene(SceneUpdateContext& context) { TRACE_EVENT_INSTANT0("flutter", "cache miss, creating"); // If we can't find an existing retained surface, create one. SceneUpdateContext::Frame frame(context, frameRRect_, color_, elevation_, - total_elevation_, viewport_depth_, this); + total_elevation_, this); for (auto& layer : layers()) { if (layer->needs_painting()) { frame.AddPaintLayer(layer.get()); @@ -115,7 +109,7 @@ void PhysicalShapeLayer::Paint(PaintContext& context) const { if (elevation_ != 0) { DrawShadow(context.leaf_nodes_canvas, path_, shadow_color_, elevation_, - SkColorGetA(color_) != 0xff, device_pixel_ratio_); + SkColorGetA(color_) != 0xff, context.frame_device_pixel_ratio); } // Call drawPath without clip if possible for better performance. diff --git a/flow/layers/physical_shape_layer.h b/flow/layers/physical_shape_layer.h index 1b5564c3662b3..38af3e45d4e27 100644 --- a/flow/layers/physical_shape_layer.h +++ b/flow/layers/physical_shape_layer.h @@ -13,8 +13,6 @@ class PhysicalShapeLayer : public ContainerLayer { public: PhysicalShapeLayer(SkColor color, SkColor shadow_color, - SkScalar device_pixel_ratio, - float viewport_depth, float elevation, const SkPath& path, Clip clip_behavior); @@ -42,10 +40,6 @@ class PhysicalShapeLayer : public ContainerLayer { private: SkColor color_; SkColor shadow_color_; - SkScalar device_pixel_ratio_; -#if defined(OS_FUCHSIA) - float viewport_depth_ = 0.0f; -#endif float elevation_ = 0.0f; float total_elevation_ = 0.0f; SkPath path_; diff --git a/flow/layers/physical_shape_layer_unittests.cc b/flow/layers/physical_shape_layer_unittests.cc index b9382318e8378..3a5a13d25ec6a 100644 --- a/flow/layers/physical_shape_layer_unittests.cc +++ b/flow/layers/physical_shape_layer_unittests.cc @@ -18,8 +18,6 @@ using PhysicalShapeLayerTest = LayerTest; TEST_F(PhysicalShapeLayerTest, PaintingEmptyLayerDies) { auto layer = std::make_shared(SK_ColorBLACK, SK_ColorBLACK, - 1.0f, // pixel ratio - 1.0f, // depth 0.0f, // elevation SkPath(), Clip::none); @@ -38,8 +36,6 @@ TEST_F(PhysicalShapeLayerTest, PaintBeforePreollDies) { auto mock_layer = std::make_shared(child_path, SkPaint()); auto layer = std::make_shared(SK_ColorBLACK, SK_ColorBLACK, - 1.0f, // pixel ratio - 1.0f, // depth 0.0f, // elevation SkPath(), Clip::none); layer->Add(mock_layer); @@ -54,8 +50,6 @@ TEST_F(PhysicalShapeLayerTest, NonEmptyLayer) { layer_path.addRect(5.0f, 6.0f, 20.5f, 21.5f); auto layer = std::make_shared(SK_ColorGREEN, SK_ColorBLACK, - 1.0f, // pixel ratio - 1.0f, // depth 0.0f, // elevation layer_path, Clip::none); layer->Preroll(preroll_context(), SkMatrix()); @@ -80,20 +74,14 @@ TEST_F(PhysicalShapeLayerTest, ChildrenLargerThanPath) { SkPath child2_path; child2_path.addRect(3, 2, 5, 15).close(); auto child1 = std::make_shared(SK_ColorRED, SK_ColorBLACK, - 1.0f, // pixel ratio - 1.0f, // depth 0.0f, // elevation child1_path, Clip::none); auto child2 = std::make_shared(SK_ColorBLUE, SK_ColorBLACK, - 1.0f, // pixel ratio - 1.0f, // depth 0.0f, // elevation child2_path, Clip::none); auto layer = std::make_shared(SK_ColorGREEN, SK_ColorBLACK, - 1.0f, // pixel ratio - 1.0f, // depth 0.0f, // elevation layer_path, Clip::none); layer->Add(child1); @@ -133,10 +121,7 @@ TEST_F(PhysicalShapeLayerTest, ElevationSimple) { SkPath layer_path; layer_path.addRect(0, 0, 8, 8).close(); auto layer = std::make_shared( - SK_ColorGREEN, SK_ColorBLACK, - 1.0f, // pixel ratio - 1.0f, // depth - initial_elevation, layer_path, Clip::none); + SK_ColorGREEN, SK_ColorBLACK, initial_elevation, layer_path, Clip::none); layer->Preroll(preroll_context(), SkMatrix()); // The Fuchsia system compositor handles all elevated PhysicalShapeLayers and @@ -190,10 +175,8 @@ TEST_F(PhysicalShapeLayerTest, ElevationComplex) { std::shared_ptr layers[4]; for (int i = 0; i < 4; i += 1) { layers[i] = std::make_shared( - SK_ColorBLACK, SK_ColorBLACK, - 1.0f, // pixel ratio - 1.0f, // depth - initial_elevations[i], layer_path, Clip::none); + SK_ColorBLACK, SK_ColorBLACK, initial_elevations[i], layer_path, + Clip::none); } layers[0]->Add(layers[1]); layers[0]->Add(layers[2]); diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index f9388ad98d4b8..a3bbb20b9b05a 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -175,7 +175,9 @@ void RasterCache::Prepare(PrerollContext* context, context->ui_time, context->texture_registry, context->has_platform_view ? nullptr : context->raster_cache, - context->checkerboard_offscreen_layers}; + context->checkerboard_offscreen_layers, + context->frame_physical_depth, + context->frame_device_pixel_ratio}; if (layer->needs_painting()) { layer->Paint(paintContext); } diff --git a/flow/scene_update_context.cc b/flow/scene_update_context.cc index 1c85bb658aacb..e6d74935f6e83 100644 --- a/flow/scene_update_context.cc +++ b/flow/scene_update_context.cc @@ -212,7 +212,9 @@ SceneUpdateContext::ExecutePaintTasks(CompositorContext::ScopedFrame& frame) { frame.context().ui_time(), frame.context().texture_registry(), &frame.context().raster_cache(), - false}; + false, + frame_physical_depth_, + frame_device_pixel_ratio_}; canvas->restoreToCount(1); canvas->save(); canvas->clear(task.background_color); @@ -301,13 +303,13 @@ SceneUpdateContext::Frame::Frame(SceneUpdateContext& context, SkColor color, float local_elevation, float world_elevation, - float depth, Layer* layer) : Shape(context), rrect_(rrect), color_(color), paint_bounds_(SkRect::MakeEmpty()), layer_(layer) { + const float depth = context.frame_physical_depth(); if (depth > -1 && world_elevation > depth) { // TODO(mklim): Deal with bounds overflow more elegantly. We'd like to be // able to have developers specify the behavior here to alternatives besides diff --git a/flow/scene_update_context.h b/flow/scene_update_context.h index c61f5670ba7f2..989fa83fb21bf 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -117,7 +117,6 @@ class SceneUpdateContext { SkColor color, float local_elevation = 0.0f, float parent_elevation = 0.0f, - float depth = 0.0f, Layer* layer = nullptr); virtual ~Frame(); @@ -152,6 +151,17 @@ class SceneUpdateContext { } const fuchsia::ui::gfx::MetricsPtr& metrics() const { return metrics_; } + void set_dimensions(const SkISize& frame_physical_size, + float frame_physical_depth, + float frame_device_pixel_ratio) { + frame_physical_size_ = frame_physical_size; + frame_physical_depth_ = frame_physical_depth; + frame_device_pixel_ratio_ = frame_device_pixel_ratio; + } + const SkISize& frame_size() const { return frame_physical_size_; } + float frame_physical_depth() const { return frame_physical_depth_; } + float frame_device_pixel_ratio() const { return frame_device_pixel_ratio_; } + // TODO(chinmaygarde): This method must submit the surfaces as soon as paint // tasks are done. However, given that there is no support currently for // Vulkan semaphores, we need to submit all the surfaces after an explicit @@ -225,6 +235,10 @@ class SceneUpdateContext { SurfaceProducer* const surface_producer_; fuchsia::ui::gfx::MetricsPtr metrics_; + SkISize frame_physical_size_; + float frame_physical_depth_ = 0.0f; + float frame_device_pixel_ratio_ = + 1.0f; // Ratio between logical and physical pixels. std::vector paint_tasks_; diff --git a/flow/testing/layer_test.h b/flow/testing/layer_test.h index e38d690a2eeb7..ab51af38af563 100644 --- a/flow/testing/layer_test.h +++ b/flow/testing/layer_test.h @@ -38,8 +38,11 @@ class LayerTestBase : public CanvasTestBase { mutators_stack_, TestT::mock_canvas().imageInfo().colorSpace(), kGiantRect, /* cull_rect */ raster_time_, ui_time_, texture_registry_, - false, /* checkerboard_offscreen_layers */ - 0.0f /* total_elevation */ + false, /* checkerboard_offscreen_layers */ + 100.0f, /* frame_physical_depth */ + 1.0f, /* frame_device_pixel_ratio */ + 0.0f, /* total_elevation */ + false, /* has_platform_view */ }), paint_context_({ TestT::mock_canvas().internal_canvas(), /* internal_nodes_canvas */ @@ -49,6 +52,8 @@ class LayerTestBase : public CanvasTestBase { raster_time_, ui_time_, texture_registry_, nullptr, /* raster_cache */ false, /* checkerboard_offscreen_layers */ + 100.0f, /* frame_physical_depth */ + 1.0f, /* frame_device_pixel_ratio */ }) {} TextureRegistry& texture_regitry() { return texture_registry_; } diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index fd3e86c7e5176..d02a232ac0433 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -7,6 +7,8 @@ #include "flutter/fml/trace_event.h" #include "flutter/lib/ui/painting/image.h" #include "flutter/lib/ui/painting/picture.h" +#include "flutter/lib/ui/ui_dart_state.h" +#include "flutter/lib/ui/window/window.h" #include "third_party/skia/include/core/SkImageInfo.h" #include "third_party/skia/include/core/SkSurface.h" #include "third_party/tonic/converter/dart_converter.h" @@ -36,13 +38,19 @@ fml::RefPtr Scene::create(std::shared_ptr rootLayer, Scene::Scene(std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, - 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( + bool checkerboardOffscreenLayers) { + auto viewport_metrics = UIDartState::Current()->window()->viewport_metrics(); + + layer_tree_ = std::make_unique( + SkISize::Make(viewport_metrics.physical_width, + viewport_metrics.physical_height), + static_cast(viewport_metrics.physical_depth), + static_cast(viewport_metrics.device_pixel_ratio)); + layer_tree_->set_root_layer(std::move(rootLayer)); + layer_tree_->set_rasterizer_tracing_threshold(rasterizerTracingThreshold); + layer_tree_->set_checkerboard_raster_cache_images( checkerboardRasterCacheImages); - m_layerTree->set_checkerboard_offscreen_layers(checkerboardOffscreenLayers); + layer_tree_->set_checkerboard_offscreen_layers(checkerboardOffscreenLayers); } Scene::~Scene() {} @@ -56,11 +64,11 @@ Dart_Handle Scene::toImage(uint32_t width, Dart_Handle raw_image_callback) { TRACE_EVENT0("flutter", "Scene::toImage"); - if (!m_layerTree) { + if (!layer_tree_) { return tonic::ToDart("Scene did not contain a layer tree."); } - auto picture = m_layerTree->Flatten(SkRect::MakeWH(width, height)); + auto picture = layer_tree_->Flatten(SkRect::MakeWH(width, height)); if (!picture) { return tonic::ToDart("Could not flatten scene into a layer tree."); } @@ -69,7 +77,7 @@ Dart_Handle Scene::toImage(uint32_t width, } std::unique_ptr Scene::takeLayerTree() { - return std::move(m_layerTree); + return std::move(layer_tree_); } } // namespace flutter diff --git a/lib/ui/compositing/scene.h b/lib/ui/compositing/scene.h index 2d280a534db63..42c972d8d4ee3 100644 --- a/lib/ui/compositing/scene.h +++ b/lib/ui/compositing/scene.h @@ -45,7 +45,7 @@ class Scene : public RefCountedDartWrappable { bool checkerboardRasterCacheImages, bool checkerboardOffscreenLayers); - std::unique_ptr m_layerTree; + std::unique_ptr layer_tree_; }; } // namespace flutter diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 5b7148803c2c2..61b74f743a9f4 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -23,8 +23,6 @@ #include "flutter/fml/build_config.h" #include "flutter/lib/ui/painting/matrix.h" #include "flutter/lib/ui/painting/shader.h" -#include "flutter/lib/ui/ui_dart_state.h" -#include "flutter/lib/ui/window/window.h" #include "third_party/skia/include/core/SkColorFilter.h" #include "third_party/tonic/converter/dart_converter.h" #include "third_party/tonic/dart_args.h" @@ -176,12 +174,6 @@ fml::RefPtr SceneBuilder::pushPhysicalShape(const CanvasPath* path, int clipBehavior) { auto layer = std::make_shared( static_cast(color), static_cast(shadow_color), - static_cast(UIDartState::Current() - ->window() - ->viewport_metrics() - .device_pixel_ratio), - static_cast( - UIDartState::Current()->window()->viewport_metrics().physical_depth), static_cast(elevation), path->path(), static_cast(clipBehavior)); PushLayer(layer); diff --git a/lib/ui/window/viewport_metrics.cc b/lib/ui/window/viewport_metrics.cc index f18972992189a..0b6dab6d4c1e0 100644 --- a/lib/ui/window/viewport_metrics.cc +++ b/lib/ui/window/viewport_metrics.cc @@ -4,8 +4,9 @@ #include "flutter/lib/ui/window/viewport_metrics.h" +#include "flutter/fml/logging.h" + namespace flutter { -ViewportMetrics::ViewportMetrics() = default; ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio, double p_physical_width, @@ -39,6 +40,10 @@ ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio, physical_system_gesture_inset_bottom( p_physical_system_gesture_inset_bottom), physical_system_gesture_inset_left(p_physical_system_gesture_inset_left) { + // Ensure we don't have nonsensical dimensions. + FML_DCHECK(physical_width >= 0); + FML_DCHECK(physical_height >= 0); + FML_DCHECK(device_pixel_ratio > 0); } ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio, @@ -68,8 +73,11 @@ ViewportMetrics::ViewportMetrics(double p_device_pixel_ratio, physical_view_inset_bottom(p_physical_view_inset_bottom), physical_view_inset_left(p_physical_view_inset_left), physical_view_inset_front(p_physical_view_inset_front), - physical_view_inset_back(p_physical_view_inset_back) {} - -ViewportMetrics::ViewportMetrics(const ViewportMetrics& other) = default; + physical_view_inset_back(p_physical_view_inset_back) { + // Ensure we don't have nonsensical dimensions. + FML_DCHECK(physical_width >= 0); + FML_DCHECK(physical_height >= 0); + FML_DCHECK(device_pixel_ratio > 0); +} } // namespace flutter diff --git a/lib/ui/window/viewport_metrics.h b/lib/ui/window/viewport_metrics.h index 5ca8b78eaa064..f60adbfcee110 100644 --- a/lib/ui/window/viewport_metrics.h +++ b/lib/ui/window/viewport_metrics.h @@ -16,7 +16,8 @@ namespace flutter { static const double kUnsetDepth = 1.7976931348623157e+308; struct ViewportMetrics { - ViewportMetrics(); + ViewportMetrics() = default; + ViewportMetrics(const ViewportMetrics& other) = default; // Create a 2D ViewportMetrics instance. ViewportMetrics(double p_device_pixel_ratio, @@ -51,8 +52,6 @@ struct ViewportMetrics { double p_physical_view_inset_bottom, double p_physical_view_inset_left); - ViewportMetrics(const ViewportMetrics& other); - double device_pixel_ratio = 1.0; double physical_width = 0; double physical_height = 0; diff --git a/shell/common/engine.cc b/shell/common/engine.cc index c0d64b9bfc42d..543ed34767c5c 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -437,13 +437,12 @@ void Engine::Render(std::unique_ptr layer_tree) { if (!layer_tree) return; - SkISize frame_size = SkISize::Make(viewport_metrics_.physical_width, - viewport_metrics_.physical_height); - if (frame_size.isEmpty()) + // Ensure frame dimensions are sane. + if (layer_tree->frame_size().isEmpty() || + layer_tree->frame_physical_depth() <= 0.0f || + layer_tree->frame_device_pixel_ratio() <= 0.0f) return; - layer_tree->set_frame_size(frame_size); - layer_tree->set_device_pixel_ratio(viewport_metrics_.device_pixel_ratio); animator_->Render(std::move(layer_tree)); } diff --git a/shell/common/persistent_cache_unittests.cc b/shell/common/persistent_cache_unittests.cc index 62b36e786875b..4948a3359bc69 100644 --- a/shell/common/persistent_cache_unittests.cc +++ b/shell/common/persistent_cache_unittests.cc @@ -56,7 +56,7 @@ TEST_F(ShellTest, CacheSkSLWorks) { SkPath path; path.addCircle(50, 50, 20); auto physical_shape_layer = std::make_shared( - SK_ColorRED, SK_ColorBLUE, 1.0f, 1.0f, 1.0f, path, Clip::antiAlias); + SK_ColorRED, SK_ColorBLUE, 1.0f, path, Clip::antiAlias); root->Add(physical_shape_layer); }; PumpOneFrame(shell.get(), 100, 100, builder); diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 97a382dcacbbd..3a0d3e150a207 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -143,14 +143,22 @@ void ShellTest::PumpOneFrame(Shell* shell, double width, double height, LayerTreeBuilder builder) { + PumpOneFrame(shell, + flutter::ViewportMetrics{1, width, height, flutter::kUnsetDepth, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + std::move(builder)); +} + +void ShellTest::PumpOneFrame(Shell* shell, + flutter::ViewportMetrics viewport_metrics, + LayerTreeBuilder builder) { // Set viewport to nonempty, and call Animator::BeginFrame to make the layer // tree pipeline nonempty. Without either of this, the layer tree below // won't be rasterized. fml::AutoResetWaitableEvent latch; shell->GetTaskRunners().GetUITaskRunner()->PostTask( - [&latch, engine = shell->weak_engine_, width, height]() { - engine->SetViewportMetrics( - {1, width, height, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}); + [&latch, engine = shell->weak_engine_, viewport_metrics]() { + engine->SetViewportMetrics(std::move(viewport_metrics)); engine->animator_->BeginFrame(fml::TimePoint::Now(), fml::TimePoint::Now()); latch.Signal(); @@ -161,8 +169,12 @@ void ShellTest::PumpOneFrame(Shell* shell, // Call |Render| to rasterize a layer tree and trigger |OnFrameRasterized| fml::WeakPtr runtime_delegate = shell->weak_engine_; shell->GetTaskRunners().GetUITaskRunner()->PostTask( - [&latch, runtime_delegate, &builder]() { - auto layer_tree = std::make_unique(); + [&latch, runtime_delegate, &builder, viewport_metrics]() { + auto layer_tree = std::make_unique( + SkISize::Make(viewport_metrics.physical_width, + viewport_metrics.physical_height), + static_cast(viewport_metrics.physical_depth), + static_cast(viewport_metrics.device_pixel_ratio)); SkMatrix identity; identity.setIdentity(); auto root_layer = std::make_shared(identity); diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index 0ae092a1c91bf..ebf9283378707 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -58,7 +58,9 @@ class ShellTest : public ThreadTest { double width = 1, double height = 1, LayerTreeBuilder = {}); - + static void PumpOneFrame(Shell* shell, + flutter::ViewportMetrics viewport_metrics, + LayerTreeBuilder = {}); static void DispatchFakePointerData(Shell* shell); static void DispatchPointerData(Shell* shell, std::unique_ptr packet); diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 636b879178d88..3074e71aaf4df 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -602,6 +602,27 @@ TEST_F(ShellTest, WaitForFirstFrame) { fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000)); ASSERT_TRUE(result.ok()); + + DestroyShell(std::move(shell)); +} + +TEST_F(ShellTest, WaitForFirstFrameZeroSizeFrame) { + auto settings = CreateSettingsForFixture(); + std::unique_ptr shell = CreateShell(settings); + + // Create the surface needed by rasterizer + PlatformViewNotifyCreated(shell.get()); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("emptyMain"); + + RunEngine(shell.get(), std::move(configuration)); + PumpOneFrame(shell.get(), {1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}); + fml::Status result = + shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000)); + ASSERT_FALSE(result.ok()); + ASSERT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded); + DestroyShell(std::move(shell)); } @@ -618,7 +639,9 @@ TEST_F(ShellTest, WaitForFirstFrameTimeout) { RunEngine(shell.get(), std::move(configuration)); fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(10)); + ASSERT_FALSE(result.ok()); ASSERT_EQ(result.code(), fml::StatusCode::kDeadlineExceeded); + DestroyShell(std::move(shell)); } @@ -641,6 +664,7 @@ TEST_F(ShellTest, WaitForFirstFrameMultiple) { result = shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1)); ASSERT_TRUE(result.ok()); } + DestroyShell(std::move(shell)); } @@ -666,10 +690,12 @@ TEST_F(ShellTest, WaitForFirstFrameInlined) { task_runner->PostTask([&shell, &event] { fml::Status result = shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1000)); + ASSERT_FALSE(result.ok()); ASSERT_EQ(result.code(), fml::StatusCode::kFailedPrecondition); event.Signal(); }); ASSERT_FALSE(event.WaitWithTimeout(fml::TimeDelta::FromMilliseconds(1000))); + DestroyShell(std::move(shell), std::move(task_runners)); } diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index c8560a6d11e7a..3631dee2563ab 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1604,7 +1604,6 @@ TEST_F(EmbedderTest, CustomCompositorMustWorkWithCustomTaskRunner) { event.width = 800; event.height = 600; event.pixel_ratio = 1.0; - ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); From 065aaf683ce7ef1745accb79eab6b24815851225 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Tue, 26 Nov 2019 12:50:02 -0800 Subject: [PATCH 3/6] Maintain layer stack inside of SceneBuilder --- flow/layers/container_layer.cc | 1 - flow/layers/layer.cc | 7 ++- flow/layers/layer.h | 9 +--- lib/ui/compositing/scene_builder.cc | 72 ++++++++++++----------------- lib/ui/compositing/scene_builder.h | 13 +++--- lib/ui/painting/engine_layer.cc | 2 - lib/ui/painting/engine_layer.h | 2 +- 7 files changed, 41 insertions(+), 65 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index fdb13411a9ac8..be9e831b08819 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -9,7 +9,6 @@ namespace flutter { ContainerLayer::ContainerLayer() {} void ContainerLayer::Add(std::shared_ptr layer) { - layer->set_parent(this); layers_.push_back(std::move(layer)); } diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index b729f582a0a9a..5ad6a6bee1499 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -10,10 +10,9 @@ namespace flutter { Layer::Layer() - : parent_(nullptr), - needs_system_composite_(false), - paint_bounds_(SkRect::MakeEmpty()), - unique_id_(NextUniqueID()) {} + : paint_bounds_(SkRect::MakeEmpty()), + unique_id_(NextUniqueID()), + needs_system_composite_(false) {} Layer::~Layer() = default; diff --git a/flow/layers/layer.h b/flow/layers/layer.h index ba91353140889..a9c4a3aa818ce 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -42,8 +42,6 @@ static constexpr SkRect kGiantRect = SkRect::MakeLTRB(-1E9F, -1E9F, 1E9F, 1E9F); // This should be an exact copy of the Clip enum in painting.dart. enum Clip { none, hardEdge, antiAlias, antiAliasWithSaveLayer }; -class ContainerLayer; - struct PrerollContext { RasterCache* raster_cache; GrContext* gr_context; @@ -137,10 +135,6 @@ class Layer { virtual void UpdateScene(SceneUpdateContext& context); #endif - ContainerLayer* parent() const { return parent_; } - - void set_parent(ContainerLayer* parent) { parent_ = parent; } - bool needs_system_composite() const { return needs_system_composite_; } void set_needs_system_composite(bool value) { needs_system_composite_ = value; @@ -159,10 +153,9 @@ class Layer { uint64_t unique_id() const { return unique_id_; } private: - ContainerLayer* parent_; - bool needs_system_composite_; SkRect paint_bounds_; uint64_t unique_id_; + bool needs_system_composite_; static uint64_t NextUniqueID(); diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 61b74f743a9f4..216e1c9284874 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -78,7 +78,12 @@ void SceneBuilder::RegisterNatives(tonic::DartLibraryNatives* natives) { }); } -SceneBuilder::SceneBuilder() = default; +SceneBuilder::SceneBuilder() { + // Add a ContainerLayer as the root layer, so that AddLayer operations are + // always valid. + PushLayer(std::make_shared()); +} + SceneBuilder::~SceneBuilder() = default; fml::RefPtr SceneBuilder::pushTransform( @@ -181,33 +186,24 @@ fml::RefPtr SceneBuilder::pushPhysicalShape(const CanvasPath* path, } void SceneBuilder::addRetained(fml::RefPtr retainedLayer) { - if (!current_layer_) { - return; - } - current_layer_->Add(retainedLayer->Layer()); + AddLayer(retainedLayer->Layer()); } void SceneBuilder::pop() { - if (!current_layer_) { - return; - } - current_layer_ = current_layer_->parent(); + PopLayer(); } void SceneBuilder::addPicture(double dx, double dy, Picture* picture, int hints) { - if (!current_layer_) { - return; - } SkPoint offset = SkPoint::Make(dx, dy); SkRect pictureRect = picture->picture()->cullRect(); pictureRect.offset(offset.x(), offset.y()); auto layer = std::make_unique( offset, UIDartState::CreateGPUObject(picture->picture()), !!(hints & 1), !!(hints & 2)); - current_layer_->Add(std::move(layer)); + AddLayer(std::move(layer)); } void SceneBuilder::addTexture(double dx, @@ -216,12 +212,9 @@ void SceneBuilder::addTexture(double dx, double height, int64_t textureId, bool freeze) { - if (!current_layer_) { - return; - } auto layer = std::make_unique( SkPoint::Make(dx, dy), SkSize::Make(width, height), textureId, freeze); - current_layer_->Add(std::move(layer)); + AddLayer(std::move(layer)); } void SceneBuilder::addPlatformView(double dx, @@ -229,12 +222,9 @@ void SceneBuilder::addPlatformView(double dx, double width, double height, int64_t viewId) { - if (!current_layer_) { - return; - } auto layer = std::make_unique( SkPoint::Make(dx, dy), SkSize::Make(width, height), viewId); - current_layer_->Add(std::move(layer)); + AddLayer(std::move(layer)); } #if defined(OS_FUCHSIA) @@ -244,13 +234,10 @@ void SceneBuilder::addChildScene(double dx, double height, SceneHost* sceneHost, bool hitTestable) { - if (!current_layer_) { - return; - } auto layer = std::make_unique( sceneHost->id(), SkPoint::Make(dx, dy), SkSize::Make(width, height), hitTestable); - current_layer_->Add(std::move(layer)); + AddLayer(std::move(layer)); } #endif // defined(OS_FUCHSIA) @@ -259,14 +246,11 @@ void SceneBuilder::addPerformanceOverlay(uint64_t enabledOptions, double right, double top, double bottom) { - if (!current_layer_) { - return; - } SkRect rect = SkRect::MakeLTRB(left, top, right, bottom); auto layer = std::make_unique(enabledOptions); layer->set_paint_bounds(rect); - current_layer_->Add(std::move(layer)); + AddLayer(std::move(layer)); } void SceneBuilder::setRasterizerTracingThreshold(uint32_t frameInterval) { @@ -282,29 +266,33 @@ void SceneBuilder::setCheckerboardOffscreenLayers(bool checkerboard) { } fml::RefPtr SceneBuilder::build() { + FML_DCHECK(layer_stack_.size() >= 1); + fml::RefPtr scene = Scene::create( - std::move(root_layer_), rasterizer_tracing_threshold_, + layer_stack_[0], rasterizer_tracing_threshold_, checkerboard_raster_cache_images_, checkerboard_offscreen_layers_); - ClearDartWrapper(); + ClearDartWrapper(); // may delete this object. return scene; } -void SceneBuilder::PushLayer(std::shared_ptr layer) { +void SceneBuilder::AddLayer(std::shared_ptr layer) { FML_DCHECK(layer); - if (!root_layer_) { - root_layer_ = std::move(layer); - current_layer_ = root_layer_.get(); - return; + if (!layer_stack_.empty()) { + layer_stack_.back()->Add(std::move(layer)); } +} - if (!current_layer_) { - return; - } +void SceneBuilder::PushLayer(std::shared_ptr layer) { + AddLayer(layer); + layer_stack_.push_back(std::move(layer)); +} - flutter::ContainerLayer* newLayer = layer.get(); - current_layer_->Add(std::move(layer)); - current_layer_ = newLayer; +void SceneBuilder::PopLayer() { + // We never pop the root layer, so that AddLayer operations are always valid. + if (layer_stack_.size() > 1) { + layer_stack_.pop_back(); + } } } // namespace flutter diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index d378876d960a2..a634087174e2f 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -8,8 +8,9 @@ #include #include -#include +#include +#include "flutter/flow/layers/container_layer.h" #include "flutter/lib/ui/compositing/scene.h" #include "flutter/lib/ui/dart_wrapper.h" #include "flutter/lib/ui/painting/color_filter.h" @@ -35,7 +36,6 @@ class SceneBuilder : public RefCountedDartWrappable { static fml::RefPtr create() { return fml::MakeRefCounted(); } - ~SceneBuilder() override; fml::RefPtr pushTransform(tonic::Float64List& matrix4); @@ -98,7 +98,6 @@ class SceneBuilder : public RefCountedDartWrappable { #endif void setRasterizerTracingThreshold(uint32_t frameInterval); - void setCheckerboardRasterCacheImages(bool checkerboard); void setCheckerboardOffscreenLayers(bool checkerboard); @@ -109,15 +108,15 @@ class SceneBuilder : public RefCountedDartWrappable { private: SceneBuilder(); - std::shared_ptr root_layer_; - flutter::ContainerLayer* current_layer_ = nullptr; + void AddLayer(std::shared_ptr layer); + void PushLayer(std::shared_ptr layer); + void PopLayer(); + std::vector> layer_stack_; int rasterizer_tracing_threshold_ = 0; bool checkerboard_raster_cache_images_ = false; bool checkerboard_offscreen_layers_ = false; - void PushLayer(std::shared_ptr layer); - FML_DISALLOW_COPY_AND_ASSIGN(SceneBuilder); }; diff --git a/lib/ui/painting/engine_layer.cc b/lib/ui/painting/engine_layer.cc index 7ac9c98497f41..0868b42cc2928 100644 --- a/lib/ui/painting/engine_layer.cc +++ b/lib/ui/painting/engine_layer.cc @@ -4,8 +4,6 @@ #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" diff --git a/lib/ui/painting/engine_layer.h b/lib/ui/painting/engine_layer.h index 1dc565c5193c2..a679ef2fe50f3 100644 --- a/lib/ui/painting/engine_layer.h +++ b/lib/ui/painting/engine_layer.h @@ -7,7 +7,7 @@ #include "flutter/lib/ui/dart_wrapper.h" -#include "flutter/flow/layers/layer.h" +#include "flutter/flow/layers/container_layer.h" namespace tonic { class DartLibraryNatives; From 2277d7d8096928ac98ce23cefeaa59074ac4bcab Mon Sep 17 00:00:00 2001 From: David Worsham Date: Tue, 26 Nov 2019 12:54:14 -0800 Subject: [PATCH 4/6] Remove EnsureSingleChild --- flow/layers/container_layer.cc | 2 +- flow/layers/container_layer.h | 2 +- flow/layers/opacity_layer.cc | 58 +++++++++++++------------- flow/layers/opacity_layer.h | 18 +++----- flow/layers/opacity_layer_unittests.cc | 2 +- 5 files changed, 37 insertions(+), 45 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index be9e831b08819..39372f6c84dd8 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -9,7 +9,7 @@ namespace flutter { ContainerLayer::ContainerLayer() {} void ContainerLayer::Add(std::shared_ptr layer) { - layers_.push_back(std::move(layer)); + layers_.emplace_back(std::move(layer)); } void ContainerLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index a0c054b1ff15c..df5be5e8b9466 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -14,7 +14,7 @@ class ContainerLayer : public Layer { public: ContainerLayer(); - void Add(std::shared_ptr layer); + virtual void Add(std::shared_ptr layer); void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void Paint(PaintContext& context) const override; diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 1b407307fd068..810aa4e4c8752 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -4,36 +4,33 @@ #include "flutter/flow/layers/opacity_layer.h" -#include "flutter/flow/layers/transform_layer.h" +#include "flutter/fml/trace_event.h" +#include "third_party/skia/include/core/SkPaint.h" namespace flutter { OpacityLayer::OpacityLayer(int alpha, const SkPoint& offset) - : alpha_(alpha), offset_(offset) {} - -void OpacityLayer::EnsureSingleChild() { - FML_DCHECK(layers().size() > 0); // OpacityLayer should never be a leaf - - if (layers().size() == 1) { - return; - } - - // Be careful: SkMatrix's default constructor doesn't initialize the matrix to - // identity. Hence we have to explicitly call SkMatrix::setIdentity. - SkMatrix identity; - identity.setIdentity(); - auto new_child = std::make_shared(identity); + : alpha_(alpha), offset_(offset) { + // Ensure OpacityLayer has only one direct child. + // + // This is needed to ensure that retained rendering can always be applied to + // save the costly saveLayer. + // + // Any children will be actually added as children of this empty + // ContainerLayer. + ContainerLayer::Add(std::make_shared()); +} - for (auto& child : layers()) { - new_child->Add(child); - } - ClearChildren(); - Add(new_child); +void OpacityLayer::Add(std::shared_ptr layer) { + GetChildContainer()->Add(std::move(layer)); } void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { TRACE_EVENT0("flutter", "OpacityLayer::Preroll"); - EnsureSingleChild(); + + ContainerLayer* container = GetChildContainer(); + FML_DCHECK(!container->layers().empty()); // OpacityLayer can't be a leaf. + SkMatrix child_matrix = matrix; child_matrix.postTranslate(offset_.fX, offset_.fY); context->mutators_stack.PushTransform( @@ -43,16 +40,14 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { context->mutators_stack.Pop(); context->mutators_stack.Pop(); set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY)); - // See |EnsureSingleChild|. - FML_DCHECK(layers().size() == 1); + if (!context->has_platform_view && context->raster_cache && SkRect::Intersects(context->cull_rect, paint_bounds())) { - Layer* child = layers()[0].get(); SkMatrix ctm = child_matrix; #ifndef SUPPORT_FRACTIONAL_TRANSLATION ctm = RasterCache::GetIntegralTransCTM(ctm); #endif - context->raster_cache->Prepare(context, child, ctm); + context->raster_cache->Prepare(context, container, ctm); } } @@ -71,13 +66,10 @@ void OpacityLayer::Paint(PaintContext& context) const { context.leaf_nodes_canvas->getTotalMatrix())); #endif - // See |EnsureSingleChild|. - FML_DCHECK(layers().size() == 1); - if (context.raster_cache) { + ContainerLayer* container = GetChildContainer(); const SkMatrix& ctm = context.leaf_nodes_canvas->getTotalMatrix(); - RasterCacheResult child_cache = - context.raster_cache->Get(layers()[0].get(), ctm); + RasterCacheResult child_cache = context.raster_cache->Get(container, ctm); if (child_cache.is_valid()) { child_cache.draw(*context.leaf_nodes_canvas, &paint); return; @@ -103,4 +95,10 @@ void OpacityLayer::Paint(PaintContext& context) const { PaintChildren(context); } +ContainerLayer* OpacityLayer::GetChildContainer() const { + FML_DCHECK(layers().size() == 1); + + return static_cast(layers()[0].get()); +} + } // namespace flutter diff --git a/flow/layers/opacity_layer.h b/flow/layers/opacity_layer.h index f1c18c51918e7..92ae4cd1eadc7 100644 --- a/flow/layers/opacity_layer.h +++ b/flow/layers/opacity_layer.h @@ -11,8 +11,8 @@ namespace flutter { // Don't add an OpacityLayer with no children to the layer tree. Painting an // OpacityLayer is very costly due to the saveLayer call. If there's no child, -// having the OpacityLayer or not has the same effect. In debug_unopt build, the -// |EnsureSingleChild| will assert if there are no children. +// having the OpacityLayer or not has the same effect. In debug_unopt build, +// |Preroll| will assert if there are no children. class OpacityLayer : public ContainerLayer { public: // An offset is provided here because OpacityLayer.addToScene method in the @@ -27,6 +27,8 @@ class OpacityLayer : public ContainerLayer { // the propagation as repainting the OpacityLayer is expensive. OpacityLayer(int alpha, const SkPoint& offset); + void Add(std::shared_ptr layer) override; + void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void Paint(PaintContext& context) const override; @@ -35,19 +37,11 @@ class OpacityLayer : public ContainerLayer { // session scene hierarchy. private: + ContainerLayer* GetChildContainer() const; + int alpha_; SkPoint offset_; - // Restructure (if necessary) OpacityLayer to have only one child. - // - // This is needed to ensure that retained rendering can always be applied to - // save the costly saveLayer. - // - // If there are multiple children, this creates a new identity TransformLayer, - // sets all children to be the TransformLayer's children, and sets that - // TransformLayer as the single child of this OpacityLayer. - void EnsureSingleChild(); - FML_DISALLOW_COPY_AND_ASSIGN(OpacityLayer); }; diff --git a/flow/layers/opacity_layer_unittests.cc b/flow/layers/opacity_layer_unittests.cc index 6ca78a8538fdf..8410d7767c890 100644 --- a/flow/layers/opacity_layer_unittests.cc +++ b/flow/layers/opacity_layer_unittests.cc @@ -20,7 +20,7 @@ TEST_F(OpacityLayerTest, LeafLayer) { std::make_shared(SK_AlphaOPAQUE, SkPoint::Make(0.0f, 0.0f)); EXPECT_DEATH_IF_SUPPORTED(layer->Preroll(preroll_context(), SkMatrix()), - "layers\\(\\)\\.size\\(\\) > 0"); + "\\!container->layers\\(\\)\\.empty\\(\\)"); } TEST_F(OpacityLayerTest, PaintingEmptyLayerDies) { From c4df6dfeedb3ee2885aca1b786d3023050c501b4 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Tue, 26 Nov 2019 14:02:52 -0800 Subject: [PATCH 5/6] Centralize system-composite and elevation logic --- ci/licenses_golden/licenses_flutter | 4 ++ flow/BUILD.gn | 4 ++ flow/layers/elevated_container_layer.cc | 49 +++++++++++++++++++ flow/layers/elevated_container_layer.h | 34 +++++++++++++ .../layers/fuchsia_system_composited_layer.cc | 41 ++++++++++++++++ flow/layers/fuchsia_system_composited_layer.h | 34 +++++++++++++ flow/layers/physical_shape_layer.cc | 42 ++++++++-------- flow/layers/physical_shape_layer.h | 41 +++++++++++----- flow/scene_update_context.cc | 19 +------ flow/scene_update_context.h | 3 +- 10 files changed, 219 insertions(+), 52 deletions(-) create mode 100644 flow/layers/elevated_container_layer.cc create mode 100644 flow/layers/elevated_container_layer.h create mode 100644 flow/layers/fuchsia_system_composited_layer.cc create mode 100644 flow/layers/fuchsia_system_composited_layer.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 299844a2a98a4..7eec11ad58215 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -48,6 +48,10 @@ FILE: ../../../flutter/flow/layers/color_filter_layer_unittests.cc FILE: ../../../flutter/flow/layers/container_layer.cc FILE: ../../../flutter/flow/layers/container_layer.h FILE: ../../../flutter/flow/layers/container_layer_unittests.cc +FILE: ../../../flutter/flow/layers/elevated_container_layer.cc +FILE: ../../../flutter/flow/layers/elevated_container_layer.h +FILE: ../../../flutter/flow/layers/fuchsia_system_composited_layer.cc +FILE: ../../../flutter/flow/layers/fuchsia_system_composited_layer.h FILE: ../../../flutter/flow/layers/layer.cc FILE: ../../../flutter/flow/layers/layer.h FILE: ../../../flutter/flow/layers/layer_tree.cc diff --git a/flow/BUILD.gn b/flow/BUILD.gn index 137aa76d57811..f3ed537fe9113 100644 --- a/flow/BUILD.gn +++ b/flow/BUILD.gn @@ -28,6 +28,8 @@ source_set("flow") { "layers/color_filter_layer.h", "layers/container_layer.cc", "layers/container_layer.h", + "layers/elevated_container_layer.cc", + "layers/elevated_container_layer.h", "layers/layer.cc", "layers/layer.h", "layers/layer_tree.cc", @@ -76,6 +78,8 @@ source_set("flow") { sources += [ "layers/child_scene_layer.cc", "layers/child_scene_layer.h", + "layers/fuchsia_system_composited_layer.cc", + "layers/fuchsia_system_composited_layer.h", "scene_update_context.cc", "scene_update_context.h", "view_holder.cc", diff --git a/flow/layers/elevated_container_layer.cc b/flow/layers/elevated_container_layer.cc new file mode 100644 index 0000000000000..cd68b06713992 --- /dev/null +++ b/flow/layers/elevated_container_layer.cc @@ -0,0 +1,49 @@ +// 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/elevated_container_layer.h" + +namespace flutter { +namespace { + +float ClampElevation(float elevation, + float parent_elevation, + float max_elevation) { + // TODO(mklim): Deal with bounds overflow more elegantly. We'd like to be + // able to have developers specify the behavior here to alternatives besides + // clamping, like normalization on some arbitrary curve. + float clamped_elevation = elevation; + if (max_elevation > -1 && (parent_elevation + elevation) > max_elevation) { + // Clamp the local z coordinate at our max bound. Take into account the + // parent z position here to fix clamping in cases where the child is + // overflowing because of its parents. + clamped_elevation = max_elevation - parent_elevation; + } + + return clamped_elevation; +} + +} // namespace + +ElevatedContainerLayer::ElevatedContainerLayer(float elevation) + : elevation_(elevation), clamped_elevation_(elevation) {} + +void ElevatedContainerLayer::Preroll(PrerollContext* context, + const SkMatrix& matrix) { + TRACE_EVENT0("flutter", "ElevatedContainerLayer::Preroll"); + + // Track total elevation as we walk the tree, in order to deal with bounds + // overflow in z. + parent_elevation_ = context->total_elevation; + clamped_elevation_ = ClampElevation(elevation_, parent_elevation_, + context->frame_physical_depth); + context->total_elevation += clamped_elevation_; + + ContainerLayer::Preroll(context, matrix); + + // Restore the elevation for our parent. + context->total_elevation = parent_elevation_; +} + +} // namespace flutter diff --git a/flow/layers/elevated_container_layer.h b/flow/layers/elevated_container_layer.h new file mode 100644 index 0000000000000..9c7a8b051f118 --- /dev/null +++ b/flow/layers/elevated_container_layer.h @@ -0,0 +1,34 @@ +// 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. + +#ifndef FLUTTER_FLOW_LAYERS_ELEVATED_CONTAINER_LAYER_H_ +#define FLUTTER_FLOW_LAYERS_ELEVATED_CONTAINER_LAYER_H_ + +#include "flutter/flow/layers/container_layer.h" + +namespace flutter { + +class ElevatedContainerLayer : public ContainerLayer { + public: + ElevatedContainerLayer(float elevation); + ~ElevatedContainerLayer() override = default; + + void Preroll(PrerollContext* context, const SkMatrix& matrix) override; + + float elevation() const { return clamped_elevation_; } + float total_elevation() const { + return parent_elevation_ + clamped_elevation_; + } + + private: + float parent_elevation_ = 0.0f; + float elevation_ = 0.0f; + float clamped_elevation_ = 0.0f; + + FML_DISALLOW_COPY_AND_ASSIGN(ElevatedContainerLayer); +}; + +} // namespace flutter + +#endif // FLUTTER_FLOW_LAYERS_ELEVATED_CONTAINER_LAYER_H_ diff --git a/flow/layers/fuchsia_system_composited_layer.cc b/flow/layers/fuchsia_system_composited_layer.cc new file mode 100644 index 0000000000000..68209b0820b6d --- /dev/null +++ b/flow/layers/fuchsia_system_composited_layer.cc @@ -0,0 +1,41 @@ +// 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/fuchsia_system_composited_layer.h" + +namespace flutter { + +FuchsiaSystemCompositedLayer::FuchsiaSystemCompositedLayer(SkColor color, + float elevation) + : ElevatedContainerLayer(elevation), color_(color) {} + +void FuchsiaSystemCompositedLayer::UpdateScene(SceneUpdateContext& context) { + FML_DCHECK(needs_system_composite()); + + // Retained rendering: speedup by reusing a retained entity node if + // possible. When an entity node is reused, no paint layer is added to the + // frame so we won't call Paint. + LayerRasterCacheKey key(unique_id(), context.Matrix()); + if (context.HasRetainedNode(key)) { + TRACE_EVENT_INSTANT0("flutter", "retained layer cache hit"); + const scenic::EntityNode& retained_node = context.GetRetainedNode(key); + FML_DCHECK(context.top_entity()); + FML_DCHECK(retained_node.session() == context.session()); + context.top_entity()->embedder_node().AddChild(retained_node); + return; + } + + TRACE_EVENT_INSTANT0("flutter", "retained cache miss, creating"); + // If we can't find an existing retained surface, create one. + SceneUpdateContext::Frame frame(context, rrect_, color_, elevation(), this); + for (auto& layer : layers()) { + if (layer->needs_painting()) { + frame.AddPaintLayer(layer.get()); + } + } + + ContainerLayer::UpdateScene(context); +} + +} // namespace flutter diff --git a/flow/layers/fuchsia_system_composited_layer.h b/flow/layers/fuchsia_system_composited_layer.h new file mode 100644 index 0000000000000..f2ceeb2536b6e --- /dev/null +++ b/flow/layers/fuchsia_system_composited_layer.h @@ -0,0 +1,34 @@ +// 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. + +#ifndef FLUTTER_FLOW_LAYERS_FUCHSIA_SYSTEM_COMPOSITED_LAYER_H_ +#define FLUTTER_FLOW_LAYERS_FUCHSIA_SYSTEM_COMPOSITED_LAYER_H_ + +#include "flutter/flow/layers/elevated_container_layer.h" +#include "flutter/flow/scene_update_context.h" + +namespace flutter { + +class FuchsiaSystemCompositedLayer : public ElevatedContainerLayer { + public: + static bool can_system_composite() { return true; } + + FuchsiaSystemCompositedLayer(SkColor color, float elevation); + + void UpdateScene(SceneUpdateContext& context) override; + + void set_dimensions(SkRRect rrect) { rrect_ = rrect; } + + SkColor color() const { return color_; } + + private: + SkRRect rrect_ = SkRRect::MakeEmpty(); + SkColor color_ = SK_ColorTRANSPARENT; + + FML_DISALLOW_COPY_AND_ASSIGN(FuchsiaSystemCompositedLayer); +}; + +} // namespace flutter + +#endif // FLUTTER_FLOW_LAYERS_FUCHSIA_SYSTEM_COMPOSITED_LAYER_H_ diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index c5012b8bb6489..5fa5dc8076620 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -17,9 +17,8 @@ PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, float elevation, const SkPath& path, Clip clip_behavior) - : color_(color), + : PhysicalShapeLayerBase(color, elevation), shadow_color_(shadow_color), - elevation_(elevation), path_(path), isRect_(false), clip_behavior_(clip_behavior) { @@ -42,30 +41,33 @@ PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, // an SkPath. frameRRect_ = SkRRect::MakeRect(path.getBounds()); } + + set_dimensions(frameRRect_); } void PhysicalShapeLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { TRACE_EVENT0("flutter", "PhysicalShapeLayer::Preroll"); - context->total_elevation += elevation_; - total_elevation_ = context->total_elevation; - SkRect child_paint_bounds; - PrerollChildren(context, matrix, &child_paint_bounds); - context->total_elevation -= elevation_; - if (elevation_ == 0) { + PhysicalShapeLayerBase::Preroll(context, matrix); + + if (elevation() == 0) { set_paint_bounds(path_.getBounds()); } else { -#if defined(OS_FUCHSIA) - // Let the system compositor draw all shadows for us. - set_needs_system_composite(true); -#else + if (PhysicalShapeLayerBase::can_system_composite()) { + set_needs_system_composite(true); + return; + } +//#if defined(OS_FUCHSIA) +// // Let the system compositor draw all shadows for us. +// set_needs_system_composite(true); +//#else // We will draw the shadow in Paint(), so add some margin to the paint // bounds to leave space for the shadow. We fill this whole region and clip // children to it so we don't need to join the child paint bounds. - set_paint_bounds(ComputeShadowBounds(path_.getBounds(), elevation_, + set_paint_bounds(ComputeShadowBounds(path_.getBounds(), elevation(), context->frame_device_pixel_ratio)); -#endif // defined(OS_FUCHSIA) +//#endif // defined(OS_FUCHSIA) } } @@ -90,8 +92,8 @@ void PhysicalShapeLayer::UpdateScene(SceneUpdateContext& context) { TRACE_EVENT_INSTANT0("flutter", "cache miss, creating"); // If we can't find an existing retained surface, create one. - SceneUpdateContext::Frame frame(context, frameRRect_, color_, elevation_, - total_elevation_, this); + SceneUpdateContext::Frame frame(context, frameRRect_, color(), elevation(), + this); for (auto& layer : layers()) { if (layer->needs_painting()) { frame.AddPaintLayer(layer.get()); @@ -107,14 +109,14 @@ void PhysicalShapeLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "PhysicalShapeLayer::Paint"); FML_DCHECK(needs_painting()); - if (elevation_ != 0) { - DrawShadow(context.leaf_nodes_canvas, path_, shadow_color_, elevation_, - SkColorGetA(color_) != 0xff, context.frame_device_pixel_ratio); + if (elevation() != 0) { + DrawShadow(context.leaf_nodes_canvas, path_, shadow_color_, elevation(), + SkColorGetA(color()) != 0xff, context.frame_device_pixel_ratio); } // Call drawPath without clip if possible for better performance. SkPaint paint; - paint.setColor(color_); + paint.setColor(color()); paint.setAntiAlias(true); if (clip_behavior_ != Clip::antiAliasWithSaveLayer) { context.leaf_nodes_canvas->drawPath(path_, paint); diff --git a/flow/layers/physical_shape_layer.h b/flow/layers/physical_shape_layer.h index 38af3e45d4e27..63c24e05d7d68 100644 --- a/flow/layers/physical_shape_layer.h +++ b/flow/layers/physical_shape_layer.h @@ -5,18 +5,33 @@ #ifndef FLUTTER_FLOW_LAYERS_PHYSICAL_SHAPE_LAYER_H_ #define FLUTTER_FLOW_LAYERS_PHYSICAL_SHAPE_LAYER_H_ -#include "flutter/flow/layers/container_layer.h" +#include "flutter/flow/layers/elevated_container_layer.h" +#if defined(OS_FUCHSIA) +#include "flutter/flow/layers/fuchsia_system_composited_layer.h" +#endif namespace flutter { -class PhysicalShapeLayer : public ContainerLayer { +#if !defined(OS_FUCHSIA) +class PhysicalShapeLayerBase : public ElevatedContainerLayer { public: - PhysicalShapeLayer(SkColor color, - SkColor shadow_color, - float elevation, - const SkPath& path, - Clip clip_behavior); + static bool can_system_composite() { return false; } + + PhysicalShapeLayerBase(SkColor color, float elevation) + : ElevatedContainerLayer(elevation), color_(color) {} + + void set_dimensions(SkRRect rrect) { } + SkColor color() const { return color_; } + + private: + SkColor color_; +}; +#else +using PhysicalShapeLayerBase = FuchsiaSystemCompositedLayer; +#endif +class PhysicalShapeLayer : public PhysicalShapeLayerBase { + public: static SkRect ComputeShadowBounds(const SkRect& bounds, float elevation, float pixel_ratio); @@ -27,21 +42,21 @@ class PhysicalShapeLayer : public ContainerLayer { bool transparentOccluder, SkScalar dpr); - void Preroll(PrerollContext* context, const SkMatrix& matrix) override; +PhysicalShapeLayer(SkColor color, + SkColor shadow_color, + float elevation, + const SkPath& path, + Clip clip_behavior); + void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void Paint(PaintContext& context) const override; #if defined(OS_FUCHSIA) void UpdateScene(SceneUpdateContext& context) override; #endif // defined(OS_FUCHSIA) - float total_elevation() const { return total_elevation_; } - private: - SkColor color_; SkColor shadow_color_; - float elevation_ = 0.0f; - float total_elevation_ = 0.0f; SkPath path_; bool isRect_; SkRRect frameRRect_; diff --git a/flow/scene_update_context.cc b/flow/scene_update_context.cc index e6d74935f6e83..c52f1ca9eb4ae 100644 --- a/flow/scene_update_context.cc +++ b/flow/scene_update_context.cc @@ -301,29 +301,14 @@ SceneUpdateContext::Shape::Shape(SceneUpdateContext& context) SceneUpdateContext::Frame::Frame(SceneUpdateContext& context, const SkRRect& rrect, SkColor color, - float local_elevation, - float world_elevation, + float elevation, Layer* layer) : Shape(context), rrect_(rrect), color_(color), paint_bounds_(SkRect::MakeEmpty()), layer_(layer) { - const float depth = context.frame_physical_depth(); - if (depth > -1 && world_elevation > depth) { - // TODO(mklim): Deal with bounds overflow more elegantly. We'd like to be - // able to have developers specify the behavior here to alternatives besides - // clamping, like normalization on some arbitrary curve. - - // Clamp the local z coordinate at our max bound. Take into account the - // parent z position here to fix clamping in cases where the child is - // overflowing because of its parents. - const float parent_elevation = world_elevation - local_elevation; - local_elevation = depth - parent_elevation; - } - if (local_elevation != 0.0) { - entity_node().SetTranslation(0.f, 0.f, -local_elevation); - } + entity_node().SetTranslation(0.f, 0.f, -elevation); } SceneUpdateContext::Frame::~Frame() { diff --git a/flow/scene_update_context.h b/flow/scene_update_context.h index 989fa83fb21bf..6961d6f72c554 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -115,8 +115,7 @@ class SceneUpdateContext { Frame(SceneUpdateContext& context, const SkRRect& rrect, SkColor color, - float local_elevation = 0.0f, - float parent_elevation = 0.0f, + float elevation = 0.0f, Layer* layer = nullptr); virtual ~Frame(); From ea67e5b0b930ebf552bc7dbd678a35ee6a129c39 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Tue, 26 Nov 2019 15:49:59 -0800 Subject: [PATCH 6/6] Wire up OpacityLayer to Scenic --- flow/layers/child_scene_layer.cc | 17 +++- .../layers/fuchsia_system_composited_layer.cc | 20 ++++- flow/layers/fuchsia_system_composited_layer.h | 5 +- flow/layers/layer.h | 5 +- flow/layers/opacity_layer.cc | 71 +++++++++++---- flow/layers/opacity_layer.h | 41 +++++++-- flow/layers/physical_shape_layer.cc | 42 +-------- flow/layers/physical_shape_layer.h | 19 ++-- flow/layers/physical_shape_layer_unittests.cc | 25 ------ flow/scene_update_context.cc | 90 +++++++++---------- flow/scene_update_context.h | 38 ++++---- 11 files changed, 199 insertions(+), 174 deletions(-) diff --git a/flow/layers/child_scene_layer.cc b/flow/layers/child_scene_layer.cc index e5652a2c87889..10b5a277db575 100644 --- a/flow/layers/child_scene_layer.cc +++ b/flow/layers/child_scene_layer.cc @@ -20,10 +20,25 @@ ChildSceneLayer::ChildSceneLayer(zx_koid_t layer_id, void ChildSceneLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { TRACE_EVENT0("flutter", "ChildSceneLayer::Preroll"); set_needs_system_composite(true); + + // An alpha "hole punch" is required if the frame behind us is not opaque. + if (!context->is_opaque) { + set_paint_bounds( + SkRect::MakeXYWH(offset_.fX, offset_.fY, size_.fWidth, size_.fHeight)); + } } void ChildSceneLayer::Paint(PaintContext& context) const { - FML_NOTREACHED() << "This layer never needs painting."; + TRACE_EVENT0("flutter", "ChildSceneLayer::Paint"); + FML_DCHECK(needs_painting()); + + // If we are being rendered into our own frame using the system compositor, + // then it is neccesary to "punch a hole" in the canvas/frame behind us so + // that group opacity looks correct. + SkPaint paint; + paint.setColor(SK_ColorTRANSPARENT); + paint.setBlendMode(SkBlendMode::kSrc); + context.leaf_nodes_canvas->drawRect(paint_bounds(), paint); } void ChildSceneLayer::UpdateScene(SceneUpdateContext& context) { diff --git a/flow/layers/fuchsia_system_composited_layer.cc b/flow/layers/fuchsia_system_composited_layer.cc index 68209b0820b6d..8c4a1b2ae26e3 100644 --- a/flow/layers/fuchsia_system_composited_layer.cc +++ b/flow/layers/fuchsia_system_composited_layer.cc @@ -7,8 +7,21 @@ namespace flutter { FuchsiaSystemCompositedLayer::FuchsiaSystemCompositedLayer(SkColor color, + SkAlpha opacity, float elevation) - : ElevatedContainerLayer(elevation), color_(color) {} + : ElevatedContainerLayer(elevation), color_(color), opacity_(opacity) {} + +void FuchsiaSystemCompositedLayer::Preroll(PrerollContext* context, + const SkMatrix& matrix) { + TRACE_EVENT0("flutter", "FuchsiaSystemCompositedLayer::Preroll"); + + const float parent_is_opaque = context->is_opaque; + context->mutators_stack.PushOpacity(opacity_); + context->is_opaque = parent_is_opaque && (opacity_ == SK_AlphaOPAQUE); + ElevatedContainerLayer::Preroll(context, matrix); + context->is_opaque = parent_is_opaque; + context->mutators_stack.Pop(); +} void FuchsiaSystemCompositedLayer::UpdateScene(SceneUpdateContext& context) { FML_DCHECK(needs_system_composite()); @@ -28,14 +41,15 @@ void FuchsiaSystemCompositedLayer::UpdateScene(SceneUpdateContext& context) { TRACE_EVENT_INSTANT0("flutter", "retained cache miss, creating"); // If we can't find an existing retained surface, create one. - SceneUpdateContext::Frame frame(context, rrect_, color_, elevation(), this); + SceneUpdateContext::Frame frame(context, rrect_, color_, opacity_ / 255.0f, + elevation(), this); for (auto& layer : layers()) { if (layer->needs_painting()) { frame.AddPaintLayer(layer.get()); } } - ContainerLayer::UpdateScene(context); + ElevatedContainerLayer::UpdateScene(context); } } // namespace flutter diff --git a/flow/layers/fuchsia_system_composited_layer.h b/flow/layers/fuchsia_system_composited_layer.h index f2ceeb2536b6e..2fe00ee6d550f 100644 --- a/flow/layers/fuchsia_system_composited_layer.h +++ b/flow/layers/fuchsia_system_composited_layer.h @@ -14,17 +14,20 @@ class FuchsiaSystemCompositedLayer : public ElevatedContainerLayer { public: static bool can_system_composite() { return true; } - FuchsiaSystemCompositedLayer(SkColor color, float elevation); + FuchsiaSystemCompositedLayer(SkColor color, SkAlpha opacity, float elevation); + void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void UpdateScene(SceneUpdateContext& context) override; void set_dimensions(SkRRect rrect) { rrect_ = rrect; } SkColor color() const { return color_; } + SkAlpha opacity() const { return opacity_; } private: SkRRect rrect_ = SkRRect::MakeEmpty(); SkColor color_ = SK_ColorTRANSPARENT; + SkAlpha opacity_ = SK_AlphaOPAQUE; FML_DISALLOW_COPY_AND_ASSIGN(FuchsiaSystemCompositedLayer); }; diff --git a/flow/layers/layer.h b/flow/layers/layer.h index a9c4a3aa818ce..731509876b611 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -60,10 +60,11 @@ struct PrerollContext { float frame_physical_depth; float frame_device_pixel_ratio; - // These allow us to track properties like elevation and opacity which stack - // with each other during Preroll. + // These allow us to track properties like elevation, opacity, and the + // prescence of a platform view during Preroll. float total_elevation = 0.0f; bool has_platform_view = false; + bool is_opaque = true; }; // Represents a single composited layer. Created on the UI thread but then diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 810aa4e4c8752..5592f40f72f8c 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -9,8 +9,29 @@ namespace flutter { -OpacityLayer::OpacityLayer(int alpha, const SkPoint& offset) - : alpha_(alpha), offset_(offset) { +// The OpacityLayer has no real "elevation", but we want to avoid Z-fighting +// when using the system compositor. Choose a small but non-zero value for +// this. +constexpr float kOpacityElevationWhenUsingSystemCompositor = 0.01f; + +#if !defined(OS_FUCHSIA) +void OpacityLayerBase::Preroll(PrerollContext* context, + const SkMatrix& matrix) { + const float parent_is_opaque = context->is_opaque; + + context->mutators_stack.PushOpacity(opacity_); + context->is_opaque = parent_is_opaque && (opacity_ == SK_AlphaOPAQUE); + ContainerLayer::Preroll(context, matrix); + context->is_opaque = parent_is_opaque; + context->mutators_stack.Pop(); +} +#endif + +OpacityLayer::OpacityLayer(SkAlpha opacity, const SkPoint& offset) + : OpacityLayerBase(SK_ColorTRANSPARENT, + opacity, + kOpacityElevationWhenUsingSystemCompositor), + offset_(offset) { // Ensure OpacityLayer has only one direct child. // // This is needed to ensure that retained rendering can always be applied to @@ -31,32 +52,53 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { ContainerLayer* container = GetChildContainer(); FML_DCHECK(!container->layers().empty()); // OpacityLayer can't be a leaf. + // Factor in the offset during Preroll. |OpacityLayerBase| will handle the + // opacity. SkMatrix child_matrix = matrix; child_matrix.postTranslate(offset_.fX, offset_.fY); context->mutators_stack.PushTransform( SkMatrix::MakeTrans(offset_.fX, offset_.fY)); - context->mutators_stack.PushOpacity(alpha_); - ContainerLayer::Preroll(context, child_matrix); - context->mutators_stack.Pop(); + OpacityLayerBase::Preroll(context, child_matrix); context->mutators_stack.Pop(); - set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY)); - if (!context->has_platform_view && context->raster_cache && - SkRect::Intersects(context->cull_rect, paint_bounds())) { - SkMatrix ctm = child_matrix; + // When using the system compositor, do not include the offset since we are + // rendering as a separate piece of geometry and the offset will be baked into + // that geometry's transform. + if (OpacityLayerBase::can_system_composite()) { + set_dimensions(SkRRect::MakeRect(paint_bounds())); + set_needs_system_composite(true); + } else { + set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY)); + + if (!context->has_platform_view && context->raster_cache && + SkRect::Intersects(context->cull_rect, paint_bounds())) { + SkMatrix ctm = child_matrix; #ifndef SUPPORT_FRACTIONAL_TRANSLATION - ctm = RasterCache::GetIntegralTransCTM(ctm); + ctm = RasterCache::GetIntegralTransCTM(ctm); #endif - context->raster_cache->Prepare(context, container, ctm); + context->raster_cache->Prepare(context, container, ctm); + } } } +#if defined(OS_FUCHSIA) + +void OpacityLayer::UpdateScene(SceneUpdateContext& context) { + SceneUpdateContext::Transform transform( + context, SkMatrix::MakeTrans(offset_.fX, offset_.fY)); + + // OpacityLayerBase will handle applying the opacity itself. + OpacityLayerBase::UpdateScene(context); +} + +#endif + void OpacityLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "OpacityLayer::Paint"); FML_DCHECK(needs_painting()); SkPaint paint; - paint.setAlpha(alpha_); + paint.setAlpha(opacity()); SkAutoCanvasRestore save(context.internal_nodes_canvas, true); context.internal_nodes_canvas->translate(offset_.fX, offset_.fY); @@ -83,8 +125,7 @@ void OpacityLayer::Paint(PaintContext& context) const { // RasterCache::GetIntegralTransCTM optimization. // // Note that the following lines are only accessible when the raster cache is - // not available (e.g., when we're using the software backend in golden - // tests). + // not available, or when a cache miss occurs. SkRect saveLayerBounds; paint_bounds() .makeOffset(-offset_.fX, -offset_.fY) @@ -92,7 +133,7 @@ void OpacityLayer::Paint(PaintContext& context) const { Layer::AutoSaveLayer save_layer = Layer::AutoSaveLayer::Create(context, saveLayerBounds, &paint); - PaintChildren(context); + OpacityLayerBase::Paint(context); } ContainerLayer* OpacityLayer::GetChildContainer() const { diff --git a/flow/layers/opacity_layer.h b/flow/layers/opacity_layer.h index 92ae4cd1eadc7..f4951e0557c70 100644 --- a/flow/layers/opacity_layer.h +++ b/flow/layers/opacity_layer.h @@ -5,15 +5,42 @@ #ifndef FLUTTER_FLOW_LAYERS_OPACITY_LAYER_H_ #define FLUTTER_FLOW_LAYERS_OPACITY_LAYER_H_ -#include "flutter/flow/layers/container_layer.h" +#include "flutter/flow/layers/elevated_container_layer.h" +#if defined(OS_FUCHSIA) +#include "flutter/flow/layers/fuchsia_system_composited_layer.h" +#endif namespace flutter { +#if !defined(OS_FUCHSIA) +class OpacityLayerBase : public ContainerLayer { + public: + static bool can_system_composite() { return false; } + + OpacityLayerBase(SkColor color, SkAlpha opacity, float elevation) + : color_(color), opacity_(opacity) {} + + void Preroll(PrerollContext* context, const SkMatrix& matrix) override; + + void set_dimensions(SkRRect rrect) {} + + SkColor color() const { return color_; } + SkAlpha opacity() const { return opacity_; } + float elevation() const { return 0; } + + private: + SkColor color_; + SkAlpha opacity_; +}; +#else +using OpacityLayerBase = FuchsiaSystemCompositedLayer; +#endif + // Don't add an OpacityLayer with no children to the layer tree. Painting an // OpacityLayer is very costly due to the saveLayer call. If there's no child, // having the OpacityLayer or not has the same effect. In debug_unopt build, // |Preroll| will assert if there are no children. -class OpacityLayer : public ContainerLayer { +class OpacityLayer : public OpacityLayerBase { public: // An offset is provided here because OpacityLayer.addToScene method in the // Flutter framework can take an optional offset argument. @@ -25,21 +52,19 @@ class OpacityLayer : public ContainerLayer { // the retained rendering inefficient as a small offset change could propagate // to many leaf layers. Therefore we try to capture that offset here to stop // the propagation as repainting the OpacityLayer is expensive. - OpacityLayer(int alpha, const SkPoint& offset); + OpacityLayer(SkAlpha alpha, const SkPoint& offset); void Add(std::shared_ptr layer) override; void Preroll(PrerollContext* context, const SkMatrix& matrix) override; - +#if defined(OS_FUCHSIA) + void UpdateScene(SceneUpdateContext& context) override; +#endif void Paint(PaintContext& context) const override; - // TODO(chinmaygarde): Once SCN-139 is addressed, introduce a new node in the - // session scene hierarchy. - private: ContainerLayer* GetChildContainer() const; - int alpha_; SkPoint offset_; FML_DISALLOW_COPY_AND_ASSIGN(OpacityLayer); diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 5fa5dc8076620..fed7a5fbe7163 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -17,7 +17,7 @@ PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, float elevation, const SkPath& path, Clip clip_behavior) - : PhysicalShapeLayerBase(color, elevation), + : PhysicalShapeLayerBase(color, SK_AlphaOPAQUE, elevation), shadow_color_(shadow_color), path_(path), isRect_(false), @@ -58,53 +58,15 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, set_needs_system_composite(true); return; } -//#if defined(OS_FUCHSIA) -// // Let the system compositor draw all shadows for us. -// set_needs_system_composite(true); -//#else + // We will draw the shadow in Paint(), so add some margin to the paint // bounds to leave space for the shadow. We fill this whole region and clip // children to it so we don't need to join the child paint bounds. set_paint_bounds(ComputeShadowBounds(path_.getBounds(), elevation(), context->frame_device_pixel_ratio)); -//#endif // defined(OS_FUCHSIA) } } -#if defined(OS_FUCHSIA) - -void PhysicalShapeLayer::UpdateScene(SceneUpdateContext& context) { - FML_DCHECK(needs_system_composite()); - TRACE_EVENT0("flutter", "PhysicalShapeLayer::UpdateScene"); - - // Retained rendering: speedup by reusing a retained entity node if possible. - // When an entity node is reused, no paint layer is added to the frame so we - // won't call PhysicalShapeLayer::Paint. - LayerRasterCacheKey key(unique_id(), context.Matrix()); - if (context.HasRetainedNode(key)) { - TRACE_EVENT_INSTANT0("flutter", "retained layer cache hit"); - const scenic::EntityNode& retained_node = context.GetRetainedNode(key); - FML_DCHECK(context.top_entity()); - FML_DCHECK(retained_node.session() == context.session()); - context.top_entity()->entity_node().AddChild(retained_node); - return; - } - - TRACE_EVENT_INSTANT0("flutter", "cache miss, creating"); - // If we can't find an existing retained surface, create one. - SceneUpdateContext::Frame frame(context, frameRRect_, color(), elevation(), - this); - for (auto& layer : layers()) { - if (layer->needs_painting()) { - frame.AddPaintLayer(layer.get()); - } - } - - UpdateSceneChildren(context); -} - -#endif // defined(OS_FUCHSIA) - void PhysicalShapeLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "PhysicalShapeLayer::Paint"); FML_DCHECK(needs_painting()); diff --git a/flow/layers/physical_shape_layer.h b/flow/layers/physical_shape_layer.h index 63c24e05d7d68..ac54a7bbc6761 100644 --- a/flow/layers/physical_shape_layer.h +++ b/flow/layers/physical_shape_layer.h @@ -6,29 +6,24 @@ #define FLUTTER_FLOW_LAYERS_PHYSICAL_SHAPE_LAYER_H_ #include "flutter/flow/layers/elevated_container_layer.h" -#if defined(OS_FUCHSIA) -#include "flutter/flow/layers/fuchsia_system_composited_layer.h" -#endif namespace flutter { -#if !defined(OS_FUCHSIA) class PhysicalShapeLayerBase : public ElevatedContainerLayer { public: static bool can_system_composite() { return false; } - PhysicalShapeLayerBase(SkColor color, float elevation) + PhysicalShapeLayerBase(SkColor color, SkAlpha opacity, float elevation) : ElevatedContainerLayer(elevation), color_(color) {} - void set_dimensions(SkRRect rrect) { } + void set_dimensions(SkRRect rrect) {} + SkColor color() const { return color_; } + SkAlpha opacity() const { return SK_AlphaOPAQUE; } private: SkColor color_; }; -#else -using PhysicalShapeLayerBase = FuchsiaSystemCompositedLayer; -#endif class PhysicalShapeLayer : public PhysicalShapeLayerBase { public: @@ -42,7 +37,7 @@ class PhysicalShapeLayer : public PhysicalShapeLayerBase { bool transparentOccluder, SkScalar dpr); -PhysicalShapeLayer(SkColor color, + PhysicalShapeLayer(SkColor color, SkColor shadow_color, float elevation, const SkPath& path, @@ -51,10 +46,6 @@ PhysicalShapeLayer(SkColor color, void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void Paint(PaintContext& context) const override; -#if defined(OS_FUCHSIA) - void UpdateScene(SceneUpdateContext& context) override; -#endif // defined(OS_FUCHSIA) - private: SkColor shadow_color_; SkPath path_; diff --git a/flow/layers/physical_shape_layer_unittests.cc b/flow/layers/physical_shape_layer_unittests.cc index 3a5a13d25ec6a..36107560fa4ed 100644 --- a/flow/layers/physical_shape_layer_unittests.cc +++ b/flow/layers/physical_shape_layer_unittests.cc @@ -124,24 +124,13 @@ TEST_F(PhysicalShapeLayerTest, ElevationSimple) { SK_ColorGREEN, SK_ColorBLACK, initial_elevation, layer_path, Clip::none); layer->Preroll(preroll_context(), SkMatrix()); - // The Fuchsia system compositor handles all elevated PhysicalShapeLayers and - // their shadows , so we do not do any painting there. -#if defined(OS_FUCHSIA) - EXPECT_EQ(layer->paint_bounds(), kEmptyRect); - EXPECT_FALSE(layer->needs_painting()); - EXPECT_TRUE(layer->needs_system_composite()); -#else EXPECT_EQ(layer->paint_bounds(), PhysicalShapeLayer::ComputeShadowBounds(layer_path.getBounds(), initial_elevation, 1.0f)); EXPECT_TRUE(layer->needs_painting()); EXPECT_FALSE(layer->needs_system_composite()); -#endif EXPECT_EQ(layer->total_elevation(), initial_elevation); - // The Fuchsia system compositor handles all elevated PhysicalShapeLayers and - // their shadows , so we do not use the direct |Paint()| path there. -#if !defined(OS_FUCHSIA) SkPaint layer_paint; layer_paint.setColor(SK_ColorGREEN); layer_paint.setAntiAlias(true); @@ -152,7 +141,6 @@ TEST_F(PhysicalShapeLayerTest, ElevationSimple) { {MockCanvas::DrawCall{0, MockCanvas::DrawShadowData{layer_path}}, MockCanvas::DrawCall{ 0, MockCanvas::DrawPathData{layer_path, layer_paint}}})); -#endif } TEST_F(PhysicalShapeLayerTest, ElevationComplex) { @@ -184,27 +172,15 @@ TEST_F(PhysicalShapeLayerTest, ElevationComplex) { layers[0]->Preroll(preroll_context(), SkMatrix()); for (int i = 0; i < 4; i += 1) { - // On Fuchsia, the system compositor handles all elevated - // PhysicalShapeLayers and their shadows , so we do not do any painting - // there. -#if defined(OS_FUCHSIA) - EXPECT_EQ(layers[i]->paint_bounds(), kEmptyRect); - EXPECT_FALSE(layers[i]->needs_painting()); - EXPECT_TRUE(layers[i]->needs_system_composite()); -#else EXPECT_EQ(layers[i]->paint_bounds(), (PhysicalShapeLayer::ComputeShadowBounds( layer_path.getBounds(), initial_elevations[i], 1.0f /* pixel_ratio */))); EXPECT_TRUE(layers[i]->needs_painting()); EXPECT_FALSE(layers[i]->needs_system_composite()); -#endif EXPECT_EQ(layers[i]->total_elevation(), total_elevations[i]); } - // The Fuchsia system compositor handles all elevated PhysicalShapeLayers and - // their shadows , so we do not use the direct |Paint()| path there. -#if !defined(OS_FUCHSIA) SkPaint layer_paint; layer_paint.setColor(SK_ColorBLACK); layer_paint.setAntiAlias(true); @@ -224,7 +200,6 @@ TEST_F(PhysicalShapeLayerTest, ElevationComplex) { MockCanvas::DrawCall{0, MockCanvas::DrawShadowData{layer_path}}, MockCanvas::DrawCall{ 0, MockCanvas::DrawPathData{layer_path, layer_paint}}})); -#endif } } // namespace testing diff --git a/flow/scene_update_context.cc b/flow/scene_update_context.cc index c52f1ca9eb4ae..5ffcc4b272ce9 100644 --- a/flow/scene_update_context.cc +++ b/flow/scene_update_context.cc @@ -7,6 +7,7 @@ #include "flutter/flow/layers/layer.h" #include "flutter/flow/matrix_decomposition.h" #include "flutter/fml/trace_event.h" +#include "include/core/SkColor.h" namespace flutter { @@ -59,18 +60,16 @@ void SceneUpdateContext::CreateFrame(scenic::EntityNode entity_node, scenic::ShapeNode shape_node, const SkRRect& rrect, SkColor color, + float opacity, const SkRect& paint_bounds, std::vector paint_layers, Layer* layer) { - // Frames always clip their children. - SetEntityNodeClipPlanes(entity_node, rrect.getBounds()); - // TODO(SCN-1274): SetClip() will be deleted. - entity_node.SetClip(0u, true /* clip to self */); - // We don't need a shape if the frame is zero size. if (rrect.isEmpty()) return; + SetEntityNodeClipPlanes(entity_node, rrect.getBounds()); + // isEmpty should account for this, but we are adding these experimental // checks to validate if this is the root cause for b/144933519. if (std::isnan(rrect.width()) || std::isnan(rrect.height())) { @@ -102,7 +101,9 @@ void SceneUpdateContext::CreateFrame(scenic::EntityNode entity_node, // Check whether a solid color will suffice. if (paint_layers.empty()) { - SetShapeColor(shape_node, color); + scenic::Material material(session_); + SetMaterialColor(material, color, opacity); + shape_node.SetMaterial(material); return; } @@ -110,43 +111,38 @@ void SceneUpdateContext::CreateFrame(scenic::EntityNode entity_node, const float scale_x = ScaleX(); const float scale_y = ScaleY(); - // Apply a texture to the whole shape. - SetShapeTextureAndColor(shape_node, color, scale_x, scale_y, shape_bounds, - std::move(paint_layers), layer, - std::move(entity_node)); -} - -void SceneUpdateContext::SetShapeTextureAndColor( - scenic::ShapeNode& shape_node, - SkColor color, - SkScalar scale_x, - SkScalar scale_y, - const SkRect& paint_bounds, - std::vector paint_layers, - Layer* layer, - scenic::EntityNode entity_node) { scenic::Image* image = GenerateImageIfNeeded( - color, scale_x, scale_y, paint_bounds, std::move(paint_layers), layer, + color, scale_x, scale_y, shape_bounds, std::move(paint_layers), layer, std::move(entity_node)); if (image != nullptr) { scenic::Material material(session_); + + // The final shape's color is material_color * texture_color. The passed in + // material color was already used as a background when generating the + // texture, so set the model color to |SK_ColorWHITE| in order to allow + // using the texture's color unmodified. + SetMaterialColor(material, SK_ColorWHITE, opacity); material.SetTexture(*image); shape_node.SetMaterial(material); return; } - SetShapeColor(shape_node, color); -} + // No texture was needed, so apply a solid color to the whole shape. + if (SkColorGetA(color) != 0 && opacity != 0.0f) { + scenic::Material material(session_); -void SceneUpdateContext::SetShapeColor(scenic::ShapeNode& shape_node, - SkColor color) { - if (SkColorGetA(color) == 0) + SetMaterialColor(material, color, opacity); + shape_node.SetMaterial(material); return; + } +} - scenic::Material material(session_); +void SceneUpdateContext::SetMaterialColor(scenic::Material& material, + SkColor color, + float opacity) { + const SkAlpha color_alpha = (SkAlpha)(SkColorGetA(color) * opacity); material.SetColor(SkColorGetR(color), SkColorGetG(color), SkColorGetB(color), - SkColorGetA(color)); - shape_node.SetMaterial(material); + color_alpha); } scenic::Image* SceneUpdateContext::GenerateImageIfNeeded( @@ -235,6 +231,7 @@ SceneUpdateContext::Entity::Entity(SceneUpdateContext& context) entity_node_(context.session()) { if (previous_entity_) previous_entity_->embedder_node().AddChild(entity_node_); + context.top_entity_ = this; } @@ -293,28 +290,37 @@ SceneUpdateContext::Transform::~Transform() { context().top_scale_y_ = previous_scale_y_; } -SceneUpdateContext::Shape::Shape(SceneUpdateContext& context) - : Entity(context), shape_node_(context.session()) { - entity_node().AddChild(shape_node_); +SceneUpdateContext::Clip::Clip(SceneUpdateContext& context, + const SkRect& shape_bounds) + : Entity(context) { + SetEntityNodeClipPlanes(entity_node(), shape_bounds); } SceneUpdateContext::Frame::Frame(SceneUpdateContext& context, const SkRRect& rrect, SkColor color, + float opacity, float elevation, Layer* layer) - : Shape(context), + : Entity(context), + opacity_node_(context.session()), + shape_node_(context.session()), + layer_(layer), rrect_(rrect), - color_(color), paint_bounds_(SkRect::MakeEmpty()), - layer_(layer) { + color_(color), + opacity_(opacity) { entity_node().SetTranslation(0.f, 0.f, -elevation); + + entity_node().AddChild(shape_node_); + entity_node().AddChild(opacity_node_); + opacity_node_.SetOpacity(opacity_); } SceneUpdateContext::Frame::~Frame() { - context().CreateFrame(std::move(entity_node()), std::move(shape_node()), - rrect_, color_, paint_bounds_, std::move(paint_layers_), - layer_); + context().CreateFrame(std::move(entity_node()), std::move(shape_node_), + rrect_, color_, opacity_, paint_bounds_, + std::move(paint_layers_), layer_); } void SceneUpdateContext::Frame::AddPaintLayer(Layer* layer) { @@ -323,10 +329,4 @@ void SceneUpdateContext::Frame::AddPaintLayer(Layer* layer) { paint_bounds_.join(layer->paint_bounds()); } -SceneUpdateContext::Clip::Clip(SceneUpdateContext& context, - const SkRect& shape_bounds) - : Entity(context) { - SetEntityNodeClipPlanes(entity_node(), shape_bounds); -} - } // namespace flutter diff --git a/flow/scene_update_context.h b/flow/scene_update_context.h index 6961d6f72c554..c992fa20bba36 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -89,25 +89,20 @@ class SceneUpdateContext { float scale_x, float scale_y, float scale_z); - virtual ~Transform(); + ~Transform() override; private: float const previous_scale_x_; float const previous_scale_y_; }; - class Shape : public Entity { + class Clip : public Entity { public: - Shape(SceneUpdateContext& context); - virtual ~Shape() = default; - - scenic::ShapeNode& shape_node() { return shape_node_; } - - private: - scenic::ShapeNode shape_node_; + Clip(SceneUpdateContext& context, const SkRect& shape_bounds); + ~Clip() override = default; }; - class Frame : public Shape { + class Frame : public Entity { public: // When layer is not nullptr, the frame is associated with a layer subtree // rooted with that layer. The frame may then create a surface that will be @@ -115,25 +110,25 @@ class SceneUpdateContext { Frame(SceneUpdateContext& context, const SkRRect& rrect, SkColor color, + float opacity = 1.0f, float elevation = 0.0f, Layer* layer = nullptr); - virtual ~Frame(); + ~Frame() override; + scenic::ContainerNode& embedder_node() override { return opacity_node_; } void AddPaintLayer(Layer* layer); private: - const SkRRect rrect_; - SkColor const color_; + scenic::OpacityNodeHACK opacity_node_; + scenic::ShapeNode shape_node_; std::vector paint_layers_; - SkRect paint_bounds_; Layer* layer_; - }; - class Clip : public Entity { - public: - Clip(SceneUpdateContext& context, const SkRect& shape_bounds); - ~Clip() = default; + SkRRect rrect_; + SkRect paint_bounds_; + SkColor color_; + float opacity_; }; SceneUpdateContext(scenic::Session* session, @@ -206,6 +201,7 @@ class SceneUpdateContext { scenic::ShapeNode shape_node, const SkRRect& rrect, SkColor color, + float opacity, const SkRect& paint_bounds, std::vector paint_layers, Layer* layer); @@ -217,7 +213,9 @@ class SceneUpdateContext { std::vector paint_layers, Layer* layer, scenic::EntityNode entity_node); - void SetShapeColor(scenic::ShapeNode& shape_node, SkColor color); + void SetMaterialColor(scenic::Material& material, + SkColor color, + float opacity); scenic::Image* GenerateImageIfNeeded(SkColor color, SkScalar scale_x, SkScalar scale_y,