Merge lp:~afrantzis/mir/fix-1332632-input-for-resized-surfaces into lp:mir

[Alexandros Frantzis]

server: Ensure default input region is updated when surface is resized

server: Ensure default input region is updated when surface is resized

This MP also clarifies the behavior of set_input_region() with both tests and doxygen comments.

[Cemil Azizoglu (cemil-azizoglu)]

Looks good.

review: Approve

[Alan Griffiths (alan-griffiths)]

OK

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1724
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2007/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/713/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/719
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/713/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/528/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/525/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2125/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2007/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/802/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/714/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/720
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/714/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/186/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/186/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2143/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/809/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/737
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/743
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/736/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/193
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/193/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/193
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/193/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2235
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2235/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1899/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/8982

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1725
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2024/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/740
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/746
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/739
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/545
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/545/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/543
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/543/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2240
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2240/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1901
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/8987

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2024/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/814/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/744
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/750/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/743
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/198
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/198/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/198
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/198/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2253
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2253/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1906
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9000

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/821/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/754
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/760
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/753/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/205/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/205
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/205/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2277
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2277/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1915/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9024

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Failure is lp:1335873 (which has yet to be reproduced)

Re-approving

[Daniel van Vugt (vanvugt)]

Appears to have landed last week(?)

revno: 1728 [merge]
author: Alexandros Frantzis <email address hidden>
committer: Tarmac
branch nick: development-branch
timestamp: Fri 2014-06-27 13:40:34 +0000
message:
  server: Ensure default input region is updated when surface is resized. Fixes: https://bugs.launchpad.net/bugs/1332632.

  Approved by PS Jenkins bot, Alan Griffiths, Cemil Azizoglu.

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'include/server/mir/scene/surface.h'
--- include/server/mir/scene/surface.h	2014-06-23 02:57:39 +0000
+++ include/server/mir/scene/surface.h	2014-06-27 08:55:40 +0000
@@ -65,6 +65,18 @@
     virtual void show() = 0;
     virtual void move_to(geometry::Point const& top_left) = 0;
     virtual void take_input_focus(std::shared_ptr<shell::InputTargeter> const& targeter) = 0;
+
+    /**
+     * Sets the input region for this surface.
+     *
+     * The input region is expressed in coordinates relative to the surface (i.e.,
+     * use (0,0) for the top left point of the surface).
+     *
+     * By default the input region is the whole surface. To unset a custom input region
+     * and revert to the default set an empty input region, i.e., set_input_region({}).
+     * To disable input set a non-empty region containing an empty rectangle, i.e.,
+     * set_input_region({geom::Rectangle{}}).
+     */
     virtual void set_input_region(std::vector<geometry::Rectangle> const& region) = 0;
     virtual void allow_framedropping(bool) = 0;
     virtual void resize(geometry::Size const& size) = 0;

=== modified file 'src/server/scene/basic_surface.cpp'
--- src/server/scene/basic_surface.cpp	2014-06-24 14:12:25 +0000
+++ src/server/scene/basic_surface.cpp	2014-06-27 08:55:40 +0000
@@ -153,7 +153,7 @@
     hidden(false),
     input_mode(mi::InputReceptionMode::normal),
     nonrectangular(nonrectangular),
-    input_rectangles{geom::Rectangle{geom::Point{0, 0}, surface_rect.size}},
+    custom_input_rectangles(),
     surface_buffer_stream(buffer_stream),
     server_input_channel(input_channel),
     input_sender(input_sender),
@@ -277,7 +277,7 @@
 void ms::BasicSurface::set_input_region(std::vector<geom::Rectangle> const& input_rectangles)
 {
     std::unique_lock<std::mutex> lock(guard);
-    this->input_rectangles = input_rectangles;
+    custom_input_rectangles = input_rectangles;
 }
 
 void ms::BasicSurface::resize(geom::Size const& desired_size)
@@ -324,16 +324,20 @@
 
     if (hidden)
         return false;
-    
+
     // Restrict to bounding rectangle
     if (!surface_rect.contains(point))
         return false;
 
+    // No custom input region means effective input region is whole surface
+    if (custom_input_rectangles.empty())
+        return true;
+
     // TODO: Perhaps creates some issues with transformation.
     auto local_point = geom::Point{geom::X{point.x.as_uint32_t()-surface_rect.top_left.x.as_uint32_t()},
                                    geom::Y{point.y.as_uint32_t()-surface_rect.top_left.y.as_uint32_t()}};
 
-    for (auto const& rectangle : input_rectangles)
+    for (auto const& rectangle : custom_input_rectangles)
     {
         if (rectangle.contains(local_point))
         {

=== modified file 'src/server/scene/basic_surface.h'
--- src/server/scene/basic_surface.h	2014-06-23 22:25:17 +0000
+++ src/server/scene/basic_surface.h	2014-06-27 08:55:40 +0000
@@ -167,7 +167,7 @@
     bool hidden;
     input::InputReceptionMode input_mode;
     const bool nonrectangular;
-    std::vector<geometry::Rectangle> input_rectangles;
+    std::vector<geometry::Rectangle> custom_input_rectangles;
     std::shared_ptr<compositor::BufferStream> const surface_buffer_stream;
     std::shared_ptr<input::InputChannel> const server_input_channel;
     std::shared_ptr<input::InputSender> const input_sender;

=== modified file 'tests/unit-tests/scene/test_basic_surface.cpp'
--- tests/unit-tests/scene/test_basic_surface.cpp	2014-06-23 22:25:17 +0000
+++ tests/unit-tests/scene/test_basic_surface.cpp	2014-06-27 08:55:40 +0000
@@ -79,8 +79,8 @@
 
 struct BasicSurfaceTest : public testing::Test
 {
-    std::string name{"aa"};
-    geom::Rectangle rect{{4,7},{5,9}};
+    std::string const name{"aa"};
+    geom::Rectangle const rect{{4,7},{5,9}};
 
     testing::NiceMock<MockCallback> mock_callback;
     std::function<void()> null_change_cb{[]{}};
@@ -383,6 +383,78 @@
     }
 }
 
+TEST_F(BasicSurfaceTest, updates_default_input_region_when_surface_is_resized_to_larger_size)
+{
+    geom::Rectangle const new_rect{rect.top_left,{10,10}};
+    surface.resize(new_rect.size);
+
+    for (auto x = new_rect.top_left.x.as_int() - 1;
+         x <= new_rect.top_right().x.as_int();
+         x++)
+    {
+        for (auto y = new_rect.top_left.y.as_int() - 1;
+             y <= new_rect.bottom_left().y.as_int();
+             y++)
+        {
+            auto const test_pt = geom::Point{x, y};
+            auto const contains = surface.input_area_contains(test_pt);
+            if (new_rect.contains(test_pt))
+            {
+                EXPECT_TRUE(contains) << " point = " << test_pt;
+            }
+            else
+            {
+                EXPECT_FALSE(contains) << " point = " << test_pt;
+            }
+        }
+    }
+}
+
+TEST_F(BasicSurfaceTest, updates_default_input_region_when_surface_is_resized_to_smaller_size)
+{
+    geom::Rectangle const new_rect{rect.top_left,{2,2}};
+    surface.resize(new_rect.size);
+
+    for (auto x = rect.top_left.x.as_int() - 1;
+         x <= rect.top_right().x.as_int();
+         x++)
+    {
+        for (auto y = rect.top_left.y.as_int() - 1;
+             y <= rect.bottom_left().y.as_int();
+             y++)
+        {
+            auto const test_pt = geom::Point{x, y};
+            auto const contains = surface.input_area_contains(test_pt);
+            if (new_rect.contains(test_pt))
+            {
+                EXPECT_TRUE(contains) << " point = " << test_pt;
+            }
+            else
+            {
+                EXPECT_FALSE(contains) << " point = " << test_pt;
+            }
+        }
+    }
+}
+
+TEST_F(BasicSurfaceTest, restores_default_input_region_when_setting_empty_input_region)
+{
+    std::vector<geom::Rectangle> const rectangles = {
+        {{geom::X{0}, geom::Y{0}}, {geom::Width{1}, geom::Height{1}}}, //region0
+    };
+
+    surface.set_input_region(rectangles);
+    EXPECT_FALSE(surface.input_area_contains(rect.bottom_right() - geom::Displacement{1,1}));
+
+    surface.set_input_region({});
+    EXPECT_TRUE(surface.input_area_contains(rect.bottom_right() - geom::Displacement{1,1}));
+}
+
+TEST_F(BasicSurfaceTest, disables_input_when_setting_input_region_with_empty_rectangle)
+{
+    surface.set_input_region({geom::Rectangle()});
+    EXPECT_FALSE(surface.input_area_contains(rect.top_left));
+}
 
 TEST_F(BasicSurfaceTest, reception_mode_is_normal_by_default)
 {