Merge lp:~albaguirre/mir/fix-1498540 into lp:mir

[Alberto Aguirre]

Make DisplayInputRegion return only the first display view area.

Bounding all display view areas results in wrong touch screen coordinate scaling on most common
setups (phone/tablets/laptops that have a touchscreen in the primary display and support external displays).

Make DisplayInputRegion return only the first display view area.

Bounding all display view areas results in wrong touch screen coordinate scaling on most common
setups (phone/tablets/laptops that have a touchscreen in the primary display and support external displays).

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2962
http://jenkins.qa.ubuntu.com/job/mir-ci/5006/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3019
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4054/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/12/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1157/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/12/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4055
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4055/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6746/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23630
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/13/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5006/rebuild

review: Needs Fixing (continuous-integration)

[Alexandros Frantzis (afrantzis)]

OK as a quick fix.

+ //TODO: This region is mainly used for scaling touchscreen coordinates, so the caller
+ // probably wants the full list of rectangles. Additional work is needed
+ // to group a touchscreen with a display.

+1

Should we be worried about the "mainly" part? What other uses does it currently have that may be affected by this change?

Mainly approve, but need some info :)

review: Needs Information

[Alberto Aguirre (albaguirre)]

It's also used by mir::input::android::PointerController which passes the bounds to CursorInputMapper in the 3rd_party android input code. I don't quite understand what it does with the information however.

Testing on my laptop with mouse attached (which I would assume is a pointer controller?) dragging windows and clicking on both displays works just fine.

[Alexandros Frantzis (afrantzis)]

OK.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2963
http://jenkins.qa.ubuntu.com/job/mir-ci/5027/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4137
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3044
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4079
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/37/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1178
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1178/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/37/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4080
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4080/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6765
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23674
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/38/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5027/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

It doesn't make things right, but arguably makes them less wrong.

review: Approve

=== modified file 'src/server/input/display_input_region.cpp'
--- src/server/input/display_input_region.cpp	2015-04-28 07:54:10 +0000
+++ src/server/input/display_input_region.cpp	2015-09-25 14:16:19 +0000
@@ -46,7 +46,12 @@
             });
     });
 
-    return rectangles.bounding_rectangle();
+    //TODO: This region is mainly used for scaling touchscreen coordinates, so the caller
+    // probably wants the full list of rectangles. Additional work is needed
+    // to group a touchscreen with a display. So for now, just return the view area
+    // of the first display, as that matches the most common systems (laptops with touchscreens,
+    // phone/tablets with touchscreens).
+    return *rectangles.begin();
 }
 
 void mi::DisplayInputRegion::confine(geom::Point& point)

=== modified file 'tests/unit-tests/input/test_display_input_region.cpp'
--- tests/unit-tests/input/test_display_input_region.cpp	2015-06-25 03:00:08 +0000
+++ tests/unit-tests/input/test_display_input_region.cpp	2015-09-25 14:16:19 +0000
@@ -42,7 +42,7 @@
 
 TEST(DisplayInputRegionTest, returns_correct_bounding_rectangle)
 {
-    geom::Rectangle const expected_bounding_rect{geom::Point{0,0}, geom::Size{900,700}};
+    geom::Rectangle const expected_bounding_rect{geom::Point{0,0}, geom::Size{800,600}};
     auto stub_display = std::make_shared<mtd::StubDisplay>(rects);
 
     mi::DisplayInputRegion input_region{stub_display};