Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1186833005:
Fix touch screen on Linux with or without flag --touch-devices (Closed)
DescriptionFix touch screen on Linux with or without flag --touch-devices
Using flag --touch-devices=device id or not using it, touch screen should work on Linux,
but now only the touch screens with the ids after the this flag will be set as
touch screens, and we only use touch inputs from master pointer not from the
slave devices, so we are only receiving touch inputs if we set touch-devices equal to
the master pointer id.
BUG=486492
Committed: https://crrev.com/9c44eff0121856eaa54559515418c4dc06a18250
Cr-Commit-Position: refs/heads/master@{#335785}
Patch Set 1 : #Patch Set 2 : #Patch Set 3 : #
Total comments: 8
Patch Set 4 : #
Total comments: 1
Patch Set 5 : Change comment #
Messages
Total messages: 28 (13 generated)
lanwei@chromium.org changed reviewers: + sadrul@chromium.org
lanwei@chromium.org changed reviewers: + tdresser@chromium.org
Patchset #1 (id:1) has been deleted
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1186833005/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM.
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Patchset #3 (id:60001) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1186833005/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... File ui/events/devices/x11/touch_factory_x11.cc (right): https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... ui/events/devices/x11/touch_factory_x11.cc:106: device_master_id_list_[devinfo.deviceid] = devinfo.attachment; We should do this only for XISlavePointer device? https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... ui/events/devices/x11/touch_factory_x11.cc:138: touch_device_list_[device_master_id_list_[devinfo.deviceid]] = true; We should do this only for XISlavePointer? Also, can this directly use |devinfo.attachment|, instead of going through |device_master_id_list_|? https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... ui/events/devices/x11/touch_factory_x11.cc:228: for (std::vector<int>::const_iterator iter = devices.begin(); Since you are here: can you change this to: for (int deviceid : devices) { ... } https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... ui/events/devices/x11/touch_factory_x11.cc:235: touch_device_lookup_[device_master_id_list_[*iter]] = true; Check to see if |*iter| exists in |device_master_id_list_| first?
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1186833005/120001
Patchset #4 (id:100001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... File ui/events/devices/x11/touch_factory_x11.cc (right): https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... ui/events/devices/x11/touch_factory_x11.cc:106: device_master_id_list_[devinfo.deviceid] = devinfo.attachment; On 2015/06/17 03:33:52, sadrul wrote: > We should do this only for XISlavePointer device? Done. https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... ui/events/devices/x11/touch_factory_x11.cc:138: touch_device_list_[device_master_id_list_[devinfo.deviceid]] = true; On 2015/06/17 03:33:52, sadrul wrote: > We should do this only for XISlavePointer? > > Also, can this directly use |devinfo.attachment|, instead of going through > |device_master_id_list_|? Done. https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... ui/events/devices/x11/touch_factory_x11.cc:228: for (std::vector<int>::const_iterator iter = devices.begin(); On 2015/06/17 03:33:52, sadrul wrote: > Since you are here: can you change this to: > > for (int deviceid : devices) { ... } Done. https://codereview.chromium.org/1186833005/diff/80001/ui/events/devices/x11/t... ui/events/devices/x11/touch_factory_x11.cc:235: touch_device_lookup_[device_master_id_list_[*iter]] = true; On 2015/06/17 03:33:52, sadrul wrote: > Check to see if |*iter| exists in |device_master_id_list_| first? Done.
lgtm https://codereview.chromium.org/1186833005/diff/120001/ui/events/devices/x11/... File ui/events/devices/x11/touch_factory_x11.cc (right): https://codereview.chromium.org/1186833005/diff/120001/ui/events/devices/x11/... ui/events/devices/x11/touch_factory_x11.cc:234: // When we set the device through the "--touch-devices" flag to salve *slave
The CQ bit was checked by lanwei@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tdresser@chromium.org, sadrul@chromium.org Link to the patchset: https://codereview.chromium.org/1186833005/#ps140001 (title: "Change comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1186833005/140001
Message was sent while issue was closed.
Committed patchset #5 (id:140001)
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/9c44eff0121856eaa54559515418c4dc06a18250 Cr-Commit-Position: refs/heads/master@{#335785} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
