Merge lp:~mir-team/mir/custom-input-regions-scale-with-surface-size into lp:mir

[Robert Carr]

Scale input rectangles as the surface resizes.

This branch is a preparation for client specified input regions.

To refresh, the requirement is that clients can list regions of their surface which are eligible to receive input and regions which input falls through.

One important question is...what happens when the surface resizes but the client has yet to supply new input rectangles? It seems clear the input rectangles should be scaled to the new surface size...if the client wishes to increase or decrease the resolution of the input mesh a new input shape could be applied.

Recall, we already have input rectangles on the server side, though mostly in an unused format. This branch makes them scale according to the surface size when they were applied.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2702
http://jenkins.qa.ubuntu.com/job/mir-ci/4219/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3028
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/547
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2976
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/375
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/375/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2976
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2976/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5791
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21526

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

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

Another option is that the client is *not* resized without supplying new input rectangles, or that a resize clears any existing rectangles. The client is then required to submit new input extents before any resize is rendered.

I think that the “just scale up the rectangles” approach isn't going to be useful; that's only useful if the client responds to a resize by drawing exactly the same thing but bigger. Clients are almost always going to respond to a resize by drawing more stuff, rather than the same stuff but bigger.

Although if we want the server to be able to resize surfaces - *and* have the user interact with them - without blocking on the client this is necessary.

Hm. I think I'm a -0 on this. Do we need to do this?

review: Needs Information

[Daniel van Vugt (vanvugt)]

I agree, this is not going to be useful.

What I learned very quickly when implementing resizing was that the dominant use case when resizing a surface is that the widgets within that surface mostly don't move (relative to the top left corner). So that's why you see we never scale the surface contents with resizing, but clamp-to-edge instead (stick to the left and top edges, filling the right/bottom with identical pixels till the client supplies a new buffer of the correct size).

As we don't scale our pixels during resizing I don't think it makes any sense to scale the input regions either. Correct behaviour will almost always be: Leave things exactly where they were (relative to top-left) until the client provides information to the contrary.

review: Disapprove

[Kevin DuBois (kdub)]

18 + auto x_scale = surface_rect.size.width.as_float() / size_when_input_rectangles_set.width.as_float();
19 + auto y_scale = surface_rect.size.height.as_float() / size_when_input_rectangles_set.height.as_float();

seems like a geom::Width / geom::Width should make a geom::Scalar (would save some line length)

[Robert Carr (robertcarr)]

I've been mulling over the concerns and whether this is really necessary...

I think it is true that clients will mostly respond by drawing more/different stuff rather than the same stuff but bigger. I have a suspicion that input shapes may mostly scale though, e.g. circular apps?

I've realized though that any sort of races I was worrying about with input shape not being up to date during resize are pretty marginal...you don't generally interact with a surface while it's being resized. I wonder if its necessary and worthwhile to make the input shape setting at least atomic with the buffer exchange though...

[Daniel van Vugt (vanvugt)]

It's true you don't interact with a surface mid-resizing, but that's a less important point.

More importantly real apps don't scale their widgets when resized, most of the time. You can see this for yourself on a desktop -- just resize some apps. Most widgets stay still and some (right/bottom aligned) widgets will move, but none of them scale (in the vast majority of cases). And if the widgets are not scaling then the input areas should not scale.

Atomicity I think is unimportant. We still have at least a frame or two lag between the client and server when resizing a window, but not much more. That still means the client is catching up much faster than you can move your mouse to it, so those brief frames of lag during a rapid resize don't need pixel-accurate input regions.

[Robert Carr (robertcarr)]

It seems like these apps (most real apps I use on my desktop) don't use input regions though and examples I can think of (OSK, Circular/elliptical freestyle windows) would benefit from the region scaling.

[Daniel van Vugt (vanvugt)]

^ That last paragraph is misleading. We already have the most pixel-accurate input regions without scaling them.

[Daniel van Vugt (vanvugt)]

Come to think of it, the solution is simple:

1. Client-specified region is always intersected with the surface region to get the final result.
2. The default (when client hasn't specified) input region is an "infinite region". But intersected with the surface region that gives you just the surface.
3. Right answer every time.

[Kevin DuBois (kdub)]

+1, sounds reasonable

=== modified file 'src/server/scene/basic_surface.cpp'
--- src/server/scene/basic_surface.cpp	2015-06-19 15:44:57 +0000
+++ src/server/scene/basic_surface.cpp	2015-06-26 20:00:08 +0000
@@ -245,7 +245,8 @@
 void ms::BasicSurface::set_input_region(std::vector<geom::Rectangle> const& input_rectangles)
 {
     std::unique_lock<std::mutex> lock(guard);
-    custom_input_rectangles = input_rectangles;
+    custom_input_rectangles  = input_rectangles;
+    size_when_input_rectangles_set = surface_rect.size;
 }
 
 void ms::BasicSurface::resize(geom::Size const& desired_size)
@@ -304,9 +305,13 @@
     // TODO: Perhaps creates some issues with transformation.
     auto local_point = geom::Point{0, 0} + (point-surface_rect.top_left);
 
+    auto x_scale = surface_rect.size.width.as_float() / size_when_input_rectangles_set.width.as_float();
+    auto y_scale = surface_rect.size.height.as_float() / size_when_input_rectangles_set.height.as_float();
+
     for (auto const& rectangle : custom_input_rectangles)
     {
-        if (rectangle.contains(local_point))
+        auto scaled_point = geom::Point{local_point.x.as_float() / x_scale, local_point.y.as_float() / y_scale};
+        if (rectangle.contains(scaled_point))
         {
             return true;
         }
 
=== modified file 'src/server/scene/basic_surface.h'
--- src/server/scene/basic_surface.h	2015-06-17 05:20:42 +0000
+++ src/server/scene/basic_surface.h	2015-06-26 20:00:08 +0000
@@ -166,6 +166,7 @@
     input::InputReceptionMode input_mode;
     const bool nonrectangular;
     std::vector<geometry::Rectangle> custom_input_rectangles;
+    geometry::Size size_when_input_rectangles_set;
     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	2015-06-25 03:00:08 +0000
+++ tests/unit-tests/scene/test_basic_surface.cpp	2015-06-26 20:00:08 +0000
@@ -488,6 +488,55 @@
     EXPECT_FALSE(surface.input_area_contains(rect.top_left));
 }
 
+// We create a 2x2 surface with a ((0, 0), (1, 1)) input rectangle.
+// as we stretch the surface we expect the input rectangle to still fill only the upper
+// left corner
+TEST_F(BasicSurfaceTest, input_regions_scale_with_size)
+{
+    surface.move_to({0, 0});
+    surface.resize({2, 2});
+
+    geom::Rectangle input_rect{{0, 0}, {1, 1}};
+    surface.set_input_region({input_rect});
+
+    EXPECT_TRUE(surface.input_area_contains(geom::Point{0, 0}));
+    EXPECT_FALSE(surface.input_area_contains(geom::Point{1, 1}));
+
+    surface.resize({10, 10});
+    EXPECT_TRUE(surface.input_area_contains(geom::Point{4, 4}));
+    EXPECT_FALSE(surface.input_area_contains(geom::Point{5, 5}));
+}
+
+TEST_F(BasicSurfaceTest, multiple_input_rectangles_scale_with_size)
+{
+    surface.move_to({0, 0});
+    surface.resize({10, 10});
+
+    geom::Rectangle input_rect_1 = {{0, 0}, {10, 3}};
+    geom::Rectangle input_rect_2 = {{0, 0}, {3, 10}};
+    surface.set_input_region({input_rect_1, input_rect_2});
+
+    // Diagonal
+    EXPECT_TRUE(surface.input_area_contains({0, 0}));
+    EXPECT_TRUE(surface.input_area_contains({1, 1}));
+    EXPECT_TRUE(surface.input_area_contains({2, 2}));
+    EXPECT_FALSE(surface.input_area_contains({3, 3}));
+
+    // Along the sides
+    EXPECT_TRUE(surface.input_area_contains({4, 2}));
+    EXPECT_TRUE(surface.input_area_contains({2, 4}));
+
+    surface.resize({100, 100});
+    EXPECT_TRUE(surface.input_area_contains({0, 0}));
+    EXPECT_TRUE(surface.input_area_contains({11, 11}));
+    EXPECT_TRUE(surface.input_area_contains({14, 13}));
+    EXPECT_TRUE(surface.input_area_contains({29, 27}));
+    EXPECT_FALSE(surface.input_area_contains({30, 30}));
+
+    EXPECT_TRUE(surface.input_area_contains({43, 24}));
+    EXPECT_TRUE(surface.input_area_contains({23, 46}));
+}
+
 TEST_F(BasicSurfaceTest, reception_mode_is_normal_by_default)
 {
     EXPECT_EQ(mi::InputReceptionMode::normal, surface.reception_mode());