Issue metadata
Sign in to add a comment
|
touch adjustment range is shrink due to zoom-for-dsf |
||||||||||||||||||||||
Issue descriptionTouch adjustment range on android is suppose to be radius 16 in dip. [1] After enabling zoom-for-dsf, the radius is 16 physical pixel. which makes it ~5 css pixel, so it feels like disabled on Android. [1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/page/touch_adjustment.cc?sq=package:chromium&l=47
,
Oct 12
Maybe we need to fix drag DragThreshold too. although mouse is rarely used on Android, other platform might already be affected. https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/input/mouse_event_manager.cc?sq=package:chromium&dr=CSs&g=0&l=76
,
Oct 12
,
Oct 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e08cca3a55e90a852284e196cb3b1212252d08f3 commit e08cca3a55e90a852284e196cb3b1212252d08f3 Author: Ella Ge <eirage@chromium.org> Date: Tue Oct 16 21:28:07 2018 Make touch adjustment range upperbound respect zoom_factor We used to have the touch adjustment range as 32dip in diameter (16dip in radius) for Android. After the zoom-for-dsf change, the pointerevent width & height became in physical pixel. It makes the actually range in dip much smaller on Android. This CL changes the adjustment range to be 32 CSS pixel, and apply the zoom_factor before compare with the pointerevent width&height. On Android, there is no ctrl +/- page zoom, so CSS pixel is dip. There is no change in behavior. On Desktop (aura), the adjustment upperbound is affected as it wasn't consider zooming before but now it does. Therefore, the bound will be affected by zoom_factor. On aura, there wasn't an upper bound for adjustment range before the unified touch adjustment change, which means the adjustment range is always the pointerevent width & height (physical pixel not changed). When we zoom in, the upper bound is increased, but the adjustment range will follow the finger size. So I think it's ok to make the bound changing with zooming as well on aura. Bug: 894961 Change-Id: I466d6b187359f13c82c62860086cb9eeef8cb2a5 Reviewed-on: https://chromium-review.googlesource.com/c/1279204 Commit-Queue: Ella Ge <eirage@chromium.org> Reviewed-by: Rick Byers <rbyers@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Cr-Commit-Position: refs/heads/master@{#600126} [add] https://crrev.com/e08cca3a55e90a852284e196cb3b1212252d08f3/third_party/WebKit/LayoutTests/fast/hidpi/static/pointerevents/pointerevent_touch-adjustment_click_target.html [modify] https://crrev.com/e08cca3a55e90a852284e196cb3b1212252d08f3/third_party/blink/renderer/core/input/event_handler.cc [modify] https://crrev.com/e08cca3a55e90a852284e196cb3b1212252d08f3/third_party/blink/renderer/core/input/pointer_event_manager.cc [modify] https://crrev.com/e08cca3a55e90a852284e196cb3b1212252d08f3/third_party/blink/renderer/core/page/touch_adjustment.cc [modify] https://crrev.com/e08cca3a55e90a852284e196cb3b1212252d08f3/third_party/blink/renderer/core/page/touch_adjustment.h
,
Oct 16
,
Oct 16
Requesting merge c#4 to M71. It's for fixing a regression in M71. The change is small and not risky.
,
Oct 17
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9bff915bdfef66c46742cb1db2df17c59485ae17 commit 9bff915bdfef66c46742cb1db2df17c59485ae17 Author: Ella Ge <eirage@chromium.org> Date: Thu Oct 18 21:22:49 2018 Make touch adjustment range upperbound respect zoom_factor We used to have the touch adjustment range as 32dip in diameter (16dip in radius) for Android. After the zoom-for-dsf change, the pointerevent width & height became in physical pixel. It makes the actually range in dip much smaller on Android. This CL changes the adjustment range to be 32 CSS pixel, and apply the zoom_factor before compare with the pointerevent width&height. On Android, there is no ctrl +/- page zoom, so CSS pixel is dip. There is no change in behavior. On Desktop (aura), the adjustment upperbound is affected as it wasn't consider zooming before but now it does. Therefore, the bound will be affected by zoom_factor. On aura, there wasn't an upper bound for adjustment range before the unified touch adjustment change, which means the adjustment range is always the pointerevent width & height (physical pixel not changed). When we zoom in, the upper bound is increased, but the adjustment range will follow the finger size. So I think it's ok to make the bound changing with zooming as well on aura. Bug: 894961 Change-Id: I466d6b187359f13c82c62860086cb9eeef8cb2a5 Reviewed-on: https://chromium-review.googlesource.com/c/1279204 Commit-Queue: Ella Ge <eirage@chromium.org> Reviewed-by: Rick Byers <rbyers@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#600126}(cherry picked from commit e08cca3a55e90a852284e196cb3b1212252d08f3) Reviewed-on: https://chromium-review.googlesource.com/c/1289681 Reviewed-by: Ella Ge <eirage@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#134} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [add] https://crrev.com/9bff915bdfef66c46742cb1db2df17c59485ae17/third_party/WebKit/LayoutTests/fast/hidpi/static/pointerevents/pointerevent_touch-adjustment_click_target.html [modify] https://crrev.com/9bff915bdfef66c46742cb1db2df17c59485ae17/third_party/blink/renderer/core/input/event_handler.cc [modify] https://crrev.com/9bff915bdfef66c46742cb1db2df17c59485ae17/third_party/blink/renderer/core/input/pointer_event_manager.cc [modify] https://crrev.com/9bff915bdfef66c46742cb1db2df17c59485ae17/third_party/blink/renderer/core/page/touch_adjustment.cc [modify] https://crrev.com/9bff915bdfef66c46742cb1db2df17c59485ae17/third_party/blink/renderer/core/page/touch_adjustment.h
,
Oct 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1db3dc04678b2815a8e86612961385ac3814ad0a commit 1db3dc04678b2815a8e86612961385ac3814ad0a Author: Ella Ge <eirage@chromium.org> Date: Fri Oct 19 17:00:56 2018 make touch adjustment bound in dip Previous CL crrev.com/c/1279204 changed the touch adjustment bound to CSS pixel; however, kMaxAdjustmentSize is a bound on the finger size in case noisy drivers or errant touches causing degenerate hit-tests that hit too many elements. The bound should be in dip so it's corresponding to a physical size. Therefore, kMaxAdjustmentSize should be in unscaled dip, and we need to convert it to scaled physical pixel to compare with touch_area, which is in root frame coordinates (scaled physical pixel). Bug: 894961 Change-Id: Ib515e55fd027ba7c17db7488d872be82b91f954a Reviewed-on: https://chromium-review.googlesource.com/c/1287201 Commit-Queue: Ella Ge <eirage@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#601192} [modify] https://crrev.com/1db3dc04678b2815a8e86612961385ac3814ad0a/third_party/blink/renderer/core/BUILD.gn [modify] https://crrev.com/1db3dc04678b2815a8e86612961385ac3814ad0a/third_party/blink/renderer/core/input/event_handler.cc [modify] https://crrev.com/1db3dc04678b2815a8e86612961385ac3814ad0a/third_party/blink/renderer/core/input/pointer_event_manager.cc [modify] https://crrev.com/1db3dc04678b2815a8e86612961385ac3814ad0a/third_party/blink/renderer/core/layout/hit_test_location.h [modify] https://crrev.com/1db3dc04678b2815a8e86612961385ac3814ad0a/third_party/blink/renderer/core/page/touch_adjustment.cc [modify] https://crrev.com/1db3dc04678b2815a8e86612961385ac3814ad0a/third_party/blink/renderer/core/page/touch_adjustment.h [add] https://crrev.com/1db3dc04678b2815a8e86612961385ac3814ad0a/third_party/blink/renderer/core/page/touch_adjustment_test.cc
,
Oct 19
Requesting merge c#9 to M71. It's a follow up for c#4(merged). It’s a small change with test, should not be risky.
,
Oct 20
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/faef8fb04d80de35f97990ad373f888896015ba0 commit faef8fb04d80de35f97990ad373f888896015ba0 Author: Ella Ge <eirage@chromium.org> Date: Sun Oct 21 03:54:39 2018 make touch adjustment bound in dip Previous CL crrev.com/c/1279204 changed the touch adjustment bound to CSS pixel; however, kMaxAdjustmentSize is a bound on the finger size in case noisy drivers or errant touches causing degenerate hit-tests that hit too many elements. The bound should be in dip so it's corresponding to a physical size. Therefore, kMaxAdjustmentSize should be in unscaled dip, and we need to convert it to scaled physical pixel to compare with touch_area, which is in root frame coordinates (scaled physical pixel). Bug: 894961 Change-Id: Ib515e55fd027ba7c17db7488d872be82b91f954a Reviewed-on: https://chromium-review.googlesource.com/c/1287201 Commit-Queue: Ella Ge <eirage@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#601192}(cherry picked from commit 1db3dc04678b2815a8e86612961385ac3814ad0a) Reviewed-on: https://chromium-review.googlesource.com/c/1293040 Reviewed-by: Ella Ge <eirage@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#188} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/faef8fb04d80de35f97990ad373f888896015ba0/third_party/blink/renderer/core/BUILD.gn [modify] https://crrev.com/faef8fb04d80de35f97990ad373f888896015ba0/third_party/blink/renderer/core/input/event_handler.cc [modify] https://crrev.com/faef8fb04d80de35f97990ad373f888896015ba0/third_party/blink/renderer/core/input/pointer_event_manager.cc [modify] https://crrev.com/faef8fb04d80de35f97990ad373f888896015ba0/third_party/blink/renderer/core/layout/hit_test_location.h [modify] https://crrev.com/faef8fb04d80de35f97990ad373f888896015ba0/third_party/blink/renderer/core/page/touch_adjustment.cc [modify] https://crrev.com/faef8fb04d80de35f97990ad373f888896015ba0/third_party/blink/renderer/core/page/touch_adjustment.h [add] https://crrev.com/faef8fb04d80de35f97990ad373f888896015ba0/third_party/blink/renderer/core/page/touch_adjustment_test.cc
,
Oct 21
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9bff915bdfef66c46742cb1db2df17c59485ae17 Commit: 9bff915bdfef66c46742cb1db2df17c59485ae17 Author: eirage@chromium.org Commiter: eirage@chromium.org Date: 2018-10-18 21:22:49 +0000 UTC Make touch adjustment range upperbound respect zoom_factor We used to have the touch adjustment range as 32dip in diameter (16dip in radius) for Android. After the zoom-for-dsf change, the pointerevent width & height became in physical pixel. It makes the actually range in dip much smaller on Android. This CL changes the adjustment range to be 32 CSS pixel, and apply the zoom_factor before compare with the pointerevent width&height. On Android, there is no ctrl +/- page zoom, so CSS pixel is dip. There is no change in behavior. On Desktop (aura), the adjustment upperbound is affected as it wasn't consider zooming before but now it does. Therefore, the bound will be affected by zoom_factor. On aura, there wasn't an upper bound for adjustment range before the unified touch adjustment change, which means the adjustment range is always the pointerevent width & height (physical pixel not changed). When we zoom in, the upper bound is increased, but the adjustment range will follow the finger size. So I think it's ok to make the bound changing with zooming as well on aura. Bug: 894961 Change-Id: I466d6b187359f13c82c62860086cb9eeef8cb2a5 Reviewed-on: https://chromium-review.googlesource.com/c/1279204 Commit-Queue: Ella Ge <eirage@chromium.org> Reviewed-by: Rick Byers <rbyers@chromium.org> Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#600126}(cherry picked from commit e08cca3a55e90a852284e196cb3b1212252d08f3) Reviewed-on: https://chromium-review.googlesource.com/c/1289681 Reviewed-by: Ella Ge <eirage@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#134} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Oct 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/faef8fb04d80de35f97990ad373f888896015ba0 Commit: faef8fb04d80de35f97990ad373f888896015ba0 Author: eirage@chromium.org Commiter: eirage@chromium.org Date: 2018-10-21 03:54:39 +0000 UTC make touch adjustment bound in dip Previous CL crrev.com/c/1279204 changed the touch adjustment bound to CSS pixel; however, kMaxAdjustmentSize is a bound on the finger size in case noisy drivers or errant touches causing degenerate hit-tests that hit too many elements. The bound should be in dip so it's corresponding to a physical size. Therefore, kMaxAdjustmentSize should be in unscaled dip, and we need to convert it to scaled physical pixel to compare with touch_area, which is in root frame coordinates (scaled physical pixel). Bug: 894961 Change-Id: Ib515e55fd027ba7c17db7488d872be82b91f954a Reviewed-on: https://chromium-review.googlesource.com/c/1287201 Commit-Queue: Ella Ge <eirage@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#601192}(cherry picked from commit 1db3dc04678b2815a8e86612961385ac3814ad0a) Reviewed-on: https://chromium-review.googlesource.com/c/1293040 Reviewed-by: Ella Ge <eirage@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#188} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by eirage@chromium.org
, Oct 12