From 7da39ea87d26fd8803a6fe0111ccd31e047e1326 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Wed, 18 Dec 2019 15:14:22 -0800 Subject: [PATCH] Reland Wire up Opacity on Fuchsia This reverts commit 6ea69a0d4339dd153899bb6c299689f1dd43329d. On top of the revert, it reverted a commit in the PR: https://github.com/flutter/engine/pull/14024 This reverts commit ea67e5b0b930ebf552bc7dbd678a35ee6a129c39. --- ci/licenses_golden/licenses_flutter | 4 + flow/BUILD.gn | 4 + flow/layers/container_layer.cc | 3 +- flow/layers/container_layer.h | 2 +- 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/layer.cc | 7 +- flow/layers/layer.h | 22 +++--- flow/layers/layer_tree.cc | 62 +++++++++++----- flow/layers/layer_tree.h | 20 +++-- flow/layers/layer_tree_unittests.cc | 19 +++-- flow/layers/opacity_layer.cc | 58 +++++++-------- flow/layers/opacity_layer.h | 18 ++--- flow/layers/opacity_layer_unittests.cc | 2 +- flow/layers/physical_shape_layer.cc | 51 ++++++------- flow/layers/physical_shape_layer.h | 47 +++++++----- flow/layers/physical_shape_layer_unittests.cc | 31 ++------ flow/raster_cache.cc | 4 +- flow/scene_update_context.cc | 23 ++---- flow/scene_update_context.h | 19 ++++- flow/testing/layer_test.h | 9 ++- flow/view_holder.cc | 20 ++--- flow/view_holder.h | 4 - lib/ui/compositing.dart | 11 --- lib/ui/compositing/scene.cc | 26 ++++--- lib/ui/compositing/scene.h | 2 +- lib/ui/compositing/scene_builder.cc | 73 ++++++------------- lib/ui/compositing/scene_builder.h | 13 ++-- lib/ui/compositing/scene_host.cc | 16 +--- lib/ui/compositing/scene_host.h | 11 +-- lib/ui/painting/engine_layer.cc | 2 - lib/ui/painting/engine_layer.h | 2 +- lib/ui/window/viewport_metrics.cc | 16 +++- lib/ui/window/viewport_metrics.h | 5 +- lib/web_ui/lib/src/ui/compositing.dart | 12 --- 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 - 43 files changed, 498 insertions(+), 342 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 16602861dd5c6..14e30b096fc11 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/container_layer.cc b/flow/layers/container_layer.cc index fdb13411a9ac8..39372f6c84dd8 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -9,8 +9,7 @@ namespace flutter { ContainerLayer::ContainerLayer() {} void ContainerLayer::Add(std::shared_ptr layer) { - layer->set_parent(this); - 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/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/layer.cc b/flow/layers/layer.cc index 036dd87105bd5..d4bd809d5f2cf 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 a055d9fd99f36..ca7eecd5507fc 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; @@ -53,11 +51,18 @@ struct PrerollContext { SkRect cull_rect; bool surface_needs_readback; - // 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; }; @@ -117,6 +122,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 @@ -153,10 +162,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; @@ -175,10 +180,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/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index 0ba8ece54e5ae..dd9e6b4f02b01 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) {} @@ -47,7 +51,9 @@ bool 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()); return context.surface_needs_readback; @@ -57,12 +63,22 @@ bool 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( @@ -106,7 +122,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); @@ -130,17 +148,19 @@ 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 - false, // layer reads from surface - 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 + false, // layer reads from surface + 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(); @@ -152,11 +172,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 61c1f929c55ca..21ba509f09ae6 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); // Perform a preroll pass on the tree and return information about // the tree that affects rendering this frame. @@ -49,8 +50,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_; } @@ -76,18 +77,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 eb598d17000dc..9851cebf26d0e 100644 --- a/flow/layers/layer_tree_unittests.cc +++ b/flow/layers/layer_tree_unittests.cc @@ -17,14 +17,17 @@ 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, true, 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, + true, + nullptr)) {} LayerTree& layer_tree() { return layer_tree_; } CompositorContext::ScopedFrame& frame() { return *scoped_frame_.get(); } diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 2bc0b77f43781..32f57179a3756 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( @@ -45,16 +42,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); } } @@ -73,13 +68,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; @@ -105,4 +97,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 6d57ea1c1f33d..f7cc77ffe2ddd 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) { diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 7299f119f761a..a3d1f70f0c09d 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -14,18 +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), + : PhysicalShapeLayerBase(color, elevation), 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), clip_behavior_(clip_behavior) { @@ -48,33 +41,35 @@ 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"); + Layer::AutoPrerollSaveLayerState save = Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer()); + PhysicalShapeLayerBase::Preroll(context, matrix); - 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) { + 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_, - device_pixel_ratio_)); -#endif // defined(OS_FUCHSIA) + set_paint_bounds(ComputeShadowBounds(path_.getBounds(), elevation(), + context->frame_device_pixel_ratio)); + //#endif // defined(OS_FUCHSIA) } } @@ -99,8 +94,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_, viewport_depth_, this); + SceneUpdateContext::Frame frame(context, frameRRect_, color(), elevation(), + this); for (auto& layer : layers()) { if (layer->needs_painting()) { frame.AddPaintLayer(layer.get()); @@ -116,14 +111,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, 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 ac15234ad2d82..34b3c59fe355b 100644 --- a/flow/layers/physical_shape_layer.h +++ b/flow/layers/physical_shape_layer.h @@ -5,20 +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, - SkScalar device_pixel_ratio, - float viewport_depth, - 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); @@ -29,8 +42,13 @@ 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; bool UsesSaveLayer() const { @@ -41,17 +59,8 @@ class PhysicalShapeLayer : public ContainerLayer { void UpdateScene(SceneUpdateContext& context) override; #endif // defined(OS_FUCHSIA) - float total_elevation() const { return total_elevation_; } - 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_; bool isRect_; SkRRect frameRRect_; diff --git a/flow/layers/physical_shape_layer_unittests.cc b/flow/layers/physical_shape_layer_unittests.cc index a29cc70fc8120..d13116c9c44a0 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]); @@ -251,10 +234,10 @@ static bool ReadbackResult(PrerollContext* context, const SkMatrix initial_matrix = SkMatrix(); const SkRect layer_bounds = SkRect::MakeXYWH(0.5, 1.0, 5.0, 6.0); const SkPath layer_path = SkPath().addRect(layer_bounds); - auto layer = std::make_shared(SK_ColorGREEN, - SK_ColorBLACK, 1.0f, 1.0f, - 0.0f, // elevation - layer_path, clip_behavior); + auto layer = + std::make_shared(SK_ColorGREEN, SK_ColorBLACK, + 0.0f, // elevation + layer_path, clip_behavior); if (child != nullptr) { layer->Add(child); } 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..c52f1ca9eb4ae 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); @@ -299,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 depth, + float elevation, Layer* layer) : Shape(context), rrect_(rrect), color_(color), paint_bounds_(SkRect::MakeEmpty()), layer_(layer) { - 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 c61f5670ba7f2..6961d6f72c554 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -115,9 +115,7 @@ class SceneUpdateContext { Frame(SceneUpdateContext& context, const SkRRect& rrect, SkColor color, - float local_elevation = 0.0f, - float parent_elevation = 0.0f, - float depth = 0.0f, + float elevation = 0.0f, Layer* layer = nullptr); virtual ~Frame(); @@ -152,6 +150,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 +234,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 590cef72c73ad..593dec1836823 100644 --- a/flow/testing/layer_test.h +++ b/flow/testing/layer_test.h @@ -39,8 +39,11 @@ class LayerTestBase : public CanvasTestBase { kGiantRect, /* cull_rect */ false, /* layer reads from surface */ 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 */ @@ -50,6 +53,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/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.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 332213ed559d8..216e1c9284874 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" @@ -181,12 +179,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); @@ -194,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, @@ -229,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, @@ -242,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) @@ -257,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) @@ -272,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) { @@ -295,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/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/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; 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/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(); - } } 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 d43fa564cc87b..2557d8ca73bed 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)); const auto frame_begin_time = fml::TimePoint::Now(); const auto frame_end_time = frame_begin_time + fml::TimeDelta::FromSecondsF(1.0 / 60.0); @@ -163,8 +171,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 b3e5429670e8f..ddbbc94ae1899 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -59,7 +59,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 69512053f233a..02b0edf6e0760 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -611,6 +611,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)); } @@ -627,7 +648,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)); } @@ -650,6 +673,7 @@ TEST_F(ShellTest, WaitForFirstFrameMultiple) { result = shell->WaitForFirstFrame(fml::TimeDelta::FromMilliseconds(1)); ASSERT_TRUE(result.ok()); } + DestroyShell(std::move(shell)); } @@ -675,10 +699,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 4634de8f1029a..fe192ca00a72c 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1601,7 +1601,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());