Mir

Merge lp:~vanvugt/mir/fix-1610054 into lp:mir

Proposed by Daniel van Vugt on 2016-08-05
Status: Merged
Approved by: Daniel van Vugt on 2016-08-10
Approved revision: 3636
Merged at revision: 3638
Proposed branch: lp:~vanvugt/mir/fix-1610054
Merge into: lp:mir
Diff against target: 208 lines (+74/-7)
6 files modified
src/platforms/mesa/server/kms/cursor.cpp (+9/-0)
src/platforms/mesa/server/kms/cursor.h (+1/-0)
src/platforms/mesa/server/kms/display.cpp (+13/-3)
src/platforms/mesa/server/kms/real_kms_output.cpp (+2/-2)
tests/unit-tests/graphics/mesa/kms/test_cursor.cpp (+25/-2)
tests/unit-tests/graphics/mesa/kms/test_real_kms_output.cpp (+24/-0)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-1610054
Reviewer Review Type Date Requested Status
Cemil Azizoglu (community) 2016-08-05 Approve on 2016-08-08
Mir CI Bot continuous-integration Approve on 2016-08-08
Review via email: mp+302121@code.launchpad.net

Commit message

On those devices that have KMS but no hardware cursor support,
fall back automatically to the software cursor (LP: #1610054)

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3634
https://mir-jenkins.ubuntu.com/job/mir-ci/1403/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/1710/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1763
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1754
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1754
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1754
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1731
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1731/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/1731
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1731/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/1731
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/1731/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1731/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1731/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/1731
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1731/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/1731
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1731/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
Daniel van Vugt (vanvugt) wrote :

Unrelated CI failure - 08:18:06 ERROR: Connection was broken: java.io.EOFException

lp:~vanvugt/mir/fix-1610054 updated on 2016-08-08
3635. By Daniel van Vugt on 2016-08-08

Merge latest trunk

3636. By Daniel van Vugt on 2016-08-08

More correct unit test expectation change

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

PASSED: Continuous integration, rev:3636
https://mir-jenkins.ubuntu.com/job/mir-ci/1404/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1712
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1765
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1756
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1756
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1756
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1733
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1733/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/1733
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/1733/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/1733
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/1733/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1733
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1733/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/1733
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1733/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/1733
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/1733/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Cemil Azizoglu (cemil-azizoglu) wrote :

Looks good

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/kms/cursor.cpp'
2--- src/platforms/mesa/server/kms/cursor.cpp 2016-01-29 08:18:22 +0000
3+++ src/platforms/mesa/server/kms/cursor.cpp 2016-08-08 03:36:45 +0000
4@@ -112,12 +112,15 @@
5 output_container(output_container),
6 current_position(),
7 visible(true),
8+ last_set_failed(false),
9 buffer(gbm),
10 buffer_width(gbm_bo_get_width(buffer)),
11 buffer_height(gbm_bo_get_height(buffer)),
12 current_configuration(current_configuration)
13 {
14 show(*initial_image);
15+ if (last_set_failed)
16+ throw std::runtime_error("Initial KMS cursor set failed");
17 }
18
19 mgm::Cursor::~Cursor() noexcept
20@@ -270,6 +273,8 @@
21 if (!visible)
22 return;
23
24+ bool set_on_all_outputs = true;
25+
26 for_each_used_output([&](KMSOutput& output, geom::Rectangle const& output_rect, MirOrientation orientation)
27 {
28 if (output_rect.contains(position))
29@@ -284,6 +289,8 @@
30 if (force_state || !output.has_cursor()) // TODO - or if orientation had changed - then set buffer..
31 {
32 output.set_cursor(buffer);
33+ if (!output.has_cursor())
34+ set_on_all_outputs = false;
35 }
36 }
37 else
38@@ -294,4 +301,6 @@
39 }
40 }
41 });
42+
43+ last_set_failed = !set_on_all_outputs;
44 }
45
46=== modified file 'src/platforms/mesa/server/kms/cursor.h'
47--- src/platforms/mesa/server/kms/cursor.h 2016-01-29 08:18:22 +0000
48+++ src/platforms/mesa/server/kms/cursor.h 2016-08-08 03:36:45 +0000
49@@ -97,6 +97,7 @@
50 geometry::Displacement hotspot;
51
52 bool visible;
53+ bool last_set_failed;
54
55 struct GBMBOWrapper
56 {
57
58=== modified file 'src/platforms/mesa/server/kms/display.cpp'
59--- src/platforms/mesa/server/kms/display.cpp 2016-08-01 09:37:45 +0000
60+++ src/platforms/mesa/server/kms/display.cpp 2016-08-08 03:36:45 +0000
61@@ -360,9 +360,19 @@
62 Display& display;
63 };
64
65- cursor = locked_cursor = std::make_shared<Cursor>(gbm->device, output_container,
66- std::make_shared<KMSCurrentConfiguration>(*this),
67- initial_image);
68+ try
69+ {
70+ locked_cursor = std::make_shared<Cursor>(gbm->device,
71+ output_container,
72+ std::make_shared<KMSCurrentConfiguration>(*this),
73+ initial_image);
74+ }
75+ catch (std::runtime_error const&)
76+ {
77+ // That's OK, we don't need a hardware cursor. Returning null
78+ // is allowed and will trigger a fallback to software.
79+ }
80+ cursor = locked_cursor;
81 }
82
83 return locked_cursor;
84
85=== modified file 'src/platforms/mesa/server/kms/real_kms_output.cpp'
86--- src/platforms/mesa/server/kms/real_kms_output.cpp 2016-05-26 02:12:27 +0000
87+++ src/platforms/mesa/server/kms/real_kms_output.cpp 2016-08-08 03:36:45 +0000
88@@ -187,6 +187,7 @@
89 {
90 if (current_crtc)
91 {
92+ has_cursor_ = true;
93 if (auto result = drmModeSetCursor(
94 drm_fd,
95 current_crtc->crtc_id,
96@@ -194,11 +195,10 @@
97 gbm_bo_get_width(buffer),
98 gbm_bo_get_height(buffer)))
99 {
100+ has_cursor_ = false;
101 mir::log_warning("set_cursor: drmModeSetCursor failed (%s)",
102 strerror(-result));
103 }
104-
105- has_cursor_ = true;
106 }
107 }
108
109
110=== modified file 'tests/unit-tests/graphics/mesa/kms/test_cursor.cpp'
111--- tests/unit-tests/graphics/mesa/kms/test_cursor.cpp 2016-01-29 08:18:22 +0000
112+++ tests/unit-tests/graphics/mesa/kms/test_cursor.cpp 2016-08-08 03:36:45 +0000
113@@ -60,6 +60,13 @@
114 {11, std::make_shared<testing::NiceMock<MockKMSOutput>>()},
115 {12, std::make_shared<testing::NiceMock<MockKMSOutput>>()}}
116 {
117+ // These need to be established before Cursor construction:
118+ for (auto& entry : outputs)
119+ {
120+ auto& out = *entry.second;
121+ ON_CALL(out, has_cursor())
122+ .WillByDefault(Return(true));
123+ }
124 }
125
126 std::shared_ptr<mgm::KMSOutput> get_kms_output_for(uint32_t connector_id)
127@@ -458,7 +465,7 @@
128 EXPECT_CALL(*output_container.outputs[12], clear_cursor());
129
130 /* No checking of existing cursor state */
131- EXPECT_CALL(*output_container.outputs[10], has_cursor()).Times(0);
132+ EXPECT_CALL(*output_container.outputs[10], has_cursor()).Times(1);
133 EXPECT_CALL(*output_container.outputs[11], has_cursor()).Times(0);
134 EXPECT_CALL(*output_container.outputs[12], has_cursor()).Times(0);
135
136@@ -469,13 +476,27 @@
137 output_container.verify_and_clear_expectations();
138 }
139
140+TEST_F(MesaCursorTest, construction_fails_if_initial_set_fails)
141+{
142+ using namespace testing;
143+
144+ EXPECT_CALL(*output_container.outputs[10], has_cursor())
145+ .WillOnce(Return(false));
146+
147+ EXPECT_THROW(
148+ mgm::Cursor cursor_tmp(mock_gbm.fake_gbm.device, output_container,
149+ std::make_shared<StubCurrentConfiguration>(),
150+ std::make_shared<StubCursorImage>())
151+ , std::runtime_error);
152+}
153
154 TEST_F(MesaCursorTest, move_to_sets_clears_cursor_if_needed)
155 {
156 using namespace testing;
157
158 EXPECT_CALL(*output_container.outputs[10], has_cursor())
159- .WillOnce(Return(false));
160+ .WillOnce(Return(false))
161+ .WillOnce(Return(true));
162 EXPECT_CALL(*output_container.outputs[10], set_cursor(_));
163
164 EXPECT_CALL(*output_container.outputs[11], has_cursor())
165@@ -637,6 +658,8 @@
166 EXPECT_CALL(*output_container.outputs[11], move_cursor(geom::Point{50,25}));
167 EXPECT_CALL(*output_container.outputs[10], clear_cursor());
168 EXPECT_CALL(*output_container.outputs[11], clear_cursor());
169+ EXPECT_CALL(*output_container.outputs[12], has_cursor())
170+ .WillRepeatedly(Return(false));
171 EXPECT_CALL(*output_container.outputs[12], clear_cursor());
172
173 cursor.move_to({150, 75});
174
175=== modified file 'tests/unit-tests/graphics/mesa/kms/test_real_kms_output.cpp'
176--- tests/unit-tests/graphics/mesa/kms/test_real_kms_output.cpp 2016-06-02 05:33:50 +0000
177+++ tests/unit-tests/graphics/mesa/kms/test_real_kms_output.cpp 2016-08-08 03:36:45 +0000
178@@ -324,6 +324,30 @@
179 });
180 }
181
182+TEST_F(RealKMSOutputTest, has_no_cursor_if_no_hardware_support)
183+{ // Regression test related to LP: #1610054
184+ using namespace testing;
185+
186+ EXPECT_CALL(mock_drm, drmModeSetCursor(_, _, _, _, _))
187+ .Times(1)
188+ .WillOnce(Return(-ENXIO));
189+ union gbm_bo_handle some_handle;
190+ some_handle.u64 = 0xbaadf00d;
191+ ON_CALL(mock_gbm, gbm_bo_get_handle(_)).WillByDefault(Return(some_handle));
192+ ON_CALL(mock_gbm, gbm_bo_get_width(_)).WillByDefault(Return(34));
193+ ON_CALL(mock_gbm, gbm_bo_get_height(_)).WillByDefault(Return(56));
194+
195+ setup_outputs_connected_crtc();
196+
197+ mgm::RealKMSOutput output{mock_drm.fake_drm.fd(), connector_ids[0],
198+ mt::fake_shared(mock_page_flipper)};
199+
200+ EXPECT_TRUE(output.set_crtc(987));
201+ struct gbm_bo *dummy = reinterpret_cast<struct gbm_bo*>(0x1234567);
202+ output.set_cursor(dummy);
203+ EXPECT_FALSE(output.has_cursor());
204+}
205+
206 TEST_F(RealKMSOutputTest, clear_crtc_throws_if_drm_call_fails)
207 {
208 mir::FatalErrorStrategy on_error{mir::fatal_error_except};

Subscribers

People subscribed via source and target branches