Separate layer-specific data in DisplayListBuilder::SaveInfo

ci/licenses_golden/excluded_files

@@ -39,11 +39,13 @@
 ../../../flutter/display_list/effects/dl_image_filter_unittests.cc
 ../../../flutter/display_list/effects/dl_mask_filter_unittests.cc
 ../../../flutter/display_list/effects/dl_path_effect_unittests.cc
+../../../flutter/display_list/geometry/dl_geometry_types_unittests.cc
 ../../../flutter/display_list/geometry/dl_region_unittests.cc
 ../../../flutter/display_list/geometry/dl_rtree_unittests.cc
 ../../../flutter/display_list/skia/dl_sk_conversions_unittests.cc
 ../../../flutter/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
 ../../../flutter/display_list/testing
+../../../flutter/display_list/utils/dl_accumulation_rect_unittests.cc
 ../../../flutter/display_list/utils/dl_matrix_clip_tracker_unittests.cc
 ../../../flutter/docs
 ../../../flutter/examples

display_list/BUILD.gn

@@ -120,10 +120,12 @@ if (enable_unittests) {
       "effects/dl_image_filter_unittests.cc",
       "effects/dl_mask_filter_unittests.cc",
       "effects/dl_path_effect_unittests.cc",
+      "geometry/dl_geometry_types_unittests.cc",
       "geometry/dl_region_unittests.cc",
       "geometry/dl_rtree_unittests.cc",
       "skia/dl_sk_conversions_unittests.cc",
       "skia/dl_sk_paint_dispatcher_unittests.cc",
+      "utils/dl_accumulation_rect_unittests.cc",
       "utils/dl_matrix_clip_tracker_unittests.cc",
     ]
 

display_list/dl_builder.h

@@ -248,6 +248,8 @@ class DisplayListBuilder final : public virtual DlCanvas,
   sk_sp<DisplayList> Build();
 
  private:
+  void Init(bool prepare_rtree);
+
   // This method exposes the internal stateful DlOpReceiver implementation
   // of the DisplayListBuilder, primarily for testing purposes. Its use
   // is obsolete and forbidden in every other case and is only shared to a
@@ -512,7 +514,49 @@ class DisplayListBuilder final : public virtual DlCanvas,
     std::vector<int> indices;
   };
 
-  // The SaveInfo class stores internal data for both Save and SaveLayer calls
+  struct LayerInfo {
+    LayerInfo(const std::shared_ptr<const DlImageFilter>& filter,
+              size_t rtree_rects_start_index)
+        : filter(filter),
+          rtree_rects_start_index(rtree_rects_start_index) {}
+
+    // The filter that will be applied to the contents of the saveLayer
+    // when it is restored into the parent layer.
+    const std::shared_ptr<const DlImageFilter> filter;
+
+    // The index of the rtree rects when the saveLayer was called, used
+    // only in the case that the saveLayer has a filter so that the
+    // accumulated rects can be updated in the corresponding restore call.
+    const size_t rtree_rects_start_index = 0;
+
+    // The bounds accumulator for the entire DisplayList, relative to its root
+    // (not used when accumulating rects for an rtree, though)
+    AccumulationRect global_space_accumulator;
+
+    // The bounds accumulator to set/verify the bounds of the most recently
+    // invoked saveLayer call, relative to the root of that saveLayer
+    AccumulationRect layer_local_accumulator;
+
+    DlBlendMode max_blend_mode = DlBlendMode::kClear;
+
+    bool opacity_incompatible_op_detected = false;
+    bool affects_transparent_layer = false;
+    bool contains_backdrop_filter = false;
+
+    bool is_group_opacity_compatible() const {
+      return !opacity_incompatible_op_detected &&
+             !layer_local_accumulator.overlap_detected();
+    }
+
+    void update_blend_mode(DlBlendMode mode) {
+      if (max_blend_mode < mode) {
+        max_blend_mode = mode;
+      }
+    }
+  };
+
+  // The SaveInfo class stores internal data common to both Save and
+  // SaveLayer calls
   class SaveInfo {
    public:
     // For vector reallocation calls to copy vector data
@@ -521,78 +565,43 @@ class DisplayListBuilder final : public virtual DlCanvas,
 
     // For constructor (root layer) initialization
     explicit SaveInfo(const DlRect& cull_rect)
-        : is_root_layer(true),
-          is_save_layer(true),
+        : is_save_layer(true),
           global_state(cull_rect),
           layer_state(cull_rect),
-          layer_local_accumulator(new AccumulationRect()) {}
+          layer_info(new LayerInfo(nullptr, 0u)) {}
 
     // For regular save calls:
     // Passing a pointer to the parent_info so as to distinguish this
-    // call from the copy constructor used above in vector reallocations
+    // call from the copy constructors used during vector reallocations
     explicit SaveInfo(const SaveInfo* parent_info)
-        : is_root_layer(false),
-          is_save_layer(false),
+        : is_save_layer(false),
           has_deferred_save_op(true),
           global_state(parent_info->global_state),
           layer_state(parent_info->layer_state),
-          global_space_accumulator(parent_info->global_space_accumulator),
-          layer_local_accumulator(parent_info->layer_local_accumulator) {}
+          layer_info(parent_info->layer_info) {}
 
     // For saveLayer calls:
     explicit SaveInfo(const SaveInfo* parent_info,
                       const std::shared_ptr<const DlImageFilter>& filter,
                       int rtree_rect_index)
-        : is_root_layer(false),
-          is_save_layer(true),
-          rtree_rects_start_index(rtree_rect_index),
+        : is_save_layer(true),
           global_state(parent_info->global_state),
           layer_state(kMaxCullRect),
-          global_space_accumulator(parent_info->global_space_accumulator),
-          layer_local_accumulator(new AccumulationRect()),
-          filter(filter) {}
-
-    bool is_group_opacity_compatible() const {
-      return !opacity_incompatible_op_detected &&
-             !layer_local_accumulator->overlap_detected();
-    }
-
-    // Records the given bounds after transforming by the global and
-    // layer matrices.
-    bool AccumulateBoundsLocal(const SkRect& bounds);
+          layer_info(new LayerInfo(filter, rtree_rect_index)) {}
 
-    // Simply transfers the local bounds to the parent
-    void TransferBoundsToParent(const SaveInfo& parent);
-
-    void update_blend_mode(DlBlendMode mode) {
-      if (max_blend_mode < mode) {
-        max_blend_mode = mode;
-      }
-    }
-
-    const bool is_root_layer;
     const bool is_save_layer;
 
     bool has_deferred_save_op = false;
     bool is_nop = false;
-    bool opacity_incompatible_op_detected = false;
-    bool is_unbounded = false;
-    bool affects_transparent_layer = false;
-    bool contains_backdrop_filter = false;
-
-    DlBlendMode max_blend_mode = DlBlendMode::kClear;
 
-    // The offset into the buffer where the associated save op is recorded
-    // (which is not necessarily the same as when the Save() method is called)
-    size_t save_offset = 0;
     // The depth when the save call is recorded, used to compute the total
     // depth of its content when the associated restore is called.
     uint32_t save_depth = 0;
 
-    // The index of the rtree rects when the saveLayer was called, used
-    // only in the case that the saveLayer has a filter so that the
-    // accumulated rects can be updated in the corresponding restore call.
-    const size_t rtree_rects_start_index = 0;
+    // The offset into the buffer where the associated save op is recorded
+    // (which is not necessarily the same as when the Save() method is called
+    // due to deferred saves)
+    size_t save_offset = 0;
 
     // The transform and clip accumulated since the root of the DisplayList
     DisplayListMatrixClipState global_state;
@@ -601,30 +610,14 @@ class DisplayListBuilder final : public virtual DlCanvas,
     // used to compute and update its bounds when the restore is called.
     DisplayListMatrixClipState layer_state;
 
-    // Not every layer needs its own accumulator(s). In particular, the
-    // global accumulator is only used if we are not construting an rtree.
-    // Regular save calls will share both accumulators with their parent.
-    // Additionally, a saveLayer will separate its global accumulator from
-    // its parent (if not constructing an rtree) when it has a filter which
-    // requires it to post-adjust the bounds accumulated while recording
-    // its content. Finally, every saveLayer has its own local accumulator.
-    //
-    // All accumulations could occur in the local layer space, and then be
-    // transformed and accumulated into the parent as each layer is restored,
-    // but that technique would compound the bounds errors that happen when
-    // a list of transforms is performed serially on a rectangle (mainly
-    // when multiple rotation or skew transforms are involved).
-
-    // The bounds accumulator for the entire DisplayList, relative to its root
-    std::shared_ptr<AccumulationRect> global_space_accumulator;
+    std::shared_ptr<LayerInfo> layer_info;
 
-    // The bounds accumulator to set/verify the bounds of the most recently
-    // invoked saveLayer call, relative to the root of that saveLayer
-    std::shared_ptr<AccumulationRect> layer_local_accumulator;
+    // Records the given bounds after transforming by the global and
+    // layer matrices.
+    bool AccumulateBoundsLocal(const SkRect& bounds);
 
-    // The filter that will be applied to the contents of the saveLayer
-    // when it is restored into the parent layer.
-    const std::shared_ptr<const DlImageFilter> filter;
+    // Simply transfers the local bounds to the parent
+    void TransferBoundsToParent(const SaveInfo& parent);
   };
 
   const DlRect original_cull_rect_;
@@ -634,7 +627,7 @@ class DisplayListBuilder final : public virtual DlCanvas,
   DlPaint current_;
 
   // Returns a reference to the SaveInfo structure at the top of the current
-  // save_stack state. Note that the clip and matrix state can be accessed
+  // save_stack vector. Note that the clip and matrix state can be accessed
   // more directly through global_state() and layer_state().
   SaveInfo& current_info() { return save_stack_.back(); }
   const SaveInfo& current_info() const { return save_stack_.back(); }
@@ -646,6 +639,23 @@ class DisplayListBuilder final : public virtual DlCanvas,
     return *std::prev(save_stack_.end(), 2);
   }
 
+  // Returns a reference to the LayerInfo structure at the top of the current
+  // save_stack vector. Note that the clip and matrix state can be accessed
+  // more directly through global_state() and layer_state().
+  LayerInfo& current_layer() { return *save_stack_.back().layer_info; }
+  const LayerInfo& current_layer() const {
+    return *save_stack_.back().layer_info;
+  }
+
+  // Returns a reference to the LayerInfo structure just below the top
+  // of the current save_stack state.
+  LayerInfo& parent_layer() {
+    return *std::prev(save_stack_.end(), 2)->layer_info;
+  }
+  const LayerInfo& parent_layer() const {
+    return *std::prev(save_stack_.end(), 2)->layer_info;
+  }
+
   // Returns a reference to the matrix and clip state for the entire
   // DisplayList. The initial transform of this state is identity and
   // the initial cull_rect is the root original_cull_rect supplied
@@ -673,11 +683,8 @@ class DisplayListBuilder final : public virtual DlCanvas,
     return current_info().layer_state;
   }
 
-  void RestoreLayer(const SaveInfo& current_info,
-                    SaveInfo& parent_info);
-  void TransferLayerBounds(const SaveInfo& current_info,
-                           SaveInfo& parent_info,
-                           const SkRect& content_bounds);
+  void RestoreLayer();
+  void TransferLayerBounds(const SkRect& content_bounds);
   bool AdjustRTreeRects(RTreeData& data,
                         const DlImageFilter& filter,
                         const SkMatrix& matrix,
@@ -709,7 +716,7 @@ class DisplayListBuilder final : public virtual DlCanvas,
   // that has determined its compatibility as indicated by |compatible|.
   void UpdateLayerOpacityCompatibility(bool compatible) {
     if (!compatible) {
-      current_info().opacity_incompatible_op_detected = true;
+      current_layer().opacity_incompatible_op_detected = true;
     }
   }
 
@@ -770,10 +777,10 @@ class DisplayListBuilder final : public virtual DlCanvas,
       case OpResult::kPreservesTransparency:
         break;
       case OpResult::kAffectsAll:
-        current_info().affects_transparent_layer = true;
+        current_layer().affects_transparent_layer = true;
         break;
     }
-    current_info().update_blend_mode(mode);
+    current_layer().update_blend_mode(mode);
   }
   void UpdateLayerResult(OpResult result, bool uses_attributes = true) {
     UpdateLayerResult(result, uses_attributes ? current_.getBlendMode()
@@ -794,7 +801,7 @@ class DisplayListBuilder final : public virtual DlCanvas,
 
   // Records the fact that we encountered an op that either could not
   // estimate its bounds or that fills all of the destination space.
-  bool AccumulateUnbounded(SaveInfo& layer);
+  bool AccumulateUnbounded(const SaveInfo& save);
   bool AccumulateUnbounded() {
     return AccumulateUnbounded(current_info());
   }

display_list/geometry/dl_geometry_types.h

@@ -20,18 +20,33 @@ using DlScalar = impeller::Scalar;
 using DlDegrees = impeller::Degrees;
 using DlRadians = impeller::Radians;
 
-using DlISize = impeller::ISize32;
+using DlPoint = impeller::Point;
+using DlIPoint = impeller::IPoint32;
 using DlSize = impeller::Size;
+using DlISize = impeller::ISize32;
 using DlRect = impeller::Rect;
 using DlIRect = impeller::IRect32;
 using DlMatrix = impeller::Matrix;
 
+static_assert(sizeof(SkPoint) == sizeof(DlPoint));
+static_assert(sizeof(SkIPoint) == sizeof(DlIPoint));
+static_assert(sizeof(SkSize) == sizeof(DlSize));
+static_assert(sizeof(SkISize) == sizeof(DlISize));
 static_assert(sizeof(SkRect) == sizeof(DlRect));
+static_assert(sizeof(SkIRect) == sizeof(DlIRect));
+
+inline const DlPoint& ToDlPoint(const SkPoint& point) {
+  return *reinterpret_cast<const DlPoint*>(&point);
+}
 
 inline const DlRect& ToDlRect(const SkRect& rect) {
   return *reinterpret_cast<const DlRect*>(&rect);
 }
 
+inline const DlISize& ToDlISize(const SkISize& size) {
+  return *reinterpret_cast<const DlISize*>(&size);
+}
+
 inline constexpr DlMatrix ToDlMatrix(const SkMatrix& matrix) {
   // clang-format off
   return DlMatrix::MakeColumn(
@@ -49,6 +64,10 @@ inline constexpr DlMatrix ToDlMatrix(const SkM44& matrix) {
   return dl_matrix;
 }
 
+inline const SkPoint& ToSkPoint(const DlPoint& point) {
+  return *reinterpret_cast<const SkPoint*>(&point);
+}
+
 inline const SkRect& ToSkRect(const DlRect& rect) {
   return *reinterpret_cast<const SkRect*>(&rect);
 }

display_list/geometry/dl_geometry_types_unittests.cc

@@ -0,0 +1,54 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/display_list/geometry/dl_geometry_types.h"
+#include "gtest/gtest.h"
+
+namespace flutter {
+namespace testing {
+
+TEST(DisplayListGeometryTypes, PointConversion) {
+  SkPoint sk_p = SkPoint::Make(1.0f, 2.0f);
+  DlPoint dl_p = DlPoint(1.0f, 2.0f);
+
+  EXPECT_EQ(sk_p, ToSkPoint(dl_p));
+  EXPECT_EQ(ToDlPoint(sk_p), dl_p);
+
+  sk_p = SkPoint::Make(1.0f, 2.0f);
+  dl_p = DlPoint(1.0f, 3.0f);
+
+  EXPECT_NE(sk_p, ToSkPoint(dl_p));
+  EXPECT_NE(ToDlPoint(sk_p), dl_p);
+}
+
+TEST(DisplayListGeometryTypes, RectConversion) {
+  SkRect sk_r = SkRect::MakeLTRB(1.0f, 2.0f, 3.0f, 4.0f);
+  DlRect dl_r = DlRect::MakeLTRB(1.0f, 2.0f, 3.0f, 4.0f);
+
+  EXPECT_EQ(sk_r, ToSkRect(dl_r));
+  EXPECT_EQ(ToDlRect(sk_r), dl_r);
+
+  sk_r = SkRect::MakeLTRB(1.0f, 2.0f, 3.0f, 4.0f);
+  dl_r = DlRect::MakeLTRB(1.0f, 2.0f, 3.0f, 5.0f);
+
+  EXPECT_NE(sk_r, ToSkRect(dl_r));
+  EXPECT_NE(ToDlRect(sk_r), dl_r);
+}
+
+TEST(DisplayListGeometryTypes, ISizeConversion) {
+  SkISize sk_s = SkISize::Make(1.0f, 2.0f);
+  DlISize dl_s = DlISize(1.0f, 2.0f);
+
+  EXPECT_EQ(sk_s, ToSkISize(dl_s));
+  EXPECT_EQ(ToDlISize(sk_s), dl_s);
+
+  sk_s = SkISize::Make(1.0f, 2.0f);
+  dl_s = DlISize(1.0f, 3.0f);
+
+  EXPECT_NE(sk_s, ToSkISize(dl_s));
+  EXPECT_NE(ToDlISize(sk_s), dl_s);
+}
+
+}  // namespace testing
+}  // namespace flutter