Touch transform controller now handles more than 2 displays

Touch transform controller now handles more than 2 displays

Touch transform controller did not work if more than 2 displays were
connected. This led to some touchscreens not working in such scenarios.

BUG=684178
COMPONENT=TouchTransformController, ChromeOS

Review-Url: https://codereview.chromium.org/2653493008
Cr-Commit-Position: refs/heads/master@{#445981}
Committed: https://chromium.googlesource.com/chromium/src/+/7830924e9af15a90939d656ee538aa145c2dd643

[malaykeshav, 2017-01-24 00:47:00]

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

[commit-bot: I haz the power, 2017-01-24 00:47:58]

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

[malaykeshav, 2017-01-24 00:54:00]

malaykeshav@chromium.org changed reviewers:
+ oshima@chromium.org

[malaykeshav, 2017-01-24 00:54:00]

PTAL

[commit-bot: I haz the power, 2017-01-24 01:20:51]

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

[commit-bot: I haz the power, 2017-01-24 01:20:51]

Dry run: This issue passed the CQ dry run.

[oshima, 2017-01-24 18:22:08]

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
File ui/display/manager/chromeos/touch_transform_controller.cc (right):

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
ui/display/manager/chromeos/touch_transform_controller.cc:155: DisplayIdList
GetCurrentDisplayIdList(const DisplayManager* display_manager) {
DisplayManager already has GetCurrentDisplayIdList, so let's call it
GetConnectedDisplayIdList to be more explicit.

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
ui/display/manager/chromeos/touch_transform_controller.cc:160: return
display_manager->GetCurrentDisplayIdList();
This returns single display when mirrored. is it ok?

[malaykeshav, 2017-01-24 21:20:53]

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
File ui/display/manager/chromeos/touch_transform_controller.cc (right):

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
ui/display/manager/chromeos/touch_transform_controller.cc:155: DisplayIdList
GetCurrentDisplayIdList(const DisplayManager* display_manager) {
On 2017/01/24 at 18:22:07, oshima wrote:
> DisplayManager already has GetCurrentDisplayIdList, so let's call it
GetConnectedDisplayIdList to be more explicit.

I think GetCurrentDisplayIdList() is still a better name since it amends the
DispayManager's GetCurrentDisplayIdList which does not work if the number of
displays connected is < 2.

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
ui/display/manager/chromeos/touch_transform_controller.cc:160: return
display_manager->GetCurrentDisplayIdList();
On 2017/01/24 at 18:22:07, oshima wrote:
> This returns single display when mirrored. is it ok?

No. It returns 2 IDs if I am not wrong.

[oshima, 2017-01-25 01:17:24]

lgtm

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
File ui/display/manager/chromeos/touch_transform_controller.cc (right):

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
ui/display/manager/chromeos/touch_transform_controller.cc:156: if
(display_manager->num_connected_displays() == 0)
DCHECK because this condition is already checked on caller side.

https://codereview.chromium.org/2653493008/diff/1/ui/display/manager/chromeos...
ui/display/manager/chromeos/touch_transform_controller.cc:160: return
display_manager->GetCurrentDisplayIdList();
On 2017/01/24 21:20:53, malaykeshav wrote:
> On 2017/01/24 at 18:22:07, oshima wrote:
> > This returns single display when mirrored. is it ok?
> 
> No. It returns 2 IDs if I am not wrong.

You're right. please disregard the comment.

[malaykeshav, 2017-01-25 08:22:19]

The CQ bit was checked by malaykeshav@chromium.org

[malaykeshav, 2017-01-25 08:22:20]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/2653493008/#ps20001
(title: "Touch transform controller now handles more than 2 displays")

[commit-bot: I haz the power, 2017-01-25 08:22:31]

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 08:57:14]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485332539496760,
"parent_rev": "c5a725556b091a3db0970bb42c903ae08c6496a0", "commit_rev":
"7830924e9af15a90939d656ee538aa145c2dd643"}

[commit-bot: I haz the power, 2017-01-25 08:57:44]

Description was changed from

==========
Touch transform controller now handles more than 2 displays

Touch transform controller did not work if more than 2 displays were
connected. This led to some touchscreens not working in such scenarios.

BUG=684178
COMPONENT=TouchTransformController, ChromeOS
==========

to

==========
Touch transform controller now handles more than 2 displays

Touch transform controller did not work if more than 2 displays were
connected. This led to some touchscreens not working in such scenarios.

BUG=684178
COMPONENT=TouchTransformController, ChromeOS

Review-Url: https://codereview.chromium.org/2653493008
Cr-Commit-Position: refs/heads/master@{#445981}
Committed:
https://chromium.googlesource.com/chromium/src/+/7830924e9af15a90939d656ee538...
==========

[commit-bot: I haz the power, 2017-01-25 08:57:45]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/7830924e9af15a90939d656ee538...