New issue
Advanced search Search tips

Issue 853912 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 21
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug

Blocking:
issue 836905



Sign in to add a comment

Make PaintTouchActionRects pass all fast/events/touch tests

Project Member Reported by pdr@chromium.org, Jun 18 2018

Issue description

We should pass all tests in fast/events/touch with PaintTouchActionRects:
rtd third_party/WebKit/LayoutTests/fast/events/touch/ --additional-driver-flag=--enable-blink-features=PaintTouchActionRects
 
Labels: Test-Layout
Here are the remaining failures:

* fast/events/touch/compositor-touch-hit-rects-global.html
This is testing a special-case of the entire document/body having touch action rects. I think these results are correct even though they are different.

* fast/events/touch/compositor-touch-hit-rects-many.html
PaintTouchActionRects does not have logic for handling the pathological case of many many rects. I think this is actually okay and we can accept that there can be many touch action rects.

These failures need investigation:
* fast/events/touch/compositor-touch-hit-rects-non-composited-scroll.html
* fast/events/touch/compositor-touch-hit-rects-scroll.html
* fast/events/touch/compositor-touch-hit-rects-squashing.html
* fast/events/touch/compositor-touch-hit-rects.html

Some of these may be due to how PaintTouchActionRects fixes scrolling ( https://crbug.com/826746 ) by putting the rects in the scrolling contents layer. Some of these are likely bugs that need investigation.
Owner: ----
Status: Available (was: Assigned)
Owner: xidac...@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 24

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

commit ba00eb2c96e32ec793bef33c56d53eba96edd2d5
Author: Xida Chen <xidachen@chromium.org>
Date: Tue Jul 24 01:17:20 2018

Create a virtual test suite for layout test under fast/events/touch

The feature PaintTouchActionRects could introduce different behavior
than today's code of computing the touch action rects. In order to
prevent regression, this CL creates a new test suite for the tests
under fast/events/touch/. The virtual suite run those tests with
--enable-blink-features=PaintTouchActionRects.

Bug:  853912 
Change-Id: Ia07e93892e88dfb9e6c2ea078784da3985ef26a4
Reviewed-on: https://chromium-review.googlesource.com/1146764
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577388}
[modify] https://crrev.com/ba00eb2c96e32ec793bef33c56d53eba96edd2d5/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/ba00eb2c96e32ec793bef33c56d53eba96edd2d5/third_party/WebKit/LayoutTests/VirtualTestSuites
[add] https://crrev.com/ba00eb2c96e32ec793bef33c56d53eba96edd2d5/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/README.txt
[add] https://crrev.com/ba00eb2c96e32ec793bef33c56d53eba96edd2d5/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-global-expected.txt
[add] https://crrev.com/ba00eb2c96e32ec793bef33c56d53eba96edd2d5/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-many-expected.txt
[add] https://crrev.com/ba00eb2c96e32ec793bef33c56d53eba96edd2d5/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-scroll-expected.txt

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 26

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

commit be1adc2d776a6ece1cbd085caf197be71f76e0be
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Jul 26 01:03:10 2018

Account for paint offset when compute touch action rects

Right now the touch action rects are compute during the pre-paint tree
walk, and we do account for the paint offset with regards to the layout
object. However, when we project that to the graphics layer's coordinate
space, we need to account for the paint offset of the graphics layer
with regards to the layout object.

We add a new layout test specially for this.

Bug:  853912 
Change-Id: I6054f564c7e3f71f6c4c874180802112b5dd37c4
Reviewed-on: https://chromium-review.googlesource.com/1149456
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578153}
[add] https://crrev.com/be1adc2d776a6ece1cbd085caf197be71f76e0be/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-with-negative-child-expected.txt
[add] https://crrev.com/be1adc2d776a6ece1cbd085caf197be71f76e0be/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-with-negative-child.html
[modify] https://crrev.com/be1adc2d776a6ece1cbd085caf197be71f76e0be/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-scroll-expected.txt
[modify] https://crrev.com/be1adc2d776a6ece1cbd085caf197be71f76e0be/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 26

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

commit 646f2eb0c2ee3737a493ddc948d000fd9911b106
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Jul 26 14:52:02 2018

Account for graphics layer's offset with regards to its transform node

In this previous CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1149456
When projecting a touch-action rect to a graphics layer's space, the
offset with regards to the layout object is accounted for. What we
really need is the offset with regards to its transform node.

The correctness can be proved by passing an existing test.

Bug:  853912 
Change-Id: I534bd2aa52552bc2ac12a0eec259d47c9cb73574
Reviewed-on: https://chromium-review.googlesource.com/1150928
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578301}
[modify] https://crrev.com/646f2eb0c2ee3737a493ddc948d000fd9911b106/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/646f2eb0c2ee3737a493ddc948d000fd9911b106/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 27

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

commit 7f010897fcfbda4a62daaea1fc59b3120b2b2d95
Author: Xida Chen <xidachen@chromium.org>
Date: Fri Jul 27 19:29:22 2018

Record HitTestData in SVGShapePainter

Right now none of SVG related painters call RecordHitTestData, that's
why the "svgline" test case in the compositor-touch-hit-rects.html is
failing.

This CL adds a RecordHitTestData API in SVGShapePainter. It will fix
the failing test. However, any SVG related painters should have such an
API. It will be addressed in follow-up CLs together with new test cases.

Bug:  853912 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I7b8aa3869d8703a38a4f3265e4f1fc2744b6bd6a
Reviewed-on: https://chromium-review.googlesource.com/1151616
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578747}
[modify] https://crrev.com/7f010897fcfbda4a62daaea1fc59b3120b2b2d95/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/7f010897fcfbda4a62daaea1fc59b3120b2b2d95/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-svg-expected.txt
[add] https://crrev.com/7f010897fcfbda4a62daaea1fc59b3120b2b2d95/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-svg.html
[add] https://crrev.com/7f010897fcfbda4a62daaea1fc59b3120b2b2d95/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-svg-expected.txt
[modify] https://crrev.com/7f010897fcfbda4a62daaea1fc59b3120b2b2d95/third_party/blink/renderer/core/paint/svg_shape_painter.cc
[modify] https://crrev.com/7f010897fcfbda4a62daaea1fc59b3120b2b2d95/third_party/blink/renderer/core/paint/svg_shape_painter.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 28

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

commit d8d5fff44042614afd211c5c1185877bc95aa2ab
Author: Xida Chen <xidachen@chromium.org>
Date: Sat Jul 28 00:33:26 2018

[LayoutTest] Mark compositor-touch-hit-rects-non-composited-scroll.html pass

Under paint-touchaction-rects, the new result is actually correct. This
CL changes the expected.txt and mark this test pass.

A new test is added to show that with PaintTouchActionRects, the result
is correct with clipping.

Bug:  853912 
Change-Id: Id6138081e9a782333edc2911a9e9f53997f0b564
Reviewed-on: https://chromium-review.googlesource.com/1151709
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578876}
[modify] https://crrev.com/d8d5fff44042614afd211c5c1185877bc95aa2ab/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/d8d5fff44042614afd211c5c1185877bc95aa2ab/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-non-composited-scroll-overflow-with-handler-expected.txt
[add] https://crrev.com/d8d5fff44042614afd211c5c1185877bc95aa2ab/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-non-composited-scroll-overflow-with-handler.html
[add] https://crrev.com/d8d5fff44042614afd211c5c1185877bc95aa2ab/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-non-composited-scroll-expected.txt
[add] https://crrev.com/d8d5fff44042614afd211c5c1185877bc95aa2ab/third_party/WebKit/LayoutTests/virtual/paint-touchaction-rects/fast/events/touch/compositor-touch-hit-rects-non-composited-scroll-overflow-with-handler-expected.txt

Status: Assigned (was: Available)
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 16

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

commit a1c594c829ab0c8df636cdbcf4c4220fc33aa8b2
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Aug 16 18:01:29 2018

Fix touch action rect computation for continuation

This CL fixes the touch action rects computation for continuation
case. In this case, we need to mark white listed touch action
changed for a object's continuation.

The correctness can be verified by passing an existing test.

Bug:  853912 
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I88a5bd5989d2dbf42c921fd71ca2ca5aa5f893c6
Reviewed-on: https://chromium-review.googlesource.com/1154888
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583727}
[modify] https://crrev.com/a1c594c829ab0c8df636cdbcf4c4220fc33aa8b2/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/a1c594c829ab0c8df636cdbcf4c4220fc33aa8b2/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-continuation-expected.txt
[add] https://crrev.com/a1c594c829ab0c8df636cdbcf4c4220fc33aa8b2/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-continuation.html
[modify] https://crrev.com/a1c594c829ab0c8df636cdbcf4c4220fc33aa8b2/third_party/blink/renderer/core/frame/event_handler_registry.cc
[modify] https://crrev.com/a1c594c829ab0c8df636cdbcf4c4220fc33aa8b2/third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc

Status: Fixed (was: Assigned)
This can be closed now.

Sign in to add a comment