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

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef PropertyTreeState_h
 #define PropertyTreeState_h
 
 #include "platform/graphics/paint/ClipPaintPropertyNode.h"
 #include "platform/graphics/paint/EffectPaintPropertyNode.h"
 #include "platform/graphics/paint/ScrollPaintPropertyNode.h"
 #include "platform/graphics/paint/TransformPaintPropertyNode.h"
 #include "wtf/HashFunctions.h"
 #include "wtf/HashTraits.h"
 
 namespace blink {
 
 // A complete set of paint properties including those that are inherited from
 // 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,
                     const EffectPaintPropertyNode* effect,
                     const ScrollPaintPropertyNode* scroll)
       : m_transform(transform),
         m_clip(clip),
         m_effect(effect),
         m_scroll(scroll) {
     DCHECK(!m_transform || !m_transform->hasOneRef());
     DCHECK(!m_clip || !m_clip->hasOneRef());
     DCHECK(!m_effect || !m_effect->hasOneRef());
     DCHECK(!m_scroll || !m_scroll->hasOneRef());
   }
 
+  bool hasDirectCompositingReasons() const;
+
   const TransformPaintPropertyNode* transform() const {
     DCHECK(!m_transform || !m_transform->hasOneRef());
     return m_transform.get();
   }
   void setTransform(const TransformPaintPropertyNode* node) {
     m_transform = node;
     DCHECK(!node->hasOneRef());
   }
 
   const ClipPaintPropertyNode* clip() const {
@@ skipping 16 matching lines @@
 
   const ScrollPaintPropertyNode* scroll() const {
     DCHECK(!m_scroll || !m_scroll->hasOneRef());
     return m_scroll.get();
   }
   void setScroll(const ScrollPaintPropertyNode* node) {
     m_scroll = node;
     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;
   RefPtr<const EffectPaintPropertyNode> m_effect;
   RefPtr<const ScrollPaintPropertyNode> m_scroll;
 };
 
 inline bool operator==(const PropertyTreeState& a, const PropertyTreeState& b) {
   return a.transform() == b.transform() && a.clip() == b.clip() &&
          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