[SPv175] Clear paint property tree node change flags after painting

The change flags are for the current painting only. Clear them to avoid
unnecessary raster invalidation for paint property change in later
paintings.

Bug: 803867
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I2aebbd1e6342931d34bd2566983c9c22bdeabfc0
Reviewed-on: https://chromium-review.googlesource.com/920581Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537402}

third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp

@@ -291,7 +291,17 @@ IntRect GraphicsLayer::InterestRect() {
 }
 
 void GraphicsLayer::PaintRecursively() {
-  PaintRecursivelyInternal();
+  Vector<GraphicsLayer*> repainted_layers;
+  PaintRecursivelyInternal(repainted_layers);
+
+  if (RuntimeEnabledFeatures::SlimmingPaintV175Enabled()) {
+    // Clear paint property change flags that are for this paint only.
+    for (auto* layer : repainted_layers) {
+      for (const auto& chunk :
+           layer->GetPaintController().GetPaintArtifact().PaintChunks())
+        chunk.properties.property_tree_state.ClearChangedToRoot();
+    }
+  }
 
 #if DCHECK_IS_ON()
   if (VLOG_IS_ON(2)) {
@@ -308,23 +318,26 @@ void GraphicsLayer::PaintRecursively() {
 #endif
 }
 
-void GraphicsLayer::PaintRecursivelyInternal() {
-  if (DrawsContent())
-    Paint(nullptr);
+void GraphicsLayer::PaintRecursivelyInternal(
+    Vector<GraphicsLayer*>& repainted_layers) {
+  if (DrawsContent()) {
+    if (Paint(nullptr))
+      repainted_layers.push_back(this);
+  }
 
   if (MaskLayer())
-    MaskLayer()->PaintRecursivelyInternal();
+    MaskLayer()->PaintRecursivelyInternal(repainted_layers);
   if (ContentsClippingMaskLayer())
-    ContentsClippingMaskLayer()->PaintRecursivelyInternal();
+    ContentsClippingMaskLayer()->PaintRecursivelyInternal(repainted_layers);
 
   for (auto* child : Children())
-    child->PaintRecursivelyInternal();
+    child->PaintRecursivelyInternal(repainted_layers);
 }
 
-void GraphicsLayer::Paint(const IntRect* interest_rect,
+bool GraphicsLayer::Paint(const IntRect* interest_rect,
                           GraphicsContext::DisabledMode disabled_mode) {
   if (!PaintWithoutCommit(interest_rect, disabled_mode))
-    return;
+    return false;
 
 #if DCHECK_IS_ON()
   if (VLOG_IS_ON(2)) {
@@ -357,6 +370,8 @@ void GraphicsLayer::Paint(const IntRect* interest_rect,
       layer_->Layer()->Invalidate();
     }
   }
+
+  return true;
 }
 
 bool GraphicsLayer::PaintWithoutCommit(

third_party/WebKit/Source/platform/graphics/GraphicsLayer.h

@@ -254,7 +254,8 @@ class PLATFORM_EXPORT GraphicsLayer : public cc::LayerClient,
 
   IntRect InterestRect();
   void PaintRecursively();
-  void Paint(const IntRect* interest_rect,
+  // Returns true if this layer is repainted.
+  bool Paint(const IntRect* interest_rect,
              GraphicsContext::DisabledMode = GraphicsContext::kNothingDisabled);
 
   // cc::LayerClient implementation.
@@ -309,7 +310,7 @@ class PLATFORM_EXPORT GraphicsLayer : public cc::LayerClient,
                      PaintingControlSetting = kPaintDefaultBehavior) final;
   size_t ApproximateUnsharedMemoryUsage() const final;
 
-  void PaintRecursivelyInternal();
+  void PaintRecursivelyInternal(Vector<GraphicsLayer*>& repainted_layers);
 
   // Returns true if PaintController::paintArtifact() changed and needs commit.
   bool PaintWithoutCommit(

third_party/WebKit/Source/platform/graphics/GraphicsLayerTest.cpp

@@ -34,12 +34,15 @@
 #include "platform/animation/CompositorKeyframeModel.h"
 #include "platform/animation/CompositorTargetProperty.h"
 #include "platform/graphics/CompositorElementId.h"
+#include "platform/graphics/paint/PaintControllerTest.h"
 #include "platform/graphics/paint/PropertyTreeState.h"
+#include "platform/graphics/paint/ScopedPaintChunkProperties.h"
 #include "platform/graphics/test/FakeScrollableArea.h"
 #include "platform/scheduler/child/web_scheduler.h"
 #include "platform/scroll/ScrollableArea.h"
 #include "platform/testing/FakeGraphicsLayer.h"
 #include "platform/testing/FakeGraphicsLayerClient.h"
+#include "platform/testing/PaintTestConfigurations.h"
 #include "platform/testing/WebLayerTreeViewImplForTesting.h"
 #include "platform/transforms/Matrix3DTransformOperation.h"
 #include "platform/transforms/RotateTransformOperation.h"
@@ -54,7 +57,8 @@
 
 namespace blink {
 
-class GraphicsLayerTest : public ::testing::Test {
+class GraphicsLayerTest : public ::testing::Test,
+                          public PaintTestConfigurations {
  public:
   GraphicsLayerTest() {
     clip_layer_ = WTF::WrapUnique(new FakeGraphicsLayer(client_));
@@ -109,6 +113,10 @@ class GraphicsLayerTest : public ::testing::Test {
   std::unique_ptr<WebLayerTreeViewImplForTesting> layer_tree_view_;
 };
 
+INSTANTIATE_TEST_CASE_P(All,
+                        GraphicsLayerTest,
+                        ::testing::Values(0, kSlimmingPaintV175));
+
 class AnimationPlayerForTesting : public CompositorAnimationPlayerClient {
  public:
   AnimationPlayerForTesting() {
@@ -122,7 +130,7 @@ class AnimationPlayerForTesting : public CompositorAnimationPlayerClient {
   std::unique_ptr<CompositorAnimationPlayer> compositor_player_;
 };
 
-TEST_F(GraphicsLayerTest, updateLayerShouldFlattenTransformWithAnimations) {
+TEST_P(GraphicsLayerTest, updateLayerShouldFlattenTransformWithAnimations) {
   ASSERT_FALSE(platform_layer_->HasTickingAnimationForTesting());
 
   std::unique_ptr<CompositorFloatAnimationCurve> curve =
@@ -177,7 +185,7 @@ TEST_F(GraphicsLayerTest, updateLayerShouldFlattenTransformWithAnimations) {
   host.RemoveTimeline(*compositor_timeline.get());
 }
 
-TEST_F(GraphicsLayerTest, Paint) {
+TEST_P(GraphicsLayerTest, Paint) {
   IntRect interest_rect(1, 2, 3, 4);
   EXPECT_TRUE(PaintWithoutCommit(*graphics_layer_, &interest_rect));
   graphics_layer_->GetPaintController().CommitNewDisplayItems();
@@ -200,4 +208,42 @@ TEST_F(GraphicsLayerTest, Paint) {
   EXPECT_FALSE(PaintWithoutCommit(*graphics_layer_, &interest_rect));
 }
 
+TEST_P(GraphicsLayerTest, PaintRecursively) {
+  if (!RuntimeEnabledFeatures::SlimmingPaintV175Enabled())
+    return;
+
+  IntRect interest_rect(1, 2, 3, 4);
+  auto transform_root = TransformPaintPropertyNode::Root();
+  auto transform1 = TransformPaintPropertyNode::Create(
+      transform_root, TransformationMatrix().Translate(10, 20), FloatPoint3D());
+  auto transform2 = TransformPaintPropertyNode::Create(
+      transform1, TransformationMatrix().Scale(2), FloatPoint3D());
+
+  client_.SetPainter([&](const GraphicsLayer* layer, GraphicsContext& context,
+                         GraphicsLayerPaintingPhase, const IntRect&) {
+    {
+      ScopedPaintChunkProperties properties(
+          context.GetPaintController(), transform1, *layer, kBackgroundType);
+      PaintControllerTestBase::DrawRect(context, *layer, kBackgroundType,
+                                        interest_rect);
+    }
+    {
+      ScopedPaintChunkProperties properties(
+          context.GetPaintController(), transform2, *layer, kForegroundType);
+      PaintControllerTestBase::DrawRect(context, *layer, kForegroundType,
+                                        interest_rect);
+    }
+  });
+
+  transform1->Update(transform_root, TransformationMatrix().Translate(20, 30),
+                     FloatPoint3D());
+  EXPECT_TRUE(transform1->Changed(*transform_root));
+  EXPECT_TRUE(transform2->Changed(*transform_root));
+  client_.SetNeedsRepaint(true);
+  graphics_layer_->PaintRecursively();
+
+  EXPECT_FALSE(transform1->Changed(*transform_root));
+  EXPECT_FALSE(transform2->Changed(*transform_root));
+}
+
 }  // namespace blink

third_party/WebKit/Source/platform/graphics/compositing/CompositedLayerRasterInvalidator.cpp

@@ -316,12 +316,16 @@ void CompositedLayerRasterInvalidator::Generate(
   if (!layer_bounds_was_empty && !layer_bounds_.IsEmpty())
     GenerateRasterInvalidations(paint_chunks, new_chunks_info, layer_state);
 
-  paint_chunks_info_.clear();
-  std::swap(paint_chunks_info_, new_chunks_info);
+  paint_chunks_info_ = std::move(new_chunks_info);
   if (tracking_info_) {
-    tracking_info_->old_client_debug_names.clear();
-    std::swap(tracking_info_->old_client_debug_names,
-              tracking_info_->new_client_debug_names);
+    tracking_info_->old_client_debug_names =
+        std::move(tracking_info_->new_client_debug_names);
+  }
+
+  for (const auto* chunk : paint_chunks) {
+    chunk->client_is_just_created = false;
+    chunk->raster_invalidation_rects.clear();
+    chunk->raster_invalidation_tracking.clear();
   }
 }
 

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

@@ -789,14 +789,9 @@ void PaintArtifactCompositor::Update(
 
   g_s_property_tree_sequence_number++;
 
-  for (const auto& chunk : paint_artifact.PaintChunks()) {
+  // Clear paint property change flags that are for this update only.
+  for (const auto& chunk : paint_artifact.PaintChunks())
     chunk.properties.property_tree_state.ClearChangedToRoot();
-    // TODO(wangxianzhu): This will be unnecessary if we don't call
-    // PaintArtifactCompositor::Update() when paint artifact is unchanged.
-    chunk.client_is_just_created = false;
-    chunk.raster_invalidation_rects.clear();
-    chunk.raster_invalidation_tracking.clear();
-  }
 
 #if DCHECK_IS_ON()
   if (VLOG_IS_ON(2)) {

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

@@ -18,6 +18,25 @@ class PaintControllerTestBase : public ::testing::Test {
  public:
   PaintControllerTestBase() : paint_controller_(PaintController::Create()) {}
 
+  static void DrawNothing(GraphicsContext& context,
+                          const DisplayItemClient& client,
+                          DisplayItem::Type type) {
+    if (DrawingRecorder::UseCachedDrawingIfPossible(context, client, type))
+      return;
+    DrawingRecorder recorder(context, client, type);
+  }
+
+  template <typename Rect>
+  static void DrawRect(GraphicsContext& context,
+                       const DisplayItemClient& client,
+                       DisplayItem::Type type,
+                       const Rect& bounds) {
+    if (DrawingRecorder::UseCachedDrawingIfPossible(context, client, type))
+      return;
+    DrawingRecorder recorder(context, client, type);
+    context.DrawRect(RoundedIntRect(FloatRect(bounds)));
+  }
+
  protected:
   PaintController& GetPaintController() { return *paint_controller_; }
 
@@ -42,25 +61,6 @@ class PaintControllerTestBase : public ::testing::Test {
     return paint_controller_->GetSubsequenceMarkers(client);
   }
 
-  static void DrawNothing(GraphicsContext& context,
-                          const DisplayItemClient& client,
-                          DisplayItem::Type type) {
-    if (DrawingRecorder::UseCachedDrawingIfPossible(context, client, type))
-      return;
-    DrawingRecorder recorder(context, client, type);
-  }
-
-  template <typename Rect>
-  static void DrawRect(GraphicsContext& context,
-                       const DisplayItemClient& client,
-                       DisplayItem::Type type,
-                       const Rect& bounds) {
-    if (DrawingRecorder::UseCachedDrawingIfPossible(context, client, type))
-      return;
-    DrawingRecorder recorder(context, client, type);
-    context.DrawRect(RoundedIntRect(FloatRect(bounds)));
-  }
-
  private:
   std::unique_ptr<PaintController> paint_controller_;
 };

third_party/WebKit/Source/platform/testing/FakeGraphicsLayerClient.h

@@ -24,10 +24,13 @@ class FakeGraphicsLayerClient : public GraphicsLayerClient {
   bool NeedsRepaint(const GraphicsLayer&) const override {
     return needs_repaint_;
   }
-  void PaintContents(const GraphicsLayer*,
-                     GraphicsContext&,
-                     GraphicsLayerPaintingPhase,
-                     const IntRect&) const override {}
+  void PaintContents(const GraphicsLayer* layer,
+                     GraphicsContext& context,
+                     GraphicsLayerPaintingPhase phase,
+                     const IntRect& rect) const override {
+    if (painter_)
+      painter_(layer, context, phase, rect);
+  }
 
   void SetIsTrackingRasterInvalidations(bool is_tracking_raster_invalidations) {
     is_tracking_raster_invalidations_ = is_tracking_raster_invalidations;
@@ -35,7 +38,14 @@ class FakeGraphicsLayerClient : public GraphicsLayerClient {
 
   void SetNeedsRepaint(bool needs_repaint) { needs_repaint_ = needs_repaint; }
 
+  using Painter = std::function<void(const GraphicsLayer*,
+                                     GraphicsContext&,
+                                     GraphicsLayerPaintingPhase,
+                                     const IntRect&)>;
+  void SetPainter(const Painter& painter) { painter_ = painter; }
+
  private:
+  Painter painter_ = nullptr;
   bool is_tracking_raster_invalidations_ = false;
   bool needs_repaint_ = false;
 };