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

third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.h

Index: third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.h
diff --git a/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.h b/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.h
index 0204844235482c817b3b3089078fd63d1a110951..1c2c07cb2bb66d553f8b5f55d3756b384d564c47 100644
--- a/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.h
+++ b/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.h
@@ -18,7 +18,7 @@ namespace blink {
 // other objects.  RefPtrs are used to guard against use-after-free bugs and
 // DCHECKs ensure PropertyTreeState never retains the last reference to a
 // property tree node.
-class PropertyTreeState {
+class PLATFORM_EXPORT PropertyTreeState {
  public:
   PropertyTreeState(const TransformPaintPropertyNode* transform,
                     const ClipPaintPropertyNode* clip,
@@ -34,6 +34,8 @@ class PropertyTreeState {
     DCHECK(!m_scroll || !m_scroll->hasOneRef());
   }
 
+  bool hasDirectCompositingReasons() const;
+
   const TransformPaintPropertyNode* transform() const {
     DCHECK(!m_transform || !m_transform->hasOneRef());
     return m_transform.get();
@@ -70,6 +72,15 @@ class PropertyTreeState {
     DCHECK(!node->hasOneRef());
   }
 
+  enum InnermostNode {
+    None,  // None means that all nodes are their root values
+    Transform,
+    Clip,
+    Effect,
+  };
+
+  InnermostNode innermostNode() const;

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

Please add a comment describing this and the iterator below.

It looks like innermostNode is sort of the current "lowest common ancestor" in
the property tree, and you're using that to define an iteration order for the
PropertyTreeStateIterator to traverse up the tree--is that correct?

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

Done.

The innermostNode is the first of the transform, clip or effect to be applied.
There is always a total order on them.

[trchen, 2016/12/20 23:49:37]

The canonical order is:
chunk.transform.screen_matrix
chunk.clip.combined_screen_region
chunk.effect
chunk.effect.outputClip.combined_screen_region
chunk.effect.parent
chunk.effect.parent.outputClip.combined_screen_region
chunk.effect.parent.parent
...

The innermostNode implementation seems to already include some reordering to
improve edge bleeding. That's one reason I feel this belong to PAC because it
involves compositing decision.

[chrishtr, 2016/12/20 23:58:10]

I'm not sure what your point is here. Is there a bug in my algorithm?

+
  private:
   RefPtr<const TransformPaintPropertyNode> m_transform;
   RefPtr<const ClipPaintPropertyNode> m_clip;
@@ -82,6 +93,16 @@ inline bool operator==(const PropertyTreeState& a, const PropertyTreeState& b) {
          a.effect() == b.effect() && a.scroll() == b.scroll();
 }
 
+class PLATFORM_EXPORT PropertyTreeStateIterator {

[pdr., 2016/12/20 22:50:58]

Can you add a comment or file a bug about the invariant asserts we talked about?
Basically, assert here (or in the PropertyTreeState ctor) that:
a) the clip's transform is equal to or lower than the current transform
b) the effect's clip is equal to or lower than the current clip
c) the effect's transform is equal to or lower than the current transform

I think the iteration algorithm is clever and clean, but it's still a lot to
page into a brain. Heavily asserting these core invariants will save future
refactorers from frighteningly tricky bugs.

[chrishtr, 2016/12/20 23:48:31]

These asserts are a good idea, will add them in another patch.

+ public:
+  PropertyTreeStateIterator(const PropertyTreeState& properties)
+      : m_properties(properties) {}
+  const PropertyTreeState* next();
+
+ private:
+  PropertyTreeState m_properties;
+};
+
 }  // namespace blink
 
 #endif  // PropertyTreeState_h