fix issue with layer bounds rejection when rendering a tree to a DL

Author: flar

display_list/display_list_builder.cc

@@ -710,6 +710,21 @@ SkRect DisplayListBuilder::getLocalClipBounds() {
   return kMaxCullRect_;
 }
 
+bool DisplayListBuilder::quickReject(const SkRect& bounds) const {
+  if (bounds.isEmpty()) {
+    return true;
+  }
+  SkMatrix matrix = getTransform();
+  // We don't need the inverse, but this method tells us if the matrix
+  // is singular in which case we can reject all rendering.
+  if (!matrix.invert(nullptr)) {
+    return true;
+  }
+  SkRect dev_bounds;
+  matrix.mapRect(bounds).roundOut(&dev_bounds);
+  return !current_layer_->clip_bounds.intersects(dev_bounds);
+}
+
 void DisplayListBuilder::drawPaint() {
   Push<DrawPaintOp>(0, 1);
   CheckLayerOpacityCompatibility();

display_list/display_list_builder.h

@@ -206,7 +206,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
   /// Returns the 3x3 partial perspective transform representing all transform
   /// operations executed so far in this DisplayList within the enclosing
   /// save stack.
-  SkMatrix getTransform() { return current_layer_->matrix.asM33(); }
+  SkMatrix getTransform() const { return current_layer_->matrix.asM33(); }
 
   void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override;
   void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override;
@@ -221,6 +221,11 @@ class DisplayListBuilder final : public virtual Dispatcher,
   /// recorded rendering operations are interpreted.
   SkRect getLocalClipBounds();
 
+  /// Return true iff the supplied bounds are easily shown to be outside
+  /// of the current clip bounds. This method may conservatively return
+  /// false if it cannot make the determination.
+  bool quickReject(const SkRect& bounds) const;
+
   void drawPaint() override;
   void drawPaint(const DlPaint& paint);
   void drawColor(DlColor color, DlBlendMode mode) override;

flow/layers/clip_path_layer_unittests.cc

@@ -370,7 +370,7 @@ TEST_F(ClipPathLayerTest, OpacityInheritancePainting) {
   auto mock2 = MockLayer::MakeOpacityCompatible(path2);
   auto layer_clip = SkPath()
                         .addRect(SkRect::MakeLTRB(5, 5, 25, 25))
-                        .addOval(SkRect::MakeLTRB(20, 20, 40, 50));
+                        .addOval(SkRect::MakeLTRB(45, 45, 55, 55));
   auto clip_path_layer =
       std::make_shared<ClipPathLayer>(layer_clip, Clip::antiAlias);
   clip_path_layer->Add(mock1);

flow/layers/layer.h

@@ -240,15 +240,7 @@ class Layer {
       // See https://github.com/flutter/flutter/issues/81419
       return true;
     }
-    if (!context.state_stack.needs_painting()) {
-      return false;
-    }
-    // Workaround for Skia bug (quickReject does not reject empty bounds).
-    // https://bugs.chromium.org/p/skia/issues/detail?id=10951
-    if (paint_bounds_.isEmpty()) {
-      return false;
-    }
-    return !context.canvas->quickReject(paint_bounds_);
+    return context.state_stack.needs_painting(paint_bounds_);
   }
 
   // Propagated unique_id of the first layer in "chain" of replacement layers

flow/layers/layer_state_stack.cc

@@ -66,7 +66,7 @@ AutoRestore LayerStateStack::applyState(const SkRect& bounds,
 }
 
 SkPaint* LayerStateStack::RenderingAttributes::fill(SkPaint& paint,
-                                                    DlBlendMode mode) {
+                                                    DlBlendMode mode) const {
   SkPaint* ret = nullptr;
   if (opacity < SK_Scalar1) {
     paint.setAlphaf(std::max(opacity, 0.0f));
@@ -94,7 +94,7 @@ SkPaint* LayerStateStack::RenderingAttributes::fill(SkPaint& paint,
 }
 
 DlPaint* LayerStateStack::RenderingAttributes::fill(DlPaint& paint,
-                                                    DlBlendMode mode) {
+                                                    DlBlendMode mode) const {
   DlPaint* ret = nullptr;
   if (opacity < SK_Scalar1) {
     paint.setOpacity(std::max(opacity, 0.0f));
@@ -117,6 +117,27 @@ DlPaint* LayerStateStack::RenderingAttributes::fill(DlPaint& paint,
   return ret;
 }
 
+bool LayerStateStack::needs_painting(const SkRect& bounds) const {
+  if (!needs_painting()) {
+    // Answer based on outstanding attributes...
+    return false;
+  }
+  if (canvas_) {
+    // Workaround for Skia bug (quickReject does not reject empty bounds).
+    // https://bugs.chromium.org/p/skia/issues/detail?id=10951
+    if (bounds.isEmpty()) {
+      return false;
+    }
+    return !canvas_->quickReject(bounds);
+  }
+  if (builder_) {
+    return !builder_->quickReject(bounds);
+  }
+  // We could track the attributes ourselves, but this method is
+  // unlikely to be called without a canvas or builder to back it up.
+  return true;
+}
+
 MutatorContext LayerStateStack::save() {
   auto ret = MutatorContext(this);
   state_stack_.emplace_back(std::make_unique<SaveEntry>());

flow/layers/layer_state_stack.h

@@ -140,9 +140,17 @@ class LayerStateStack {
   // Fill the provided paint object with any oustanding attributes and
   // return a pointer to it, or return a nullptr if there were no
   // outstanding attributes to paint with.
-  DlPaint* fill(DlPaint& paint) { return outstanding_.fill(paint); }
+  DlPaint* fill(DlPaint& paint) const { return outstanding_.fill(paint); }
 
-  bool needs_painting() { return outstanding_.opacity > 0; }
+  // Tests if painting content with the current outstanding attributes
+  // will produce any content.
+  bool needs_painting() const { return outstanding_.opacity > 0; }
+
+  // Tests if painting content with the given bounds will produce any output.
+  // This method also tests whether the outstanding attributes will allow
+  // output to be produced, but then goes on to test if the supplied bounds
+  // will fall within the current clip bounds based on the transform.
+  bool needs_painting(const SkRect& bounds) const;
 
   // Saves the current state of the state stack and returns a
   // MutatorContext which can be used to manipulate the state.
@@ -214,10 +222,12 @@ class LayerStateStack {
     std::shared_ptr<const DlColorFilter> color_filter;
     std::shared_ptr<const DlImageFilter> image_filter;
 
-    SkPaint* fill(SkPaint& paint, DlBlendMode mode = DlBlendMode::kSrcOver);
-    DlPaint* fill(DlPaint& paint, DlBlendMode mode = DlBlendMode::kSrcOver);
+    SkPaint* fill(SkPaint& paint,
+                  DlBlendMode mode = DlBlendMode::kSrcOver) const;
+    DlPaint* fill(DlPaint& paint,
+                  DlBlendMode mode = DlBlendMode::kSrcOver) const;
 
-    bool operator==(const RenderingAttributes& other) {
+    bool operator==(const RenderingAttributes& other) const {
       return save_layer_bounds == other.save_layer_bounds &&
              opacity == other.opacity &&
              Equals(color_filter, other.color_filter) &&