Merge lp:~cemil-azizoglu/mir/fix-1590959 into lp:mir

[Cemil Azizoglu]

Readjust requested size to that of the actual display if it's larger in either dimension.

Readjust requested size to that of the actual display if it's larger in either dimension. Also, a border margin of 150 pixels is allowed to account for the launcher icons menu, title bar, etc. so it can still fit in the display.

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3717
https://mir-jenkins.ubuntu.com/job/mir-ci/1832/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2305/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2368
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2359
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2359
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2359
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2333/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2333
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2333/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1832/rebuild

review: Needs Fixing (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3718
https://mir-jenkins.ubuntu.com/job/mir-ci/1833/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2306/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2369
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2360
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2360
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2360
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2334/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2334/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2334/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2334/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2334/console

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1833/rebuild

review: Needs Fixing (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3719
https://mir-jenkins.ubuntu.com/job/mir-ci/1837/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2314/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2377
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2368
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2368
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2368
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2342
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2342/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2342
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2342/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2342/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2342
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2342/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2342
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2342/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1837/rebuild

review: Needs Fixing (continuous-integration)

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3719
https://mir-jenkins.ubuntu.com/job/mir-ci/1842/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2323
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2386
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2378
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2378
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2378
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2351
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2351/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2351
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2351/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2351
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2351/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2351
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2351/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2351
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2351/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1842/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

[Alan Griffiths (alan-griffiths)]

OK I suppose.

I'd prefer clipping to only apply to the default (so that I can set what I like) and for clipping to apply independently to width and height. E.g.

$ bin/mir_demo_server --x11-displays 10000x10

Ends up with a height greater than requested

review: Approve

[Kevin DuBois (kdub)]

+ auto const border = 150; //must match the border value in clip_to_display()
Redundant comment? If the production code changes, the test will tell us when run.

nit:
+ if ((screen_width < requested_size.width.as_uint32_t()+border) ||
+ (screen_height < requested_size.height.as_uint32_t()+border))
spaces between +

review: Approve

=== modified file 'src/platforms/mesa/server/x11/graphics/display.cpp'
--- src/platforms/mesa/server/x11/graphics/display.cpp	2016-09-28 04:31:08 +0000
+++ src/platforms/mesa/server/x11/graphics/display.cpp	2016-09-28 04:31:09 +0000
@@ -39,6 +39,30 @@
 
 namespace
 {
+geom::Size clip_to_display(Display *dpy, geom::Size requested_size)
+{
+    unsigned int screen_width, screen_height, uint_dummy;
+    int int_dummy;
+    Window window_dummy;
+    int const border = 150;
+
+    XGetGeometry(dpy, XDefaultRootWindow(dpy), &window_dummy, &int_dummy, &int_dummy,
+        &screen_width, &screen_height, &uint_dummy, &uint_dummy);
+
+    mir::log_info("Screen resolution = %dx%d", screen_width, screen_height);
+
+    if ((screen_width < requested_size.width.as_uint32_t()+border) ||
+        (screen_height < requested_size.height.as_uint32_t()+border))
+    {
+        mir::log_info(" ... is smaller than the requested size (%dx%d) plus border (%d). Clipping to (%dx%d).",
+            requested_size.width.as_uint32_t(), requested_size.height.as_uint32_t(), border,
+            screen_width-border, screen_height-border);
+        return geom::Size{screen_width-border, screen_height-border};
+    }
+
+    return requested_size;
+}
+
 auto get_pixel_width(Display *dpy)
 {
     auto screen = XDefaultScreenOfDisplay(dpy);
@@ -114,6 +138,7 @@
     r_mask = visInfo->red_mask;
 
     XSetWindowAttributes attr;
+    std::memset(&attr, 0, sizeof(attr));
     attr.background_pixel = 0;
     attr.border_pixel = 0;
     attr.colormap = XCreateColormap(x_dpy, root, visInfo->visual, AllocNone);
@@ -200,12 +225,12 @@
 }
 
 mgx::Display::Display(::Display* x_dpy,
-                      geom::Size const size,
+                      geom::Size const requested_size,
                       std::shared_ptr<GLConfig> const& gl_config,
                       std::shared_ptr<DisplayReport> const& report)
     : shared_egl{*gl_config},
       x_dpy{x_dpy},
-      size{size},
+      actual_size{clip_to_display(x_dpy, requested_size)},
       gl_config{gl_config},
       pixel_width{get_pixel_width(x_dpy)},
       pixel_height{get_pixel_height(x_dpy)},
@@ -216,7 +241,7 @@
 
     win = std::make_unique<X11Window>(x_dpy,
                                       shared_egl.display(),
-                                      size,
+                                      actual_size,
                                       shared_egl.config());
 
     auto red_mask = win->red_mask();
@@ -226,7 +251,7 @@
     display_buffer = std::make_unique<mgx::DisplayBuffer>(
                          x_dpy,
                          *win,
-                         size,
+                         actual_size,
                          shared_egl.context(),
                          report,
                          orientation,
@@ -249,7 +274,7 @@
 std::unique_ptr<mg::DisplayConfiguration> mgx::Display::configuration() const
 {
     return std::make_unique<mgx::DisplayConfiguration>(
-        pf, size, geom::Size{size.width * pixel_width, size.height * pixel_height}, orientation);
+        pf, actual_size, geom::Size{actual_size.width * pixel_width, actual_size.height * pixel_height}, orientation);
 }
 
 void mgx::Display::configure(mg::DisplayConfiguration const& new_configuration)

=== modified file 'src/platforms/mesa/server/x11/graphics/display.h'
--- src/platforms/mesa/server/x11/graphics/display.h	2016-09-28 04:31:08 +0000
+++ src/platforms/mesa/server/x11/graphics/display.h	2016-09-28 04:31:09 +0000
@@ -66,7 +66,7 @@
 {
 public:
     explicit Display(::Display* x_dpy,
-                     geometry::Size const size,
+                     geometry::Size const requested_size,
                      std::shared_ptr<GLConfig> const& gl_config,
                      std::shared_ptr<DisplayReport> const& report);
     ~Display() noexcept;
@@ -101,7 +101,7 @@
 private:
     helpers::EGLHelper shared_egl;
     ::Display* const x_dpy;
-    mir::geometry::Size const size;
+    mir::geometry::Size const actual_size;
     std::shared_ptr<GLConfig> const gl_config;
     float pixel_width;
     float pixel_height;

=== modified file 'tests/include/mir/test/doubles/mock_x11.h'
--- tests/include/mir/test/doubles/mock_x11.h	2016-06-28 08:24:02 +0000
+++ tests/include/mir/test/doubles/mock_x11.h	2016-09-28 04:31:09 +0000
@@ -88,7 +88,8 @@
     MOCK_METHOD9(XGrabPointer, int(Display*, Window, Bool, unsigned int, int, int, Window, Cursor, Time));
     MOCK_METHOD2(XUngrabPointer, int(Display*, Time));
     MOCK_METHOD3(XInternAtom, Atom(Display*, const char*, Bool));
-    MOCK_METHOD4(XSetWMProtocols, Status(Display *, Window, Atom*, int));
+    MOCK_METHOD4(XSetWMProtocols, Status(Display*, Window, Atom*, int));
+    MOCK_METHOD9(XGetGeometry, Status(Display*, Drawable, Window*, int*, int*, unsigned int*, unsigned int*, unsigned int*, unsigned int*));
 
     FakeX11Resources fake_x11;
 };

=== modified file 'tests/mir_test_doubles/mock_x11.cpp'
--- tests/mir_test_doubles/mock_x11.cpp	2016-06-28 08:24:02 +0000
+++ tests/mir_test_doubles/mock_x11.cpp	2016-09-28 04:31:09 +0000
@@ -57,8 +57,8 @@
     motion_event_return.type = MotionNotify;
     enter_notify_event_return.type = EnterNotify;
     leave_notify_event_return.type = LeaveNotify;
-    screen.width = 1024;
-    screen.height = 768;
+    screen.width = 2880;
+    screen.height = 1800;
     screen.mwidth = 338;
     screen.mwidth = 270;
 }
@@ -91,6 +91,11 @@
                                      {
                                          return fake_x11.pending_events;
                                      }));
+
+    ON_CALL(*this, XGetGeometry(fake_x11.display,_,_,_,_,_,_,_,_))
+    .WillByDefault(DoAll(SetArgPointee<5>(fake_x11.screen.width),
+                         SetArgPointee<6>(fake_x11.screen.height),
+                         Return(1)));
 }
 
 mtd::MockX11::~MockX11()
@@ -237,3 +242,15 @@
 {
     return global_mock->XDefaultScreenOfDisplay(display);
 }
+
+Status XGetGeometry(Display* display, Drawable d,
+                    Window* root_return,
+                    int* x_return, int* y_return,
+                    unsigned int* width_return, unsigned int* height_return,
+                    unsigned int* border_width_return, unsigned int* depth_return)
+{
+    return global_mock->XGetGeometry(display, d, root_return,
+                                     x_return, y_return,
+                                     width_return, height_return,
+                                     border_width_return, depth_return);
+}

=== modified file 'tests/unit-tests/platforms/mesa/x11/test_display.cpp'
--- tests/unit-tests/platforms/mesa/x11/test_display.cpp	2016-09-28 04:31:08 +0000
+++ tests/unit-tests/platforms/mesa/x11/test_display.cpp	2016-09-28 04:31:09 +0000
@@ -93,6 +93,11 @@
         mock_x11.fake_x11.screen.mwidth = mm.width.as_int();
         mock_x11.fake_x11.screen.mheight = mm.height.as_int();
         size = window;
+
+        ON_CALL(mock_x11, XGetGeometry(mock_x11.fake_x11.display,_,_,_,_,_,_,_,_))
+        .WillByDefault(DoAll(SetArgPointee<5>(mock_x11.fake_x11.screen.width),
+                             SetArgPointee<6>(mock_x11.fake_x11.screen.height),
+                             Return(1)));
     }
     std::shared_ptr<mgx::Display> create_display()
     {
@@ -151,7 +156,7 @@
 
 TEST_F(X11DisplayTest, calculates_physical_size_of_display_based_on_default_screen)
 {
-    auto const pixel = geom::Size{2560, 1080};
+    auto const pixel = geom::Size{2880, 1800};
     auto const mm = geom::Size{677, 290};
     auto const window = geom::Size{1280, 1024};
     auto const pixel_width = float(mm.width.as_int()) / float(pixel.width.as_int());
@@ -175,7 +180,7 @@
 
 TEST_F(X11DisplayTest, reports_a_resolution_that_matches_the_window_size)
 {
-    auto const pixel = geom::Size{2560, 1080};
+    auto const pixel = geom::Size{2880, 1800};
     auto const mm = geom::Size{677, 290};
     auto const window = geom::Size{1280, 1024};
 
@@ -193,3 +198,24 @@
 
     EXPECT_THAT(reported_resolution, Eq(window));
 }
+
+TEST_F(X11DisplayTest, adjusts_resolution_with_respect_to_screen_size)
+{
+    auto const pixel = geom::Size{1000, 1000};
+    auto const mm = geom::Size{677, 290};
+    auto const window = geom::Size{1280, 1024};
+    auto const border = 150; //must match the border value in clip_to_display()
+
+    setup_x11_screen(pixel, mm, window);
+
+    auto display = create_display();
+    auto config = display->configuration();
+    geom::Size reported_resolution;
+    config->for_each_output(
+        [&reported_resolution](mg::DisplayConfigurationOutput const& output)
+        {
+            reported_resolution = output.modes[0].size;
+        });
+
+    EXPECT_THAT(reported_resolution, Eq(geom::Size{pixel.width.as_uint32_t()-border, pixel.height.as_uint32_t()-border}));
+}