[PE] Output names and visual rects for traced value of DisplayItemList

Visual rects are important for correct rendering. Without them we may
be confused by the correct SkPictures producing wrong rendering.

Trace-viewer has already supported visual-rects but incorrectly.
https://chromium-review.googlesource.com/c/catapult/+/963858 will fix
that.

Also output display item names which trace-viewer already supports.

Bug: 816854
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: If819b4d9ba49ee583c0f7e25e97773e9cd9fde6f
Reviewed-on: https://chromium-review.googlesource.com/965166
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544116}

cc/base/rtree.h

@@ -71,6 +71,10 @@ class RTree {
   // Returns the total bounds of all items in this rtree.
   gfx::Rect GetBounds() const;
 
+  // Returns respective bounds of all items in this rtree in the order of items.
+  // Production code except tracing should not use this method.
+  std::vector<gfx::Rect> GetAllBoundsForTracing() const;
+
   void Reset();
 
  private:
@@ -118,6 +122,9 @@ class RTree {
   Branch<T> BuildRecursive(std::vector<Branch<T>>* branches, int level);
   Node<T>* AllocateNodeAtLevel(int level);
 
+  void GetAllBoundsRecursive(Node<T>* root,
+                             std::vector<gfx::Rect>* results) const;
+
   // This is the count of data elements (rather than total nodes in the tree)
   size_t num_data_elements_ = 0u;
   Branch<T> root_;
@@ -329,6 +336,25 @@ gfx::Rect RTree<T>::GetBounds() const {
   return root_.bounds;
 }
 
+template <typename T>
+std::vector<gfx::Rect> RTree<T>::GetAllBoundsForTracing() const {
+  std::vector<gfx::Rect> results;
+  if (num_data_elements_ > 0)
+    GetAllBoundsRecursive(root_.subtree, &results);
+  return results;
+}
+
+template <typename T>
+void RTree<T>::GetAllBoundsRecursive(Node<T>* node,
+                                     std::vector<gfx::Rect>* results) const {
+  for (uint16_t i = 0; i < node->num_children; ++i) {
+    if (node->level == 0)
+      results->push_back(node->children[i].bounds);
+    else
+      GetAllBoundsRecursive(node->children[i].subtree, results);
+  }
+}
+
 template <typename T>
 void RTree<T>::Reset() {
   num_data_elements_ = 0;

cc/base/rtree_unittest.cc

@@ -110,6 +110,7 @@ TEST(RTreeTest, SortedResults) {
 TEST(RTreeTest, GetBoundsEmpty) {
   RTree<size_t> rtree;
   EXPECT_EQ(gfx::Rect(), rtree.GetBounds());
+  EXPECT_TRUE(rtree.GetAllBoundsForTracing().empty());
 }
 
 TEST(RTreeTest, GetBoundsNonOverlapping) {
@@ -121,6 +122,7 @@ TEST(RTreeTest, GetBoundsNonOverlapping) {
   rtree.Build(rects);
 
   EXPECT_EQ(gfx::Rect(5, 6, 19, 20), rtree.GetBounds());
+  EXPECT_EQ(rects, rtree.GetAllBoundsForTracing());
 }
 
 TEST(RTreeTest, GetBoundsOverlapping) {
@@ -132,6 +134,7 @@ TEST(RTreeTest, GetBoundsOverlapping) {
   rtree.Build(rects);
 
   EXPECT_EQ(gfx::Rect(0, 0, 10, 10), rtree.GetBounds());
+  EXPECT_EQ(rects, rtree.GetAllBoundsForTracing());
 }
 
 TEST(RTreeTest, BuildAfterReset) {
@@ -147,10 +150,12 @@ TEST(RTreeTest, BuildAfterReset) {
   // Resetting should give the same as an empty rtree.
   rtree.Reset();
   EXPECT_EQ(gfx::Rect(), rtree.GetBounds());
+  EXPECT_TRUE(rtree.GetAllBoundsForTracing().empty());
 
   // Should be able to rebuild from a reset rtree.
   rtree.Build(rects);
   EXPECT_EQ(gfx::Rect(0, 0, 10, 10), rtree.GetBounds());
+  EXPECT_EQ(rects, rtree.GetAllBoundsForTracing());
 }
 
 TEST(RTreeTest, Payload) {

cc/paint/display_item_list.cc

@@ -115,8 +115,12 @@ DisplayItemList::CreateTracedValue(bool include_items) const {
     state->BeginArray("items");
 
     PlaybackParams params(nullptr, SkMatrix::I());
+    const auto& bounds = rtree_.GetAllBoundsForTracing();
+    size_t i = 0;
     for (const PaintOp* op : PaintOpBuffer::Iterator(&paint_op_buffer_)) {
       state->BeginDictionary();
+      state->SetString("name", PaintOpTypeToString(op->GetType()));
+      MathUtil::AddToTracedValue("visual_rect", bounds[i++], state.get());
 
       SkPictureRecorder recorder;
       SkCanvas* canvas =
@@ -124,9 +128,11 @@ DisplayItemList::CreateTracedValue(bool include_items) const {
       op->Raster(canvas, params);
       sk_sp<SkPicture> picture = recorder.finishRecordingAsPicture();
 
-      std::string b64_picture;
-      PictureDebugUtil::SerializeAsBase64(picture.get(), &b64_picture);
-      state->SetString("skp64", b64_picture);
+      if (picture->approximateOpCount()) {
+        std::string b64_picture;
+        PictureDebugUtil::SerializeAsBase64(picture.get(), &b64_picture);
+        state->SetString("skp64", b64_picture);
+      }
 
       state->EndDictionary();
     }

cc/paint/display_item_list_unittest.cc

@@ -64,6 +64,20 @@ bool CompareN32Pixels(void* actual_pixels,
 
 }  // namespace
 
+#define EXPECT_TRACED_RECT(x, y, width, height, rect_list) \
+  do {                                                     \
+    ASSERT_EQ(4u, rect_list->GetSize());                   \
+    double d;                                              \
+    EXPECT_TRUE((rect_list)->GetDouble(0, &d));            \
+    EXPECT_EQ(x, d);                                       \
+    EXPECT_TRUE((rect_list)->GetDouble(1, &d));            \
+    EXPECT_EQ(y, d);                                       \
+    EXPECT_TRUE((rect_list)->GetDouble(2, &d));            \
+    EXPECT_EQ(width, d);                                   \
+    EXPECT_TRUE((rect_list)->GetDouble(3, &d));            \
+    EXPECT_EQ(height, d);                                  \
+  } while (false)
+
 TEST(DisplayItemListTest, SingleUnpairedRange) {
   gfx::Rect layer_rect(100, 100);
   PaintFlags blue_flags;
@@ -399,15 +413,10 @@ TEST(DisplayItemListTest, AsValueWithNoOps) {
     // The real contents of the traced value is in here.
     {
       const base::ListValue* list;
-      double d;
 
       // The layer_rect field is present by empty.
       ASSERT_TRUE(params_dict->GetList("layer_rect", &list));
-      ASSERT_EQ(4u, list->GetSize());
-      EXPECT_TRUE(list->GetDouble(0, &d) && d == 0) << d;
-      EXPECT_TRUE(list->GetDouble(1, &d) && d == 0) << d;
-      EXPECT_TRUE(list->GetDouble(2, &d) && d == 0) << d;
-      EXPECT_TRUE(list->GetDouble(3, &d) && d == 0) << d;
+      EXPECT_TRACED_RECT(0, 0, 0, 0, list);
 
       // The items list is there but empty.
       ASSERT_TRUE(params_dict->GetList("items", &list));
@@ -426,15 +435,10 @@ TEST(DisplayItemListTest, AsValueWithNoOps) {
     // The real contents of the traced value is in here.
     {
       const base::ListValue* list;
-      double d;
 
       // The layer_rect field is present by empty.
       ASSERT_TRUE(params_dict->GetList("layer_rect", &list));
-      ASSERT_EQ(4u, list->GetSize());
-      EXPECT_TRUE(list->GetDouble(0, &d) && d == 0) << d;
-      EXPECT_TRUE(list->GetDouble(1, &d) && d == 0) << d;
-      EXPECT_TRUE(list->GetDouble(2, &d) && d == 0) << d;
-      EXPECT_TRUE(list->GetDouble(3, &d) && d == 0) << d;
+      EXPECT_TRACED_RECT(0, 0, 0, 0, list);
 
       // The items list is not there since we asked for no ops.
       ASSERT_FALSE(params_dict->GetList("items", &list));
@@ -463,10 +467,11 @@ TEST(DisplayItemListTest, AsValueWithOps) {
     PaintFlags red_paint;
     red_paint.setColor(SK_ColorRED);
 
-    list->push<SaveOp>();
+    list->push<SaveLayerOp>(nullptr, &red_paint);
     list->push<TranslateOp>(static_cast<float>(offset.x()),
                             static_cast<float>(offset.y()));
     list->push<DrawRectOp>(SkRect::MakeWH(4, 4), red_paint);
+    list->push<RestoreOp>();
 
     list->EndPaintOfUnpaired(bounds);
   }
@@ -490,28 +495,35 @@ TEST(DisplayItemListTest, AsValueWithOps) {
 
     // The real contents of the traced value is in here.
     {
-      const base::ListValue* list;
-      double d;
-
+      const base::ListValue* layer_rect;
       // The layer_rect field is present and has the bounds of the rtree.
-      ASSERT_TRUE(params_dict->GetList("layer_rect", &list));
-      ASSERT_EQ(4u, list->GetSize());
-      EXPECT_TRUE(list->GetDouble(0, &d) && d == 2) << d;
-      EXPECT_TRUE(list->GetDouble(1, &d) && d == 3) << d;
-      EXPECT_TRUE(list->GetDouble(2, &d) && d == 8) << d;
-      EXPECT_TRUE(list->GetDouble(3, &d) && d == 9) << d;
+      ASSERT_TRUE(params_dict->GetList("layer_rect", &layer_rect));
+      EXPECT_TRACED_RECT(2, 3, 8, 9, layer_rect);
 
       // The items list has 3 things in it since we built 3 visual rects.
-      ASSERT_TRUE(params_dict->GetList("items", &list));
-      EXPECT_EQ(6u, list->GetSize());
+      const base::ListValue* items;
+      ASSERT_TRUE(params_dict->GetList("items", &items));
+      ASSERT_EQ(7u, items->GetSize());
+
+      const char* expected_names[] = {"Save",      "Concat",   "SaveLayer",
+                                      "Translate", "DrawRect", "Restore",
+                                      "Restore"};
+      bool expected_has_skp[] = {false, true, true, true, true, false, false};
 
-      for (int i = 0; i < 6; ++i) {
+      for (int i = 0; i < 7; ++i) {
         const base::DictionaryValue* item_dict;
+        ASSERT_TRUE(items->GetDictionary(i, &item_dict));
 
-        ASSERT_TRUE(list->GetDictionary(i, &item_dict));
+        const base::ListValue* visual_rect;
+        ASSERT_TRUE(item_dict->GetList("visual_rect", &visual_rect));
+        EXPECT_TRACED_RECT(2, 3, 8, 9, visual_rect);
 
-        // The SkPicture for each item exists.
-        EXPECT_TRUE(
+        std::string name;
+        EXPECT_TRUE(item_dict->GetString("name", &name));
+        EXPECT_EQ(expected_names[i], name);
+
+        EXPECT_EQ(
+            expected_has_skp[i],
             item_dict->GetString("skp64", static_cast<std::string*>(nullptr)));
       }
     }
@@ -528,15 +540,9 @@ TEST(DisplayItemListTest, AsValueWithOps) {
     // The real contents of the traced value is in here.
     {
       const base::ListValue* list;
-      double d;
-
       // The layer_rect field is present and has the bounds of the rtree.
       ASSERT_TRUE(params_dict->GetList("layer_rect", &list));
-      ASSERT_EQ(4u, list->GetSize());
-      EXPECT_TRUE(list->GetDouble(0, &d) && d == 2) << d;
-      EXPECT_TRUE(list->GetDouble(1, &d) && d == 3) << d;
-      EXPECT_TRUE(list->GetDouble(2, &d) && d == 8) << d;
-      EXPECT_TRUE(list->GetDouble(3, &d) && d == 9) << d;
+      EXPECT_TRACED_RECT(2, 3, 8, 9, list);
 
       // The items list is not present since we asked for no ops.
       ASSERT_FALSE(params_dict->GetList("items", &list));