Add tangentialPressure and twist properties to PointerEvent on Windows

Add tangentialPressure and twist properties to PointerEvent on Windows

We have added the properties of tangentialPressure and twist to WebMouseEvent for stylus type,
now we are adding them to PointerDetails in ui::MouseEvent.

BUG=649376
Review-Url: https://codereview.chromium.org/2647253002
Cr-Commit-Position: refs/heads/master@{#446523}
Committed: https://chromium.googlesource.com/chromium/src/+/19f10eb68d3cdc96ce8543ee307899b147fa0632

[lanwei, 2017-01-23 01:22:11]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 01:22:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-23 01:52:21]

Description was changed from

==========
pressure win

BUG=
==========

to

==========
Add tangentialPressure and twist properties to PointerEvent on Windows

We have added the properties of tangentialPressure and twist to WebMouseEvent
for stylus type, now we are adding them to PointerDetails in ui::MouseEvent.

BUG=649376
==========

[lanwei, 2017-01-23 01:52:21]

lanwei@chromium.org changed reviewers:
+ dtapuska@chromium.org, mustaq@chromium.org

[lanwei, 2017-01-23 02:03:51]

Description was changed from

==========
Add tangentialPressure and twist properties to PointerEvent on Windows

We have added the properties of tangentialPressure and twist to WebMouseEvent
for stylus type, now we are adding them to PointerDetails in ui::MouseEvent.

BUG=649376
==========

to

==========
Add tangentialPressure and twist properties to PointerEvent on Windows

We have added the properties of tangentialPressure and twist to WebMouseEvent
for stylus type,
now we are adding them to PointerDetails in ui::MouseEvent.

BUG=649376
==========

[commit-bot: I haz the power, 2017-01-23 02:26:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-23 02:26:36]

Dry run: This issue passed the CQ dry run.

[dtapuska, 2017-01-23 19:38:44]

On 2017/01/23 18:47:22, lanwei wrote:

lgtm

[mustaq, 2017-01-23 20:23:56]

https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
File ui/events/blink/web_input_event.cc (right):

https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
ui/events/blink/web_input_event.cc:430: webkit_event.tangentialPressure =
event.pointer_details().tangentialPressure;
I think it's better to clamp the incoming values as you did in
web_input_event_builders_mac.mm, to avoid passing on out of bound values from
device drivers.

https://codereview.chromium.org/2647253002/diff/1/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/2647253002/diff/1/ui/events/event.h#newcode415
ui/events/event.h:415: PointerDetails(EventPointerType pointer_type,
Please update this method and events_win.cc to pass on the new params, even if
they are dummy for now (as for tilt_x, tilt_y).

[lanwei, 2017-01-25 01:25:38]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 01:26:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-25 01:34:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-25 01:34:24]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[lanwei, 2017-01-25 02:06:07]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 02:07:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-25 02:30:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-25 02:30:34]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[lanwei, 2017-01-25 03:46:38]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 03:47:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-25 04:50:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-25 04:50:12]

Dry run: This issue passed the CQ dry run.

[lanwei, 2017-01-25 15:48:42]

Patchset #2 (id:20001) has been deleted

[lanwei, 2017-01-25 15:48:49]

Patchset #2 (id:40001) has been deleted

[lanwei, 2017-01-25 15:53:52]

https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
File ui/events/blink/web_input_event.cc (right):

https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
ui/events/blink/web_input_event.cc:430: webkit_event.tangentialPressure =
event.pointer_details().tangentialPressure;
On 2017/01/23 20:23:56, mustaq wrote:
> I think it's better to clamp the incoming values as you did in
> web_input_event_builders_mac.mm, to avoid passing on out of bound values from
> device drivers.

https://msdn.microsoft.com/en-us/library/windows/desktop/hh454909(v=vs.85).aspx
Windows clearly states that the rotation(twist) is in the range of [0,359]. I
feel it is better to check the range when we get the data from the low level
event from OS. What do you think?

https://codereview.chromium.org/2647253002/diff/1/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/2647253002/diff/1/ui/events/event.h#newcode415
ui/events/event.h:415: PointerDetails(EventPointerType pointer_type,
On 2017/01/23 20:23:56, mustaq wrote:
> Please update this method and events_win.cc to pass on the new params, even if
> they are dummy for now (as for tilt_x, tilt_y).

Done.

[mustaq, 2017-01-25 16:48:53]

Just realized this: both web_input_event.cc and web_input_event_builders_win.cc
have WebMouseEventBuilder::Build(). I believe only the latter affects Windows
builds. Please check if this is the case.

Sorry for missing this before.

https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
File ui/events/blink/web_input_event.cc (right):

https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
ui/events/blink/web_input_event.cc:430: webkit_event.tangentialPressure =
event.pointer_details().tangentialPressure;
On 2017/01/25 15:53:51, lanwei wrote:
> On 2017/01/23 20:23:56, mustaq wrote:
> > I think it's better to clamp the incoming values as you did in
> > web_input_event_builders_mac.mm, to avoid passing on out of bound values
from
> > device drivers.
> 
>
https://msdn.microsoft.com/en-us/library/windows/desktop/hh454909(v=vs.85).aspx
> Windows clearly states that the rotation(twist) is in the range of [0,359]. I
> feel it is better to check the range when we get the data from the low level
> event from OS. What do you think?

I remember seeing a DCHECK failure for an old Android device in a similar case
for touch. Wanted to avoid the same in case some bad driver is not following the
spec.

I am fine either ways since the spec is clear.

[lanwei, 2017-01-25 18:34:08]

On 2017/01/25 16:48:53, mustaq wrote:
> Just realized this: both web_input_event.cc and
web_input_event_builders_win.cc
> have WebMouseEventBuilder::Build(). I believe only the latter affects Windows
> builds. Please check if this is the case.
> 
> Sorry for missing this before.

https://codesearch.chromium.org/chromium/src/ui/events/blink/web_input_event....
https://codereview.chromium.org/2648683003/diff/1/ui/views/win/hwnd_message_h...
This is a patch that we plan to create a ui::mouseevent from WM_Pointer message,
so we will not pass a native event.
We will call WebMouseEventBuilder::Build in web_input_event_builders_win.cc,
only when mouseevent has a native event, event.native_event().message, but since
we do not have
a native event, then web_input_event_builders_win.cc will never be called. We
will just need to call MakeWebMouseEventFromUiEvent in web_input_event.cc.



> 
>
https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
> File ui/events/blink/web_input_event.cc (right):
> 
>
https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
> ui/events/blink/web_input_event.cc:430: webkit_event.tangentialPressure =
> event.pointer_details().tangentialPressure;
> On 2017/01/25 15:53:51, lanwei wrote:
> > On 2017/01/23 20:23:56, mustaq wrote:
> > > I think it's better to clamp the incoming values as you did in
> > > web_input_event_builders_mac.mm, to avoid passing on out of bound values
> from
> > > device drivers.
> > 
> >
>
https://msdn.microsoft.com/en-us/library/windows/desktop/hh454909(v=vs.85).aspx
> > Windows clearly states that the rotation(twist) is in the range of [0,359].
I
> > feel it is better to check the range when we get the data from the low level
> > event from OS. What do you think?
> 
> I remember seeing a DCHECK failure for an old Android device in a similar case
> for touch. Wanted to avoid the same in case some bad driver is not following
the
> spec.
> 
> I am fine either ways since the spec is clear.

[mustaq, 2017-01-25 19:12:58]

On 2017/01/25 18:34:08, lanwei wrote:
> On 2017/01/25 16:48:53, mustaq wrote:
> > Just realized this: both web_input_event.cc and
> web_input_event_builders_win.cc
> > have WebMouseEventBuilder::Build(). I believe only the latter affects
Windows
> > builds. Please check if this is the case.
> > 
> > Sorry for missing this before.
> 
>
https://codesearch.chromium.org/chromium/src/ui/events/blink/web_input_event....
>
https://codereview.chromium.org/2648683003/diff/1/ui/views/win/hwnd_message_h...
> This is a patch that we plan to create a ui::mouseevent from WM_Pointer
message,
> so we will not pass a native event.
> We will call WebMouseEventBuilder::Build in web_input_event_builders_win.cc,
> only when mouseevent has a native event, event.native_event().message, but
since
> we do not have
> a native event, then web_input_event_builders_win.cc will never be called. We
> will just need to call MakeWebMouseEventFromUiEvent in web_input_event.cc.
> 
> 
> 
> > 
> >
>
https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
> > File ui/events/blink/web_input_event.cc (right):
> > 
> >
>
https://codereview.chromium.org/2647253002/diff/1/ui/events/blink/web_input_e...
> > ui/events/blink/web_input_event.cc:430: webkit_event.tangentialPressure =
> > event.pointer_details().tangentialPressure;
> > On 2017/01/25 15:53:51, lanwei wrote:
> > > On 2017/01/23 20:23:56, mustaq wrote:
> > > > I think it's better to clamp the incoming values as you did in
> > > > web_input_event_builders_mac.mm, to avoid passing on out of bound values
> > from
> > > > device drivers.
> > > 
> > >
> >
>
https://msdn.microsoft.com/en-us/library/windows/desktop/hh454909(v=vs.85).aspx
> > > Windows clearly states that the rotation(twist) is in the range of
[0,359].
> I
> > > feel it is better to check the range when we get the data from the low
level
> > > event from OS. What do you think?
> > 
> > I remember seeing a DCHECK failure for an old Android device in a similar
case
> > for touch. Wanted to avoid the same in case some bad driver is not following
> the
> > spec.
> > 
> > I am fine either ways since the spec is clear.

Thanks for clarifying, this makes sense now. LGTM.

[lanwei, 2017-01-25 19:16:18]

lanwei@chromium.org changed reviewers:
+ sadrul@chromium.org

[lanwei, 2017-01-25 19:20:56]

lanwei@chromium.org changed reviewers:
+ dcheng@chromium.org

[lanwei, 2017-01-25 19:23:23]

sadrul@ could you please take a look at the files under ui/,

dcheng@ could you please take a look at ui/events/mojo/event_struct_traits.cc,
thank you?

[dcheng, 2017-01-26 02:18:43]

struct traits rs lgtm

[sadrul, 2017-01-26 02:24:08]

https://codereview.chromium.org/2647253002/diff/60001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/2647253002/diff/60001/ui/events/event.h#newco...
ui/events/event.h:421: float tangentialPressure,
tangential_pressure (everywhere else in this CL)

https://codereview.chromium.org/2647253002/diff/60001/ui/events/event.h#newco...
ui/events/event.h:422: int twist)
Can these have default values now?

https://codereview.chromium.org/2647253002/diff/60001/ui/events/event.h#newco...
ui/events/event.h:471: float tangentialPressure = 0.0;
tangential_pressure

[lanwei, 2017-01-26 03:57:28]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 03:58:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-26 04:02:01]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 04:03:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lanwei, 2017-01-26 04:04:28]

Patchset #3 (id:80001) has been deleted

[lanwei, 2017-01-26 04:34:41]

The CQ bit was checked by lanwei@chromium.org to run a CQ dry run

[lanwei, 2017-01-26 04:35:10]

Patchset #3 (id:100001) has been deleted

[commit-bot: I haz the power, 2017-01-26 04:35:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 05:40:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 05:40:55]

Dry run: This issue passed the CQ dry run.

[lanwei, 2017-01-26 15:53:52]

https://codereview.chromium.org/2647253002/diff/60001/ui/events/event.h
File ui/events/event.h (right):

https://codereview.chromium.org/2647253002/diff/60001/ui/events/event.h#newco...
ui/events/event.h:421: float tangentialPressure,
On 2017/01/26 02:24:08, sadrul wrote:
> tangential_pressure (everywhere else in this CL)
Done, thanks for catching this!

https://codereview.chromium.org/2647253002/diff/60001/ui/events/event.h#newco...
ui/events/event.h:422: int twist)
On 2017/01/26 02:24:08, sadrul wrote:
> Can these have default values now?

Done.

[sadrul, 2017-01-27 00:23:18]

lgtm

[lanwei, 2017-01-27 01:10:54]

The CQ bit was checked by lanwei@chromium.org

[lanwei, 2017-01-27 01:10:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, mustaq@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2647253002/#ps120001
(title: "rotation")

[commit-bot: I haz the power, 2017-01-27 01:11:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 01:27:32]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1485479454108360,
"parent_rev": "9865cb667e579e2cd63214d9a423bf028a4dbd90", "commit_rev":
"19f10eb68d3cdc96ce8543ee307899b147fa0632"}

[commit-bot: I haz the power, 2017-01-27 01:28:10]

Description was changed from

==========
Add tangentialPressure and twist properties to PointerEvent on Windows

We have added the properties of tangentialPressure and twist to WebMouseEvent
for stylus type,
now we are adding them to PointerDetails in ui::MouseEvent.

BUG=649376
==========

to

==========
Add tangentialPressure and twist properties to PointerEvent on Windows

We have added the properties of tangentialPressure and twist to WebMouseEvent
for stylus type,
now we are adding them to PointerDetails in ui::MouseEvent.

BUG=649376

Review-Url: https://codereview.chromium.org/2647253002
Cr-Commit-Position: refs/heads/master@{#446523}
Committed:
https://chromium.googlesource.com/chromium/src/+/19f10eb68d3cdc96ce8543ee3078...
==========

[commit-bot: I haz the power, 2017-01-27 01:28:11]

Committed patchset #3 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/19f10eb68d3cdc96ce8543ee3078...