[engine] changes to DlVertices::Builder and Stopwatch visualizer. (#57031)

Collection of changes to DlVertices::Builder and the stopwatch visualizer.

At a high level:

* improve performance of the stopwatch visualizer by pre-allocating storage (and sharing it across both visualizers), lookup up font once, and cache the debug frame rate used. Updates to use Dl types instead of SkTypes.

* Change DlVerticesBuilder to allow storing the bounds and use that in the visualizer, since we already know them. Make FML_CHECKS into dchecks, as the dart:ui vertices will already bounds check correctly - so these should only be necessary for debugging engine changes.

Author: Jonah Williams

display_list/dl_vertices.cc

@@ -43,7 +43,8 @@ std::shared_ptr<DlVertices> DlVertices::Make(
     const SkPoint texture_coordinates[],
     const DlColor colors[],
     int index_count,
-    const uint16_t indices[]) {
+    const uint16_t indices[],
+    const DlRect* bounds) {
   if (!vertices || vertex_count <= 0) {
     vertex_count = 0;
     texture_coordinates = nullptr;
@@ -75,6 +76,9 @@ std::shared_ptr<DlVertices> DlVertices::Make(
   if (indices) {
     builder.store_indices(indices);
   }
+  if (bounds != nullptr) {
+    builder.store_bounds(*bounds);
+  }
 
   return builder.build();
 }
@@ -223,53 +227,53 @@ static void store_points(char* dst, int offset, const float* src, int count) {
 }
 
 void DlVertices::Builder::store_vertices(const SkPoint vertices[]) {
-  FML_CHECK(is_valid());
-  FML_CHECK(needs_vertices_);
+  FML_DCHECK(is_valid());
+  FML_DCHECK(needs_vertices_);
   char* pod = reinterpret_cast<char*>(vertices_.get());
   size_t bytes = vertices_->vertex_count_ * sizeof(vertices[0]);
   memcpy(pod + vertices_->vertices_offset_, vertices, bytes);
   needs_vertices_ = false;
 }
 
 void DlVertices::Builder::store_vertices(const float vertices[]) {
-  FML_CHECK(is_valid());
-  FML_CHECK(needs_vertices_);
+  FML_DCHECK(is_valid());
+  FML_DCHECK(needs_vertices_);
   char* pod = reinterpret_cast<char*>(vertices_.get());
   store_points(pod, vertices_->vertices_offset_, vertices,
                vertices_->vertex_count_);
   needs_vertices_ = false;
 }
 
 void DlVertices::Builder::store_texture_coordinates(const SkPoint coords[]) {
-  FML_CHECK(is_valid());
-  FML_CHECK(needs_texture_coords_);
+  FML_DCHECK(is_valid());
+  FML_DCHECK(needs_texture_coords_);
   char* pod = reinterpret_cast<char*>(vertices_.get());
   size_t bytes = vertices_->vertex_count_ * sizeof(coords[0]);
   memcpy(pod + vertices_->texture_coordinates_offset_, coords, bytes);
   needs_texture_coords_ = false;
 }
 
 void DlVertices::Builder::store_texture_coordinates(const float coords[]) {
-  FML_CHECK(is_valid());
-  FML_CHECK(needs_texture_coords_);
+  FML_DCHECK(is_valid());
+  FML_DCHECK(needs_texture_coords_);
   char* pod = reinterpret_cast<char*>(vertices_.get());
   store_points(pod, vertices_->texture_coordinates_offset_, coords,
                vertices_->vertex_count_);
   needs_texture_coords_ = false;
 }
 
 void DlVertices::Builder::store_colors(const DlColor colors[]) {
-  FML_CHECK(is_valid());
-  FML_CHECK(needs_colors_);
+  FML_DCHECK(is_valid());
+  FML_DCHECK(needs_colors_);
   char* pod = reinterpret_cast<char*>(vertices_.get());
   size_t bytes = vertices_->vertex_count_ * sizeof(colors[0]);
   memcpy(pod + vertices_->colors_offset_, colors, bytes);
   needs_colors_ = false;
 }
 
 void DlVertices::Builder::store_colors(const uint32_t colors[]) {
-  FML_CHECK(is_valid());
-  FML_CHECK(needs_colors_);
+  FML_DCHECK(is_valid());
+  FML_DCHECK(needs_colors_);
   char* pod = reinterpret_cast<char*>(vertices_.get());
   DlColor* dlcolors_ptr =
       reinterpret_cast<DlColor*>(pod + vertices_->colors_offset_);
@@ -280,29 +284,37 @@ void DlVertices::Builder::store_colors(const uint32_t colors[]) {
 }
 
 void DlVertices::Builder::store_indices(const uint16_t indices[]) {
-  FML_CHECK(is_valid());
-  FML_CHECK(needs_indices_);
+  FML_DCHECK(is_valid());
+  FML_DCHECK(needs_indices_);
   char* pod = reinterpret_cast<char*>(vertices_.get());
   size_t bytes = vertices_->index_count_ * sizeof(indices[0]);
   memcpy(pod + vertices_->indices_offset_, indices, bytes);
   needs_indices_ = false;
 }
 
+void DlVertices::Builder::store_bounds(DlRect bounds) {
+  vertices_->bounds_ = SkRect::MakeLTRB(bounds.GetLeft(), bounds.GetTop(),
+                                        bounds.GetRight(), bounds.GetBottom());
+  needs_bounds_ = false;
+}
+
 std::shared_ptr<DlVertices> DlVertices::Builder::build() {
-  FML_CHECK(is_valid());
+  FML_DCHECK(is_valid());
   if (vertices_->vertex_count() <= 0) {
     // We set this to true in the constructor to make sure that they
     // call store_vertices() only once, but if there are no vertices
     // then we will not object to them never having stored any vertices
     needs_vertices_ = false;
   }
-  FML_CHECK(!needs_vertices_);
-  FML_CHECK(!needs_texture_coords_);
-  FML_CHECK(!needs_colors_);
-  FML_CHECK(!needs_indices_);
-
-  vertices_->bounds_ =
-      compute_bounds(vertices_->vertices(), vertices_->vertex_count_);
+  FML_DCHECK(!needs_vertices_);
+  FML_DCHECK(!needs_texture_coords_);
+  FML_DCHECK(!needs_colors_);
+  FML_DCHECK(!needs_indices_);
+
+  if (needs_bounds_) {
+    vertices_->bounds_ =
+        compute_bounds(vertices_->vertices(), vertices_->vertex_count_);
+  }
 
   return std::move(vertices_);
 }

display_list/dl_vertices.h

@@ -155,6 +155,9 @@ class DlVertices {
     /// promised by (index_count > 0).
     void store_indices(const uint16_t indices[]);
 
+    /// @brief Overwrite the internal bounds with a precomputed bounding rect.
+    void store_bounds(DlRect bounds);
+
     /// @brief Finalizes and the constructed DlVertices object.
     ///
     /// fails if any of the optional data promised in the constructor is
@@ -167,6 +170,7 @@ class DlVertices {
     bool needs_texture_coords_;
     bool needs_colors_;
     bool needs_indices_;
+    bool needs_bounds_ = true;
   };
 
   //--------------------------------------------------------------------------
@@ -183,7 +187,8 @@ class DlVertices {
                                           const SkPoint texture_coordinates[],
                                           const DlColor colors[],
                                           int index_count = 0,
-                                          const uint16_t indices[] = nullptr);
+                                          const uint16_t indices[] = nullptr,
+                                          const DlRect* bounds = nullptr);
 
   /// Returns the size of the object including all of the inlined data.
   size_t size() const;

flow/layers/performance_overlay_layer.cc

@@ -34,26 +34,25 @@ void VisualizeStopWatch(DlCanvas* canvas,
                         bool show_graph,
                         bool show_labels,
                         const std::string& label_prefix,
-                        const std::string& font_path) {
+                        std::vector<DlPoint>& point_storage,
+                        std::vector<DlColor>& color_storage,
+                        const SkFont& font) {
   const int label_x = 8;    // distance from x
   const int label_y = -10;  // distance from y+height
 
   if (show_graph) {
-    SkRect visualization_rect = SkRect::MakeXYWH(x, y, width, height);
-    std::unique_ptr<StopwatchVisualizer> visualizer;
-
+    DlRect visualization_rect = DlRect::MakeXYWH(x, y, width, height);
     if (impeller_enabled) {
-      visualizer = std::make_unique<DlStopwatchVisualizer>(stopwatch);
+      DlStopwatchVisualizer(stopwatch, point_storage, color_storage)
+          .Visualize(canvas, visualization_rect);
     } else {
-      visualizer = std::make_unique<SkStopwatchVisualizer>(stopwatch);
+      SkStopwatchVisualizer(stopwatch).Visualize(canvas, visualization_rect);
     }
-
-    visualizer->Visualize(canvas, visualization_rect);
   }
 
   if (show_labels) {
-    auto text = PerformanceOverlayLayer::MakeStatisticsText(
-        stopwatch, label_prefix, font_path);
+    auto text = PerformanceOverlayLayer::MakeStatisticsText(stopwatch, font,
+                                                            label_prefix);
     // Historically SK_ColorGRAY (== 0xFF888888) was used here
     DlPaint paint(DlColor(0xFF888888));
 #ifdef IMPELLER_SUPPORTS_RENDERING
@@ -69,24 +68,28 @@ void VisualizeStopWatch(DlCanvas* canvas,
 
 }  // namespace
 
-sk_sp<SkTextBlob> PerformanceOverlayLayer::MakeStatisticsText(
-    const Stopwatch& stopwatch,
-    const std::string& label_prefix,
-    const std::string& font_path) {
-  SkFont font;
+// static
+SkFont PerformanceOverlayLayer::MakeStatisticsFont(std::string_view font_path) {
   sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
   if (font_path == "") {
     if (sk_sp<SkTypeface> face = font_mgr->matchFamilyStyle(nullptr, {})) {
-      font = SkFont(face, 15);
+      return SkFont(face, 15);
     } else {
       // In Skia's Android fontmgr, matchFamilyStyle can return null instead
       // of falling back to a default typeface. If that's the case, we can use
       // legacyMakeTypeface, which *does* use that default typeface.
-      font = SkFont(font_mgr->legacyMakeTypeface(nullptr, {}), 15);
+      return SkFont(font_mgr->legacyMakeTypeface(nullptr, {}), 15);
     }
   } else {
-    font = SkFont(font_mgr->makeFromFile(font_path.c_str()), 15);
+    return SkFont(font_mgr->makeFromFile(font_path.data()), 15);
   }
+}
+
+// static
+sk_sp<SkTextBlob> PerformanceOverlayLayer::MakeStatisticsText(
+    const Stopwatch& stopwatch,
+    const SkFont& font,
+    std::string_view label_prefix) {
   // Make sure there's not an empty typeface returned, or we won't see any text.
   FML_DCHECK(font.getTypeface()->countGlyphs() > 0);
 
@@ -134,16 +137,22 @@ void PerformanceOverlayLayer::Paint(PaintContext& context) const {
   SkScalar width = paint_bounds().width() - (padding * 2);
   SkScalar height = paint_bounds().height() / 2;
   auto mutator = context.state_stack.save();
+  // Cached storage for vertex output.
+  std::vector<DlPoint> vertices_storage;
+  std::vector<DlColor> color_storage;
+  SkFont font = MakeStatisticsFont(font_path_);
 
-  VisualizeStopWatch(
-      context.canvas, context.impeller_enabled, context.raster_time, x, y,
-      width, height - padding, options_ & kVisualizeRasterizerStatistics,
-      options_ & kDisplayRasterizerStatistics, "Raster", font_path_);
+  VisualizeStopWatch(context.canvas, context.impeller_enabled,
+                     context.raster_time, x, y, width, height - padding,
+                     options_ & kVisualizeRasterizerStatistics,
+                     options_ & kDisplayRasterizerStatistics, "Raster",
+                     vertices_storage, color_storage, font);
 
   VisualizeStopWatch(context.canvas, context.impeller_enabled, context.ui_time,
                      x, y + height, width, height - padding,
                      options_ & kVisualizeEngineStatistics,
-                     options_ & kDisplayEngineStatistics, "UI", font_path_);
+                     options_ & kDisplayEngineStatistics, "UI",
+                     vertices_storage, color_storage, font);
 }
 
 }  // namespace flutter

flow/layers/performance_overlay_layer.h

@@ -22,9 +22,11 @@ const int kVisualizeEngineStatistics = 1 << 3;
 
 class PerformanceOverlayLayer : public Layer {
  public:
+  static SkFont MakeStatisticsFont(std::string_view font_path);
+
   static sk_sp<SkTextBlob> MakeStatisticsText(const Stopwatch& stopwatch,
-                                              const std::string& label_prefix,
-                                              const std::string& font_path);
+                                              const SkFont& font,
+                                              std::string_view label_prefix);
 
   bool IsReplacing(DiffContext* context, const Layer* layer) const override {
     return layer->as_performance_overlay_layer() != nullptr;

flow/layers/performance_overlay_layer_unittests.cc

@@ -199,6 +199,7 @@ TEST_F(PerformanceOverlayLayerTest, SimpleRasterizerStatistics) {
   const SkRect layer_bounds = SkRect::MakeLTRB(0.0f, 0.0f, 64.0f, 64.0f);
   const uint64_t overlay_opts = kDisplayRasterizerStatistics;
   auto layer = std::make_shared<PerformanceOverlayLayer>(overlay_opts);
+  auto font = PerformanceOverlayLayer::MakeStatisticsFont("");
 
   // TODO(): Note calling code has to call set_paint_bounds right now.  Make
   // this a constructor parameter and move the set_paint_bounds into Preroll
@@ -210,7 +211,7 @@ TEST_F(PerformanceOverlayLayerTest, SimpleRasterizerStatistics) {
 
   layer->Paint(display_list_paint_context());
   auto overlay_text = PerformanceOverlayLayer::MakeStatisticsText(
-      display_list_paint_context().raster_time, "Raster", "");
+      display_list_paint_context().raster_time, font, "Raster");
   auto overlay_text_data = overlay_text->serialize(SkSerialProcs{});
   // Historically SK_ColorGRAY (== 0xFF888888) was used here
   DlPaint text_paint(DlColor(0xFF888888));

flow/stopwatch.h

@@ -106,7 +106,7 @@ class StopwatchVisualizer {
   ///
   /// @param      canvas  The canvas to draw on.
   /// @param[in]  rect    The rectangle to draw in.
-  virtual void Visualize(DlCanvas* canvas, const SkRect& rect) const = 0;
+  virtual void Visualize(DlCanvas* canvas, const DlRect& rect) const = 0;
 
   FML_DISALLOW_COPY_AND_ASSIGN(StopwatchVisualizer);