Scale touch event radius

Scale touch event radius

Touch event's position resolution could be quite different than the
display's resolution, e.g.  the display could be set as 1920x1080
while the touchscreen is reporting touch position range at 32767x32767.
Touch event's radius is reported in the units the same as touch position.
While we are doing touch position scaling, we should also do the same
for touch radius.

BUG=392172, 233245
TEST=touch radius is scaled to be reasonable value for HP 23TM
     touch monitor.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285132

[oshima, 2014-07-22 21:41:18]

ash/ lgtm with a nit

https://codereview.chromium.org/412553005/diff/20001/ash/touch/touch_transfor...
File ash/touch/touch_transformer_controller.cc (right):

https://codereview.chromium.org/412553005/diff/20001/ash/touch/touch_transfor...
ash/touch/touch_transformer_controller.cc:37: double
TouchTransformerController::GetTouchRadiusScale(
Maybe TouchResolutionScale is better name?

[Yufeng Shen (Slow to review), 2014-07-22 21:52:04]

https://codereview.chromium.org/412553005/diff/20001/ash/touch/touch_transfor...
File ash/touch/touch_transformer_controller.cc (right):

https://codereview.chromium.org/412553005/diff/20001/ash/touch/touch_transfor...
ash/touch/touch_transformer_controller.cc:37: double
TouchTransformerController::GetTouchRadiusScale(
On 2014/07/22 21:41:18, oshima wrote:
> Maybe TouchResolutionScale is better name?

Done.

[sadrul, 2014-07-22 22:29:58]

A few questions: (1) what kind of changes would we need in athena for this same
fix? (2) what kind of changes would we need for non-x11 platforms (e.g. ozone)
for this same fix? (3) Can you explain why the transform used for touch-event's
location is not sufficient for updating the touch-event's radius?

[Yufeng Shen (Slow to review), 2014-07-22 23:03:15]

On 2014/07/22 22:29:58, sadrul wrote:
> A few questions: (1) what kind of changes would we need in athena for this
same
> fix?

Lets talk about this in person.


> (2) what kind of changes would we need for non-x11 platforms (e.g. ozone)
> for this same fix?

I think we need to expose an API of DeviceDataManager for querying the
touchscreen
resolution. And a DeviceDataManagerOzone can implement that for ozone.

Then in TouchTransformerController::GetTouchResolutionScale, we query the
touchscreen
resolution through DeviceDataManager, instead of DeviceDataManagerX11.

> (3) Can you explain why the transform used for touch-event's
> location is not sufficient for updating the touch-event's radius?

For location transform, the touch-event's location received by chrome is
already mapped by X to the framebuffer size, so it is about mapping
from framebuffer size to display size.

For radius transform, X does not scale radius, so we get the radius
that's in the touchscreen's original reporting range, and we have to
scale it to be compatible with the display size.

Say we have one touchdisplay. The display resolution is 1920x1080, and
the touch events reporting range is 30000x40000. When chrome gets the
touch event from X, its position is already in 1920x1080 range, so the
location transformer actually don't have to do anything (in this single
monitor case). But the touch radius is still in the 30000x40000 space, e.g.
the max radius could be (the longest diagonal line) 50000 / 2 = 25000,
which is wrong if you put it into 1920x1080 display resolution.

[sadrul, 2014-07-23 18:40:17]

LGTM. Thanks for explaining.

https://codereview.chromium.org/412553005/diff/40001/ui/events/x/events_x.cc
File ui/events/x/events_x.cc (right):

https://codereview.chromium.org/412553005/diff/40001/ui/events/x/events_x.cc#...
ui/events/x/events_x.cc:290: if (xev->type != GenericEvent)
Make this a DCHECK_EQ instead?

[Yufeng Shen (Slow to review), 2014-07-23 19:11:20]

https://codereview.chromium.org/412553005/diff/40001/ui/events/x/events_x.cc
File ui/events/x/events_x.cc (right):

https://codereview.chromium.org/412553005/diff/40001/ui/events/x/events_x.cc#...
ui/events/x/events_x.cc:290: if (xev->type != GenericEvent)
On 2014/07/23 18:40:16, sadrul wrote:
> Make this a DCHECK_EQ instead?

Done.

[Yufeng Shen (Slow to review), 2014-07-23 19:11:22]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2014-07-23 19:14:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/miletus@chromium.org/412553005/60001

[commit-bot: I haz the power, 2014-07-24 03:00:25]

Change committed as 285132