Mir

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

Proposed by Cemil Azizoglu on 2016-09-27
Status: Merged
Approved by: Daniel van Vugt on 2016-10-03
Approved revision: 3719
Merged at revision: 3733
Proposed branch: lp:~cemil-azizoglu/mir/fix-1590959
Merge into: lp:mir
Prerequisite: lp:~cemil-azizoglu/mir/x11-better-display-handling
Diff against target: 222 lines (+81/-12)
5 files modified
src/platforms/mesa/server/x11/graphics/display.cpp (+30/-5)
src/platforms/mesa/server/x11/graphics/display.h (+2/-2)
tests/include/mir/test/doubles/mock_x11.h (+2/-1)
tests/mir_test_doubles/mock_x11.cpp (+19/-2)
tests/unit-tests/platforms/mesa/x11/test_display.cpp (+28/-2)
To merge this branch: bzr merge lp:~cemil-azizoglu/mir/fix-1590959
Reviewer Review Type Date Requested Status
Kevin DuBois (community) Approve on 2016-09-30
Alan Griffiths Approve on 2016-09-30
Alexandros Frantzis (community) 2016-09-27 Approve on 2016-09-29
Mir CI Bot continuous-integration 2016-09-27 Approve on 2016-09-28
Review via email: mp+306961@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
lp:~cemil-azizoglu/mir/fix-1590959 updated on 2016-09-27
3718. By Cemil Azizoglu on 2016-09-27

make it rvalue reference

Mir CI Bot (mir-ci-bot) wrote :

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)
lp:~cemil-azizoglu/mir/fix-1590959 updated on 2016-09-28
3719. By Cemil Azizoglu on 2016-09-28

hmm do away with std::move

Mir CI Bot (mir-ci-bot) wrote :

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) wrote :

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) wrote :

Looks good.

review: Approve
Alan Griffiths (alan-griffiths) wrote :

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) wrote :

+ 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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/platforms/mesa/server/x11/graphics/display.cpp'
2--- src/platforms/mesa/server/x11/graphics/display.cpp 2016-09-28 04:31:08 +0000
3+++ src/platforms/mesa/server/x11/graphics/display.cpp 2016-09-28 04:31:09 +0000
4@@ -39,6 +39,30 @@
5
6 namespace
7 {
8+geom::Size clip_to_display(Display *dpy, geom::Size requested_size)
9+{
10+ unsigned int screen_width, screen_height, uint_dummy;
11+ int int_dummy;
12+ Window window_dummy;
13+ int const border = 150;
14+
15+ XGetGeometry(dpy, XDefaultRootWindow(dpy), &window_dummy, &int_dummy, &int_dummy,
16+ &screen_width, &screen_height, &uint_dummy, &uint_dummy);
17+
18+ mir::log_info("Screen resolution = %dx%d", screen_width, screen_height);
19+
20+ if ((screen_width < requested_size.width.as_uint32_t()+border) ||
21+ (screen_height < requested_size.height.as_uint32_t()+border))
22+ {
23+ mir::log_info(" ... is smaller than the requested size (%dx%d) plus border (%d). Clipping to (%dx%d).",
24+ requested_size.width.as_uint32_t(), requested_size.height.as_uint32_t(), border,
25+ screen_width-border, screen_height-border);
26+ return geom::Size{screen_width-border, screen_height-border};
27+ }
28+
29+ return requested_size;
30+}
31+
32 auto get_pixel_width(Display *dpy)
33 {
34 auto screen = XDefaultScreenOfDisplay(dpy);
35@@ -114,6 +138,7 @@
36 r_mask = visInfo->red_mask;
37
38 XSetWindowAttributes attr;
39+ std::memset(&attr, 0, sizeof(attr));
40 attr.background_pixel = 0;
41 attr.border_pixel = 0;
42 attr.colormap = XCreateColormap(x_dpy, root, visInfo->visual, AllocNone);
43@@ -200,12 +225,12 @@
44 }
45
46 mgx::Display::Display(::Display* x_dpy,
47- geom::Size const size,
48+ geom::Size const requested_size,
49 std::shared_ptr<GLConfig> const& gl_config,
50 std::shared_ptr<DisplayReport> const& report)
51 : shared_egl{*gl_config},
52 x_dpy{x_dpy},
53- size{size},
54+ actual_size{clip_to_display(x_dpy, requested_size)},
55 gl_config{gl_config},
56 pixel_width{get_pixel_width(x_dpy)},
57 pixel_height{get_pixel_height(x_dpy)},
58@@ -216,7 +241,7 @@
59
60 win = std::make_unique<X11Window>(x_dpy,
61 shared_egl.display(),
62- size,
63+ actual_size,
64 shared_egl.config());
65
66 auto red_mask = win->red_mask();
67@@ -226,7 +251,7 @@
68 display_buffer = std::make_unique<mgx::DisplayBuffer>(
69 x_dpy,
70 *win,
71- size,
72+ actual_size,
73 shared_egl.context(),
74 report,
75 orientation,
76@@ -249,7 +274,7 @@
77 std::unique_ptr<mg::DisplayConfiguration> mgx::Display::configuration() const
78 {
79 return std::make_unique<mgx::DisplayConfiguration>(
80- pf, size, geom::Size{size.width * pixel_width, size.height * pixel_height}, orientation);
81+ pf, actual_size, geom::Size{actual_size.width * pixel_width, actual_size.height * pixel_height}, orientation);
82 }
83
84 void mgx::Display::configure(mg::DisplayConfiguration const& new_configuration)
85
86=== modified file 'src/platforms/mesa/server/x11/graphics/display.h'
87--- src/platforms/mesa/server/x11/graphics/display.h 2016-09-28 04:31:08 +0000
88+++ src/platforms/mesa/server/x11/graphics/display.h 2016-09-28 04:31:09 +0000
89@@ -66,7 +66,7 @@
90 {
91 public:
92 explicit Display(::Display* x_dpy,
93- geometry::Size const size,
94+ geometry::Size const requested_size,
95 std::shared_ptr<GLConfig> const& gl_config,
96 std::shared_ptr<DisplayReport> const& report);
97 ~Display() noexcept;
98@@ -101,7 +101,7 @@
99 private:
100 helpers::EGLHelper shared_egl;
101 ::Display* const x_dpy;
102- mir::geometry::Size const size;
103+ mir::geometry::Size const actual_size;
104 std::shared_ptr<GLConfig> const gl_config;
105 float pixel_width;
106 float pixel_height;
107
108=== modified file 'tests/include/mir/test/doubles/mock_x11.h'
109--- tests/include/mir/test/doubles/mock_x11.h 2016-06-28 08:24:02 +0000
110+++ tests/include/mir/test/doubles/mock_x11.h 2016-09-28 04:31:09 +0000
111@@ -88,7 +88,8 @@
112 MOCK_METHOD9(XGrabPointer, int(Display*, Window, Bool, unsigned int, int, int, Window, Cursor, Time));
113 MOCK_METHOD2(XUngrabPointer, int(Display*, Time));
114 MOCK_METHOD3(XInternAtom, Atom(Display*, const char*, Bool));
115- MOCK_METHOD4(XSetWMProtocols, Status(Display *, Window, Atom*, int));
116+ MOCK_METHOD4(XSetWMProtocols, Status(Display*, Window, Atom*, int));
117+ MOCK_METHOD9(XGetGeometry, Status(Display*, Drawable, Window*, int*, int*, unsigned int*, unsigned int*, unsigned int*, unsigned int*));
118
119 FakeX11Resources fake_x11;
120 };
121
122=== modified file 'tests/mir_test_doubles/mock_x11.cpp'
123--- tests/mir_test_doubles/mock_x11.cpp 2016-06-28 08:24:02 +0000
124+++ tests/mir_test_doubles/mock_x11.cpp 2016-09-28 04:31:09 +0000
125@@ -57,8 +57,8 @@
126 motion_event_return.type = MotionNotify;
127 enter_notify_event_return.type = EnterNotify;
128 leave_notify_event_return.type = LeaveNotify;
129- screen.width = 1024;
130- screen.height = 768;
131+ screen.width = 2880;
132+ screen.height = 1800;
133 screen.mwidth = 338;
134 screen.mwidth = 270;
135 }
136@@ -91,6 +91,11 @@
137 {
138 return fake_x11.pending_events;
139 }));
140+
141+ ON_CALL(*this, XGetGeometry(fake_x11.display,_,_,_,_,_,_,_,_))
142+ .WillByDefault(DoAll(SetArgPointee<5>(fake_x11.screen.width),
143+ SetArgPointee<6>(fake_x11.screen.height),
144+ Return(1)));
145 }
146
147 mtd::MockX11::~MockX11()
148@@ -237,3 +242,15 @@
149 {
150 return global_mock->XDefaultScreenOfDisplay(display);
151 }
152+
153+Status XGetGeometry(Display* display, Drawable d,
154+ Window* root_return,
155+ int* x_return, int* y_return,
156+ unsigned int* width_return, unsigned int* height_return,
157+ unsigned int* border_width_return, unsigned int* depth_return)
158+{
159+ return global_mock->XGetGeometry(display, d, root_return,
160+ x_return, y_return,
161+ width_return, height_return,
162+ border_width_return, depth_return);
163+}
164
165=== modified file 'tests/unit-tests/platforms/mesa/x11/test_display.cpp'
166--- tests/unit-tests/platforms/mesa/x11/test_display.cpp 2016-09-28 04:31:08 +0000
167+++ tests/unit-tests/platforms/mesa/x11/test_display.cpp 2016-09-28 04:31:09 +0000
168@@ -93,6 +93,11 @@
169 mock_x11.fake_x11.screen.mwidth = mm.width.as_int();
170 mock_x11.fake_x11.screen.mheight = mm.height.as_int();
171 size = window;
172+
173+ ON_CALL(mock_x11, XGetGeometry(mock_x11.fake_x11.display,_,_,_,_,_,_,_,_))
174+ .WillByDefault(DoAll(SetArgPointee<5>(mock_x11.fake_x11.screen.width),
175+ SetArgPointee<6>(mock_x11.fake_x11.screen.height),
176+ Return(1)));
177 }
178 std::shared_ptr<mgx::Display> create_display()
179 {
180@@ -151,7 +156,7 @@
181
182 TEST_F(X11DisplayTest, calculates_physical_size_of_display_based_on_default_screen)
183 {
184- auto const pixel = geom::Size{2560, 1080};
185+ auto const pixel = geom::Size{2880, 1800};
186 auto const mm = geom::Size{677, 290};
187 auto const window = geom::Size{1280, 1024};
188 auto const pixel_width = float(mm.width.as_int()) / float(pixel.width.as_int());
189@@ -175,7 +180,7 @@
190
191 TEST_F(X11DisplayTest, reports_a_resolution_that_matches_the_window_size)
192 {
193- auto const pixel = geom::Size{2560, 1080};
194+ auto const pixel = geom::Size{2880, 1800};
195 auto const mm = geom::Size{677, 290};
196 auto const window = geom::Size{1280, 1024};
197
198@@ -193,3 +198,24 @@
199
200 EXPECT_THAT(reported_resolution, Eq(window));
201 }
202+
203+TEST_F(X11DisplayTest, adjusts_resolution_with_respect_to_screen_size)
204+{
205+ auto const pixel = geom::Size{1000, 1000};
206+ auto const mm = geom::Size{677, 290};
207+ auto const window = geom::Size{1280, 1024};
208+ auto const border = 150; //must match the border value in clip_to_display()
209+
210+ setup_x11_screen(pixel, mm, window);
211+
212+ auto display = create_display();
213+ auto config = display->configuration();
214+ geom::Size reported_resolution;
215+ config->for_each_output(
216+ [&reported_resolution](mg::DisplayConfigurationOutput const& output)
217+ {
218+ reported_resolution = output.modes[0].size;
219+ });
220+
221+ EXPECT_THAT(reported_resolution, Eq(geom::Size{pixel.width.as_uint32_t()-border, pixel.height.as_uint32_t()-border}));
222+}

Subscribers

People subscribed via source and target branches