From af511babc76354cd283efa3d3f80fdd4169f9ecb Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 5 Dec 2019 16:25:55 -0800 Subject: [PATCH] Do not paint a layer's children if the children were not prerolled (#14149) Prerolling a layer can have side effects. In particular, PlatformViewLayer::Preroll will call view_embedder->PrerollCompositeEmbeddedView. Clip layers will check whether the layer's children are all clipped and if so will skip calling Preroll on the children. However, the Paint implementation in these layers was always calling Paint on their children. This could result in a call to PlatformViewLayer::Paint without a corresponding call to PlatformViewLayer::Preroll. This translates to a CompositeEmbeddedView call without a PrerollCompositeEmbeddedView call on the affected view_id. The EmbedderExternalViewEmbedder implementation does not allow that. With this change, clip layers will only call PaintChildren if the preroll called PrerollChildren. See https://github.com/flutter/flutter/issues/46111 --- flow/layers/clip_path_layer.cc | 6 +++++- flow/layers/clip_path_layer.h | 1 + flow/layers/clip_rect_layer.cc | 6 +++++- flow/layers/clip_rect_layer.h | 1 + flow/layers/clip_rrect_layer.cc | 6 +++++- flow/layers/clip_rrect_layer.h | 1 + 6 files changed, 18 insertions(+), 3 deletions(-) diff --git a/flow/layers/clip_path_layer.cc b/flow/layers/clip_path_layer.cc index 3957837d0..4353debfc 100644 --- a/flow/layers/clip_path_layer.cc +++ b/flow/layers/clip_path_layer.cc @@ -20,7 +20,8 @@ ClipPathLayer::ClipPathLayer(const SkPath& clip_path, Clip clip_behavior) void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect previous_cull_rect = context->cull_rect; SkRect clip_path_bounds = clip_path_.getBounds(); - if (context->cull_rect.intersect(clip_path_bounds)) { + children_inside_clip_ = context->cull_rect.intersect(clip_path_bounds); + if (children_inside_clip_) { context->mutators_stack.PushClipPath(clip_path_); SkRect child_paint_bounds = SkRect::MakeEmpty(); PrerollChildren(context, matrix, &child_paint_bounds); @@ -49,6 +50,9 @@ void ClipPathLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "ClipPathLayer::Paint"); FML_DCHECK(needs_painting()); + if (!children_inside_clip_) + return; + SkAutoCanvasRestore save(context.internal_nodes_canvas, true); context.internal_nodes_canvas->clipPath(clip_path_, clip_behavior_ != Clip::hardEdge); diff --git a/flow/layers/clip_path_layer.h b/flow/layers/clip_path_layer.h index c21e53c34..aac23d693 100644 --- a/flow/layers/clip_path_layer.h +++ b/flow/layers/clip_path_layer.h @@ -24,6 +24,7 @@ class ClipPathLayer : public ContainerLayer { private: SkPath clip_path_; Clip clip_behavior_; + bool children_inside_clip_ = false; FML_DISALLOW_COPY_AND_ASSIGN(ClipPathLayer); }; diff --git a/flow/layers/clip_rect_layer.cc b/flow/layers/clip_rect_layer.cc index 191132a05..9e12839ae 100644 --- a/flow/layers/clip_rect_layer.cc +++ b/flow/layers/clip_rect_layer.cc @@ -13,7 +13,8 @@ ClipRectLayer::ClipRectLayer(const SkRect& clip_rect, Clip clip_behavior) void ClipRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect previous_cull_rect = context->cull_rect; - if (context->cull_rect.intersect(clip_rect_)) { + children_inside_clip_ = context->cull_rect.intersect(clip_rect_); + if (children_inside_clip_) { context->mutators_stack.PushClipRect(clip_rect_); SkRect child_paint_bounds = SkRect::MakeEmpty(); PrerollChildren(context, matrix, &child_paint_bounds); @@ -42,6 +43,9 @@ void ClipRectLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "ClipRectLayer::Paint"); FML_DCHECK(needs_painting()); + if (!children_inside_clip_) + return; + SkAutoCanvasRestore save(context.internal_nodes_canvas, true); context.internal_nodes_canvas->clipRect(clip_rect_, clip_behavior_ != Clip::hardEdge); diff --git a/flow/layers/clip_rect_layer.h b/flow/layers/clip_rect_layer.h index 50eef22a4..f503a5933 100644 --- a/flow/layers/clip_rect_layer.h +++ b/flow/layers/clip_rect_layer.h @@ -23,6 +23,7 @@ class ClipRectLayer : public ContainerLayer { private: SkRect clip_rect_; Clip clip_behavior_; + bool children_inside_clip_ = false; FML_DISALLOW_COPY_AND_ASSIGN(ClipRectLayer); }; diff --git a/flow/layers/clip_rrect_layer.cc b/flow/layers/clip_rrect_layer.cc index e02f8d241..4a22151d6 100644 --- a/flow/layers/clip_rrect_layer.cc +++ b/flow/layers/clip_rrect_layer.cc @@ -14,7 +14,8 @@ ClipRRectLayer::ClipRRectLayer(const SkRRect& clip_rrect, Clip clip_behavior) void ClipRRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { SkRect previous_cull_rect = context->cull_rect; SkRect clip_rrect_bounds = clip_rrect_.getBounds(); - if (context->cull_rect.intersect(clip_rrect_bounds)) { + children_inside_clip_ = context->cull_rect.intersect(clip_rrect_bounds); + if (children_inside_clip_) { context->mutators_stack.PushClipRRect(clip_rrect_); SkRect child_paint_bounds = SkRect::MakeEmpty(); PrerollChildren(context, matrix, &child_paint_bounds); @@ -43,6 +44,9 @@ void ClipRRectLayer::Paint(PaintContext& context) const { TRACE_EVENT0("flutter", "ClipRRectLayer::Paint"); FML_DCHECK(needs_painting()); + if (!children_inside_clip_) + return; + SkAutoCanvasRestore save(context.internal_nodes_canvas, true); context.internal_nodes_canvas->clipRRect(clip_rrect_, clip_behavior_ != Clip::hardEdge); diff --git a/flow/layers/clip_rrect_layer.h b/flow/layers/clip_rrect_layer.h index ce1cca2b5..45daf48ba 100644 --- a/flow/layers/clip_rrect_layer.h +++ b/flow/layers/clip_rrect_layer.h @@ -24,6 +24,7 @@ class ClipRRectLayer : public ContainerLayer { private: SkRRect clip_rrect_; Clip clip_behavior_; + bool children_inside_clip_ = false; FML_DISALLOW_COPY_AND_ASSIGN(ClipRRectLayer); };