Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2020143003:
Identity the mouse pointer type from low-level events for Windows (Closed)
DescriptionWe hard coded the pointer type as mouse in ui::GetMousePointerDetailsFromNative
for events sending from Windows. Now we use a mouse message from GetMessageExtraInfo to decide
the actual input device type.
GetMessageExtraInfo needs to be mask-checked against 0xFFFFFF00, and then compared
with 0xFF515700. True when this mouse message was generated by a Tablet PC pen or touch
screen. Otherwise it is from a mouse device. Additionally, in Windows Vista or later,
the eighth bit, masked by 0x80, is used to differentiate
touch input from pen input
(0 = pen, 1 = touch).
Reference:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms703320(v=vs.85).aspx
BUG=614820
Committed: https://crrev.com/16ac979eddbdd5ccc045092c1260e42dd38191f6
Cr-Commit-Position: refs/heads/master@{#398025}
Patch Set 1 : #
Total comments: 9
Patch Set 2 : Remove touch pointer type for mouse event #Patch Set 3 : Remove touch pointer type for mouse event #Messages
Total messages: 23 (8 generated)
Description was changed from ========== win type BUG=614820 ========== to ========== We hard coded the pointer type as mouse in ui::GetMousePointerDetailsFromNative for events sending from Windows. Now we use a mouse message from GetMessageExtraInfo to decide the actual input device type. GetMessageExtraInfo needs to be mask-checked against 0xFFFFFF00, and then compared with 0xFF515700. True when this mouse message was generated by a Tablet PC pen or touch screen. Otherwise it is from a mouse device. Additionally, in Windows Vista or later, the eighth bit, masked by 0x80, is used to differentiate touch input from pen input (0 = pen, 1 = touch). Reference: https://msdn.microsoft.com/en-us/library/windows/desktop/ms703320(v=vs.85).aspx BUG=614820 ==========
lanwei@chromium.org changed reviewers: + dtapuska@chromium.org, mustaq@chromium.org, nzolghadr@chromium.org, tdresser@chromium.org
Patchset #1 (id:1) has been deleted
https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... File ui/events/win/events_win.cc (right): https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:298: LPARAM message = ::GetMessageExtraInfo(); Do we have any sort of unique id for each pen device? Can we set that as well in PointerDetails? https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:303: return PointerDetails(EventPointerType::POINTER_TYPE_TOUCH); Is this possible for this touch if to run? I mean do we filter these mouse events caused by touch points anywhere?
https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... File ui/events/win/events_win.cc (right): https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:298: LPARAM message = ::GetMessageExtraInfo(); On 2016/06/01 19:15:29, Navid Zolghadr wrote: > Do we have any sort of unique id for each pen device? Can we set that as well in > PointerDetails? "The lower 8 bits returned from GetMessageExtraInfo are variable. Of those bits, 7 (the lower 7, masked by 0x7F) are used to represent the cursor ID, zero for the mouse or a variable value for the pen ID."from Windows doc. I tested, for both two pens from Wacom, the ID is the same. The pen ID from Surface is different from Wacom. Do you think we should use this? https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:303: return PointerDetails(EventPointerType::POINTER_TYPE_TOUCH); On 2016/06/01 19:15:29, Navid Zolghadr wrote: > Is this possible for this touch if to run? I mean do we filter these mouse > events caused by touch points anywhere? I am not sure if there are some cases the events are still dispatched.
https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... File ui/events/win/events_win.cc (right): https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:303: return PointerDetails(EventPointerType::POINTER_TYPE_TOUCH); On 2016/06/01 22:54:31, lanwei wrote: > On 2016/06/01 19:15:29, Navid Zolghadr wrote: > > Is this possible for this touch if to run? I mean do we filter these mouse > > events caused by touch points anywhere? > > I am not sure if there are some cases the events are still dispatched. https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/win/hwnd_...
https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... File ui/events/win/events_win.cc (right): https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:298: LPARAM message = ::GetMessageExtraInfo(); On 2016/06/01 22:54:31, lanwei wrote: > On 2016/06/01 19:15:29, Navid Zolghadr wrote: > > Do we have any sort of unique id for each pen device? Can we set that as well > in > > PointerDetails? > > "The lower 8 bits returned from GetMessageExtraInfo are variable. Of those bits, > 7 (the lower 7, masked by 0x7F) are used to represent the cursor ID, zero for > the mouse or a variable value for the pen ID."from Windows doc. I tested, for > both two pens from Wacom, the ID is the same. The pen ID from Surface is > different from Wacom. Do you think we should use this? Although it is not complete and that does not diffrentiate the two Wacom pens I think it is still better than setting ids to 0 all the time.
On 2016/06/02 14:29:37, Navid Zolghadr wrote: > https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... > File ui/events/win/events_win.cc (right): > > https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... > ui/events/win/events_win.cc:298: LPARAM message = ::GetMessageExtraInfo(); > On 2016/06/01 22:54:31, lanwei wrote: > > On 2016/06/01 19:15:29, Navid Zolghadr wrote: > > > Do we have any sort of unique id for each pen device? Can we set that as > well > > in > > > PointerDetails? > > > > "The lower 8 bits returned from GetMessageExtraInfo are variable. Of those > bits, > > 7 (the lower 7, masked by 0x7F) are used to represent the cursor ID, zero for > > the mouse or a variable value for the pen ID."from Windows doc. I tested, for > > both two pens from Wacom, the ID is the same. The pen ID from Surface is > > different from Wacom. Do you think we should use this? > > Although it is not complete and that does not diffrentiate the two Wacom pens I > think it is still better than setting ids to 0 all the time. Let's land this change which is addressing the type problem. Then later in a separate change set the id and the plumbing to Blink. LGTM as is.
LGTM with a nit. Also: please file a bug for the missing id case. https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... File ui/events/win/events_win.cc (right): https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:26: #define MOUSEEVENTF_FROMTOUCHPEN 0xFF515700 Please move FROMTOUCHPEN above FROMTOUCH to avoid having the same constant twice.
lanwei@chromium.org changed reviewers: + ananta@chromium.org, sadrul@chromium.org
https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... File ui/events/win/events_win.cc (right): https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:26: #define MOUSEEVENTF_FROMTOUCHPEN 0xFF515700 On 2016/06/02 15:28:24, mustaq wrote: > Please move FROMTOUCHPEN above FROMTOUCH to avoid having the same constant > twice. Done. https://codereview.chromium.org/2020143003/diff/20001/ui/events/win/events_wi... ui/events/win/events_win.cc:303: return PointerDetails(EventPointerType::POINTER_TYPE_TOUCH); On 2016/06/02 13:40:40, tdresser wrote: > On 2016/06/01 22:54:31, lanwei wrote: > > On 2016/06/01 19:15:29, Navid Zolghadr wrote: > > > Is this possible for this touch if to run? I mean do we filter these mouse > > > events caused by touch points anywhere? > > > > I am not sure if there are some cases the events are still dispatched. > > https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/win/hwnd_... The mouseevent from touch will set a flag EF_FROM_TOUCH and it will be filtered at RenderWidgetHostViewAura::OnMouseEvent. I think it is safe to remove touch pointer type from mouseevent https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
lgtm
rs lgtm
On 2016/06/03 11:16:12, sadrul wrote: > rs lgtm lgtm
LGTM
The CQ bit was checked by lanwei@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org, nzolghadr@chromium.org Link to the patchset: https://codereview.chromium.org/2020143003/#ps60001 (title: "Remove touch pointer type for mouse event")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2020143003/60001
Message was sent while issue was closed.
Description was changed from ========== We hard coded the pointer type as mouse in ui::GetMousePointerDetailsFromNative for events sending from Windows. Now we use a mouse message from GetMessageExtraInfo to decide the actual input device type. GetMessageExtraInfo needs to be mask-checked against 0xFFFFFF00, and then compared with 0xFF515700. True when this mouse message was generated by a Tablet PC pen or touch screen. Otherwise it is from a mouse device. Additionally, in Windows Vista or later, the eighth bit, masked by 0x80, is used to differentiate touch input from pen input (0 = pen, 1 = touch). Reference: https://msdn.microsoft.com/en-us/library/windows/desktop/ms703320(v=vs.85).aspx BUG=614820 ========== to ========== We hard coded the pointer type as mouse in ui::GetMousePointerDetailsFromNative for events sending from Windows. Now we use a mouse message from GetMessageExtraInfo to decide the actual input device type. GetMessageExtraInfo needs to be mask-checked against 0xFFFFFF00, and then compared with 0xFF515700. True when this mouse message was generated by a Tablet PC pen or touch screen. Otherwise it is from a mouse device. Additionally, in Windows Vista or later, the eighth bit, masked by 0x80, is used to differentiate touch input from pen input (0 = pen, 1 = touch). Reference: https://msdn.microsoft.com/en-us/library/windows/desktop/ms703320(v=vs.85).aspx BUG=614820 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== We hard coded the pointer type as mouse in ui::GetMousePointerDetailsFromNative for events sending from Windows. Now we use a mouse message from GetMessageExtraInfo to decide the actual input device type. GetMessageExtraInfo needs to be mask-checked against 0xFFFFFF00, and then compared with 0xFF515700. True when this mouse message was generated by a Tablet PC pen or touch screen. Otherwise it is from a mouse device. Additionally, in Windows Vista or later, the eighth bit, masked by 0x80, is used to differentiate touch input from pen input (0 = pen, 1 = touch). Reference: https://msdn.microsoft.com/en-us/library/windows/desktop/ms703320(v=vs.85).aspx BUG=614820 ========== to ========== We hard coded the pointer type as mouse in ui::GetMousePointerDetailsFromNative for events sending from Windows. Now we use a mouse message from GetMessageExtraInfo to decide the actual input device type. GetMessageExtraInfo needs to be mask-checked against 0xFFFFFF00, and then compared with 0xFF515700. True when this mouse message was generated by a Tablet PC pen or touch screen. Otherwise it is from a mouse device. Additionally, in Windows Vista or later, the eighth bit, masked by 0x80, is used to differentiate touch input from pen input (0 = pen, 1 = touch). Reference: https://msdn.microsoft.com/en-us/library/windows/desktop/ms703320(v=vs.85).aspx BUG=614820 Committed: https://crrev.com/16ac979eddbdd5ccc045092c1260e42dd38191f6 Cr-Commit-Position: refs/heads/master@{#398025} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/16ac979eddbdd5ccc045092c1260e42dd38191f6 Cr-Commit-Position: refs/heads/master@{#398025} |
