Implement merging non-composited paint property nodes in the PACompositor.

third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h

Index: third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
diff --git a/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h b/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
index fef6b35ba364bfc6666d0c2b16f696a2527e2cc6..971bd889b27f94df42ee29d419188de4e1ca6c8d 100644
--- a/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
+++ b/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
@@ -16,6 +16,7 @@
 #include <memory>
 
 namespace cc {
+class DisplayItemList;
 class Layer;
 }
 
@@ -25,6 +26,7 @@ class Vector2dF;
 
 namespace blink {
 
+class GeometryMapper;

[pdr., 2016/12/20 06:46:30]

Can you writeup a high-level description of the overall approach in a README.md
file and reference it here?

 class JSONObject;
 class PaintArtifact;
 class WebLayer;
@@ -78,22 +80,47 @@ class PLATFORM_EXPORT PaintArtifactCompositor {
   std::unique_ptr<JSONObject> layersAsJSON(LayerTreeFlags) const;
 
  private:
+  // A pending layer is a collection of paint chunks that will end up in
+  // the same cc::Layer.
+  struct PendingLayer {
+    PendingLayer(const PaintChunk& firstPaintChunk)

[wkorman, 2016/12/20 00:28:56]

OK to put the impl for these methods in the .cpp file just for future
maintenance ease? Or do we do it this way for structs, which would make me sad.

[chrishtr, 2016/12/20 23:36:32]

Done.

+        : knownToBeOpaque(true),
+          backfaceHidden(firstPaintChunk.properties.backfaceHidden),
+          propertyTreeState(firstPaintChunk.properties.propertyTreeState) {
+      add(firstPaintChunk);
+    }
+    void add(const PaintChunk& paintChunk) {
+      DCHECK(paintChunk.properties.backfaceHidden == backfaceHidden);
+      paintChunks.append(&paintChunk);
+      bounds.unite(paintChunk.bounds);
+      knownToBeOpaque = knownToBeOpaque && paintChunk.knownToBeOpaque;
+    }
+    FloatRect bounds;
+    Vector<const PaintChunk*> paintChunks;
+    bool knownToBeOpaque;
+    bool backfaceHidden;
+    PropertyTreeState propertyTreeState;
+  };
+
   PaintArtifactCompositor();
 
   class ContentLayerClientImpl;
 
+  void collectPendingLayers(const Vector<PaintChunk>& paintChunks,

[pdr., 2016/12/20 06:46:30]

Nit: Just return Vector<PendingLayer>:
Vector<PendingLayer> collectPendingLayers(const Vector<PaintChunk>&) const;

Can you add a short comment here as well? Something about how this is the secret
formula for grouping paint chunks into pending layers.

[chrishtr, 2016/12/20 23:36:32]

Returning a vector will cause a vector copy operation, no? I'd rather not pay
that penalty.
 
Done.

+                            Vector<PendingLayer>& pendingLayers);
+
   // Builds a leaf layer that represents a single paint chunk.
   // Note: cc::Layer API assumes the layer bounds start at (0, 0), but the
   // bounding box of a paint chunk does not necessarily start at (0, 0) (and
   // could even be negative). Internally the generated layer translates the
   // paint chunk to align the bounding box to (0, 0) and return the actual
   // origin of the paint chunk in the |layerOffset| outparam.
-  scoped_refptr<cc::Layer> layerForPaintChunk(
+  scoped_refptr<cc::Layer> compositedLayerForPendingLayer(
       const PaintArtifact&,
-      const PaintChunk&,
+      const PendingLayer&,
       gfx::Vector2dF& layerOffset,
       Vector<std::unique_ptr<ContentLayerClientImpl>>& newContentLayerClients,
-      RasterInvalidationTracking*);
+      RasterInvalidationTrackingMap<const PaintChunk>*);
 
   // Finds a client among the current vector of clients that matches the paint
   // chunk's id, or otherwise allocates a new one.
@@ -101,6 +128,20 @@ class PLATFORM_EXPORT PaintArtifactCompositor {
       const PaintChunk&,
       const PaintArtifact&);
 
+  static scoped_refptr<cc::DisplayItemList> recordPendingLayer(
+      const PaintArtifact&,
+      const PendingLayer& x,

[wkorman, 2016/12/20 00:28:56]

Remove 'x', or use more meaningful name?

[chrishtr, 2016/12/20 23:36:32]

Done.

+      const gfx::Rect& combinedBounds);
+
+  static bool canMergeInto(const PaintChunk& newChunk,
+                           const PendingLayer& candidatePendingLayer);
+
+  // Returns true if |newChunk| overlaps |candidatePendingLayer| in the
+  // root property tree space.
+  static bool overlaps(const PaintChunk& newChunk,
+                       const PendingLayer& candidatePendingLayer,
+                       GeometryMapper&);
+
   scoped_refptr<cc::Layer> m_rootLayer;
   std::unique_ptr<WebLayer> m_webLayer;
   Vector<std::unique_ptr<ContentLayerClientImpl>> m_contentLayerClients;
@@ -110,6 +151,18 @@ class PLATFORM_EXPORT PaintArtifactCompositor {
   friend class StubChromeClientForSPv2;
 
   bool m_isTrackingRasterInvalidations;
+  FRIEND_TEST_ALL_PREFIXES(PaintArtifactCompositorTestWithPropertyTrees,

[wkorman, 2016/12/20 00:28:56]

I thought there was a way to have one line do this for all methods in a test
class, but I can't find it, so I may be imagining things.

[chrishtr, 2016/12/20 23:36:32]

I don't know of one either.

+                           MergeSimpleChunks);
+  FRIEND_TEST_ALL_PREFIXES(PaintArtifactCompositorTestWithPropertyTrees,
+                           Merge2DTransform);
+  FRIEND_TEST_ALL_PREFIXES(PaintArtifactCompositorTestWithPropertyTrees,
+                           MergeClip);
+  FRIEND_TEST_ALL_PREFIXES(PaintArtifactCompositorTestWithPropertyTrees,
+                           MergeOpacity);
+  FRIEND_TEST_ALL_PREFIXES(PaintArtifactCompositorTestWithPropertyTrees,
+                           MergeNested);
+  FRIEND_TEST_ALL_PREFIXES(PaintArtifactCompositorTestWithPropertyTrees,
+                           PendingLayer);
 };
 
 }  // namespace blink