Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(163)

Issue 2406263003: Make PointerEvent coordinates fractional for touch (Closed)

Created:
4 years, 2 months ago by mustaq
Modified:
4 years, 1 month ago
Reviewers:
bokan, dtapuska, Rick Byers
CC:
chromium-reviews, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make PointerEvent coordinates fractional for touch The current PointerEvent spec doesn't override the UI MouseEvent spec definition of (clientX, clientY) and (screenX, screenY) coordinates: all of them are integers. This is not great for the sites that currently use Touch Events & trying to switch to Pointer Events for touch: specially on high DPI devices Pointer Event coordinates would feel less precise than fractional Touch Event coodinates. The PEWG has already reached consensus that all coordinates in Pointer Events should be floating point values: https://github.com/w3c/pointerevents/issues/107 Edge already reports fractional client coordinates for PointerEvents (and not MouseEvents). This CL exposes fractional Pointer Event coordinates for touch input. Although the CL changes the client & screen coordinates in Mouse Event idl to floating points, the integer plumbing for mouse is kept intact so that mouse coordinates still appear unchanged to JS. Pointer Events that are fired for touches propagate appropriate fractional coordinates to JS. Intent to Implement and Ship: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/1kroHAnh3BU/uYZb6uZRBwAJ BUG=607948 Committed: https://crrev.com/48a106897274196b5897f1b0967d28c49e4ec696 Cr-Commit-Position: refs/heads/master@{#432473}

Patch Set 1 #

Patch Set 2 : Updated a layout test. #

Patch Set 3 : Fixed zoom issues #

Total comments: 2

Patch Set 4 : Fixed layout tests. #

Patch Set 5 : Updated a TODO. #

Total comments: 6

Patch Set 6 : Made mouse page & scroll coodinates fractional, updated tests. #

Patch Set 7 : Reverted Init type back to int. Updated tests. #

Patch Set 8 : Fixed layout tests again. #

Patch Set 9 : Back to double type in MouseEventInit #

Patch Set 10 : Moved all coords to DoublePoints #

Patch Set 11 : Fixed test values in {mouse,wheel}-event-constructor.html. Rebased. #

Patch Set 12 : Fixed test values in {mouse,wheel}-event-constructor.html. Rebased. #

Total comments: 4

Patch Set 13 : DoublePoint -> FloatPoint #

Patch Set 14 : FloatPoint -> DoublePoint again. #

Patch Set 15 : Rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+476 lines, -317 lines) Patch
M third_party/WebKit/LayoutTests/fast/events/constructors/mouse-event-constructor.html View 1 2 3 4 5 6 7 8 9 10 11 13 1 chunk +47 lines, -32 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/constructors/mouse-event-constructor-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 13 2 chunks +41 lines, -46 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/constructors/pointer-event-constructor.html View 1 2 3 4 5 6 7 8 9 4 chunks +8 lines, -11 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/constructors/pointer-event-constructor-expected.txt View 1 2 3 4 5 6 7 8 9 7 chunks +114 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/constructors/wheel-event-constructor.html View 1 2 3 4 5 6 7 8 9 10 11 13 1 chunk +89 lines, -52 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/constructors/wheel-event-constructor-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 13 6 chunks +56 lines, -66 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/pointerevents/touch-pointer-events.html View 1 2 3 4 5 1 chunk +7 lines, -3 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/events/pointerevents/touch-pointer-events-expected.txt View 1 2 3 4 5 1 chunk +6 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/events/MouseEvent.cpp View 1 2 3 4 5 6 7 8 9 10 3 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/events/MouseEvent.idl View 1 2 3 4 5 2 chunks +10 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/core/events/MouseEventInit.idl View 7 8 1 chunk +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/events/MouseRelatedEvent.h View 1 2 3 4 5 6 7 8 9 13 2 chunks +47 lines, -22 lines 0 comments Download
M third_party/WebKit/Source/core/events/MouseRelatedEvent.cpp View 1 2 3 4 5 6 7 8 9 10 13 8 chunks +28 lines, -39 lines 0 comments Download
M third_party/WebKit/Source/core/html/forms/RangeInputType.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/shadow/SliderThumbElement.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/page/ContextMenuController.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/web/WebInputEventConversion.cpp View 1 2 3 4 5 6 7 8 9 10 13 5 chunks +10 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/web/WebPluginContainerImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +2 lines, -3 lines 0 comments Download

Messages

Total messages: 69 (34 generated)
mustaq
ptal for early comments. This is working fine, so want to double-check if exposed fractional ...
4 years, 2 months ago (2016-10-11 18:49:40 UTC) #7
Rick Byers
Seems mostly reasonable to me. I think the key outstanding question to answer is whether ...
4 years, 2 months ago (2016-10-12 14:09:54 UTC) #8
dtapuska
On 2016/10/12 14:09:54, Rick Byers wrote: > Seems mostly reasonable to me. > > I ...
4 years, 2 months ago (2016-10-12 14:14:29 UTC) #9
dtapuska
On 2016/10/12 14:14:29, dtapuska wrote: > On 2016/10/12 14:09:54, Rick Byers wrote: > > Seems ...
4 years, 2 months ago (2016-10-12 14:15:28 UTC) #10
mustaq
ptal. rbyers@: Since we want to have touch fractional coordinates asap, we have to merge ...
4 years, 2 months ago (2016-10-12 16:00:01 UTC) #11
mustaq
> dtapuska@: Fixed all layout tests. The constructor tests were affected because > of weird ...
4 years, 2 months ago (2016-10-12 16:03:35 UTC) #12
mustaq
On 2016/10/12 16:03:35, mustaq wrote: > > dtapuska@: Fixed all layout tests. The constructor tests ...
4 years, 2 months ago (2016-10-13 14:31:40 UTC) #13
mustaq
> Seems FF still follows UI MouseEvent Spec for both clientX & screenX (emits the ...
4 years, 2 months ago (2016-10-13 14:57:22 UTC) #14
Rick Byers
On 2016/10/12 16:00:01, mustaq wrote: > ptal. > > rbyers@: Since we want to have ...
4 years, 2 months ago (2016-10-13 19:01:35 UTC) #19
Rick Byers
Found a few more issues looking at this in more detail. Sorry I didn't mention ...
4 years, 2 months ago (2016-10-13 19:08:21 UTC) #20
mustaq
https://codereview.chromium.org/2406263003/diff/80001/third_party/WebKit/LayoutTests/fast/events/constructors/mouse-event-constructor-expected.txt File third_party/WebKit/LayoutTests/fast/events/constructors/mouse-event-constructor-expected.txt (right): https://codereview.chromium.org/2406263003/diff/80001/third_party/WebKit/LayoutTests/fast/events/constructors/mouse-event-constructor-expected.txt#newcode71 third_party/WebKit/LayoutTests/fast/events/constructors/mouse-event-constructor-expected.txt:71: PASS new MouseEvent('eventType', { screenX: NaN }).screenX threw exception ...
4 years, 2 months ago (2016-10-14 14:26:58 UTC) #22
mustaq
ptal. JS parsing no longer throws exception where it used to default to zeroes. The ...
4 years, 2 months ago (2016-10-17 19:16:09 UTC) #23
mustaq
On 2016/10/17 19:16:09, mustaq wrote: > ptal. JS parsing no longer throws exception where it ...
4 years, 2 months ago (2016-10-18 14:48:05 UTC) #28
dtapuska
On 2016/10/18 14:48:05, mustaq wrote: > On 2016/10/17 19:16:09, mustaq wrote: > > ptal. JS ...
4 years, 2 months ago (2016-10-19 15:39:39 UTC) #29
Rick Byers
On 2016/10/19 15:39:39, dtapuska wrote: > On 2016/10/18 14:48:05, mustaq wrote: > > On 2016/10/17 ...
4 years, 1 month ago (2016-10-24 18:35:09 UTC) #30
mustaq
ptal.
4 years, 1 month ago (2016-11-14 19:06:42 UTC) #33
dtapuska
On 2016/11/14 19:06:42, mustaq wrote: > ptal. lgtm for now. Albeit I find it odd ...
4 years, 1 month ago (2016-11-14 19:24:10 UTC) #36
mustaq
bokan@chromium.org: Please review changes in Source/web.
4 years, 1 month ago (2016-11-14 20:01:16 UTC) #38
bokan
https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h File third_party/WebKit/Source/core/events/MouseRelatedEvent.h (right): https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h#newcode130 third_party/WebKit/Source/core/events/MouseRelatedEvent.h:130: DoublePoint m_pageLocation; Why double? We've recently settled the debate ...
4 years, 1 month ago (2016-11-15 12:55:39 UTC) #41
mustaq
https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h File third_party/WebKit/Source/core/events/MouseRelatedEvent.h (right): https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h#newcode130 third_party/WebKit/Source/core/events/MouseRelatedEvent.h:130: DoublePoint m_pageLocation; On 2016/11/15 12:55:39, bokan wrote: > Why ...
4 years, 1 month ago (2016-11-15 15:21:15 UTC) #42
bokan
lgtm % FloatPoint change https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h File third_party/WebKit/Source/core/events/MouseRelatedEvent.h (right): https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h#newcode130 third_party/WebKit/Source/core/events/MouseRelatedEvent.h:130: DoublePoint m_pageLocation; On 2016/11/15 15:21:15, ...
4 years, 1 month ago (2016-11-15 15:26:00 UTC) #43
mustaq
https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h File third_party/WebKit/Source/core/events/MouseRelatedEvent.h (right): https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h#newcode130 third_party/WebKit/Source/core/events/MouseRelatedEvent.h:130: DoublePoint m_pageLocation; On 2016/11/15 15:26:00, bokan wrote: > On ...
4 years, 1 month ago (2016-11-15 16:51:39 UTC) #44
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2406263003/260001
4 years, 1 month ago (2016-11-15 16:53:39 UTC) #47
bokan
On 2016/11/15 16:51:39, mustaq wrote: > https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h > File third_party/WebKit/Source/core/events/MouseRelatedEvent.h (right): > > https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h#newcode130 > ...
4 years, 1 month ago (2016-11-15 16:54:09 UTC) #48
mustaq
On 2016/11/15 16:54:09, bokan wrote: > On 2016/11/15 16:51:39, mustaq wrote: > > > https://codereview.chromium.org/2406263003/diff/240001/third_party/WebKit/Source/core/events/MouseRelatedEvent.h ...
4 years, 1 month ago (2016-11-15 16:58:10 UTC) #49
dtapuska
On 2016/11/15 16:58:10, mustaq wrote: > On 2016/11/15 16:54:09, bokan wrote: > > On 2016/11/15 ...
4 years, 1 month ago (2016-11-15 17:06:08 UTC) #50
bokan
On 2016/11/15 17:06:08, dtapuska wrote: > On 2016/11/15 16:58:10, mustaq wrote: > > On 2016/11/15 ...
4 years, 1 month ago (2016-11-15 17:56:24 UTC) #51
mustaq
On 2016/11/15 17:56:24, bokan wrote: > On 2016/11/15 17:06:08, dtapuska wrote: > > On 2016/11/15 ...
4 years, 1 month ago (2016-11-15 18:10:33 UTC) #53
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2406263003/280001
4 years, 1 month ago (2016-11-15 18:13:38 UTC) #56
commit-bot: I haz the power
Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clang_dbg_recipe/builds/164606)
4 years, 1 month ago (2016-11-15 18:30:44 UTC) #58
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2406263003/300001
4 years, 1 month ago (2016-11-15 19:26:53 UTC) #61
commit-bot: I haz the power
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/builds/260862)
4 years, 1 month ago (2016-11-15 19:50:27 UTC) #63
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2406263003/300001
4 years, 1 month ago (2016-11-16 12:46:31 UTC) #65
commit-bot: I haz the power
Committed patchset #15 (id:300001)
4 years, 1 month ago (2016-11-16 13:46:27 UTC) #67
commit-bot: I haz the power
4 years, 1 month ago (2016-11-16 13:52:30 UTC) #69
Message was sent while issue was closed.
Patchset 15 (id:??) landed as
https://crrev.com/48a106897274196b5897f1b0967d28c49e4ec696
Cr-Commit-Position: refs/heads/master@{#432473}

Powered by Google App Engine
This is Rietveld 408576698