From d232e52bd5d78c0b057c23fb1ad25cc2ffc6ecbf Mon Sep 17 00:00:00 2001 From: David Worsham Date: Mon, 9 Sep 2019 15:27:47 -0700 Subject: [PATCH 1/9] Remove Childview opacity --- flow/view_holder.cc | 22 +++++++--------------- flow/view_holder.h | 4 ---- lib/ui/compositing.dart | 11 ----------- lib/ui/compositing/scene_host.cc | 16 +++------------- lib/ui/compositing/scene_host.h | 1 - lib/web_ui/lib/src/ui/compositing.dart | 12 ------------ 6 files changed, 10 insertions(+), 56 deletions(-) diff --git a/flow/view_holder.cc b/flow/view_holder.cc index 688f56d0987a7..391b5da4af687 100644 --- a/flow/view_holder.cc +++ b/flow/view_holder.cc @@ -4,7 +4,9 @@ #include "flutter/flow/view_holder.h" +#include "flutter/flow/embedded_views.h" #include "flutter/fml/thread_local.h" +#include "lib/ui/scenic/cpp/resources.h" namespace { @@ -104,14 +106,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 +118,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 +148,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 9466f644aee23..174340638dcea 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 232514c0f1805..944f2e933bf2f 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..795a68a9a469c 100644 --- a/lib/ui/compositing/scene_host.h +++ b/lib/ui/compositing/scene_host.h @@ -49,7 +49,6 @@ class SceneHost : public RefCountedDartWrappable { double insetBottom, double insetLeft, bool focusable); - void setOpacity(double opacity); private: fml::RefPtr gpu_task_runner_; diff --git a/lib/web_ui/lib/src/ui/compositing.dart b/lib/web_ui/lib/src/ui/compositing.dart index def31dbc7b620..2ecbb8014c613 100644 --- a/lib/web_ui/lib/src/ui/compositing.dart +++ b/lib/web_ui/lib/src/ui/compositing.dart @@ -607,12 +607,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. @@ -624,10 +618,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 3554d0602fa5d3bcb2d29c19643145c018c7b180 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Mon, 9 Sep 2019 18:39:01 -0700 Subject: [PATCH 2/9] Wire metrics through contexts --- flow/layers/layer.h | 11 +++++++++++ flow/layers/layer_tree.cc | 26 +++++++++++++++++++++----- flow/layers/layer_tree.h | 11 ++++++----- flow/layers/physical_shape_layer.cc | 16 +++++++--------- flow/layers/physical_shape_layer.h | 2 -- flow/raster_cache.cc | 4 +++- flow/scene_update_context.cc | 4 +++- flow/scene_update_context.h | 15 +++++++++++++++ lib/ui/compositing/scene.cc | 11 +++++++++-- lib/ui/compositing/scene_builder.cc | 8 -------- shell/common/engine.cc | 8 +------- shell/common/shell_test.cc | 3 ++- 12 files changed, 78 insertions(+), 41 deletions(-) diff --git a/flow/layers/layer.h b/flow/layers/layer.h index da2dcd8b21ce3..1ebacaeb3b661 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -57,6 +57,13 @@ struct PrerollContext { const Stopwatch& ui_time; TextureRegistry& texture_registry; const bool checkerboard_offscreen_layers; + + // The folllowing allow us to make use of the scene metrics during Preroll. + float frame_depth; + float frame_pixel_ratio; + + // The following allow us to track properties like elevation and opacity + // which stack with each other during Preroll. float total_elevation = 0.0f; }; @@ -89,6 +96,10 @@ class Layer { TextureRegistry& texture_registry; const RasterCache* raster_cache; const bool checkerboard_offscreen_layers; + + // The folllowing allow us to make use of the scene metrics during Paint. + float frame_depth; + float frame_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 b031ebf8cb8bd..8ee0e69c89c57 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -9,10 +9,18 @@ #include "third_party/skia/include/core/SkPictureRecorder.h" #include "third_party/skia/include/utils/SkNWayCanvas.h" +#if defined(OS_FUCHSIA) +#include +#endif + namespace flutter { -LayerTree::LayerTree() - : frame_size_{}, +LayerTree::LayerTree(const SkISize& frame_size, + float frame_depth, + float frame_pixel_ratio) + : frame_size_(frame_size), + frame_depth_(frame_depth), + frame_pixel_ratio_(frame_pixel_ratio), rasterizer_tracing_threshold_(0), checkerboard_raster_cache_images_(false), checkerboard_offscreen_layers_(false) {} @@ -42,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_depth_, + frame_pixel_ratio_}; root_layer_->Preroll(&context, frame.root_surface_transformation()); } @@ -94,7 +104,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_depth_, + frame_pixel_ratio_}; if (root_layer_->needs_painting()) root_layer_->Paint(context); @@ -128,6 +140,8 @@ sk_sp LayerTree::Flatten(const SkRect& bounds) { unused_stopwatch, // engine time (dont care) unused_texture_registry, // texture registry (not supported) false, // checkerboard_offscreen_layers + frame_depth_, // maximum depth allowed for rendering + frame_pixel_ratio_ // ratio between logical and physical }; SkISize canvas_size = canvas->getBaseLayerSize(); @@ -143,7 +157,9 @@ sk_sp LayerTree::Flatten(const SkRect& bounds) { unused_stopwatch, // engine time (dont care) unused_texture_registry, // texture registry (not supported) nullptr, // raster cache - false // checkerboard offscreen layers + false, // checkerboard offscreen layers + frame_depth_, // maximum depth allowed for rendering + frame_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 920fd166db93b..25fa5109ab953 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -20,8 +20,9 @@ namespace flutter { class LayerTree { public: - LayerTree(); - + LayerTree(const SkISize& frame_size, + float frame_depth, + float frame_pixel_ratio); ~LayerTree(); void Preroll(CompositorContext::ScopedFrame& frame, @@ -45,8 +46,6 @@ class LayerTree { const SkISize& frame_size() const { return frame_size_; } - void set_frame_size(const SkISize& frame_size) { frame_size_ = frame_size; } - void RecordBuildTime(fml::TimePoint begin_start); fml::TimePoint build_start() const { return build_start_; } fml::TimePoint build_finish() const { return build_finish_; } @@ -72,10 +71,12 @@ class LayerTree { } private: - SkISize frame_size_; // Physical pixels. std::shared_ptr root_layer_; fml::TimePoint build_start_; fml::TimePoint build_finish_; + SkISize frame_size_; // Physical pixels. + float frame_depth_; + float frame_pixel_ratio_; // Ratio between logical and physical pixels. uint32_t rasterizer_tracing_threshold_; bool checkerboard_raster_cache_images_; bool checkerboard_offscreen_layers_; diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 21c9265ce00dd..029f2eae729ef 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -14,15 +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), - viewport_depth_(viewport_depth), elevation_(elevation), path_(path), isRect_(false), @@ -100,11 +96,13 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, // t = tangent of AOB, i.e., multiplier for elevation to extent SkRect bounds(path_.getBounds()); // tangent for x - double tx = (kLightRadius * device_pixel_ratio_ + bounds.width() * 0.5) / - kLightHeight; + double tx = + (kLightRadius * context->frame_pixel_ratio + bounds.width() * 0.5) / + kLightHeight; // tangent for y - double ty = (kLightRadius * device_pixel_ratio_ + bounds.height() * 0.5) / - kLightHeight; + double ty = + (kLightRadius * context->frame_pixel_ratio + bounds.height() * 0.5) / + kLightHeight; bounds.outset(elevation_ * tx, elevation_ * ty); set_paint_bounds(bounds); #endif // defined(OS_FUCHSIA) @@ -148,7 +146,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_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 f884fe02fc5bd..5aeb2ed915710 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); diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index e0ed193d69082..f5c789d54ce8d 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -173,7 +173,9 @@ void RasterCache::Prepare(PrerollContext* context, context->ui_time, context->texture_registry, context->raster_cache, - context->checkerboard_offscreen_layers}; + context->checkerboard_offscreen_layers, + context->frame_depth, + context->frame_pixel_ratio}; if (layer->needs_painting()) { layer->Paint(paintContext); } diff --git a/flow/scene_update_context.cc b/flow/scene_update_context.cc index 14190b5b5d631..02bda154fb636 100644 --- a/flow/scene_update_context.cc +++ b/flow/scene_update_context.cc @@ -205,7 +205,9 @@ SceneUpdateContext::ExecutePaintTasks(CompositorContext::ScopedFrame& frame) { frame.context().ui_time(), frame.context().texture_registry(), &frame.context().raster_cache(), - false}; + false, + frame_depth_, + frame_pixel_ratio_}; canvas->restoreToCount(1); canvas->save(); canvas->clear(task.background_color); diff --git a/flow/scene_update_context.h b/flow/scene_update_context.h index d4a46055967f3..a9cc790a73294 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -152,6 +152,17 @@ class SceneUpdateContext { } const fuchsia::ui::gfx::MetricsPtr& metrics() const { return metrics_; } + void set_frame_dimensions(const SkISize& frame_size, + float frame_depth, + float frame_pixel_ratio) { + frame_size_ = frame_size; + frame_depth_ = frame_depth; + frame_pixel_ratio_ = frame_pixel_ratio; + } + const SkISize& frame_size() const { return frame_size_; } + float frame_depth() const { return frame_depth_; } + float frame_pixel_ratio() const { return frame_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 +236,10 @@ class SceneUpdateContext { SurfaceProducer* const surface_producer_; fuchsia::ui::gfx::MetricsPtr metrics_; + SkISize frame_size_; // Physical pixels. + float frame_depth_ = 0.0f; + float frame_pixel_ratio_ = + 1.0f; // Ratio between logical and physical pixels. std::vector paint_tasks_; diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index fd3e86c7e5176..272244b4123c5 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,8 +38,13 @@ 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()) { + bool checkerboardOffscreenLayers) { + auto viewport_metrics = UIDartState::Current()->window()->viewport_metrics(); + + m_layerTree = std::make_unique( + SkISize::Make(viewport_metrics.physical_width, + viewport_metrics.physical_height), + viewport_metrics.physical_depth, viewport_metrics.device_pixel_ratio); m_layerTree->set_root_layer(std::move(rootLayer)); m_layerTree->set_rasterizer_tracing_threshold(rasterizerTracingThreshold); m_layerTree->set_checkerboard_raster_cache_images( 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/shell/common/engine.cc b/shell/common/engine.cc index fa3fb65c8172d..6c8d9cdb7ce17 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -424,15 +424,9 @@ void Engine::ScheduleFrame(bool regenerate_layer_tree) { } void Engine::Render(std::unique_ptr layer_tree) { - if (!layer_tree) + if (!layer_tree || layer_tree->frame_size().isEmpty()) return; - SkISize frame_size = SkISize::Make(viewport_metrics_.physical_width, - viewport_metrics_.physical_height); - if (frame_size.isEmpty()) - return; - - layer_tree->set_frame_size(frame_size); animator_->Render(std::move(layer_tree)); } diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index e541d908c5586..c69409b2e8536 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -121,7 +121,8 @@ void ShellTest::PumpOneFrame(Shell* shell) { fml::WeakPtr runtime_delegate = shell->weak_engine_; shell->GetTaskRunners().GetUITaskRunner()->PostTask( [&latch, runtime_delegate]() { - auto layer_tree = std::make_unique(); + auto layer_tree = + std::make_unique(SkISize::Make(1, 1), kUnsetDepth, 1.0f); SkMatrix identity; identity.setIdentity(); auto root_layer = std::make_shared(identity); From 1846c83dce464864a4cf98511ab6c160d8794d85 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Mon, 9 Sep 2019 22:50:48 -0700 Subject: [PATCH 3/9] Cleanup code; remove most OS_FUCHSIA --- flow/layers/clip_path_layer.cc | 29 ++++++---------- flow/layers/clip_path_layer.h | 4 --- flow/layers/clip_rect_layer.cc | 23 ++++++------ flow/layers/clip_rect_layer.h | 3 -- flow/layers/clip_rrect_layer.cc | 23 ++++++------ flow/layers/clip_rrect_layer.h | 4 --- flow/layers/container_layer.cc | 4 --- flow/layers/container_layer.h | 5 --- flow/layers/layer.cc | 24 ++++++------- flow/layers/layer.h | 32 ++++++----------- flow/layers/layer_tree.cc | 54 ++++++++++++++++------------- flow/layers/layer_tree.h | 16 ++++----- flow/layers/opacity_layer.h | 1 - flow/layers/physical_shape_layer.cc | 6 ++-- flow/layers/physical_shape_layer.h | 3 -- flow/layers/transform_layer.cc | 21 ++++++----- flow/layers/transform_layer.h | 4 --- flow/scene_update_context.h | 9 ++++- 18 files changed, 111 insertions(+), 154 deletions(-) diff --git a/flow/layers/clip_path_layer.cc b/flow/layers/clip_path_layer.cc index d08c19b34eeb9..33ff9d1d37eb2 100644 --- a/flow/layers/clip_path_layer.cc +++ b/flow/layers/clip_path_layer.cc @@ -4,12 +4,6 @@ #include "flutter/flow/layers/clip_path_layer.h" -#if defined(OS_FUCHSIA) - -#include "lib/ui/scenic/cpp/commands.h" - -#endif // defined(OS_FUCHSIA) - namespace flutter { ClipPathLayer::ClipPathLayer(const SkPath& clip_path, Clip clip_behavior) @@ -35,18 +29,6 @@ void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { context->cull_rect = previous_cull_rect; } -#if defined(OS_FUCHSIA) - -void ClipPathLayer::UpdateScene(SceneUpdateContext& context) { - FML_DCHECK(needs_system_composite()); - - // TODO(liyuqian): respect clip_behavior_ - SceneUpdateContext::Clip clip(context, clip_path_.getBounds()); - UpdateSceneChildren(context); -} - -#endif // defined(OS_FUCHSIA) - void ClipPathLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "ClipPathLayer::Paint"); FML_DCHECK(needs_painting()); @@ -64,4 +46,15 @@ void ClipPathLayer::Paint(PaintContext& context) const { } } +void ClipPathLayer::UpdateScene(SceneUpdateContext& context) { +#if defined(OS_FUCHSIA) + FML_DCHECK(needs_system_composite()); + + // TODO(liyuqian): respect clip_behavior_ + SceneUpdateContext::Clip clip(context, clip_path_.getBounds()); + + ContainerLayer::UpdateScene(context); +#endif // defined(OS_FUCHSIA) +} + } // namespace flutter diff --git a/flow/layers/clip_path_layer.h b/flow/layers/clip_path_layer.h index fd4d56f0db7f0..292ccbc480497 100644 --- a/flow/layers/clip_path_layer.h +++ b/flow/layers/clip_path_layer.h @@ -15,12 +15,8 @@ class ClipPathLayer : public ContainerLayer { ~ClipPathLayer() override; 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: SkPath clip_path_; diff --git a/flow/layers/clip_rect_layer.cc b/flow/layers/clip_rect_layer.cc index de7590624e408..b0db4e497bd08 100644 --- a/flow/layers/clip_rect_layer.cc +++ b/flow/layers/clip_rect_layer.cc @@ -28,18 +28,6 @@ void ClipRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { context->cull_rect = previous_cull_rect; } -#if defined(OS_FUCHSIA) - -void ClipRectLayer::UpdateScene(SceneUpdateContext& context) { - FML_DCHECK(needs_system_composite()); - - // TODO(liyuqian): respect clip_behavior_ - SceneUpdateContext::Clip clip(context, clip_rect_); - UpdateSceneChildren(context); -} - -#endif // defined(OS_FUCHSIA) - void ClipRectLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "ClipRectLayer::Paint"); FML_DCHECK(needs_painting()); @@ -57,4 +45,15 @@ void ClipRectLayer::Paint(PaintContext& context) const { } } +void ClipRectLayer::UpdateScene(SceneUpdateContext& context) { +#if defined(OS_FUCHSIA) + FML_DCHECK(needs_system_composite()); + + // TODO(liyuqian): respect clip_behavior_ + SceneUpdateContext::Clip clip(context, clip_rect_); + + ContainerLayer::UpdateScene(context); +#endif // defined(OS_FUCHSIA) +} + } // namespace flutter diff --git a/flow/layers/clip_rect_layer.h b/flow/layers/clip_rect_layer.h index 76c5a3f01c873..716bfca138298 100644 --- a/flow/layers/clip_rect_layer.h +++ b/flow/layers/clip_rect_layer.h @@ -16,10 +16,7 @@ class ClipRectLayer : public ContainerLayer { 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: SkRect clip_rect_; diff --git a/flow/layers/clip_rrect_layer.cc b/flow/layers/clip_rrect_layer.cc index 9899a1658732d..d53bfec3fd51a 100644 --- a/flow/layers/clip_rrect_layer.cc +++ b/flow/layers/clip_rrect_layer.cc @@ -29,18 +29,6 @@ void ClipRRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { context->cull_rect = previous_cull_rect; } -#if defined(OS_FUCHSIA) - -void ClipRRectLayer::UpdateScene(SceneUpdateContext& context) { - FML_DCHECK(needs_system_composite()); - - // TODO(liyuqian): respect clip_behavior_ - SceneUpdateContext::Clip clip(context, clip_rrect_.getBounds()); - UpdateSceneChildren(context); -} - -#endif // defined(OS_FUCHSIA) - void ClipRRectLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "ClipRRectLayer::Paint"); FML_DCHECK(needs_painting()); @@ -58,4 +46,15 @@ void ClipRRectLayer::Paint(PaintContext& context) const { } } +void ClipRRectLayer::UpdateScene(SceneUpdateContext& context) { +#if defined(OS_FUCHSIA) + FML_DCHECK(needs_system_composite()); + + // TODO(liyuqian): respect clip_behavior_ + SceneUpdateContext::Clip clip(context, clip_rrect_.getBounds()); + + ContainerLayer::UpdateScene(context); +#endif // defined(OS_FUCHSIA) +} + } // namespace flutter diff --git a/flow/layers/clip_rrect_layer.h b/flow/layers/clip_rrect_layer.h index 53f74f30a0776..6b706abb95861 100644 --- a/flow/layers/clip_rrect_layer.h +++ b/flow/layers/clip_rrect_layer.h @@ -15,12 +15,8 @@ class ClipRRectLayer : public ContainerLayer { ~ClipRRectLayer() override; 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: SkRRect clip_rrect_; diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 31a5a255afca9..de8b1922d2373 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -48,8 +48,6 @@ void ContainerLayer::PaintChildren(PaintContext& context) const { } } -#if defined(OS_FUCHSIA) - void ContainerLayer::UpdateScene(SceneUpdateContext& context) { UpdateSceneChildren(context); } @@ -66,6 +64,4 @@ void ContainerLayer::UpdateSceneChildren(SceneUpdateContext& context) { } } -#endif // defined(OS_FUCHSIA) - } // namespace flutter diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index ef1c03328d1df..11c86064f332c 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -19,9 +19,7 @@ class ContainerLayer : public Layer { void Preroll(PrerollContext* context, const SkMatrix& matrix) override; -#if defined(OS_FUCHSIA) void UpdateScene(SceneUpdateContext& context) override; -#endif // defined(OS_FUCHSIA) const std::vector>& layers() const { return layers_; } @@ -30,10 +28,7 @@ class ContainerLayer : public Layer { const SkMatrix& child_matrix, SkRect* child_paint_bounds); void PaintChildren(PaintContext& context) const; - -#if defined(OS_FUCHSIA) void UpdateSceneChildren(SceneUpdateContext& context); -#endif // defined(OS_FUCHSIA) // For OpacityLayer to restructure to have a single child. void ClearChildren() { layers_.clear(); } diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index b729f582a0a9a..752e2781c870d 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -7,17 +7,9 @@ #include "flutter/flow/paint_utils.h" #include "third_party/skia/include/core/SkColorFilter.h" -namespace flutter { - -Layer::Layer() - : parent_(nullptr), - needs_system_composite_(false), - paint_bounds_(SkRect::MakeEmpty()), - unique_id_(NextUniqueID()) {} - -Layer::~Layer() = default; +namespace { -uint64_t Layer::NextUniqueID() { +uint64_t NextUniqueID() { static std::atomic nextID(1); uint64_t id; do { @@ -26,11 +18,15 @@ uint64_t Layer::NextUniqueID() { return id; } -void Layer::Preroll(PrerollContext* context, const SkMatrix& matrix) {} +} // anonymous namespace -#if defined(OS_FUCHSIA) -void Layer::UpdateScene(SceneUpdateContext& context) {} -#endif // defined(OS_FUCHSIA) +namespace flutter { + +Layer::Layer() + : parent_(nullptr), + paint_bounds_(SkRect::MakeEmpty()), + unique_id_(NextUniqueID()), + needs_system_composite_(false) {} Layer::AutoSaveLayer::AutoSaveLayer(const PaintContext& paint_context, const SkRect& bounds, diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 1ebacaeb3b661..028286c5a35b5 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -11,6 +11,7 @@ #include "flutter/flow/embedded_views.h" #include "flutter/flow/instrumentation.h" #include "flutter/flow/raster_cache.h" +#include "flutter/flow/scene_update_context.h" #include "flutter/flow/texture.h" #include "flutter/fml/build_config.h" #include "flutter/fml/compiler_specific.h" @@ -27,14 +28,6 @@ #include "third_party/skia/include/core/SkRect.h" #include "third_party/skia/include/utils/SkNWayCanvas.h" -#if defined(OS_FUCHSIA) - -#include "flutter/flow/scene_update_context.h" //nogncheck -#include "lib/ui/scenic/cpp/resources.h" //nogncheck -#include "lib/ui/scenic/cpp/session.h" //nogncheck - -#endif // defined(OS_FUCHSIA) - namespace flutter { static constexpr SkRect kGiantRect = SkRect::MakeLTRB(-1E9F, -1E9F, 1E9F, 1E9F); @@ -43,6 +36,7 @@ static constexpr SkRect kGiantRect = SkRect::MakeLTRB(-1E9F, -1E9F, 1E9F, 1E9F); enum Clip { none, hardEdge, antiAlias, antiAliasWithSaveLayer }; class ContainerLayer; +class SceneUpdateContext; struct PrerollContext { RasterCache* raster_cache; @@ -72,9 +66,7 @@ struct PrerollContext { class Layer { public: Layer(); - virtual ~Layer(); - - virtual void Preroll(PrerollContext* context, const SkMatrix& matrix); + virtual ~Layer() = default; struct PaintContext { // When splitting the scene into multiple canvases (e.g when embedding @@ -129,15 +121,18 @@ class Layer { const SkRect bounds_; }; + // Performs pre-paint optimizations, including bounds calculation. Called + // before |Paint|. If the |paint_bounds| calculated in this method is empty, + // then |Paint| will not be called. + virtual void Preroll(PrerollContext* context, const SkMatrix& matrix) {} + + // Paints this layer onto a canvas. Not called if |paint_bounds| is empty. virtual void Paint(PaintContext& context) const = 0; -#if defined(OS_FUCHSIA) // Updates the system composited scene. - virtual void UpdateScene(SceneUpdateContext& context); -#endif + virtual void UpdateScene(SceneUpdateContext& context) {} ContainerLayer* parent() const { return parent_; } - void set_parent(ContainerLayer* parent) { parent_ = parent; } bool needs_system_composite() const { return needs_system_composite_; } @@ -146,9 +141,6 @@ class Layer { } const SkRect& paint_bounds() const { return paint_bounds_; } - - // This must be set by the time Preroll() returns otherwise the layer will - // be assumed to have empty paint bounds (paints no content). void set_paint_bounds(const SkRect& paint_bounds) { paint_bounds_ = paint_bounds; } @@ -159,11 +151,9 @@ class Layer { private: ContainerLayer* parent_; - bool needs_system_composite_; SkRect paint_bounds_; uint64_t unique_id_; - - static uint64_t NextUniqueID(); + bool needs_system_composite_; FML_DISALLOW_COPY_AND_ASSIGN(Layer); }; diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index 8ee0e69c89c57..72f542b2b2e3c 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -57,31 +57,6 @@ void LayerTree::Preroll(CompositorContext::ScopedFrame& frame, root_layer_->Preroll(&context, frame.root_surface_transformation()); } -#if defined(OS_FUCHSIA) -void LayerTree::UpdateScene(SceneUpdateContext& context, - scenic::ContainerNode& container) { - TRACE_EVENT0("flutter", "LayerTree::UpdateScene"); - const auto& metrics = context.metrics(); - 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( - SkRect::MakeWH(frame_size_.width(), frame_size_.height())), - SK_ColorTRANSPARENT); - if (root_layer_->needs_system_composite()) { - root_layer_->UpdateScene(context); - } - if (root_layer_->needs_painting()) { - frame.AddPaintLayer(root_layer_.get()); - } - container.AddChild(transform.entity_node()); -} -#endif - void LayerTree::Paint(CompositorContext::ScopedFrame& frame, bool ignore_raster_cache) const { TRACE_EVENT0("flutter", "LayerTree::Paint"); @@ -175,4 +150,33 @@ sk_sp LayerTree::Flatten(const SkRect& bounds) { return recorder.finishRecordingAsPicture(); } +void LayerTree::UpdateScene(SceneUpdateContext& context, + scenic::ContainerNode& container) { +#if defined(OS_FUCHSIA) + TRACE_EVENT0("flutter", "LayerTree::UpdateScene"); + + // Ensure the context is aware of the view metrics. + context.set_frame_dimensions(frame_size_, frame_depth_, frame_pixel_ratio_); + + const auto& metrics = context.metrics(); + 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( + SkRect::MakeWH(frame_size_.width(), frame_size_.height())), + SK_ColorTRANSPARENT); + if (root_layer_->needs_system_composite()) { + root_layer_->UpdateScene(context); + } + if (root_layer_->needs_painting()) { + frame.AddPaintLayer(root_layer_.get()); + } + container.AddChild(transform.entity_node()); +#endif +} + } // namespace flutter diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index 25fa5109ab953..a9913c5bd7ca6 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -16,8 +16,14 @@ #include "third_party/skia/include/core/SkPicture.h" #include "third_party/skia/include/core/SkSize.h" +namespace scenic { +class ContainerNode; +} // namespace scenic + namespace flutter { +class SceneUpdateContext; + class LayerTree { public: LayerTree(const SkISize& frame_size, @@ -27,19 +33,13 @@ class LayerTree { void Preroll(CompositorContext::ScopedFrame& frame, bool ignore_raster_cache = false); - -#if defined(OS_FUCHSIA) - void UpdateScene(SceneUpdateContext& context, - scenic::ContainerNode& container); -#endif - void Paint(CompositorContext::ScopedFrame& frame, bool ignore_raster_cache = false) const; - sk_sp Flatten(const SkRect& bounds); + void UpdateScene(SceneUpdateContext& context, + scenic::ContainerNode& container); Layer* root_layer() const { return root_layer_.get(); } - void set_root_layer(std::shared_ptr root_layer) { root_layer_ = std::move(root_layer); } diff --git a/flow/layers/opacity_layer.h b/flow/layers/opacity_layer.h index 795d8841ba6ed..7323a975e1693 100644 --- a/flow/layers/opacity_layer.h +++ b/flow/layers/opacity_layer.h @@ -29,7 +29,6 @@ class OpacityLayer : public ContainerLayer { ~OpacityLayer() override; void Preroll(PrerollContext* context, const SkMatrix& matrix) override; - void Paint(PaintContext& context) const override; // TODO(chinmaygarde): Once SCN-139 is addressed, introduce a new node in the diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 029f2eae729ef..4460e9da9c1d2 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -109,9 +109,8 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, } } -#if defined(OS_FUCHSIA) - void PhysicalShapeLayer::UpdateScene(SceneUpdateContext& context) { +#if defined(OS_FUCHSIA) FML_DCHECK(needs_system_composite()); // Retained rendering: speedup by reusing a retained entity node if possible. @@ -136,9 +135,8 @@ void PhysicalShapeLayer::UpdateScene(SceneUpdateContext& context) { } UpdateSceneChildren(context); -} - #endif // defined(OS_FUCHSIA) +} void PhysicalShapeLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "PhysicalShapeLayer::Paint"); diff --git a/flow/layers/physical_shape_layer.h b/flow/layers/physical_shape_layer.h index 5aeb2ed915710..d09150e94e624 100644 --- a/flow/layers/physical_shape_layer.h +++ b/flow/layers/physical_shape_layer.h @@ -28,10 +28,7 @@ class PhysicalShapeLayer : public ContainerLayer { 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 color_; diff --git a/flow/layers/transform_layer.cc b/flow/layers/transform_layer.cc index 5a7af132c68f2..383c338a03d33 100644 --- a/flow/layers/transform_layer.cc +++ b/flow/layers/transform_layer.cc @@ -50,17 +50,6 @@ void TransformLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { context->mutators_stack.Pop(); } -#if defined(OS_FUCHSIA) - -void TransformLayer::UpdateScene(SceneUpdateContext& context) { - FML_DCHECK(needs_system_composite()); - - SceneUpdateContext::Transform transform(context, transform_); - UpdateSceneChildren(context); -} - -#endif // defined(OS_FUCHSIA) - void TransformLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "TransformLayer::Paint"); FML_DCHECK(needs_painting()); @@ -71,4 +60,14 @@ void TransformLayer::Paint(PaintContext& context) const { PaintChildren(context); } +void TransformLayer::UpdateScene(SceneUpdateContext& context) { +#if defined(OS_FUCHSIA) + FML_DCHECK(needs_system_composite()); + + SceneUpdateContext::Transform transform(context, transform_); + + ContainerLayer::UpdateScene(context); +#endif // defined(OS_FUCHSIA) +} + } // namespace flutter diff --git a/flow/layers/transform_layer.h b/flow/layers/transform_layer.h index f19a963ced9fe..3f3457eef5a73 100644 --- a/flow/layers/transform_layer.h +++ b/flow/layers/transform_layer.h @@ -17,12 +17,8 @@ class TransformLayer : public ContainerLayer { ~TransformLayer() override; 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: SkMatrix transform_; diff --git a/flow/scene_update_context.h b/flow/scene_update_context.h index a9cc790a73294..803cf1582a670 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_FLOW_SCENE_UPDATE_CONTEXT_H_ #define FLUTTER_FLOW_SCENE_UPDATE_CONTEXT_H_ +#if defined(OS_FUCHSIA) || defined(__Fuchsia__) #include #include #include @@ -14,10 +15,11 @@ #include "flutter/fml/compiler_specific.h" #include "flutter/fml/logging.h" #include "flutter/fml/macros.h" -#include "lib/ui/scenic/cpp/resources.h" #include "third_party/skia/include/core/SkRect.h" #include "third_party/skia/include/core/SkSurface.h" +#include + namespace flutter { class Layer; @@ -247,5 +249,10 @@ class SceneUpdateContext { }; } // namespace flutter +#else +namespace flutter { +class SceneUpdateContext; +} +#endif // defined(OS_FUCHSIA) #endif // FLUTTER_FLOW_SCENE_UPDATE_CONTEXT_H_ From 99f459ec31df3c582fc28e5d22e287cfb994a2ff Mon Sep 17 00:00:00 2001 From: David Worsham Date: Tue, 10 Sep 2019 12:38:55 -0700 Subject: [PATCH 4/9] Centralize UpdateScene and elevation logic Move all of this to ContainerLayer --- flow/layers/backdrop_filter_layer.cc | 2 +- flow/layers/clip_path_layer.cc | 11 +- flow/layers/clip_rect_layer.cc | 18 ++- flow/layers/clip_rrect_layer.cc | 11 +- flow/layers/color_filter_layer.cc | 2 +- flow/layers/container_layer.cc | 107 +++++++++++--- flow/layers/container_layer.h | 42 ++++-- flow/layers/layer_tree.cc | 2 +- flow/layers/opacity_layer.cc | 35 +---- flow/layers/opacity_layer.h | 12 +- flow/layers/physical_shape_layer.cc | 137 +++++++++--------- flow/layers/physical_shape_layer.h | 18 +-- flow/layers/physical_shape_layer_unittests.cc | 14 +- flow/layers/shader_mask_layer.cc | 2 +- flow/layers/transform_layer.cc | 8 +- flow/scene_update_context.cc | 22 +-- flow/scene_update_context.h | 4 +- 17 files changed, 232 insertions(+), 215 deletions(-) diff --git a/flow/layers/backdrop_filter_layer.cc b/flow/layers/backdrop_filter_layer.cc index 573db97f191a2..dca842918c343 100644 --- a/flow/layers/backdrop_filter_layer.cc +++ b/flow/layers/backdrop_filter_layer.cc @@ -18,7 +18,7 @@ void BackdropFilterLayer::Paint(PaintContext& context) const { Layer::AutoSaveLayer save = Layer::AutoSaveLayer::Create( context, SkCanvas::SaveLayerRec{&paint_bounds(), nullptr, filter_.get(), 0}); - PaintChildren(context); + ContainerLayer::Paint(context); } } // namespace flutter diff --git a/flow/layers/clip_path_layer.cc b/flow/layers/clip_path_layer.cc index 33ff9d1d37eb2..06c65f376bbc0 100644 --- a/flow/layers/clip_path_layer.cc +++ b/flow/layers/clip_path_layer.cc @@ -18,11 +18,12 @@ void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect clip_path_bounds = clip_path_.getBounds(); if (context->cull_rect.intersect(clip_path_bounds)) { context->mutators_stack.PushClipPath(clip_path_); - SkRect child_paint_bounds = SkRect::MakeEmpty(); - PrerollChildren(context, matrix, &child_paint_bounds); + ContainerLayer::Preroll(context, matrix); - if (child_paint_bounds.intersect(clip_path_bounds)) { - set_paint_bounds(child_paint_bounds); + if (clip_path_bounds.intersect(paint_bounds())) { + set_paint_bounds(clip_path_bounds); + } else { + set_paint_bounds(SkRect()); } context->mutators_stack.Pop(); } @@ -40,7 +41,7 @@ void ClipPathLayer::Paint(PaintContext& context) const { if (clip_behavior_ == Clip::antiAliasWithSaveLayer) { context.internal_nodes_canvas->saveLayer(paint_bounds(), nullptr); } - PaintChildren(context); + ContainerLayer::Paint(context); if (clip_behavior_ == Clip::antiAliasWithSaveLayer) { context.internal_nodes_canvas->restore(); } diff --git a/flow/layers/clip_rect_layer.cc b/flow/layers/clip_rect_layer.cc index b0db4e497bd08..66c45806baf0a 100644 --- a/flow/layers/clip_rect_layer.cc +++ b/flow/layers/clip_rect_layer.cc @@ -15,13 +15,15 @@ ClipRectLayer::~ClipRectLayer() = default; void ClipRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect previous_cull_rect = context->cull_rect; - if (context->cull_rect.intersect(clip_rect_)) { - context->mutators_stack.PushClipRect(clip_rect_); - SkRect child_paint_bounds = SkRect::MakeEmpty(); - PrerollChildren(context, matrix, &child_paint_bounds); - - if (child_paint_bounds.intersect(clip_rect_)) { - set_paint_bounds(child_paint_bounds); + SkRect clip_rect_bounds = clip_rect_; + if (context->cull_rect.intersect(clip_rect_bounds)) { + context->mutators_stack.PushClipRect(clip_rect_bounds); + ContainerLayer::Preroll(context, matrix); + + if (clip_rect_bounds.intersect(paint_bounds())) { + set_paint_bounds(clip_rect_bounds); + } else { + set_paint_bounds(SkRect()); } context->mutators_stack.Pop(); } @@ -39,7 +41,7 @@ void ClipRectLayer::Paint(PaintContext& context) const { if (clip_behavior_ == Clip::antiAliasWithSaveLayer) { context.internal_nodes_canvas->saveLayer(clip_rect_, nullptr); } - PaintChildren(context); + ContainerLayer::Paint(context); if (clip_behavior_ == Clip::antiAliasWithSaveLayer) { context.internal_nodes_canvas->restore(); } diff --git a/flow/layers/clip_rrect_layer.cc b/flow/layers/clip_rrect_layer.cc index d53bfec3fd51a..7409129cfcb88 100644 --- a/flow/layers/clip_rrect_layer.cc +++ b/flow/layers/clip_rrect_layer.cc @@ -18,11 +18,12 @@ void ClipRRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect clip_rrect_bounds = clip_rrect_.getBounds(); if (context->cull_rect.intersect(clip_rrect_bounds)) { context->mutators_stack.PushClipRRect(clip_rrect_); - SkRect child_paint_bounds = SkRect::MakeEmpty(); - PrerollChildren(context, matrix, &child_paint_bounds); + ContainerLayer::Preroll(context, matrix); - if (child_paint_bounds.intersect(clip_rrect_bounds)) { - set_paint_bounds(child_paint_bounds); + if (clip_rrect_bounds.intersect(paint_bounds())) { + set_paint_bounds(clip_rrect_bounds); + } else { + set_paint_bounds(SkRect()); } context->mutators_stack.Pop(); } @@ -40,7 +41,7 @@ void ClipRRectLayer::Paint(PaintContext& context) const { if (clip_behavior_ == Clip::antiAliasWithSaveLayer) { context.internal_nodes_canvas->saveLayer(paint_bounds(), nullptr); } - PaintChildren(context); + ContainerLayer::Paint(context); if (clip_behavior_ == Clip::antiAliasWithSaveLayer) { context.internal_nodes_canvas->restore(); } diff --git a/flow/layers/color_filter_layer.cc b/flow/layers/color_filter_layer.cc index f838b0612b2e5..ad878407345f8 100644 --- a/flow/layers/color_filter_layer.cc +++ b/flow/layers/color_filter_layer.cc @@ -20,7 +20,7 @@ void ColorFilterLayer::Paint(PaintContext& context) const { Layer::AutoSaveLayer save = Layer::AutoSaveLayer::Create(context, paint_bounds(), &paint); - PaintChildren(context); + ContainerLayer::Paint(context); } } // namespace flutter diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index de8b1922d2373..0c44efa1240f7 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -4,13 +4,49 @@ #include "flutter/flow/layers/container_layer.h" +#include + +#include "flutter/flow/layers/transform_layer.h" + namespace flutter { -ContainerLayer::ContainerLayer() {} +static 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; +} -ContainerLayer::~ContainerLayer() = default; +ContainerLayer::ContainerLayer(bool force_single_child) { + // Place all "child" layers under a single child if requested. + if (force_single_child) { + // 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(); + single_child_ = std::make_shared(identity); + single_child_->set_parent(this); + layers_.push_back(single_child_); + } +} void ContainerLayer::Add(std::shared_ptr layer) { + // Place all "child" layers under a single child if requested. + if (single_child_) { + single_child_->Add(std::move(layer)); + return; + } + layer->set_parent(this); layers_.push_back(std::move(layer)); } @@ -18,25 +54,29 @@ void ContainerLayer::Add(std::shared_ptr layer) { void ContainerLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { TRACE_EVENT0("flutter", "ContainerLayer::Preroll"); - SkRect child_paint_bounds = SkRect::MakeEmpty(); - PrerollChildren(context, matrix, &child_paint_bounds); - set_paint_bounds(child_paint_bounds); -} + // 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_depth); + context->total_elevation += clamped_elevation_; -void ContainerLayer::PrerollChildren(PrerollContext* context, - const SkMatrix& child_matrix, - SkRect* child_paint_bounds) { + SkRect child_paint_bounds = SkRect::MakeEmpty(); for (auto& layer : layers_) { - layer->Preroll(context, child_matrix); + layer->Preroll(context, matrix); if (layer->needs_system_composite()) { set_needs_system_composite(true); } - child_paint_bounds->join(layer->paint_bounds()); + child_paint_bounds.join(layer->paint_bounds()); } + set_paint_bounds(child_paint_bounds); + + // Restore the elevation for our parent. + context->total_elevation = parent_elevation_; } -void ContainerLayer::PaintChildren(PaintContext& context) const { +void ContainerLayer::Paint(PaintContext& context) const { FML_DCHECK(needs_painting()); // Intentionally not tracing here as there should be no self-time @@ -49,19 +89,44 @@ void ContainerLayer::PaintChildren(PaintContext& context) const { } void ContainerLayer::UpdateScene(SceneUpdateContext& context) { - UpdateSceneChildren(context); -} +#if defined(OS_FUCHSIA) + if (should_render_as_frame()) { + FML_DCHECK(needs_system_composite()); -void ContainerLayer::UpdateSceneChildren(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)) { + 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; + } - // Paint all of the layers which need to be drawn into the container. - // These may be flattened down to a containing - for (auto& layer : layers_) { - if (layer->needs_system_composite()) { - layer->UpdateScene(context); + SceneUpdateContext::Frame frame(context, frame_rrect_, frame_color_, + elevation(), this); + // Paint the child layers into the Frame as well as allowing them to create + // their own scene entities. + for (auto& layer : layers()) { + if (layer->needs_painting()) { + frame.AddPaintLayer(layer.get()); + } + if (layer->needs_system_composite()) { + layer->UpdateScene(context); + } + } + } else { + // Update all of the Layers which are part of the container. This may cause + // additional scene entities to be created. + for (auto& layer : layers()) { + if (layer->needs_system_composite()) { + layer->UpdateScene(context); + } } } +#endif // defined(OS_FUCHSIA) } } // namespace flutter diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index 11c86064f332c..9fe78d6ae6ed8 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -5,36 +5,52 @@ #ifndef FLUTTER_FLOW_LAYERS_CONTAINER_LAYER_H_ #define FLUTTER_FLOW_LAYERS_CONTAINER_LAYER_H_ +#include #include + #include "flutter/flow/layers/layer.h" +#include "third_party/skia/include/core/SkColor.h" +#include "third_party/skia/include/core/SkRRect.h" namespace flutter { class ContainerLayer : public Layer { public: - ContainerLayer(); - ~ContainerLayer() override; + ContainerLayer(bool force_single_child = false); + ~ContainerLayer() override = default; void Add(std::shared_ptr layer); void Preroll(PrerollContext* context, const SkMatrix& matrix) override; - + void Paint(PaintContext& context) const override; void UpdateScene(SceneUpdateContext& context) override; - const std::vector>& layers() const { return layers_; } + bool should_render_as_frame() const { return !frame_rrect_.isEmpty(); } + void set_frame_properties(SkRRect frame_rrect, SkColor frame_color) { + frame_rrect_ = frame_rrect; + frame_color_ = frame_color; + } + + float elevation() const { return clamped_elevation_; } + float total_elevation() const { + return parent_elevation_ + clamped_elevation_; + } + void set_elevation(float elevation) { + parent_elevation_ = 0.0f; + elevation_ = elevation; + clamped_elevation_ = elevation; + } - protected: - void PrerollChildren(PrerollContext* context, - const SkMatrix& child_matrix, - SkRect* child_paint_bounds); - void PaintChildren(PaintContext& context) const; - void UpdateSceneChildren(SceneUpdateContext& context); - - // For OpacityLayer to restructure to have a single child. - void ClearChildren() { layers_.clear(); } + const std::vector>& layers() const { return layers_; } private: std::vector> layers_; + std::shared_ptr single_child_; + SkRRect frame_rrect_; + SkColor frame_color_; + float parent_elevation_ = 0.0f; + float elevation_ = 0.0f; + float clamped_elevation_ = 0.0f; FML_DISALLOW_COPY_AND_ASSIGN(ContainerLayer); }; diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index 72f542b2b2e3c..db44dfd021731 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -168,7 +168,7 @@ void LayerTree::UpdateScene(SceneUpdateContext& context, context, SkRRect::MakeRect( SkRect::MakeWH(frame_size_.width(), frame_size_.height())), - SK_ColorTRANSPARENT); + SK_ColorTRANSPARENT, /* elevation */ 0.0f); if (root_layer_->needs_system_composite()) { root_layer_->UpdateScene(context); } diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 014ee6736d92a..8c66e995286e3 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -4,37 +4,12 @@ #include "flutter/flow/layers/opacity_layer.h" -#include "flutter/flow/layers/transform_layer.h" - namespace flutter { OpacityLayer::OpacityLayer(int alpha, const SkPoint& offset) - : alpha_(alpha), offset_(offset) {} - -OpacityLayer::~OpacityLayer() = default; - -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); - - for (auto& child : layers()) { - new_child->Add(child); - } - ClearChildren(); - Add(new_child); -} + : ContainerLayer(true), alpha_(alpha), offset_(offset) {} void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { - EnsureSingleChild(); SkMatrix child_matrix = matrix; child_matrix.postTranslate(offset_.fX, offset_.fY); context->mutators_stack.PushTransform( @@ -44,8 +19,7 @@ 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->view_embedder == nullptr && context->raster_cache && SkRect::Intersects(context->cull_rect, paint_bounds())) { Layer* child = layers()[0].get(); @@ -72,9 +46,6 @@ void OpacityLayer::Paint(PaintContext& context) const { context.leaf_nodes_canvas->getTotalMatrix())); #endif - // See |EnsureSingleChild|. - FML_DCHECK(layers().size() == 1); - // Embedded platform views are changing the canvas in the middle of the paint // traversal. To make sure we paint on the right canvas, when the embedded // platform views preview is enabled (context.view_embedded is not null) we @@ -105,7 +76,7 @@ void OpacityLayer::Paint(PaintContext& context) const { Layer::AutoSaveLayer save_layer = Layer::AutoSaveLayer::Create(context, saveLayerBounds, &paint); - PaintChildren(context); + ContainerLayer::Paint(context); } } // namespace flutter diff --git a/flow/layers/opacity_layer.h b/flow/layers/opacity_layer.h index 7323a975e1693..48a8137f35a10 100644 --- a/flow/layers/opacity_layer.h +++ b/flow/layers/opacity_layer.h @@ -26,7 +26,7 @@ class OpacityLayer : public ContainerLayer { // 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() override; + ~OpacityLayer() override = default; void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void Paint(PaintContext& context) const override; @@ -38,16 +38,6 @@ class OpacityLayer : public ContainerLayer { 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/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 4460e9da9c1d2..42da85b5b5aae 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -5,12 +5,18 @@ #include "flutter/flow/layers/physical_shape_layer.h" #include "flutter/flow/paint_utils.h" +#include "include/core/SkColor.h" #include "third_party/skia/include/utils/SkShadowUtils.h" namespace flutter { -const SkScalar kLightHeight = 600; -const SkScalar kLightRadius = 800; +constexpr SkScalar kLightHeight = 600; +constexpr SkScalar kLightRadius = 800; +#if defined(OS_FUCHSIA) +constexpr bool kRenderPhysicalShapeUsingSystemCompositor = false; +#else +constexpr bool kRenderPhysicalShapeUsingSystemCompositor = false; +#endif PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, SkColor shadow_color, @@ -19,48 +25,57 @@ PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, Clip clip_behavior) : color_(color), shadow_color_(shadow_color), - elevation_(elevation), path_(path), - isRect_(false), clip_behavior_(clip_behavior) { - SkRect rect; - if (path.isRect(&rect)) { - isRect_ = true; - frameRRect_ = SkRRect::MakeRect(rect); - } else if (path.isRRect(&frameRRect_)) { - isRect_ = frameRRect_.isRect(); - } else if (path.isOval(&rect)) { - // isRRect returns false for ovals, so we need to explicitly check isOval - // as well. - frameRRect_ = SkRRect::MakeOval(rect); - } else { - // Scenic currently doesn't provide an easy way to create shapes from - // arbitrary paths. - // For shapes that cannot be represented as a rounded rectangle we - // default to use the bounding rectangle. - // TODO(amirh): fix this once we have a way to create a Scenic shape from - // an SkPath. - frameRRect_ = SkRRect::MakeRect(path.getBounds()); +#if !defined(OS_FUCHSIA) + FML_DCHECK(!kRenderPhysicalShapeUsingSystemCompositor); +#endif // !defined(OS_FUCHSIA) + + // If rendering as a separate frame using the system compositor, then make + // sure to set up the properties needed to do so. + if (kRenderPhysicalShapeUsingSystemCompositor) { + SkRect rect; + SkRRect frame_rrect; + if (path.isRect(&rect)) { + frame_rrect = SkRRect::MakeRect(rect); + } else if (path.isRRect(&frame_rrect)) { + // Nothing needed here, as isRRect will fill in frameRRect_ already. + } else if (path.isOval(&rect)) { + // isRRect returns false for ovals, so we need to explicitly check isOval + // as well. + frame_rrect = SkRRect::MakeOval(rect); + } else { + // Scenic currently doesn't provide an easy way to create shapes from + // arbitrary paths. + // For shapes that cannot be represented as a rounded rectangle we + // default to use the bounding rectangle. + // TODO(amirh): fix this once we have a way to create a Scenic shape from + // an SkPath. + frame_rrect = SkRRect::MakeRect(path.getBounds()); + } + + set_frame_properties(frame_rrect, color_); } + set_elevation(elevation); } -PhysicalShapeLayer::~PhysicalShapeLayer() = default; - void PhysicalShapeLayer::Preroll(PrerollContext* context, const SkMatrix& 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) { - set_paint_bounds(path_.getBounds()); - } else { -#if defined(OS_FUCHSIA) - // Let the system compositor draw all shadows for us. + ContainerLayer::Preroll(context, matrix); + + // Compute paint bounds based on the layer's elevation. + set_paint_bounds(path_.getBounds()); + if (elevation() == 0) { + return; + } + + // If elevation is non-zero, compute the proper paint_bounds to allow drawing + // a shadow. + if (kRenderPhysicalShapeUsingSystemCompositor) { + // Let the system compositor draw all shadows for us, by popping us out as + // a new frame. set_needs_system_composite(true); -#else + } else { // 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. @@ -103,47 +118,29 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, double ty = (kLightRadius * context->frame_pixel_ratio + bounds.height() * 0.5) / kLightHeight; - bounds.outset(elevation_ * tx, elevation_ * ty); + bounds.outset(elevation() * tx, elevation() * ty); set_paint_bounds(bounds); -#endif // defined(OS_FUCHSIA) } } -void PhysicalShapeLayer::UpdateScene(SceneUpdateContext& context) { -#if defined(OS_FUCHSIA) - 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 PhysicalShapeLayer::Paint. - LayerRasterCacheKey key(unique_id(), context.Matrix()); - if (context.HasRetainedNode(key)) { - 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; - } - - // If we can't find an existing retained surface, create one. - SceneUpdateContext::Frame frame(context, frameRRect_, color_, elevation_, - total_elevation_, viewport_depth_, 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()); - if (elevation_ != 0) { - DrawShadow(context.leaf_nodes_canvas, path_, shadow_color_, elevation_, + // The compositor will paint this layer (which is a solid color) via the + // color on |SceneUpdateContext::Frame|. + // + // The child layers will be painted into the texture used by the Frame, so + // painting them here would actually cause them to be painted on the display + // twice -- once into the current canvas (which may be inside of another + // Frame) and once into the Frame's texture (which is then drawn on top of the + // current canvas). + if (kRenderPhysicalShapeUsingSystemCompositor) { + return; + } + + if (elevation() != 0) { + DrawShadow(context.leaf_nodes_canvas, path_, shadow_color_, elevation(), SkColorGetA(color_) != 0xff, context.frame_pixel_ratio); } @@ -179,7 +176,7 @@ void PhysicalShapeLayer::Paint(PaintContext& context) const { context.leaf_nodes_canvas->drawPaint(paint); } - PaintChildren(context); + ContainerLayer::Paint(context); context.internal_nodes_canvas->restoreToCount(saveCount); } diff --git a/flow/layers/physical_shape_layer.h b/flow/layers/physical_shape_layer.h index d09150e94e624..c5c3af9fa9781 100644 --- a/flow/layers/physical_shape_layer.h +++ b/flow/layers/physical_shape_layer.h @@ -16,7 +16,10 @@ class PhysicalShapeLayer : public ContainerLayer { float elevation, const SkPath& path, Clip clip_behavior); - ~PhysicalShapeLayer() override; + ~PhysicalShapeLayer() override = default; + + void Preroll(PrerollContext* context, const SkMatrix& matrix) override; + void Paint(PaintContext& context) const override; static void DrawShadow(SkCanvas* canvas, const SkPath& path, @@ -25,24 +28,11 @@ class PhysicalShapeLayer : public ContainerLayer { bool transparentOccluder, SkScalar dpr); - void Preroll(PrerollContext* context, const SkMatrix& matrix) override; - - void Paint(PaintContext& context) const override; - void UpdateScene(SceneUpdateContext& context) override; - private: SkColor color_; SkColor shadow_color_; - SkScalar device_pixel_ratio_; - float viewport_depth_; - float elevation_ = 0.0f; - float total_elevation_ = 0.0f; SkPath path_; - bool isRect_; - SkRRect frameRRect_; Clip clip_behavior_; - - friend class PhysicalShapeLayer_TotalElevation_Test; }; } // namespace flutter diff --git a/flow/layers/physical_shape_layer_unittests.cc b/flow/layers/physical_shape_layer_unittests.cc index 972424a2fec6d..dbdda7ff40248 100644 --- a/flow/layers/physical_shape_layer_unittests.cc +++ b/flow/layers/physical_shape_layer_unittests.cc @@ -16,8 +16,6 @@ TEST(PhysicalShapeLayer, TotalElevation) { for (int i = 0; i < 4; i += 1) { layers[i] = std::make_shared(dummy_color, dummy_color, - 1.0f, // pixel ratio, - 1.0f, // depth (float)(i + 1), // elevation dummy_path, Clip::none); } @@ -40,6 +38,8 @@ TEST(PhysicalShapeLayer, TotalElevation) { unused_stopwatch, // engine time (dont care) unused_texture_registry, // texture registry (not supported) false, // checkerboard_offscreen_layers + 6.0f, // depth + 1.0f, // pixel ratio 0.0f, // total elevation }; @@ -55,14 +55,14 @@ TEST(PhysicalShapeLayer, TotalElevation) { // | \ // | layers[2] +3.0f // | | - // | layers[3] +4.0f + // | layers[3] +4.0f (clamped to 6.0f) // | // | // layers[1] + 2.0f - EXPECT_EQ(layers[0]->total_elevation_, 1.0f); - EXPECT_EQ(layers[1]->total_elevation_, 3.0f); - EXPECT_EQ(layers[2]->total_elevation_, 4.0f); - EXPECT_EQ(layers[3]->total_elevation_, 8.0f); + EXPECT_EQ(layers[0]->total_elevation(), 1.0f); + EXPECT_EQ(layers[1]->total_elevation(), 3.0f); + EXPECT_EQ(layers[2]->total_elevation(), 4.0f); + EXPECT_EQ(layers[3]->total_elevation(), 6.0f); } } // namespace flutter diff --git a/flow/layers/shader_mask_layer.cc b/flow/layers/shader_mask_layer.cc index 36e7b7332aeae..75fc9f4a3eda8 100644 --- a/flow/layers/shader_mask_layer.cc +++ b/flow/layers/shader_mask_layer.cc @@ -19,7 +19,7 @@ void ShaderMaskLayer::Paint(PaintContext& context) const { Layer::AutoSaveLayer save = Layer::AutoSaveLayer::Create(context, paint_bounds(), nullptr); - PaintChildren(context); + ContainerLayer::Paint(context); SkPaint paint; paint.setBlendMode(blend_mode_); diff --git a/flow/layers/transform_layer.cc b/flow/layers/transform_layer.cc index 383c338a03d33..eb14c51286e28 100644 --- a/flow/layers/transform_layer.cc +++ b/flow/layers/transform_layer.cc @@ -40,11 +40,9 @@ void TransformLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { context->cull_rect = kGiantRect; } - SkRect child_paint_bounds = SkRect::MakeEmpty(); - PrerollChildren(context, child_matrix, &child_paint_bounds); + ContainerLayer::Preroll(context, child_matrix); - transform_.mapRect(&child_paint_bounds); - set_paint_bounds(child_paint_bounds); + transform_.mapRect(paint_bounds()); context->cull_rect = previous_cull_rect; context->mutators_stack.Pop(); @@ -57,7 +55,7 @@ void TransformLayer::Paint(PaintContext& context) const { SkAutoCanvasRestore save(context.internal_nodes_canvas, true); context.internal_nodes_canvas->concat(transform_); - PaintChildren(context); + ContainerLayer::Paint(context); } void TransformLayer::UpdateScene(SceneUpdateContext& context) { diff --git a/flow/scene_update_context.cc b/flow/scene_update_context.cc index 02bda154fb636..0f4c414b886d8 100644 --- a/flow/scene_update_context.cc +++ b/flow/scene_update_context.cc @@ -294,29 +294,17 @@ 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); + SetEntityNodeClipPlanes(entity_node(), rrect_.getBounds()); + + entity_node().AddChild(shape_node()); } SceneUpdateContext::Frame::~Frame() { diff --git a/flow/scene_update_context.h b/flow/scene_update_context.h index 803cf1582a670..aade6d6e07193 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -117,9 +117,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(); From 471513e688b3faec1a94867e8dd28ff5dbf348a9 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Tue, 10 Sep 2019 14:36:44 -0700 Subject: [PATCH 5/9] Wire up OpacityLayer to Scenic --- flow/layers/child_scene_layer.cc | 17 +++++- flow/layers/container_layer.cc | 2 +- flow/layers/container_layer.h | 6 +- flow/layers/layer.h | 1 + flow/layers/layer_tree.cc | 2 +- flow/layers/opacity_layer.cc | 76 ++++++++++++++++++++++--- flow/layers/opacity_layer.h | 4 +- flow/layers/physical_shape_layer.cc | 16 +++++- flow/scene_update_context.cc | 88 ++++++++++++++--------------- flow/scene_update_context.h | 38 ++++++------- 10 files changed, 168 insertions(+), 82 deletions(-) diff --git a/flow/layers/child_scene_layer.cc b/flow/layers/child_scene_layer.cc index f4100be2fee69..66873e360f70c 100644 --- a/flow/layers/child_scene_layer.cc +++ b/flow/layers/child_scene_layer.cc @@ -19,10 +19,25 @@ ChildSceneLayer::ChildSceneLayer(zx_koid_t layer_id, void ChildSceneLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { 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/container_layer.cc b/flow/layers/container_layer.cc index 0c44efa1240f7..4fd171ba39956 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -106,7 +106,7 @@ void ContainerLayer::UpdateScene(SceneUpdateContext& context) { } SceneUpdateContext::Frame frame(context, frame_rrect_, frame_color_, - elevation(), this); + frame_opacity_, elevation(), this); // Paint the child layers into the Frame as well as allowing them to create // their own scene entities. for (auto& layer : layers()) { diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index 9fe78d6ae6ed8..d50abdce3aa8b 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -26,9 +26,12 @@ class ContainerLayer : public Layer { void UpdateScene(SceneUpdateContext& context) override; bool should_render_as_frame() const { return !frame_rrect_.isEmpty(); } - void set_frame_properties(SkRRect frame_rrect, SkColor frame_color) { + void set_frame_properties(SkRRect frame_rrect, + SkColor frame_color, + float frame_opacity) { frame_rrect_ = frame_rrect; frame_color_ = frame_color; + frame_opacity_ = frame_opacity; } float elevation() const { return clamped_elevation_; } @@ -51,6 +54,7 @@ class ContainerLayer : public Layer { float parent_elevation_ = 0.0f; float elevation_ = 0.0f; float clamped_elevation_ = 0.0f; + float frame_opacity_ = 1.0f; FML_DISALLOW_COPY_AND_ASSIGN(ContainerLayer); }; diff --git a/flow/layers/layer.h b/flow/layers/layer.h index 028286c5a35b5..c311ee8d454ba 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -59,6 +59,7 @@ struct PrerollContext { // The following allow us to track properties like elevation and opacity // which stack with each other during Preroll. float total_elevation = 0.0f; + bool is_opaque = true; }; // Represents a single composited layer. Created on the UI thread but then diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc index db44dfd021731..921fcc5d06da3 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -168,7 +168,7 @@ void LayerTree::UpdateScene(SceneUpdateContext& context, context, SkRRect::MakeRect( SkRect::MakeWH(frame_size_.width(), frame_size_.height())), - SK_ColorTRANSPARENT, /* elevation */ 0.0f); + SK_ColorTRANSPARENT, /* opacity */ 1.0f, /* elevation */ 0.0f); if (root_layer_->needs_system_composite()) { root_layer_->UpdateScene(context); } diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 8c66e995286e3..423c2f38d97fe 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -6,28 +6,60 @@ namespace flutter { +#if defined(OS_FUCHSIA) +constexpr bool kRenderOpacityUsingSystemCompositor = true; +#else +constexpr bool kRenderOpacityUsingSystemCompositor = false; +#endif +constexpr float kOpacityElevationWhenUsingSystemCompositor = 0.01f; + OpacityLayer::OpacityLayer(int alpha, const SkPoint& offset) - : ContainerLayer(true), alpha_(alpha), offset_(offset) {} + : ContainerLayer(true), alpha_(alpha), offset_(offset) { +#if !defined(OS_FUCHSIA) + FML_DCHECK(!kRenderOpacityUsingSystemCompositor); +#endif + + if (kRenderOpacityUsingSystemCompositor) { + set_elevation(kOpacityElevationWhenUsingSystemCompositor); + } +} void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkMatrix child_matrix = matrix; + float parent_is_opaque = context->is_opaque; child_matrix.postTranslate(offset_.fX, offset_.fY); context->mutators_stack.PushTransform( SkMatrix::MakeTrans(offset_.fX, offset_.fY)); context->mutators_stack.PushOpacity(alpha_); + context->is_opaque = parent_is_opaque && (alpha_ == 255); ContainerLayer::Preroll(context, child_matrix); + context->is_opaque = parent_is_opaque; context->mutators_stack.Pop(); context->mutators_stack.Pop(); - set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY)); - if (context->view_embedder == nullptr && context->raster_cache && - SkRect::Intersects(context->cull_rect, paint_bounds())) { - Layer* child = layers()[0].get(); - SkMatrix ctm = child_matrix; + // When using the system compositor, do not include the offset or use the + // raster cache, since we are rendering as a separate piece of geometry. + if (kRenderOpacityUsingSystemCompositor) { + set_needs_system_composite(true); + set_frame_properties(SkRRect::MakeRect(paint_bounds()), SK_ColorTRANSPARENT, + alpha_ / 255.0f); + + // If the frame behind us is opaque, don't punch a hole in it for group + // opacity. + if (context->is_opaque) { + set_paint_bounds(SkRect()); + } + } else { + set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY)); + if (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); + ctm = RasterCache::GetIntegralTransCTM(ctm); #endif - context->raster_cache->Prepare(context, child, ctm); + context->raster_cache->Prepare(context, child, ctm); + } } } @@ -35,6 +67,25 @@ void OpacityLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "OpacityLayer::Paint"); FML_DCHECK(needs_painting()); + // The compositor will paint this layer (which is |Sk_ColorWHITE| scaled by + // opacity) via the model color on |SceneUpdateContext::Frame|. + // + // The child layers will be painted into the texture used by the Frame, so + // painting them here would actually cause them to be painted on the display + // twice -- once into the current canvas (which may be inside of another + // Frame) and once into the Frame's texture (which is then drawn on top of the + // current canvas). + if (kRenderOpacityUsingSystemCompositor) { + // 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); + return; + } + SkPaint paint; paint.setAlpha(alpha_); @@ -79,4 +130,13 @@ void OpacityLayer::Paint(PaintContext& context) const { ContainerLayer::Paint(context); } +void OpacityLayer::UpdateScene(SceneUpdateContext& context) { +#if defined(OS_FUCHSIA) + SceneUpdateContext::Transform transform( + context, SkMatrix::MakeTrans(offset_.fX, offset_.fY)); + + ContainerLayer::UpdateScene(context); +#endif +} + } // namespace flutter diff --git a/flow/layers/opacity_layer.h b/flow/layers/opacity_layer.h index 48a8137f35a10..8d92b7bb40477 100644 --- a/flow/layers/opacity_layer.h +++ b/flow/layers/opacity_layer.h @@ -30,9 +30,7 @@ class OpacityLayer : public ContainerLayer { void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void Paint(PaintContext& context) const override; - - // TODO(chinmaygarde): Once SCN-139 is addressed, introduce a new node in the - // session scene hierarchy. + void UpdateScene(SceneUpdateContext& context) override; private: int alpha_; diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 42da85b5b5aae..995bc827b77ad 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -54,7 +54,7 @@ PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, frame_rrect = SkRRect::MakeRect(path.getBounds()); } - set_frame_properties(frame_rrect, color_); + set_frame_properties(frame_rrect, color_, /* opacity */ 1.0f); } set_elevation(elevation); } @@ -75,6 +75,12 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, // Let the system compositor draw all shadows for us, by popping us out as // a new frame. set_needs_system_composite(true); + + // If the frame behind us is opaque, don't punch a hole in it for group + // opacity. + if (context->is_opaque) { + set_paint_bounds(SkRect()); + } } else { // 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 @@ -136,6 +142,14 @@ void PhysicalShapeLayer::Paint(PaintContext& context) const { // Frame) and once into the Frame's texture (which is then drawn on top of the // current canvas). if (kRenderPhysicalShapeUsingSystemCompositor) { + // 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); + return; } diff --git a/flow/scene_update_context.cc b/flow/scene_update_context.cc index 0f4c414b886d8..0d531d71fccdf 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,14 +60,10 @@ 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; @@ -95,7 +92,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; } @@ -103,43 +102,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( @@ -228,6 +222,7 @@ SceneUpdateContext::Entity::Entity(SceneUpdateContext& context) entity_node_(context.session()) { if (previous_entity_) previous_entity_->embedder_node().AddChild(entity_node_); + context.top_entity_ = this; } @@ -286,31 +281,38 @@ 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); SetEntityNodeClipPlanes(entity_node(), rrect_.getBounds()); - entity_node().AddChild(shape_node()); + entity_node().AddChild(opacity_node_); + entity_node().AddChild(shape_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) { @@ -319,10 +321,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 aade6d6e07193..d7097e63d79ad 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -91,25 +91,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 @@ -117,25 +112,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, @@ -208,6 +203,7 @@ class SceneUpdateContext { scenic::ShapeNode shape_node, const SkRRect& rrect, SkColor color, + float opacity, const SkRect& paint_bounds, std::vector paint_layers, Layer* layer); @@ -219,7 +215,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, From ba18ca591063cec7cb3a640dbdb09e647f452020 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Wed, 11 Sep 2019 15:55:42 -0700 Subject: [PATCH 6/9] Addressing review feedback --- flow/layers/container_layer.cc | 11 +--- flow/layers/layer.h | 8 +-- flow/layers/layer_tree.cc | 61 ++++++++++--------- flow/layers/layer_tree.h | 11 ++-- flow/layers/physical_shape_layer.cc | 14 ++--- flow/raster_cache.cc | 4 +- flow/scene_update_context.cc | 4 +- flow/scene_update_context.h | 24 ++++---- lib/ui/compositing/scene.cc | 16 ++--- lib/ui/compositing/scene.h | 2 +- lib/ui/window/viewport_metrics.cc | 11 +++- lib/ui/window/viewport_metrics.h | 2 + shell/common/engine.cc | 8 ++- .../embedder/tests/embedder_unittests.cc | 8 +++ 14 files changed, 106 insertions(+), 78 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 4fd171ba39956..289ece362aab9 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -30,12 +30,7 @@ static float ClampElevation(float elevation, ContainerLayer::ContainerLayer(bool force_single_child) { // Place all "child" layers under a single child if requested. if (force_single_child) { - // 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(); - - single_child_ = std::make_shared(identity); + single_child_ = std::make_shared(SkMatrix::I()); single_child_->set_parent(this); layers_.push_back(single_child_); } @@ -57,8 +52,8 @@ void ContainerLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { // 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_depth); + clamped_elevation_ = ClampElevation(elevation_, parent_elevation_, + context->frame_physical_depth); context->total_elevation += clamped_elevation_; SkRect child_paint_bounds = SkRect::MakeEmpty(); diff --git a/flow/layers/layer.h b/flow/layers/layer.h index c311ee8d454ba..8a8419b2bf516 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -53,8 +53,8 @@ struct PrerollContext { const bool checkerboard_offscreen_layers; // The folllowing allow us to make use of the scene metrics during Preroll. - float frame_depth; - float frame_pixel_ratio; + float frame_physical_depth; + float frame_device_pixel_ratio; // The following allow us to track properties like elevation and opacity // which stack with each other during Preroll. @@ -91,8 +91,8 @@ class Layer { const bool checkerboard_offscreen_layers; // The folllowing allow us to make use of the scene metrics during Paint. - float frame_depth; - float frame_pixel_ratio; + 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 921fcc5d06da3..5639ef1675d7d 100644 --- a/flow/layers/layer_tree.cc +++ b/flow/layers/layer_tree.cc @@ -16,11 +16,11 @@ namespace flutter { LayerTree::LayerTree(const SkISize& frame_size, - float frame_depth, - float frame_pixel_ratio) + float frame_physical_depth, + float frame_device_pixel_ratio) : frame_size_(frame_size), - frame_depth_(frame_depth), - frame_pixel_ratio_(frame_pixel_ratio), + 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) {} @@ -51,8 +51,8 @@ void LayerTree::Preroll(CompositorContext::ScopedFrame& frame, frame.context().ui_time(), frame.context().texture_registry(), checkerboard_offscreen_layers_, - frame_depth_, - frame_pixel_ratio_}; + frame_physical_depth_, + frame_device_pixel_ratio_}; root_layer_->Preroll(&context, frame.root_surface_transformation()); } @@ -80,8 +80,8 @@ void LayerTree::Paint(CompositorContext::ScopedFrame& frame, frame.context().texture_registry(), ignore_raster_cache ? nullptr : &frame.context().raster_cache(), checkerboard_offscreen_layers_, - frame_depth_, - frame_pixel_ratio_}; + frame_physical_depth_, + frame_device_pixel_ratio_}; if (root_layer_->needs_painting()) root_layer_->Paint(context); @@ -105,18 +105,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 - frame_depth_, // maximum depth allowed for rendering - frame_pixel_ratio_ // ratio between logical and physical + 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(); @@ -128,13 +128,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 - frame_depth_, // maximum depth allowed for rendering - frame_pixel_ratio_ // ratio between logical and physical + 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 @@ -156,9 +156,14 @@ void LayerTree::UpdateScene(SceneUpdateContext& context, TRACE_EVENT0("flutter", "LayerTree::UpdateScene"); // Ensure the context is aware of the view metrics. - context.set_frame_dimensions(frame_size_, frame_depth_, frame_pixel_ratio_); + context.set_frame_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 diff --git a/flow/layers/layer_tree.h b/flow/layers/layer_tree.h index a9913c5bd7ca6..a1ea725353560 100644 --- a/flow/layers/layer_tree.h +++ b/flow/layers/layer_tree.h @@ -27,8 +27,8 @@ class SceneUpdateContext; class LayerTree { public: LayerTree(const SkISize& frame_size, - float frame_depth, - float frame_pixel_ratio); + float frame_physical_depth, + float frame_device_pixel_ratio); ~LayerTree(); void Preroll(CompositorContext::ScopedFrame& frame, @@ -45,6 +45,8 @@ class LayerTree { } const SkISize& frame_size() const { return 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_; } @@ -75,8 +77,9 @@ class LayerTree { fml::TimePoint build_start_; fml::TimePoint build_finish_; SkISize frame_size_; // Physical pixels. - float frame_depth_; - float frame_pixel_ratio_; // Ratio between logical and physical pixels. + float frame_physical_depth_; + float + frame_device_pixel_ratio_; // Ratio between logical and physical pixels. uint32_t rasterizer_tracing_threshold_; bool checkerboard_raster_cache_images_; bool checkerboard_offscreen_layers_; diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 995bc827b77ad..cc596fea25f09 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -117,13 +117,13 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, // t = tangent of AOB, i.e., multiplier for elevation to extent SkRect bounds(path_.getBounds()); // tangent for x - double tx = - (kLightRadius * context->frame_pixel_ratio + bounds.width() * 0.5) / - kLightHeight; + double tx = (kLightRadius * context->frame_device_pixel_ratio + + bounds.width() * 0.5) / + kLightHeight; // tangent for y - double ty = - (kLightRadius * context->frame_pixel_ratio + bounds.height() * 0.5) / - kLightHeight; + double ty = (kLightRadius * context->frame_device_pixel_ratio + + bounds.height() * 0.5) / + kLightHeight; bounds.outset(elevation() * tx, elevation() * ty); set_paint_bounds(bounds); } @@ -155,7 +155,7 @@ void PhysicalShapeLayer::Paint(PaintContext& context) const { if (elevation() != 0) { DrawShadow(context.leaf_nodes_canvas, path_, shadow_color_, elevation(), - SkColorGetA(color_) != 0xff, context.frame_pixel_ratio); + SkColorGetA(color_) != 0xff, context.frame_device_pixel_ratio); } // Call drawPath without clip if possible for better performance. diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc index f5c789d54ce8d..8e85f23444d08 100644 --- a/flow/raster_cache.cc +++ b/flow/raster_cache.cc @@ -174,8 +174,8 @@ void RasterCache::Prepare(PrerollContext* context, context->texture_registry, context->raster_cache, context->checkerboard_offscreen_layers, - context->frame_depth, - context->frame_pixel_ratio}; + 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 0d531d71fccdf..0e694b8e6e548 100644 --- a/flow/scene_update_context.cc +++ b/flow/scene_update_context.cc @@ -200,8 +200,8 @@ SceneUpdateContext::ExecutePaintTasks(CompositorContext::ScopedFrame& frame) { frame.context().texture_registry(), &frame.context().raster_cache(), false, - frame_depth_, - frame_pixel_ratio_}; + frame_physical_depth_, + frame_device_pixel_ratio_}; canvas->restoreToCount(1); canvas->save(); canvas->clear(task.background_color); diff --git a/flow/scene_update_context.h b/flow/scene_update_context.h index d7097e63d79ad..0c311cae850e2 100644 --- a/flow/scene_update_context.h +++ b/flow/scene_update_context.h @@ -147,16 +147,16 @@ class SceneUpdateContext { } const fuchsia::ui::gfx::MetricsPtr& metrics() const { return metrics_; } - void set_frame_dimensions(const SkISize& frame_size, - float frame_depth, - float frame_pixel_ratio) { - frame_size_ = frame_size; - frame_depth_ = frame_depth; - frame_pixel_ratio_ = frame_pixel_ratio; + void set_frame_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_size_; } - float frame_depth() const { return frame_depth_; } - float frame_pixel_ratio() const { return frame_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 @@ -234,9 +234,9 @@ class SceneUpdateContext { SurfaceProducer* const surface_producer_; fuchsia::ui::gfx::MetricsPtr metrics_; - SkISize frame_size_; // Physical pixels. - float frame_depth_ = 0.0f; - float frame_pixel_ratio_ = + 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/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index 272244b4123c5..80ee79f2f7622 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -41,15 +41,15 @@ Scene::Scene(std::shared_ptr rootLayer, bool checkerboardOffscreenLayers) { auto viewport_metrics = UIDartState::Current()->window()->viewport_metrics(); - m_layerTree = std::make_unique( + layer_tree_ = std::make_unique( SkISize::Make(viewport_metrics.physical_width, viewport_metrics.physical_height), viewport_metrics.physical_depth, viewport_metrics.device_pixel_ratio); - m_layerTree->set_root_layer(std::move(rootLayer)); - m_layerTree->set_rasterizer_tracing_threshold(rasterizerTracingThreshold); - m_layerTree->set_checkerboard_raster_cache_images( + 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() {} @@ -63,11 +63,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."); } @@ -76,7 +76,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/window/viewport_metrics.cc b/lib/ui/window/viewport_metrics.cc index f18972992189a..44e42957e3a8f 100644 --- a/lib/ui/window/viewport_metrics.cc +++ b/lib/ui/window/viewport_metrics.cc @@ -39,6 +39,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,7 +72,12 @@ 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) {} + 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); +} ViewportMetrics::ViewportMetrics(const ViewportMetrics& other) = default; diff --git a/lib/ui/window/viewport_metrics.h b/lib/ui/window/viewport_metrics.h index 5ca8b78eaa064..6868d34d87bbf 100644 --- a/lib/ui/window/viewport_metrics.h +++ b/lib/ui/window/viewport_metrics.h @@ -7,6 +7,8 @@ #include +#include "flutter/fml/logging.h" + namespace flutter { // This is the value of double.maxFinite from dart:core. diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 6c8d9cdb7ce17..b9a5602d75c0f 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -424,7 +424,13 @@ void Engine::ScheduleFrame(bool regenerate_layer_tree) { } void Engine::Render(std::unique_ptr layer_tree) { - if (!layer_tree || layer_tree->frame_size().isEmpty()) + if (!layer_tree) + return; + + // 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; animator_->Render(std::move(layer_tree)); diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 5304a959281d5..d3e4825bf36cc 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -633,6 +633,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLFramebuffer) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); @@ -724,6 +725,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLTexture) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); @@ -815,6 +817,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToSoftwareBuffer) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); @@ -1106,6 +1109,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderKnownScene) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); @@ -1283,6 +1287,7 @@ TEST_F(EmbedderTest, event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); @@ -1403,6 +1408,7 @@ TEST_F(EmbedderTest, CustomCompositorMustWorkWithCustomTaskRunner) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); @@ -1481,6 +1487,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderWithRootLayerOnly) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); @@ -1593,6 +1600,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderWithPlatformLayerOnBottom) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); From 4f2f3e14bf0c1c92f8ec03ecf418704cb12a4d9e Mon Sep 17 00:00:00 2001 From: David Worsham Date: Fri, 13 Sep 2019 13:01:13 -0700 Subject: [PATCH 7/9] Fix Windows build --- lib/ui/compositing/scene.cc | 3 ++- shell/common/shell_test.cc | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index 80ee79f2f7622..d02a232ac0433 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -44,7 +44,8 @@ Scene::Scene(std::shared_ptr rootLayer, layer_tree_ = std::make_unique( SkISize::Make(viewport_metrics.physical_width, viewport_metrics.physical_height), - viewport_metrics.physical_depth, viewport_metrics.device_pixel_ratio); + 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( diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index c69409b2e8536..645f1a9760536 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -121,8 +121,8 @@ void ShellTest::PumpOneFrame(Shell* shell) { fml::WeakPtr runtime_delegate = shell->weak_engine_; shell->GetTaskRunners().GetUITaskRunner()->PostTask( [&latch, runtime_delegate]() { - auto layer_tree = - std::make_unique(SkISize::Make(1, 1), kUnsetDepth, 1.0f); + auto layer_tree = std::make_unique( + SkISize::Make(1, 1), static_cast(kUnsetDepth), 1.0f); SkMatrix identity; identity.setIdentity(); auto root_layer = std::make_shared(identity); From f585f25d4f3645f78a066cf08197a5870feeae70 Mon Sep 17 00:00:00 2001 From: David Worsham Date: Fri, 20 Sep 2019 19:01:05 -0700 Subject: [PATCH 8/9] Address more review feedback --- flow/layers/clip_path_layer.cc | 2 +- flow/layers/clip_rect_layer.cc | 2 +- flow/layers/clip_rrect_layer.cc | 2 +- flow/layers/opacity_layer.cc | 12 ++++++++---- flow/layers/physical_shape_layer.cc | 17 ++++++++--------- flow/scene_update_context.cc | 1 - 6 files changed, 19 insertions(+), 17 deletions(-) diff --git a/flow/layers/clip_path_layer.cc b/flow/layers/clip_path_layer.cc index 06c65f376bbc0..6f3f930a4c0eb 100644 --- a/flow/layers/clip_path_layer.cc +++ b/flow/layers/clip_path_layer.cc @@ -23,7 +23,7 @@ void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { if (clip_path_bounds.intersect(paint_bounds())) { set_paint_bounds(clip_path_bounds); } else { - set_paint_bounds(SkRect()); + set_paint_bounds(SkRect::MakeEmpty()); } context->mutators_stack.Pop(); } diff --git a/flow/layers/clip_rect_layer.cc b/flow/layers/clip_rect_layer.cc index 66c45806baf0a..c4a1022dd4035 100644 --- a/flow/layers/clip_rect_layer.cc +++ b/flow/layers/clip_rect_layer.cc @@ -23,7 +23,7 @@ void ClipRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { if (clip_rect_bounds.intersect(paint_bounds())) { set_paint_bounds(clip_rect_bounds); } else { - set_paint_bounds(SkRect()); + set_paint_bounds(SkRect::MakeEmpty()); } context->mutators_stack.Pop(); } diff --git a/flow/layers/clip_rrect_layer.cc b/flow/layers/clip_rrect_layer.cc index 7409129cfcb88..70831a0236ae8 100644 --- a/flow/layers/clip_rrect_layer.cc +++ b/flow/layers/clip_rrect_layer.cc @@ -23,7 +23,7 @@ void ClipRRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { if (clip_rrect_bounds.intersect(paint_bounds())) { set_paint_bounds(clip_rrect_bounds); } else { - set_paint_bounds(SkRect()); + set_paint_bounds(SkRect::MakeEmpty()); } context->mutators_stack.Pop(); } diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 423c2f38d97fe..ec491b9d75cbc 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -16,7 +16,9 @@ constexpr float kOpacityElevationWhenUsingSystemCompositor = 0.01f; OpacityLayer::OpacityLayer(int alpha, const SkPoint& offset) : ContainerLayer(true), alpha_(alpha), offset_(offset) { #if !defined(OS_FUCHSIA) - FML_DCHECK(!kRenderOpacityUsingSystemCompositor); + static_assert(!kRenderOpacityUsingSystemCompositor, + "Delegation of OpacityLayer to the system compositor is only " + "allowed on Fuchsia"); #endif if (kRenderOpacityUsingSystemCompositor) { @@ -76,13 +78,15 @@ void OpacityLayer::Paint(PaintContext& context) const { // Frame) and once into the Frame's texture (which is then drawn on top of the // current canvas). if (kRenderOpacityUsingSystemCompositor) { - // 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. +#if defined(OS_FUCHSIA) + // On Fuchsia, 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 single-pass group opacity looks correct. SkPaint paint; paint.setColor(SK_ColorTRANSPARENT); paint.setBlendMode(SkBlendMode::kSrc); context.leaf_nodes_canvas->drawRect(paint_bounds(), paint); +#endif return; } diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index cc596fea25f09..458f364c0db2a 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -12,11 +12,7 @@ namespace flutter { constexpr SkScalar kLightHeight = 600; constexpr SkScalar kLightRadius = 800; -#if defined(OS_FUCHSIA) -constexpr bool kRenderPhysicalShapeUsingSystemCompositor = false; -#else constexpr bool kRenderPhysicalShapeUsingSystemCompositor = false; -#endif PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, SkColor shadow_color, @@ -28,7 +24,9 @@ PhysicalShapeLayer::PhysicalShapeLayer(SkColor color, path_(path), clip_behavior_(clip_behavior) { #if !defined(OS_FUCHSIA) - FML_DCHECK(!kRenderPhysicalShapeUsingSystemCompositor); + static_assert(!kRenderPhysicalShapeUsingSystemCompositor, + "Delegation of PhysicalShapeLayer to the system compositor is " + "only allowed on Fuchsia"); #endif // !defined(OS_FUCHSIA) // If rendering as a separate frame using the system compositor, then make @@ -142,14 +140,15 @@ void PhysicalShapeLayer::Paint(PaintContext& context) const { // Frame) and once into the Frame's texture (which is then drawn on top of the // current canvas). if (kRenderPhysicalShapeUsingSystemCompositor) { - // 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. +#if defined(OS_FUCHSIA) + // On Fuchsia, 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 single-pass group opacity looks correct. SkPaint paint; paint.setColor(SK_ColorTRANSPARENT); paint.setBlendMode(SkBlendMode::kSrc); context.leaf_nodes_canvas->drawRect(paint_bounds(), paint); - +#endif return; } diff --git a/flow/scene_update_context.cc b/flow/scene_update_context.cc index 0e694b8e6e548..ff3bacaa8c097 100644 --- a/flow/scene_update_context.cc +++ b/flow/scene_update_context.cc @@ -222,7 +222,6 @@ SceneUpdateContext::Entity::Entity(SceneUpdateContext& context) entity_node_(context.session()) { if (previous_entity_) previous_entity_->embedder_node().AddChild(entity_node_); - context.top_entity_ = this; } From 552afd8b76107eb2fb306831ef607b51603efb6e Mon Sep 17 00:00:00 2001 From: David Worsham Date: Wed, 25 Sep 2019 12:08:12 -0400 Subject: [PATCH 9/9] Fix broken tests --- shell/platform/embedder/tests/embedder_unittests.cc | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index d3e4825bf36cc..0959b4c3e33f5 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1409,7 +1409,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()); @@ -1784,6 +1783,7 @@ TEST_F(EmbedderTest, // Flutter still thinks it is 800 x 600. Only the root surface is rotated. event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); ASSERT_TRUE(engine.is_valid()); @@ -1818,6 +1818,7 @@ TEST_F(EmbedderTest, CanRenderSceneWithoutCustomCompositor) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); @@ -1860,6 +1861,7 @@ TEST_F(EmbedderTest, CanRenderSceneWithoutCustomCompositorWithTransformation) { // Flutter still thinks it is 800 x 600. event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); @@ -1894,6 +1896,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithoutCompositor) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); @@ -1936,6 +1939,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithoutCompositorWithXform) { // Flutter still thinks it is 800 x 600. event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); @@ -1970,6 +1974,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithCompositor) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); @@ -2013,6 +2018,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithCompositorWithXform) { // Flutter still thinks it is 800 x 600. event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); @@ -2123,6 +2129,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithCompositorOnNonRootLayer) { event.struct_size = sizeof(event); event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess); @@ -2242,6 +2249,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithCompositorOnNonRootLayerWithXform) { // Flutter still thinks it is 800 x 600. event.width = 800; event.height = 600; + event.pixel_ratio = 1.0; ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), kSuccess);