Run synthetic gestures at regular 120Hz interval |
||||||||
Issue descriptionOur synthetic gesture controller is currently generating input at approximately 60Hz. The input runs out of phase with our BeginFrame scheduling. This can lead to some frames not receiving any input during synthetic gestures, which is measured as noisy jank in our graphics smoothness metrics. The first thing we can do is raise the controller's event frequency to 120Hz so that we have an event every frame. This is also closer to how we receive input on some platforms. Another improvement for benchmarking purposes would be to ensure the event timing is at a repeatable phase relative to our BeginFrame timing.
,
Aug 20
,
Oct 1
I am going to pick up this work.
,
Oct 1
This is a risky change in the sense that when we enable compositor for the layout tests this might fail bunch of them as the events may get coalesced. Note that by default events come at a 60Hz in the web driver spec as well. Although it has a parameter that the test can change and inject it faster (which we don't support at this point) but blindly changing all event injection to 120Hz is going to cause us more problem I believe.
,
Oct 1
What kind of failures do you expect from event coalescing?
,
Oct 1
Say right now a test says move to p1 and move to p2. It also expects 2 mouse moves being received (one at p1 listener and one at p2 listener). Now that we send things at 60Hz and everything is run by main there is no coalescing. So page does get that. However, when we inject them at 120Hz and also enable compositing in layout tests, the two mouse move events might get coalesced into one and the page sees only one mouse move event at p2.
,
Oct 1
That kind of test would be pretty flaky, right? Having expectations on the precise events the page receives from the browser would almost always be flaky. So I agree that this might uncover flaky tests, but that's probably not a bad thing? :)
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/581a728b2665c85e466408c2d53371abcb7f763e commit 581a728b2665c85e466408c2d53371abcb7f763e Author: Sadrul Habib Chowdhury <sadrul@chromium.org> Date: Thu Oct 04 04:53:34 2018 synthetic gesture: Allow high-frequency dispatch. For synthesized scroll/drag/pinch gestures, dispatch the events at a high frequency (120Hz) so that there's at least one event in each frame. Dispatching events at a lower frequency (60Hz) means the timer can go out of sync with begin-frame, and so there can be frames where there was to event dispatch. Our telemetry code reports this as jank, which is incorrect. Dispatching at a higher frequency for the scroll/drag/pinch gestures resolves this issue. BUG= 783034 Change-Id: I1102f76ed743231cbb10fdb872e8c66f895fa223 Reviewed-on: https://chromium-review.googlesource.com/c/1255152 Reviewed-by: Victor Miura <vmiura@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Reviewed-by: Lan Wei <lanwei@chromium.org> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#596507} [modify] https://crrev.com/581a728b2665c85e466408c2d53371abcb7f763e/content/browser/renderer_host/input/synthetic_gesture.cc [modify] https://crrev.com/581a728b2665c85e466408c2d53371abcb7f763e/content/browser/renderer_host/input/synthetic_gesture.h [modify] https://crrev.com/581a728b2665c85e466408c2d53371abcb7f763e/content/browser/renderer_host/input/synthetic_gesture_controller.cc [modify] https://crrev.com/581a728b2665c85e466408c2d53371abcb7f763e/content/browser/renderer_host/input/synthetic_gesture_controller.h [modify] https://crrev.com/581a728b2665c85e466408c2d53371abcb7f763e/content/browser/renderer_host/input/synthetic_pointer_action.cc [modify] https://crrev.com/581a728b2665c85e466408c2d53371abcb7f763e/content/browser/renderer_host/input/synthetic_pointer_action.h [modify] https://crrev.com/581a728b2665c85e466408c2d53371abcb7f763e/content/browser/renderer_host/input/synthetic_tap_gesture.cc [modify] https://crrev.com/581a728b2665c85e466408c2d53371abcb7f763e/content/browser/renderer_host/input/synthetic_tap_gesture.h
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ae26a2e41edaa334d60048b48c496062a4bb99c1 commit ae26a2e41edaa334d60048b48c496062a4bb99c1 Author: Marijn Kruisselbrink <mek@chromium.org> Date: Thu Oct 04 17:55:42 2018 Revert "synthetic gesture: Allow high-frequency dispatch." This reverts commit 581a728b2665c85e466408c2d53371abcb7f763e. Reason for revert: Causing significant test flakiness in telemetry.internal.actions.scroll_unittest.ScrollActionTest.testScrollDistanceSlowWheel. Original change's description: > synthetic gesture: Allow high-frequency dispatch. > > For synthesized scroll/drag/pinch gestures, dispatch the events > at a high frequency (120Hz) so that there's at least one event in > each frame. Dispatching events at a lower frequency (60Hz) means > the timer can go out of sync with begin-frame, and so there can > be frames where there was to event dispatch. Our telemetry code > reports this as jank, which is incorrect. Dispatching at a higher > frequency for the scroll/drag/pinch gestures resolves this issue. > > BUG= 783034 > > Change-Id: I1102f76ed743231cbb10fdb872e8c66f895fa223 > Reviewed-on: https://chromium-review.googlesource.com/c/1255152 > Reviewed-by: Victor Miura <vmiura@chromium.org> > Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> > Reviewed-by: Lan Wei <lanwei@chromium.org> > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> > Cr-Commit-Position: refs/heads/master@{#596507} TBR=sadrul@chromium.org,vmiura@chromium.org,lanwei@chromium.org,nzolghadr@chromium.org Change-Id: I1e4d64dfc3b98a547c38f8f82a2dcd45141fd66b No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 783034 , 892036 Reviewed-on: https://chromium-review.googlesource.com/c/1261972 Reviewed-by: Marijn Kruisselbrink <mek@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/master@{#596748} [modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_gesture.cc [modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_gesture.h [modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_gesture_controller.cc [modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_gesture_controller.h [modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_pointer_action.cc [modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_pointer_action.h [modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_tap_gesture.cc [modify] https://crrev.com/ae26a2e41edaa334d60048b48c496062a4bb99c1/content/browser/renderer_host/input/synthetic_tap_gesture.h
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a66784e0fd71f686f1d7152902137619608c6c80 commit a66784e0fd71f686f1d7152902137619608c6c80 Author: Sadrul Habib Chowdhury <sadrul@chromium.org> Date: Fri Oct 05 15:29:32 2018 Reland "synthetic gesture: Allow high-frequency dispatch." This is a reland of 581a728b2665c85e466408c2d53371abcb7f763e Fix for the revert in http://crrev.com/c/1264175 Original change's description: > synthetic gesture: Allow high-frequency dispatch. > > For synthesized scroll/drag/pinch gestures, dispatch the events > at a high frequency (120Hz) so that there's at least one event in > each frame. Dispatching events at a lower frequency (60Hz) means > the timer can go out of sync with begin-frame, and so there can > be frames where there was to event dispatch. Our telemetry code > reports this as jank, which is incorrect. Dispatching at a higher > frequency for the scroll/drag/pinch gestures resolves this issue. > > BUG= 783034 > > Change-Id: I1102f76ed743231cbb10fdb872e8c66f895fa223 > Reviewed-on: https://chromium-review.googlesource.com/c/1255152 > Reviewed-by: Victor Miura <vmiura@chromium.org> > Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> > Reviewed-by: Lan Wei <lanwei@chromium.org> > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> > Cr-Commit-Position: refs/heads/master@{#596507} TBR=vmiura@, nzolghadr@, lanwei@ since reland without changes Bug: 783034 Change-Id: I7cb96177df54729b24f5414c897cca97456e9484 Reviewed-on: https://chromium-review.googlesource.com/c/1263882 Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#597137} [modify] https://crrev.com/a66784e0fd71f686f1d7152902137619608c6c80/content/browser/renderer_host/input/synthetic_gesture.cc [modify] https://crrev.com/a66784e0fd71f686f1d7152902137619608c6c80/content/browser/renderer_host/input/synthetic_gesture.h [modify] https://crrev.com/a66784e0fd71f686f1d7152902137619608c6c80/content/browser/renderer_host/input/synthetic_gesture_controller.cc [modify] https://crrev.com/a66784e0fd71f686f1d7152902137619608c6c80/content/browser/renderer_host/input/synthetic_gesture_controller.h [modify] https://crrev.com/a66784e0fd71f686f1d7152902137619608c6c80/content/browser/renderer_host/input/synthetic_pointer_action.cc [modify] https://crrev.com/a66784e0fd71f686f1d7152902137619608c6c80/content/browser/renderer_host/input/synthetic_pointer_action.h [modify] https://crrev.com/a66784e0fd71f686f1d7152902137619608c6c80/content/browser/renderer_host/input/synthetic_tap_gesture.cc [modify] https://crrev.com/a66784e0fd71f686f1d7152902137619608c6c80/content/browser/renderer_host/input/synthetic_tap_gesture.h
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4f50ce7c63326a95b47f9217571bea98b8d5ff53 commit 4f50ce7c63326a95b47f9217571bea98b8d5ff53 Author: Olga Sharonova <olka@chromium.org> Date: Tue Oct 09 12:17:09 2018 Revert "Reland "synthetic gesture: Allow high-frequency dispatch."" This reverts commit a66784e0fd71f686f1d7152902137619608c6c80. Reason for revert: FindIt suspects it caused flakiness of virtual/threaded/fast/events/pinch/pinch-zoom-pan-position-fixed.html Bug: 893506 Original change's description: > Reland "synthetic gesture: Allow high-frequency dispatch." > > This is a reland of 581a728b2665c85e466408c2d53371abcb7f763e > Fix for the revert in http://crrev.com/c/1264175 > > Original change's description: > > synthetic gesture: Allow high-frequency dispatch. > > > > For synthesized scroll/drag/pinch gestures, dispatch the events > > at a high frequency (120Hz) so that there's at least one event in > > each frame. Dispatching events at a lower frequency (60Hz) means > > the timer can go out of sync with begin-frame, and so there can > > be frames where there was to event dispatch. Our telemetry code > > reports this as jank, which is incorrect. Dispatching at a higher > > frequency for the scroll/drag/pinch gestures resolves this issue. > > > > BUG= 783034 > > > > Change-Id: I1102f76ed743231cbb10fdb872e8c66f895fa223 > > Reviewed-on: https://chromium-review.googlesource.com/c/1255152 > > Reviewed-by: Victor Miura <vmiura@chromium.org> > > Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> > > Reviewed-by: Lan Wei <lanwei@chromium.org> > > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#596507} > > TBR=vmiura@, nzolghadr@, lanwei@ since reland without changes > > Bug: 783034 > Change-Id: I7cb96177df54729b24f5414c897cca97456e9484 > Reviewed-on: https://chromium-review.googlesource.com/c/1263882 > Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> > Cr-Commit-Position: refs/heads/master@{#597137} TBR=sadrul@chromium.org,vmiura@chromium.org,lanwei@chromium.org,nzolghadr@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 783034 Change-Id: I9baf489175adae82a15777427640f6eff7e2c76d Reviewed-on: https://chromium-review.googlesource.com/c/1270717 Reviewed-by: Olga Sharonova <olka@chromium.org> Commit-Queue: Olga Sharonova <olka@chromium.org> Cr-Commit-Position: refs/heads/master@{#597886} [modify] https://crrev.com/4f50ce7c63326a95b47f9217571bea98b8d5ff53/content/browser/renderer_host/input/synthetic_gesture.cc [modify] https://crrev.com/4f50ce7c63326a95b47f9217571bea98b8d5ff53/content/browser/renderer_host/input/synthetic_gesture.h [modify] https://crrev.com/4f50ce7c63326a95b47f9217571bea98b8d5ff53/content/browser/renderer_host/input/synthetic_gesture_controller.cc [modify] https://crrev.com/4f50ce7c63326a95b47f9217571bea98b8d5ff53/content/browser/renderer_host/input/synthetic_gesture_controller.h [modify] https://crrev.com/4f50ce7c63326a95b47f9217571bea98b8d5ff53/content/browser/renderer_host/input/synthetic_pointer_action.cc [modify] https://crrev.com/4f50ce7c63326a95b47f9217571bea98b8d5ff53/content/browser/renderer_host/input/synthetic_pointer_action.h [modify] https://crrev.com/4f50ce7c63326a95b47f9217571bea98b8d5ff53/content/browser/renderer_host/input/synthetic_tap_gesture.cc [modify] https://crrev.com/4f50ce7c63326a95b47f9217571bea98b8d5ff53/content/browser/renderer_host/input/synthetic_tap_gesture.h
,
Oct 9
,
Oct 9
,
Oct 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fcb17443ee321b56a46af09659ab09b20df8d0bf commit fcb17443ee321b56a46af09659ab09b20df8d0bf Author: Sadrul Habib Chowdhury <sadrul@chromium.org> Date: Tue Oct 09 18:39:15 2018 Reland "synthetic gesture: Allow high-frequency dispatch." This is a reland of a66784e0fd71f686f1d7152902137619608c6c80 Fix for the revert landed in https://crrev.com/c/1271017 Original change's description: > Reland "synthetic gesture: Allow high-frequency dispatch." > > This is a reland of 581a728b2665c85e466408c2d53371abcb7f763e > Fix for the revert in http://crrev.com/c/1264175 > > Original change's description: > > synthetic gesture: Allow high-frequency dispatch. > > > > For synthesized scroll/drag/pinch gestures, dispatch the events > > at a high frequency (120Hz) so that there's at least one event in > > each frame. Dispatching events at a lower frequency (60Hz) means > > the timer can go out of sync with begin-frame, and so there can > > be frames where there was to event dispatch. Our telemetry code > > reports this as jank, which is incorrect. Dispatching at a higher > > frequency for the scroll/drag/pinch gestures resolves this issue. > > > > BUG= 783034 > > > > Change-Id: I1102f76ed743231cbb10fdb872e8c66f895fa223 > > Reviewed-on: https://chromium-review.googlesource.com/c/1255152 > > Reviewed-by: Victor Miura <vmiura@chromium.org> > > Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> > > Reviewed-by: Lan Wei <lanwei@chromium.org> > > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#596507} > > TBR=vmiura@, nzolghadr@, lanwei@ since reland without changes > > Bug: 783034 > Change-Id: I7cb96177df54729b24f5414c897cca97456e9484 > Reviewed-on: https://chromium-review.googlesource.com/c/1263882 > Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> > Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> > Cr-Commit-Position: refs/heads/master@{#597137} TBR=vmiura@, nzolghadr@, lanwei@ since reland without changes Bug: 783034 Change-Id: I78079e81b9f57ae985b596b2bc53b3cccc5b2e4f Reviewed-on: https://chromium-review.googlesource.com/c/1270530 Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Cr-Commit-Position: refs/heads/master@{#598003} [modify] https://crrev.com/fcb17443ee321b56a46af09659ab09b20df8d0bf/content/browser/renderer_host/input/synthetic_gesture.cc [modify] https://crrev.com/fcb17443ee321b56a46af09659ab09b20df8d0bf/content/browser/renderer_host/input/synthetic_gesture.h [modify] https://crrev.com/fcb17443ee321b56a46af09659ab09b20df8d0bf/content/browser/renderer_host/input/synthetic_gesture_controller.cc [modify] https://crrev.com/fcb17443ee321b56a46af09659ab09b20df8d0bf/content/browser/renderer_host/input/synthetic_gesture_controller.h [modify] https://crrev.com/fcb17443ee321b56a46af09659ab09b20df8d0bf/content/browser/renderer_host/input/synthetic_pointer_action.cc [modify] https://crrev.com/fcb17443ee321b56a46af09659ab09b20df8d0bf/content/browser/renderer_host/input/synthetic_pointer_action.h [modify] https://crrev.com/fcb17443ee321b56a46af09659ab09b20df8d0bf/content/browser/renderer_host/input/synthetic_tap_gesture.cc [modify] https://crrev.com/fcb17443ee321b56a46af09659ab09b20df8d0bf/content/browser/renderer_host/input/synthetic_tap_gesture.h
,
Nov 19
,
Jan 16
(6 days ago)
,
Jan 16
(6 days ago)
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by sadrul@chromium.org
, Jan 10 2018