Compute cull_rect and optimize in Layer::Preroll (flutter#6923)

This PR replaces the unused `PrerollContext::child_paint_bounds` with `PrerollContext::cull_rect` so we can prune unnecessary preroll tasks (especially cache) based on clips. This PR fixes flutter/flutter#24712 Performance test has been added (flutter/flutter#25381) to make sure that we won't regress again in the future. Note that the cull_rect here is very similar to those removed in flutter#6352 . We can't compute cull rects in SceneBuilder because of retained layers. But we can still compute and use them to optimize performance in Preroll.
yhzhan1002 · Dec 18, 2018 · ba11736 · ba11736
1 parent fbce2bf
commit ba11736
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 19 deletions.
diff --git a/flow/layers/clip_path_layer.cc b/flow/layers/clip_path_layer.cc
@@ -18,12 +18,17 @@ ClipPathLayer::ClipPathLayer(Clip clip_behavior)
 ClipPathLayer::~ClipPathLayer() = default;
 
 void ClipPathLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
-  SkRect child_paint_bounds = SkRect::MakeEmpty();
-  PrerollChildren(context, matrix, &child_paint_bounds);
-
-  if (child_paint_bounds.intersect(clip_path_.getBounds())) {
-    set_paint_bounds(child_paint_bounds);
+  SkRect previous_cull_rect = context->cull_rect;
+  SkRect clip_path_bounds = clip_path_.getBounds();
+  if (context->cull_rect.intersect(clip_path_bounds)) {
+    SkRect child_paint_bounds = SkRect::MakeEmpty();
+    PrerollChildren(context, matrix, &child_paint_bounds);
+
+    if (child_paint_bounds.intersect(clip_path_bounds)) {
+      set_paint_bounds(child_paint_bounds);
+    }
   }
+  context->cull_rect = previous_cull_rect;
 }
 
 #if defined(OS_FUCHSIA)

diff --git a/flow/layers/clip_rect_layer.cc b/flow/layers/clip_rect_layer.cc
@@ -12,12 +12,16 @@ ClipRectLayer::ClipRectLayer(Clip clip_behavior)
 ClipRectLayer::~ClipRectLayer() = default;
 
 void ClipRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
-  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 previous_cull_rect = context->cull_rect;
+  if (context->cull_rect.intersect(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);
+    }
   }
+  context->cull_rect = previous_cull_rect;
 }
 
 #if defined(OS_FUCHSIA)

diff --git a/flow/layers/clip_rrect_layer.cc b/flow/layers/clip_rrect_layer.cc
@@ -12,12 +12,17 @@ ClipRRectLayer::ClipRRectLayer(Clip clip_behavior)
 ClipRRectLayer::~ClipRRectLayer() = default;
 
 void ClipRRectLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
-  SkRect child_paint_bounds = SkRect::MakeEmpty();
-  PrerollChildren(context, matrix, &child_paint_bounds);
+  SkRect previous_cull_rect = context->cull_rect;
+  SkRect clip_rrect_bounds = clip_rrect_.getBounds();
+  if (context->cull_rect.intersect(clip_rrect_bounds)) {
+    SkRect child_paint_bounds = SkRect::MakeEmpty();
+    PrerollChildren(context, matrix, &child_paint_bounds);
 
-  if (child_paint_bounds.intersect(clip_rrect_.getBounds())) {
-    set_paint_bounds(child_paint_bounds);
+    if (child_paint_bounds.intersect(clip_rrect_bounds)) {
+      set_paint_bounds(child_paint_bounds);
+    }
   }
+  context->cull_rect = previous_cull_rect;
 }
 
 #if defined(OS_FUCHSIA)

diff --git a/flow/layers/layer.h b/flow/layers/layer.h
@@ -37,6 +37,8 @@
 
 namespace flow {
 
+static constexpr SkRect kGiantRect = SkRect::MakeLTRB(-1E9F, -1E9F, 1E9F, 1E9F);
+
 // This should be an exact copy of the Clip enum in painting.dart.
 enum Clip { none, hardEdge, antiAlias, antiAliasWithSaveLayer };
 
@@ -47,7 +49,7 @@ struct PrerollContext {
   GrContext* gr_context;
   ExternalViewEmbedder* view_embedder;
   SkColorSpace* dst_color_space;
-  SkRect child_paint_bounds;
+  SkRect cull_rect;
 
   // The following allows us to paint in the end of subtree preroll
   const Stopwatch& frame_time;

diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc
@@ -31,7 +31,7 @@ void LayerTree::Preroll(CompositorContext::ScopedFrame& frame,
       frame.gr_context(),
       frame.view_embedder(),
       color_space,
-      SkRect::MakeEmpty(),
+      kGiantRect,
       frame.context().frame_time(),
       frame.context().engine_time(),
       frame.context().texture_registry(),
@@ -113,7 +113,7 @@ sk_sp<SkPicture> LayerTree::Flatten(const SkRect& bounds) {
       nullptr,                  // gr_context  (used for the raster cache)
       nullptr,                  // external view embedder
       nullptr,                  // SkColorSpace* dst_color_space
-      SkRect::MakeEmpty(),      // SkRect child_paint_bounds
+      kGiantRect,               // SkRect cull_rect
       unused_stopwatch,         // frame time (dont care)
       unused_stopwatch,         // engine time (dont care)
       unused_texture_registry,  // texture registry (not supported)

diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc
@@ -15,7 +15,8 @@ void OpacityLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
   child_matrix.postTranslate(offset_.fX, offset_.fY);
   ContainerLayer::Preroll(context, child_matrix);
   set_paint_bounds(paint_bounds().makeOffset(offset_.fX, offset_.fY));
-  if (context->raster_cache && layers().size() == 1) {
+  if (context->raster_cache && layers().size() == 1 &&
+      SkRect::Intersects(context->cull_rect, paint_bounds())) {
     Layer* child = layers()[0].get();
     SkMatrix ctm = child_matrix;
 #ifndef SUPPORT_FRACTIONAL_TRANSLATION

diff --git a/flow/layers/transform_layer.cc b/flow/layers/transform_layer.cc
@@ -14,11 +14,21 @@ void TransformLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
   SkMatrix child_matrix;
   child_matrix.setConcat(matrix, transform_);
 
+  SkRect previous_cull_rect = context->cull_rect;
+  SkMatrix inverse_transform_;
+  if (transform_.invert(&inverse_transform_)) {
+    inverse_transform_.mapRect(&context->cull_rect);
+  } else {
+    context->cull_rect = kGiantRect;
+  }
+
   SkRect child_paint_bounds = SkRect::MakeEmpty();
   PrerollChildren(context, child_matrix, &child_paint_bounds);
 
   transform_.mapRect(&child_paint_bounds);
   set_paint_bounds(child_paint_bounds);
+
+  context->cull_rect = previous_cull_rect;
 }
 
 #if defined(OS_FUCHSIA)

diff --git a/lib/ui/geometry.dart b/lib/ui/geometry.dart
@@ -693,7 +693,7 @@ class Rect {
   /// A rectangle with left, top, right, and bottom edges all at zero.
   static final Rect zero = new Rect._();
 
-  static const double _giantScalar = 1.0E+9; // matches kGiantRect from default_layer_builder.cc
+  static const double _giantScalar = 1.0E+9; // matches kGiantRect from layer.h
 
   /// A rectangle that covers the entire coordinate space.
   ///