focus SkiaGPUObject wrappers on only those objects that need the protection

Author: flar

display_list/display_list.cc

@@ -21,7 +21,8 @@ DisplayList::DisplayList()
       nested_op_count_(0),
       unique_id_(0),
       bounds_({0, 0, 0, 0}),
-      can_apply_group_opacity_(true) {}
+      can_apply_group_opacity_(true),
+      is_ui_thread_safe_(true) {}
 
 DisplayList::DisplayList(DisplayListStorage&& storage,
                          size_t byte_count,
@@ -30,6 +31,7 @@ DisplayList::DisplayList(DisplayListStorage&& storage,
                          unsigned int nested_op_count,
                          const SkRect& bounds,
                          bool can_apply_group_opacity,
+                         bool is_ui_thread_safe,
                          sk_sp<const DlRTree> rtree)
     : storage_(std::move(storage)),
       byte_count_(byte_count),
@@ -39,6 +41,7 @@ DisplayList::DisplayList(DisplayListStorage&& storage,
       unique_id_(next_unique_id()),
       bounds_(bounds),
       can_apply_group_opacity_(can_apply_group_opacity),
+      is_ui_thread_safe_(is_ui_thread_safe),
       rtree_(std::move(rtree)) {}
 
 DisplayList::~DisplayList() {

display_list/display_list.h

@@ -262,6 +262,7 @@ class DisplayList : public SkRefCnt {
   }
 
   bool can_apply_group_opacity() const { return can_apply_group_opacity_; }
+  bool isUIThreadSafe() const { return is_ui_thread_safe_; }
 
   static void DisposeOps(uint8_t* ptr, uint8_t* end);
 
@@ -273,6 +274,7 @@ class DisplayList : public SkRefCnt {
               unsigned int nested_op_count,
               const SkRect& bounds,
               bool can_apply_group_opacity,
+              bool is_ui_thread_safe,
               sk_sp<const DlRTree> rtree);
 
   static uint32_t next_unique_id();
@@ -288,6 +290,7 @@ class DisplayList : public SkRefCnt {
   const SkRect bounds_;
 
   const bool can_apply_group_opacity_;
+  const bool is_ui_thread_safe_;
   const sk_sp<const DlRTree> rtree_;
 
   void Dispatch(DlOpReceiver& ctx,

display_list/dl_builder.cc

@@ -73,9 +73,10 @@ sk_sp<DisplayList> DisplayListBuilder::Build() {
   nested_bytes_ = nested_op_count_ = 0;
   storage_.realloc(bytes);
   bool compatible = layer_stack_.back().is_group_opacity_compatible();
-  return sk_sp<DisplayList>(new DisplayList(std::move(storage_), bytes, count,
-                                            nested_bytes, nested_count,
-                                            bounds(), compatible, rtree()));
+  bool is_safe = is_ui_thread_safe_;
+  return sk_sp<DisplayList>(
+      new DisplayList(std::move(storage_), bytes, count, nested_bytes,
+                      nested_count, bounds(), compatible, is_safe, rtree()));
 }
 
 DisplayListBuilder::DisplayListBuilder(const SkRect& cull_rect,
@@ -156,6 +157,7 @@ void DisplayListBuilder::onSetColorSource(const DlColorSource* source) {
     Push<ClearColorSourceOp>(0, 0);
   } else {
     current_.setColorSource(source->shared());
+    is_ui_thread_safe_ = is_ui_thread_safe_ && source->isUIThreadSafe();
     switch (source->type()) {
       case DlColorSourceType::kColor: {
         const DlColorColorSource* color_source = source->asColor();
@@ -923,6 +925,7 @@ void DisplayListBuilder::drawImage(const sk_sp<DlImage> image,
       ? Push<DrawImageWithAttrOp>(0, 1, image, point, sampling)
       : Push<DrawImageOp>(0, 1, image, point, sampling);
   CheckLayerOpacityCompatibility(render_with_attributes);
+  is_ui_thread_safe_ = is_ui_thread_safe_ && image->isUIThreadSafe();
   SkRect bounds = SkRect::MakeXYWH(point.fX, point.fY,  //
                                    image->width(), image->height());
   DisplayListAttributeFlags flags = render_with_attributes  //
@@ -951,6 +954,7 @@ void DisplayListBuilder::drawImageRect(const sk_sp<DlImage> image,
   Push<DrawImageRectOp>(0, 1, image, src, dst, sampling, render_with_attributes,
                         constraint);
   CheckLayerOpacityCompatibility(render_with_attributes);
+  is_ui_thread_safe_ = is_ui_thread_safe_ && image->isUIThreadSafe();
   DisplayListAttributeFlags flags = render_with_attributes
                                         ? kDrawImageRectWithPaintFlags
                                         : kDrawImageRectFlags;
@@ -979,6 +983,7 @@ void DisplayListBuilder::drawImageNine(const sk_sp<DlImage> image,
       ? Push<DrawImageNineWithAttrOp>(0, 1, image, center, dst, filter)
       : Push<DrawImageNineOp>(0, 1, image, center, dst, filter);
   CheckLayerOpacityCompatibility(render_with_attributes);
+  is_ui_thread_safe_ = is_ui_thread_safe_ && image->isUIThreadSafe();
   DisplayListAttributeFlags flags = render_with_attributes
                                         ? kDrawImageNineWithPaintFlags
                                         : kDrawImageNineFlags;
@@ -1034,6 +1039,7 @@ void DisplayListBuilder::drawAtlas(const sk_sp<DlImage> atlas,
   // on it to distribute the opacity without overlap without checking all
   // of the transforms and texture rectangles.
   UpdateLayerOpacityCompatibility(false);
+  is_ui_thread_safe_ = is_ui_thread_safe_ && atlas->isUIThreadSafe();
 
   SkPoint quad[4];
   RectBoundsAccumulator atlasBounds;
@@ -1075,6 +1081,7 @@ void DisplayListBuilder::DrawDisplayList(const sk_sp<DisplayList> display_list,
                                          SkScalar opacity) {
   DlPaint current_paint = current_;
   Push<DrawDisplayListOp>(0, 1, display_list, opacity);
+  is_ui_thread_safe_ = is_ui_thread_safe_ && display_list->isUIThreadSafe();
   // Not really necessary if the developer is interacting with us via
   // our attribute-state-less DlCanvas methods, but this avoids surprises
   // for those who may have been using the stateful Dispatcher methods.

display_list/dl_builder.h

@@ -480,6 +480,8 @@ class DisplayListBuilder final : public virtual DlCanvas,
   size_t nested_bytes_ = 0;
   int nested_op_count_ = 0;
 
+  bool is_ui_thread_safe_ = true;
+
   template <typename T, typename... Args>
   void* Push(size_t extra, int op_inc, Args&&... args);
 

display_list/effects/dl_color_source.h

@@ -109,6 +109,16 @@ class DlColorSource : public DlAttribute<DlColorSource, DlColorSourceType> {
 
   virtual bool is_opaque() const = 0;
 
+  //----------------------------------------------------------------------------
+  /// @brief      If the underlying platform data held by this object is
+  ///             held in a way that it can be stored and potentially
+  ///             released from the UI thread, this method returns true.
+  ///
+  /// @return     True if the class has no GPU related resources or if any
+  ///             that it holds are held in a thread-safe manner.
+  ///
+  virtual bool isUIThreadSafe() const = 0;
+
   // Return a DlColorColorSource pointer to this object iff it is an Color
   // type of ColorSource, otherwise return nullptr.
   virtual const DlColorColorSource* asColor() const { return nullptr; }
@@ -160,6 +170,8 @@ class DlColorColorSource final : public DlColorSource {
  public:
   DlColorColorSource(DlColor color) : color_(color) {}
 
+  bool isUIThreadSafe() const override { return true; }
+
   std::shared_ptr<DlColorSource> shared() const override {
     return std::make_shared<DlColorColorSource>(color_);
   }
@@ -215,6 +227,10 @@ class DlImageColorSource final : public SkRefCnt,
         vertical_tile_mode_(vertical_tile_mode),
         sampling_(sampling) {}
 
+  bool isUIThreadSafe() const override {
+    return image_ ? image_->isUIThreadSafe() : true;
+  }
+
   const DlImageColorSource* asImage() const override { return this; }
 
   std::shared_ptr<DlColorSource> shared() const override {
@@ -338,6 +354,8 @@ class DlLinearGradientColorSource final : public DlGradientColorSourceBase {
     return this;
   }
 
+  bool isUIThreadSafe() const override { return true; }
+
   DlColorSourceType type() const override {
     return DlColorSourceType::kLinearGradient;
   }
@@ -399,6 +417,8 @@ class DlRadialGradientColorSource final : public DlGradientColorSourceBase {
     return this;
   }
 
+  bool isUIThreadSafe() const override { return true; }
+
   std::shared_ptr<DlColorSource> shared() const override {
     return MakeRadial(center_, radius_, stop_count(), colors(), stops(),
                       tile_mode(), matrix_ptr());
@@ -460,6 +480,8 @@ class DlConicalGradientColorSource final : public DlGradientColorSourceBase {
     return this;
   }
 
+  bool isUIThreadSafe() const override { return true; }
+
   std::shared_ptr<DlColorSource> shared() const override {
     return MakeConical(start_center_, start_radius_, end_center_, end_radius_,
                        stop_count(), colors(), stops(), tile_mode(),
@@ -534,6 +556,8 @@ class DlSweepGradientColorSource final : public DlGradientColorSourceBase {
     return this;
   }
 
+  bool isUIThreadSafe() const override { return true; }
+
   std::shared_ptr<DlColorSource> shared() const override {
     return MakeSweep(center_, start_, end_, stop_count(), colors(), stops(),
                      tile_mode(), matrix_ptr());
@@ -604,6 +628,15 @@ class DlRuntimeEffectColorSource final : public DlColorSource {
         samplers_(std::move(samplers)),
         uniform_data_(std::move(uniform_data)) {}
 
+  bool isUIThreadSafe() const override {
+    for (auto sampler : samplers_) {
+      if (!sampler->isUIThreadSafe()) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   const DlRuntimeEffectColorSource* asRuntimeEffect() const override {
     return this;
   }
@@ -666,6 +699,8 @@ class DlSceneColorSource final : public DlColorSource {
                      impeller::Matrix camera_matrix)
       : node_(std::move(node)), camera_matrix_(camera_matrix) {}
 
+  bool isUIThreadSafe() const override { return true; }
+
   const DlSceneColorSource* asScene() const override { return this; }
 
   std::shared_ptr<DlColorSource> shared() const override {

display_list/image/dl_image.h

@@ -67,6 +67,16 @@ class DlImage : public SkRefCnt {
 
   virtual bool isTextureBacked() const = 0;
 
+  //----------------------------------------------------------------------------
+  /// @brief      If the underlying platform image held by this object has no
+  ///             threading requirements for the release of that image (or if
+  ///             arrangements have already been made to forward that image to
+  ///             the correct thread upon deletion), this method returns true.
+  ///
+  /// @return     True if the underlying image is held in a thread-safe manner.
+  ///
+  virtual bool isUIThreadSafe() const = 0;
+
   //----------------------------------------------------------------------------
   /// @return     The dimensions of the pixel grid.
   ///

display_list/image/dl_image_skia.cc

@@ -31,6 +31,17 @@ bool DlImageSkia::isTextureBacked() const {
   return image_ ? image_->isTextureBacked() : false;
 }
 
+// |DlImage|
+bool DlImageSkia::isUIThreadSafe() const {
+  // Technically if the image is null then we are thread-safe, and possibly
+  // if the image is constructed from a heap raster as well, but there
+  // should never be a leak of an instance of this class into any data that
+  // is shared with the UI thread, regardless of value.
+  // All images intended to be shared with the UI thread should be constructed
+  // via one of the DlImage subclasses designed for that purpose.
+  return false;
+}
+
 // |DlImage|
 SkISize DlImageSkia::dimensions() const {
   return image_ ? image_->dimensions() : SkISize::MakeEmpty();

display_list/image/dl_image_skia.h

@@ -29,6 +29,9 @@ class DlImageSkia final : public DlImage {
   // |DlImage|
   bool isTextureBacked() const override;
 
+  // |DlImage|
+  bool isUIThreadSafe() const override;
+
   // |DlImage|
   SkISize dimensions() const override;
 

flow/layers/display_list_layer.cc

@@ -16,15 +16,14 @@
 namespace flutter {
 
 DisplayListLayer::DisplayListLayer(const SkPoint& offset,
-                                   SkiaGPUObject<DisplayList> display_list,
+                                   sk_sp<DisplayList> display_list,
                                    bool is_complex,
                                    bool will_change)
     : offset_(offset), display_list_(std::move(display_list)) {
-  if (display_list_.skia_object() != nullptr) {
-    bounds_ = display_list_.skia_object()->bounds().makeOffset(offset_.x(),
-                                                               offset_.y());
+  if (display_list_) {
+    bounds_ = display_list_->bounds().makeOffset(offset_.x(), offset_.y());
     display_list_raster_cache_item_ = DisplayListRasterCacheItem::Make(
-        display_list_.skia_object(), offset_, is_complex, will_change);
+        display_list_, offset_, is_complex, will_change);
   }
 }
 
@@ -61,8 +60,8 @@ void DisplayListLayer::Diff(DiffContext* context, const Layer* old_layer) {
 bool DisplayListLayer::Compare(DiffContext::Statistics& statistics,
                                const DisplayListLayer* l1,
                                const DisplayListLayer* l2) {
-  const auto& dl1 = l1->display_list_.skia_object();
-  const auto& dl2 = l2->display_list_.skia_object();
+  const auto& dl1 = l1->display_list_;
+  const auto& dl2 = l2->display_list_;
   if (dl1.get() == dl2.get()) {
     statistics.AddSameInstancePicture();
     return true;
@@ -105,7 +104,7 @@ void DisplayListLayer::Preroll(PrerollContext* context) {
 }
 
 void DisplayListLayer::Paint(PaintContext& context) const {
-  FML_DCHECK(display_list_.skia_object());
+  FML_DCHECK(display_list_);
   FML_DCHECK(needs_painting(context));
 
   auto mutator = context.state_stack.save();
@@ -143,7 +142,7 @@ void DisplayListLayer::Paint(PaintContext& context) const {
         DlAutoCanvasRestore save(canvas, true);
         canvas->Clear(DlColor::kTransparent());
         canvas->SetTransform(ctm);
-        canvas->DrawDisplayList(display_list_.skia_object(), opacity);
+        canvas->DrawDisplayList(display_list_, opacity);
       }
       canvas->Flush();
     }
@@ -158,8 +157,7 @@ void DisplayListLayer::Paint(PaintContext& context) const {
     context.layer_snapshot_store->Add(snapshot_data);
   }
 
-  auto display_list = display_list_.skia_object();
-  context.canvas->DrawDisplayList(display_list, opacity);
+  context.canvas->DrawDisplayList(display_list_, opacity);
 }
 
 }  // namespace flutter

flow/layers/display_list_layer.h

@@ -11,7 +11,6 @@
 #include "flutter/flow/layers/display_list_raster_cache_item.h"
 #include "flutter/flow/layers/layer.h"
 #include "flutter/flow/raster_cache_item.h"
-#include "flutter/flow/skia_gpu_object.h"
 
 namespace flutter {
 
@@ -20,13 +19,11 @@ class DisplayListLayer : public Layer {
   static constexpr size_t kMaxBytesToCompare = 10000;
 
   DisplayListLayer(const SkPoint& offset,
-                   SkiaGPUObject<DisplayList> display_list,
+                   sk_sp<DisplayList> display_list,
                    bool is_complex,
                    bool will_change);
 
-  DisplayList* display_list() const {
-    return display_list_.skia_object().get();
-  }
+  DisplayList* display_list() const { return display_list_.get(); }
 
   bool IsReplacing(DiffContext* context, const Layer* layer) const override;
 
@@ -55,7 +52,7 @@ class DisplayListLayer : public Layer {
   SkPoint offset_;
   SkRect bounds_;
 
-  flutter::SkiaGPUObject<DisplayList> display_list_;
+  sk_sp<DisplayList> display_list_;
 
   static bool Compare(DiffContext::Statistics& statistics,
                       const DisplayListLayer* l1,