New issue
Advanced search Search tips

Issue 888269 link

Starred by 1 user
Status: Fixed
Owner:
sunxd@chromium.org
Closed: Oct 30
Cc:
pdr@chromium.org
xidac...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 836905



Sign in to add a comment

[PaintTouchActionRects] Repainting on every frame during scroll on touch_handler_scrolling.html

Project Member Reported by pdr@chromium.org, Sep 22 
PaintTouchActionRects will repaint on every frame on tools/perf/page_sets/tough_scheduling_cases/touch_handler_scrolling.html. This is due to the TODO in BlockPainter::RecordHitTestData: the touch action rect needs to be painted in the space of the scrolling contents layer (see: BoxPainter::PaintBoxDecorationBackground).

To reproduce: open touch_handler_scrolling.html with --show-paint-rects and notice the full-page paints occurring on every scroll.

This is a regression due to https://crrev.com/593255.

Comment 1 by xidac...@chromium.org, Oct 5

Owner: sunxd@chromium.org
Status: Assigned (was: Untriaged)
sunxd@: we should have this test covered by your new CL.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 27 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4294591ceb97fab1465d9490484041a708a9ba38

commit 4294591ceb97fab1465d9490484041a708a9ba38
Author: Xianda Sun <sunxd@chromium.org>
Date: Sat Oct 27 14:30:21 2018

[PaintTouchActionRects] Record overflow paint rect as hit test rect

Since touch action rect needs to be painted in the space of scrolling
contents layer, and we record border box rect as touch action rect in
block painter, we ended up repainting on every frame.

This CL fixes this bug by recording overflow paint rect in box painter.

Bug:  888269 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ifefac631604355e1b33be92a37ffdb0a0301cd25
Reviewed-on: https://chromium-review.googlesource.com/c/1259946
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603335}
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/block_painter.cc
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/block_painter.h
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/block_painter_test.cc
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/box_painter.cc
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/box_painter.h
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/fieldset_painter.cc
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/table_cell_painter.cc
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/table_painter.cc
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/table_painter.h
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/view_painter.cc
[modify] https://crrev.com/4294591ceb97fab1465d9490484041a708a9ba38/third_party/blink/renderer/core/paint/view_painter.h

Comment 3 by xidac...@chromium.org, Oct 30

Status: Fixed (was: Assigned)
Awesome work!
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 31 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80b2218c58bc9de49f65f56d5a5a805a03a47f62

commit 80b2218c58bc9de49f65f56d5a5a805a03a47f62
Author: Philip Rogers <pdr@chromium.org>
Date: Wed Oct 31 18:29:55 2018

[PaintTouchActionRects] Support scrolling LayoutView touch action rects

This patch is a followup to [1] and paints the LayoutView hit test
rects into the scrolling contents layer, like the background does. This
improves performance because the hit test rect does not need to be
repainted on every scroll change.

With this patch we no longer regress raster in the rendering.desktop
touch_handler_scrolling benchmark.

[1] https://crrev.com/603335

Bug:  888269 
Change-Id: I27864ed6d612c26f2258b60805778b2c6f894d62
Reviewed-on: https://chromium-review.googlesource.com/c/1303868
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Xianda Sun <sunxd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604342}
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-global-expected.txt
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/block_painter_test.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/box_painter.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/box_painter.h
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/fieldset_painter.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/table_cell_painter.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/table_painter.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/view_painter.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/view_painter.h
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/core/paint/view_painter_test.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/platform/graphics/hit_test_rect.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/platform/graphics/hit_test_rect.h
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/platform/graphics/paint/hit_test_data.cc
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/platform/graphics/paint/hit_test_data.h
[modify] https://crrev.com/80b2218c58bc9de49f65f56d5a5a805a03a47f62/third_party/blink/renderer/platform/graphics/paint/paint_chunk.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 5 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a469697e93e7d645f1927502e6c9b663bb2d3a9

commit 3a469697e93e7d645f1927502e6c9b663bb2d3a9
Author: Xianda Sun <sunxd@chromium.org>
Date: Mon Nov 05 17:59:27 2018

[PaintTouchActionRect] Calculate scrolling LayoutReplaced hit test rects

This is a followup patch to [1] and paints LayoutReplaced hit test rects
into the scrolling contents layer. In this way we don't need to repaint
on every scroll change.

[1] https://crrev.com/603335

Bug:  888269 
Change-Id: Ic8196f59b8a2f7d08285c5234ed562a61386ee6b
Reviewed-on: https://chromium-review.googlesource.com/c/1310754
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605378}
[modify] https://crrev.com/3a469697e93e7d645f1927502e6c9b663bb2d3a9/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-svg-root-expected.txt
[modify] https://crrev.com/3a469697e93e7d645f1927502e6c9b663bb2d3a9/third_party/blink/renderer/core/paint/box_painter.cc
[modify] https://crrev.com/3a469697e93e7d645f1927502e6c9b663bb2d3a9/third_party/blink/renderer/core/paint/replaced_painter.cc
[modify] https://crrev.com/3a469697e93e7d645f1927502e6c9b663bb2d3a9/third_party/blink/renderer/core/paint/replaced_painter.h
►

Sign in to add a comment