Commit 74780299 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Revert "[PE] Don't call PaintController::FinishCycle() if not repainted"

This reverts commit e71a3846.

Reason for revert: Caused performance regressions because changed
paint properties are not cleared changed flags.

Original change's description:
> [PE] Don't call PaintController::FinishCycle() if not repainted
> 
> This is mainly a performance optimization because we don't need
> FinishCycle() if PaintController has nothing changed.
> 
> By the way it can also avoid the immediate bad effect (but not the
> root cause) of bug 859294.
> 
> Bug: 859294
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: Iad3e14ee303c08185922d3d6e495c3b25ead5434
> Reviewed-on: https://chromium-review.googlesource.com/1121783
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#571803}

TBR=wangxianzhu@chromium.org,chrishtr@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 859294, 859885
Change-Id: I2a4d2a7d09cc3bf5edfbf0c6dee275581030c2c9
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1124701Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572302}
parent 09d7ef53
......@@ -2530,9 +2530,8 @@ bool LocalFrameView::UpdateLifecyclePhasesInternal(
// See crbug.com/667547
bool print_mode_enabled = frame_->GetDocument()->Printing() &&
!RuntimeEnabledFeatures::PrintBrowserEnabled();
bool repainted_paint_controller = false;
if (!print_mode_enabled)
repainted_paint_controller = PaintTree();
PaintTree();
if (!RuntimeEnabledFeatures::SlimmingPaintV2Enabled()) {
if (layout_view->Compositor()->InCompositingMode()) {
......@@ -2553,8 +2552,7 @@ bool LocalFrameView::UpdateLifecyclePhasesInternal(
// Notify the controller that the artifact has been pushed and some
// lifecycle state can be freed (such as raster invalidations).
if (repainted_paint_controller)
paint_controller_->FinishCycle();
paint_controller_->FinishCycle();
// PaintController for BlinkGenPropertyTrees is transient.
if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
paint_controller_ = nullptr;
......@@ -2725,7 +2723,7 @@ static void CollectDrawableLayersForLayerListRecursively(
CollectDrawableLayersForLayerListRecursively(context, layer->MaskLayer());
}
bool LocalFrameView::PaintTree() {
void LocalFrameView::PaintTree() {
TRACE_EVENT0("blink,benchmark", "LocalFrameView::paintTree");
SCOPED_UMA_AND_UKM_TIMER("Blink.Paint.UpdateTime", UkmMetricNames::kPaint);
......@@ -2739,7 +2737,6 @@ bool LocalFrameView::PaintTree() {
frame_view.Lifecycle().AdvanceTo(DocumentLifecycle::kInPaint);
});
bool repainted_paint_controller = false;
if (RuntimeEnabledFeatures::SlimmingPaintV2Enabled()) {
if (!paint_controller_)
paint_controller_ = PaintController::Create();
......@@ -2762,7 +2759,6 @@ bool LocalFrameView::PaintTree() {
PaintInternal(graphics_context, kGlobalPaintNormalPhase,
CullRect(LayoutRect::InfiniteIntRect()));
paint_controller_->CommitNewDisplayItems();
repainted_paint_controller = true;
}
} else {
// A null graphics layer can occur for painting of SVG images that are not
......@@ -2813,7 +2809,6 @@ bool LocalFrameView::PaintTree() {
CollectDrawableLayersForLayerListRecursively(
context, layout_view->Compositor()->PaintRootGraphicsLayer());
paint_controller_->CommitNewDisplayItems();
repainted_paint_controller = true;
}
ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
......@@ -2821,8 +2816,6 @@ bool LocalFrameView::PaintTree() {
if (auto* layout_view = frame_view.GetLayoutView())
layout_view->Layer()->ClearNeedsRepaintRecursively();
});
return repainted_paint_controller;
}
void LocalFrameView::PushPaintArtifactToCompositor(
......
......@@ -664,10 +664,7 @@ class CORE_EXPORT LocalFrameView final
void NotifyFrameRectsChangedIfNeededRecursive();
void UpdateStyleAndLayoutIfNeededRecursive();
void PrePaint();
// Returns true if paint_controller_ is repainted. The return value is not
// used for non-BlinkGenPropertyTrees/SPv2 where paint_controller_ doesn't
// exist.
bool PaintTree();
void PaintTree();
void UpdateStyleAndLayoutIfNeededRecursiveInternal();
......
......@@ -286,7 +286,7 @@ void GraphicsLayer::PaintRecursively() {
PaintRecursivelyInternal(repainted_layers);
// Notify the controllers that the artifact has been pushed and some
// lifecycle state can be updated.
// lifecycle state can be freed (such as raster invalidations).
for (auto* layer : repainted_layers) {
#if DCHECK_IS_ON()
if (VLOG_IS_ON(2))
......@@ -335,13 +335,10 @@ bool GraphicsLayer::Paint(const IntRect* interest_rect,
return false;
#endif
bool repainted = false;
if (PaintWithoutCommit(interest_rect, disabled_mode)) {
repainted = true;
if (PaintWithoutCommit(interest_rect, disabled_mode))
GetPaintController().CommitNewDisplayItems();
} else if (!needs_check_raster_invalidation_) {
else if (!needs_check_raster_invalidation_)
return false;
}
#if DCHECK_IS_ON()
if (VLOG_IS_ON(2)) {
......@@ -372,7 +369,7 @@ bool GraphicsLayer::Paint(const IntRect* interest_rect,
}
needs_check_raster_invalidation_ = false;
return repainted;
return true;
}
bool GraphicsLayer::PaintWithoutCommit(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment