From 302034ed4a92b209274cf4c8c45817a0659dea82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Tiselice?= Date: Mon, 4 May 2020 18:22:09 +0200 Subject: [PATCH 1/2] Fixed ChildSceneLayer elevation issue on Fuchsia. ChildSceneLayers would draw on top of stacked Flutter content on Fuchsia if not wrapped up in a Material widget, i.e. a PhysicalShapeLayer. This patch pushes the logic from there to all types of Layers. --- flow/layers/child_scene_layer.cc | 5 ++ flow/layers/container_layer.cc | 76 ++++++++++++++++++++++-- flow/layers/container_layer.h | 1 + flow/layers/layer.cc | 48 ++++++++++++++- flow/layers/layer.h | 6 ++ flow/layers/performance_overlay_layer.cc | 7 +++ flow/layers/performance_overlay_layer.h | 1 + flow/layers/physical_shape_layer.cc | 67 --------------------- flow/layers/physical_shape_layer.h | 7 --- flow/layers/picture_layer.cc | 5 ++ flow/layers/platform_view_layer.cc | 4 ++ flow/layers/shader_mask_layer.cc | 4 ++ flow/layers/texture_layer.cc | 4 ++ 13 files changed, 155 insertions(+), 80 deletions(-) diff --git a/flow/layers/child_scene_layer.cc b/flow/layers/child_scene_layer.cc index 4e533130c1d4b..795946e0855c9 100644 --- a/flow/layers/child_scene_layer.cc +++ b/flow/layers/child_scene_layer.cc @@ -20,6 +20,9 @@ ChildSceneLayer::ChildSceneLayer(zx_koid_t layer_id, void ChildSceneLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { TRACE_EVENT0("flutter", "ChildSceneLayer::Preroll"); set_needs_system_composite(true); + + CheckForChildLayerBelow(context); + context->child_scene_layer_exists_below = true; // An alpha "hole punch" is required if the frame behind us is not opaque. @@ -47,6 +50,8 @@ void ChildSceneLayer::UpdateScene(SceneUpdateContext& context) { TRACE_EVENT0("flutter", "ChildSceneLayer::UpdateScene"); FML_DCHECK(needs_system_composite()); + Layer::UpdateScene(context); + auto* view_holder = ViewHolder::FromId(layer_id_); FML_DCHECK(view_holder); diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 39372f6c84dd8..6470d45649c03 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -29,6 +29,11 @@ void ContainerLayer::Paint(PaintContext& context) const { void ContainerLayer::PrerollChildren(PrerollContext* context, const SkMatrix& child_matrix, SkRect* child_paint_bounds) { +#if defined(OS_FUCHSIA) + child_layer_exists_below_ = context->child_scene_layer_exists_below; + context->child_scene_layer_exists_below = false; +#endif + // Platform views have no children, so context->has_platform_view should // always be false. FML_DCHECK(!context->has_platform_view); @@ -51,6 +56,14 @@ void ContainerLayer::PrerollChildren(PrerollContext* context, } context->has_platform_view = child_has_platform_view; + +#if defined(OS_FUCHSIA) + if (child_layer_exists_below_) { + set_needs_system_composite(true); + } + context->child_scene_layer_exists_below = + context->child_scene_layer_exists_below || child_layer_exists_below_; +#endif } void ContainerLayer::PaintChildren(PaintContext& context) const { @@ -67,19 +80,72 @@ void ContainerLayer::PaintChildren(PaintContext& context) const { #if defined(OS_FUCHSIA) +void ContainerLayer::CheckForChildLayerBelow(PrerollContext* context) { + // All ContainerLayers make the check in PrerollChildren. +} + void ContainerLayer::UpdateScene(SceneUpdateContext& context) { UpdateSceneChildren(context); } void ContainerLayer::UpdateSceneChildren(SceneUpdateContext& context) { + auto update_scene_layers = [&] { + // 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); + } + } + }; + FML_DCHECK(needs_system_composite()); - // 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); + // If there is embedded Fuchsia content in the scene (a ChildSceneLayer), + // PhysicalShapeLayers that appear above the embedded content will be turned + // into their own Scenic layers. + if (child_layer_exists_below_) { + float global_scenic_elevation = + context.GetGlobalElevationForNextScenicLayer(); + float local_scenic_elevation = + global_scenic_elevation - context.scenic_elevation(); + float z_translation = -local_scenic_elevation; + + // Retained rendering: speedup by reusing a retained entity node if + // possible. When an entity node is reused, no paint layer is added to the + // frame so we won't call PhysicalShapeLayer::Paint. + LayerRasterCacheKey key(unique_id(), context.Matrix()); + if (context.HasRetainedNode(key)) { + TRACE_EVENT_INSTANT0("flutter", "retained layer cache hit"); + scenic::EntityNode* retained_node = context.GetRetainedNode(key); + FML_DCHECK(context.top_entity()); + FML_DCHECK(retained_node->session() == context.session()); + + // Re-adjust the elevation. + retained_node->SetTranslation(0.f, 0.f, z_translation); + + context.top_entity()->entity_node().AddChild(*retained_node); + return; } + + TRACE_EVENT_INSTANT0("flutter", "cache miss, creating"); + // If we can't find an existing retained surface, create one. + SceneUpdateContext::Frame frame( + context, SkRRect::MakeRect(paint_bounds()), SK_ColorTRANSPARENT, + SkScalarRoundToInt(context.alphaf() * 255), + "flutter::PhysicalShapeLayer", z_translation, this); + + frame.AddPaintLayer(this); + + // Node: UpdateSceneChildren needs to be called here so that |frame| is + // still in scope (and therefore alive) while UpdateSceneChildren is being + // called. + float scenic_elevation = context.scenic_elevation(); + context.set_scenic_elevation(scenic_elevation + local_scenic_elevation); + update_scene_layers(); + context.set_scenic_elevation(scenic_elevation); + } else { + update_scene_layers(); } } diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h index df5be5e8b9466..a0b67a59c3969 100644 --- a/flow/layers/container_layer.h +++ b/flow/layers/container_layer.h @@ -19,6 +19,7 @@ class ContainerLayer : public Layer { void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void Paint(PaintContext& context) const override; #if defined(OS_FUCHSIA) + void CheckForChildLayerBelow(PrerollContext* context) override; void UpdateScene(SceneUpdateContext& context) override; #endif // defined(OS_FUCHSIA) diff --git a/flow/layers/layer.cc b/flow/layers/layer.cc index d4bd809d5f2cf..641fef0f8f5c1 100644 --- a/flow/layers/layer.cc +++ b/flow/layers/layer.cc @@ -56,7 +56,53 @@ Layer::AutoPrerollSaveLayerState::~AutoPrerollSaveLayerState() { } #if defined(OS_FUCHSIA) -void Layer::UpdateScene(SceneUpdateContext& context) {} + +void Layer::CheckForChildLayerBelow(PrerollContext* context) { + child_layer_exists_below_ = context->child_scene_layer_exists_below; + if (child_layer_exists_below_) { + set_needs_system_composite(true); + } +} + +void Layer::UpdateScene(SceneUpdateContext& context) { + // If there is embedded Fuchsia content in the scene (a ChildSceneLayer), + // PhysicalShapeLayers that appear above the embedded content will be turned + // into their own Scenic layers. + if (child_layer_exists_below_) { + float global_scenic_elevation = + context.GetGlobalElevationForNextScenicLayer(); + float local_scenic_elevation = + global_scenic_elevation - context.scenic_elevation(); + float z_translation = -local_scenic_elevation; + + // Retained rendering: speedup by reusing a retained entity node if + // possible. When an entity node is reused, no paint layer is added to the + // frame so we won't call PhysicalShapeLayer::Paint. + LayerRasterCacheKey key(unique_id(), context.Matrix()); + if (context.HasRetainedNode(key)) { + TRACE_EVENT_INSTANT0("flutter", "retained layer cache hit"); + scenic::EntityNode* retained_node = context.GetRetainedNode(key); + FML_DCHECK(context.top_entity()); + FML_DCHECK(retained_node->session() == context.session()); + + // Re-adjust the elevation. + retained_node->SetTranslation(0.f, 0.f, z_translation); + + context.top_entity()->entity_node().AddChild(*retained_node); + return; + } + + TRACE_EVENT_INSTANT0("flutter", "cache miss, creating"); + // If we can't find an existing retained surface, create one. + SceneUpdateContext::Frame frame( + context, SkRRect::MakeRect(paint_bounds()), SK_ColorTRANSPARENT, + SkScalarRoundToInt(context.alphaf() * 255), + "flutter::PhysicalShapeLayer", z_translation, this); + + frame.AddPaintLayer(this); + } +} + #endif // defined(OS_FUCHSIA) Layer::AutoSaveLayer::AutoSaveLayer(const PaintContext& paint_context, diff --git a/flow/layers/layer.h b/flow/layers/layer.h index dfd022a350a0f..65df081cb7f59 100644 --- a/flow/layers/layer.h +++ b/flow/layers/layer.h @@ -165,6 +165,7 @@ class Layer { #if defined(OS_FUCHSIA) // Updates the system composited scene. virtual void UpdateScene(SceneUpdateContext& context); + virtual void CheckForChildLayerBelow(PrerollContext* context); #endif bool needs_system_composite() const { return needs_system_composite_; } @@ -184,6 +185,11 @@ class Layer { uint64_t unique_id() const { return unique_id_; } + protected: +#if defined(OS_FUCHSIA) + bool child_layer_exists_below_ = false; +#endif + private: SkRect paint_bounds_; uint64_t unique_id_; diff --git a/flow/layers/performance_overlay_layer.cc b/flow/layers/performance_overlay_layer.cc index dacb9428604f8..7c0c7be7bc03f 100644 --- a/flow/layers/performance_overlay_layer.cc +++ b/flow/layers/performance_overlay_layer.cc @@ -97,4 +97,11 @@ void PerformanceOverlayLayer::Paint(PaintContext& context) const { options_ & kDisplayEngineStatistics, "UI", font_path_); } +void PerformanceOverlayLayer::Preroll(PrerollContext* context, + const SkMatrix& matrix) { +#if defined(OS_FUCHSIA) + CheckForChildLayerBelow(context); +#endif +} + } // namespace flutter diff --git a/flow/layers/performance_overlay_layer.h b/flow/layers/performance_overlay_layer.h index b1434a221e688..c1f595f2ff8f9 100644 --- a/flow/layers/performance_overlay_layer.h +++ b/flow/layers/performance_overlay_layer.h @@ -30,6 +30,7 @@ class PerformanceOverlayLayer : public Layer { const char* font_path = nullptr); void Paint(PaintContext& context) const override; + void Preroll(PrerollContext* context, const SkMatrix& matrix) override; private: int options_; diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 675ec5917c5df..4f87fb23605a0 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -52,21 +52,10 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, context->total_elevation += elevation_; total_elevation_ = context->total_elevation; -#if defined(OS_FUCHSIA) - child_layer_exists_below_ = context->child_scene_layer_exists_below; - context->child_scene_layer_exists_below = false; -#endif SkRect child_paint_bounds; PrerollChildren(context, matrix, &child_paint_bounds); -#if defined(OS_FUCHSIA) - if (child_layer_exists_below_) { - set_needs_system_composite(true); - } - context->child_scene_layer_exists_below = - context->child_scene_layer_exists_below || child_layer_exists_below_; -#endif context->total_elevation -= elevation_; if (elevation_ == 0) { @@ -80,62 +69,6 @@ void PhysicalShapeLayer::Preroll(PrerollContext* context, } } -#if defined(OS_FUCHSIA) - -void PhysicalShapeLayer::UpdateScene(SceneUpdateContext& context) { - FML_DCHECK(needs_system_composite()); - TRACE_EVENT0("flutter", "PhysicalShapeLayer::UpdateScene"); - - // If there is embedded Fuchsia content in the scene (a ChildSceneLayer), - // PhysicalShapeLayers that appear above the embedded content will be turned - // into their own Scenic layers. - if (child_layer_exists_below_) { - float global_scenic_elevation = - context.GetGlobalElevationForNextScenicLayer(); - float local_scenic_elevation = - global_scenic_elevation - context.scenic_elevation(); - float z_translation = -local_scenic_elevation; - - // Retained rendering: speedup by reusing a retained entity node if - // possible. When an entity node is reused, no paint layer is added to the - // frame so we won't call PhysicalShapeLayer::Paint. - LayerRasterCacheKey key(unique_id(), context.Matrix()); - if (context.HasRetainedNode(key)) { - TRACE_EVENT_INSTANT0("flutter", "retained layer cache hit"); - scenic::EntityNode* retained_node = context.GetRetainedNode(key); - FML_DCHECK(context.top_entity()); - FML_DCHECK(retained_node->session() == context.session()); - - // Re-adjust the elevation. - retained_node->SetTranslation(0.f, 0.f, z_translation); - - context.top_entity()->entity_node().AddChild(*retained_node); - return; - } - - TRACE_EVENT_INSTANT0("flutter", "cache miss, creating"); - // If we can't find an existing retained surface, create one. - SceneUpdateContext::Frame frame(context, frameRRect_, SK_ColorTRANSPARENT, - SkScalarRoundToInt(context.alphaf() * 255), - "flutter::PhysicalShapeLayer", - z_translation, this); - - frame.AddPaintLayer(this); - - // Node: UpdateSceneChildren needs to be called here so that |frame| is - // still in scope (and therefore alive) while UpdateSceneChildren is being - // called. - float scenic_elevation = context.scenic_elevation(); - context.set_scenic_elevation(scenic_elevation + local_scenic_elevation); - ContainerLayer::UpdateSceneChildren(context); - context.set_scenic_elevation(scenic_elevation); - } else { - ContainerLayer::UpdateSceneChildren(context); - } -} - -#endif // defined(OS_FUCHSIA) - void PhysicalShapeLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "PhysicalShapeLayer::Paint"); FML_DCHECK(needs_painting()); diff --git a/flow/layers/physical_shape_layer.h b/flow/layers/physical_shape_layer.h index 106327f47ec9c..2c04368e9a81e 100644 --- a/flow/layers/physical_shape_layer.h +++ b/flow/layers/physical_shape_layer.h @@ -35,16 +35,9 @@ class PhysicalShapeLayer : public ContainerLayer { return clip_behavior_ == Clip::antiAliasWithSaveLayer; } -#if defined(OS_FUCHSIA) - void UpdateScene(SceneUpdateContext& context) override; -#endif // defined(OS_FUCHSIA) - float total_elevation() const { return total_elevation_; } private: -#if defined(OS_FUCHSIA) - bool child_layer_exists_below_ = false; -#endif SkColor color_; SkColor shadow_color_; float elevation_ = 0.0f; diff --git a/flow/layers/picture_layer.cc b/flow/layers/picture_layer.cc index c60b09264f2f1..6d41b92314ba7 100644 --- a/flow/layers/picture_layer.cc +++ b/flow/layers/picture_layer.cc @@ -19,6 +19,11 @@ PictureLayer::PictureLayer(const SkPoint& offset, void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { TRACE_EVENT0("flutter", "PictureLayer::Preroll"); + +#if defined(OS_FUCHSIA) + CheckForChildLayerBelow(context); +#endif + SkPicture* sk_picture = picture(); if (auto* cache = context->raster_cache) { diff --git a/flow/layers/platform_view_layer.cc b/flow/layers/platform_view_layer.cc index 81541b7a0cde8..81c40a99f0d89 100644 --- a/flow/layers/platform_view_layer.cc +++ b/flow/layers/platform_view_layer.cc @@ -13,6 +13,10 @@ PlatformViewLayer::PlatformViewLayer(const SkPoint& offset, void PlatformViewLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { +#if defined(OS_FUCHSIA) + CheckForChildLayerBelow(context); +#endif + set_paint_bounds(SkRect::MakeXYWH(offset_.x(), offset_.y(), size_.width(), size_.height())); diff --git a/flow/layers/shader_mask_layer.cc b/flow/layers/shader_mask_layer.cc index 157354f7314b4..9b76e1c546464 100644 --- a/flow/layers/shader_mask_layer.cc +++ b/flow/layers/shader_mask_layer.cc @@ -12,6 +12,10 @@ ShaderMaskLayer::ShaderMaskLayer(sk_sp shader, : shader_(shader), mask_rect_(mask_rect), blend_mode_(blend_mode) {} void ShaderMaskLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { +#if defined(OS_FUCHSIA) + CheckForChildLayerBelow(context); +#endif + Layer::AutoPrerollSaveLayerState save = Layer::AutoPrerollSaveLayerState::Create(context); ContainerLayer::Preroll(context, matrix); diff --git a/flow/layers/texture_layer.cc b/flow/layers/texture_layer.cc index e5df2b795dd34..00d976eeed700 100644 --- a/flow/layers/texture_layer.cc +++ b/flow/layers/texture_layer.cc @@ -17,6 +17,10 @@ TextureLayer::TextureLayer(const SkPoint& offset, void TextureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { TRACE_EVENT0("flutter", "TextureLayer::Preroll"); +#if defined(OS_FUCHSIA) + CheckForChildLayerBelow(context); +#endif + set_paint_bounds(SkRect::MakeXYWH(offset_.x(), offset_.y(), size_.width(), size_.height())); } From ba35a922f804f91879f949650bf69da7c8660173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Tiselice?= Date: Wed, 6 May 2020 11:17:10 +0200 Subject: [PATCH 2/2] Removed Preroll from performance layer. Also fixed a comment. --- flow/layers/container_layer.cc | 2 +- flow/layers/performance_overlay_layer.cc | 7 ------- flow/layers/performance_overlay_layer.h | 1 - 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/flow/layers/container_layer.cc b/flow/layers/container_layer.cc index 6470d45649c03..28ab9cfcd56e8 100644 --- a/flow/layers/container_layer.cc +++ b/flow/layers/container_layer.cc @@ -91,7 +91,7 @@ void ContainerLayer::UpdateScene(SceneUpdateContext& context) { void ContainerLayer::UpdateSceneChildren(SceneUpdateContext& context) { auto update_scene_layers = [&] { // Paint all of the layers which need to be drawn into the container. - // These may be flattened down to a containing + // These may be flattened down to a containing Scenic Frame. for (auto& layer : layers_) { if (layer->needs_system_composite()) { layer->UpdateScene(context); diff --git a/flow/layers/performance_overlay_layer.cc b/flow/layers/performance_overlay_layer.cc index 7c0c7be7bc03f..dacb9428604f8 100644 --- a/flow/layers/performance_overlay_layer.cc +++ b/flow/layers/performance_overlay_layer.cc @@ -97,11 +97,4 @@ void PerformanceOverlayLayer::Paint(PaintContext& context) const { options_ & kDisplayEngineStatistics, "UI", font_path_); } -void PerformanceOverlayLayer::Preroll(PrerollContext* context, - const SkMatrix& matrix) { -#if defined(OS_FUCHSIA) - CheckForChildLayerBelow(context); -#endif -} - } // namespace flutter diff --git a/flow/layers/performance_overlay_layer.h b/flow/layers/performance_overlay_layer.h index c1f595f2ff8f9..b1434a221e688 100644 --- a/flow/layers/performance_overlay_layer.h +++ b/flow/layers/performance_overlay_layer.h @@ -30,7 +30,6 @@ class PerformanceOverlayLayer : public Layer { const char* font_path = nullptr); void Paint(PaintContext& context) const override; - void Preroll(PrerollContext* context, const SkMatrix& matrix) override; private: int options_;