Touch move event coordinates incorrect when OOPIF has CSS scale transform |
||||
Issue descriptionChrome Version: 68.0.3440.118 OS: ChromeOS What steps will reproduce the problem? (1) See output.jsbin.com/vuzefivifa (2) Blue and red frames are identical, except that blue frame is (loaded from jsfiddle) and red frame is same-process using srcdoc. (3) If you touch at left and swipe to right of those frames, you'll see move X coordinate goes to ~1000 for red frame as expected, but only to ~500 for blue frame. An alternative repro case is at http://output.jsbin.com/lomowed/5, where an <input range> element shows the slider moving way too fast in the blue OOPIF.
,
Sep 12
Just re-reading my description, (2) meant to say blue frame "is an OOPIF"
,
Sep 12
It's quite likely this occurs due to the fact that the TouchEvents are transformed only by a translation ('delta'), which would be fine for the TouchStart, but wrong for any other TouchEvent not at the exact same position.
https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_input_event_router.cc?rcl=6cb8ed9f50e8c23c1fc6968277ffaf1d94bbd2f1&l=718
A fix for this is to instead use TransformPointToCoordSpaceForView
https://cs.chromium.org/chromium/src/content/browser/frame_host/cross_process_frame_connector.h?rcl=6cb8ed9f50e8c23c1fc6968277ffaf1d94bbd2f1&l=93
... though we had originally intended to do this and ran into reliability problems. We could re-try this though ... it would be good to revisit what the issues where there anyways.
This wouldn't happen to Mouse events since those get transformed individually according to the transformed point computed during targeting (though it could be an issue if mouse capture is locking the mouse event to some other target?).
,
Sep 12
I can put together a test CL tomorrow to see if my ideas above are correct ...
,
Sep 13
halliwell@ - I've put up an experimental CL at https://chromium-review.googlesource.com/c/chromium/src/+/1224453 that seems to fix the problem for the OOPIF case ... did you want to give it a spin with your WebView-app? You might want to try a DCHECK enabled build.
,
Sep 13
I should note ... my experimental is specific to touch screen gestures, but there's a similar fix that can be enacted for touchpad as well.
,
Sep 13
I haven't had time to test exhaustively yet, but it does appear to fix the original bug that started this :)
,
Sep 13
,
Sep 13
Note: there is an outstanding issue with Android point transformations that will affect whatever we land to fix this: https://bugs.chromium.org/p/chromium/issues/detail?id=809122
,
Sep 13
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0eb253d276542716a804b6d23090c2f137f1c282 commit 0eb253d276542716a804b6d23090c2f137f1c282 Author: W. James MacLean <wjmaclean@chromium.org> Date: Thu Oct 04 18:46:53 2018 Make test, BasicSelectionIsolatedIframe, correct. This CL corrects a deficiency in TouchSelectionControllerClientAuraSiteIsolationTest. BasicSelectionIsolatedIframe that was discovered in the process of fixing the bug listed below. The test was manufacturing its own gesture events, instead of letting the GestureRecognizer/Provider framework do it. As a result, these fake gesture events were routing improperly through RenderWidgetHostInputEventRouter, and in fact the test was passing for totally incorrect reasons. This CL modifies the touch events to be sent through the Aura event dispatching system, so that proper and consistent gesture events are inferred from the touch events. Bug: 883508 Change-Id: Ic22b402b23fc7ede598821985998d8d3afd31cb1 Reviewed-on: https://chromium-review.googlesource.com/c/1259203 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Commit-Queue: James MacLean <wjmaclean@chromium.org> Cr-Commit-Position: refs/heads/master@{#596780} [modify] https://crrev.com/0eb253d276542716a804b6d23090c2f137f1c282/content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc [modify] https://crrev.com/0eb253d276542716a804b6d23090c2f137f1c282/content/test/data/touch_selection.html
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c2469abf7b94fa0d0303aa3615899eb4bc3deb36 commit c2469abf7b94fa0d0303aa3615899eb4bc3deb36 Author: W. James MacLean <wjmaclean@chromium.org> Date: Thu Oct 04 20:20:50 2018 Convert coords per touch point with full transforms. It has long been known that the translation-only model for transforming point coordinates of Touch and TouchscreenGesture events in RenderWidgetHostInputEventRouter is insufficient in the case of non- trivial CSS transformations and OOPIFs; specifically when scale and/or rotation is involved. This CL fixes that by using full transforms when they are available via Viz hit testing. Bug: 883508 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I53c163b0c942f0eb62d5bee76245003c0f032cfc Reviewed-on: https://chromium-review.googlesource.com/c/1224453 Reviewed-by: Charlie Reis <creis@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Commit-Queue: James MacLean <wjmaclean@chromium.org> Cr-Commit-Position: refs/heads/master@{#596826} [modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/renderer_host/render_widget_host_input_event_router.cc [modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/renderer_host/render_widget_host_input_event_router.h [modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/renderer_host/render_widget_host_view_base.cc [modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/renderer_host/render_widget_host_view_base.h [modify] https://crrev.com/c2469abf7b94fa0d0303aa3615899eb4bc3deb36/content/browser/site_per_process_hit_test_browsertest.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by halliwell@chromium.org
, Sep 12