Mir

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

  1. fix-1610054
  2. Merge into development-branch
Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
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) Approve
Mir CI Bot continuous-integration Approve
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.
Revision history for this message
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)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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

Revision history for this message
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)
Revision history for this message
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
=== modified file 'src/platforms/mesa/server/kms/cursor.cpp'
--- src/platforms/mesa/server/kms/cursor.cpp 2016-01-29 08:18:22 +0000
+++ src/platforms/mesa/server/kms/cursor.cpp 2016-08-08 03:36:45 +0000
@@ -112,12 +112,15 @@
112 output_container(output_container),112 output_container(output_container),
113 current_position(),113 current_position(),
114 visible(true),114 visible(true),
115 last_set_failed(false),
115 buffer(gbm),116 buffer(gbm),
116 buffer_width(gbm_bo_get_width(buffer)),117 buffer_width(gbm_bo_get_width(buffer)),
117 buffer_height(gbm_bo_get_height(buffer)),118 buffer_height(gbm_bo_get_height(buffer)),
118 current_configuration(current_configuration)119 current_configuration(current_configuration)
119{120{
120 show(*initial_image);121 show(*initial_image);
122 if (last_set_failed)
123 throw std::runtime_error("Initial KMS cursor set failed");
121}124}
122125
123mgm::Cursor::~Cursor() noexcept126mgm::Cursor::~Cursor() noexcept
@@ -270,6 +273,8 @@
270 if (!visible)273 if (!visible)
271 return;274 return;
272275
276 bool set_on_all_outputs = true;
277
273 for_each_used_output([&](KMSOutput& output, geom::Rectangle const& output_rect, MirOrientation orientation)278 for_each_used_output([&](KMSOutput& output, geom::Rectangle const& output_rect, MirOrientation orientation)
274 {279 {
275 if (output_rect.contains(position))280 if (output_rect.contains(position))
@@ -284,6 +289,8 @@
284 if (force_state || !output.has_cursor()) // TODO - or if orientation had changed - then set buffer..289 if (force_state || !output.has_cursor()) // TODO - or if orientation had changed - then set buffer..
285 {290 {
286 output.set_cursor(buffer);291 output.set_cursor(buffer);
292 if (!output.has_cursor())
293 set_on_all_outputs = false;
287 }294 }
288 }295 }
289 else296 else
@@ -294,4 +301,6 @@
294 }301 }
295 }302 }
296 });303 });
304
305 last_set_failed = !set_on_all_outputs;
297}306}
298307
=== modified file 'src/platforms/mesa/server/kms/cursor.h'
--- src/platforms/mesa/server/kms/cursor.h 2016-01-29 08:18:22 +0000
+++ src/platforms/mesa/server/kms/cursor.h 2016-08-08 03:36:45 +0000
@@ -97,6 +97,7 @@
97 geometry::Displacement hotspot;97 geometry::Displacement hotspot;
9898
99 bool visible;99 bool visible;
100 bool last_set_failed;
100101
101 struct GBMBOWrapper102 struct GBMBOWrapper
102 {103 {
103104
=== modified file 'src/platforms/mesa/server/kms/display.cpp'
--- src/platforms/mesa/server/kms/display.cpp 2016-08-01 09:37:45 +0000
+++ src/platforms/mesa/server/kms/display.cpp 2016-08-08 03:36:45 +0000
@@ -360,9 +360,19 @@
360 Display& display;360 Display& display;
361 };361 };
362362
363 cursor = locked_cursor = std::make_shared<Cursor>(gbm->device, output_container,363 try
364 std::make_shared<KMSCurrentConfiguration>(*this),364 {
365 initial_image);365 locked_cursor = std::make_shared<Cursor>(gbm->device,
366 output_container,
367 std::make_shared<KMSCurrentConfiguration>(*this),
368 initial_image);
369 }
370 catch (std::runtime_error const&)
371 {
372 // That's OK, we don't need a hardware cursor. Returning null
373 // is allowed and will trigger a fallback to software.
374 }
375 cursor = locked_cursor;
366 }376 }
367377
368 return locked_cursor;378 return locked_cursor;
369379
=== modified file 'src/platforms/mesa/server/kms/real_kms_output.cpp'
--- src/platforms/mesa/server/kms/real_kms_output.cpp 2016-05-26 02:12:27 +0000
+++ src/platforms/mesa/server/kms/real_kms_output.cpp 2016-08-08 03:36:45 +0000
@@ -187,6 +187,7 @@
187{187{
188 if (current_crtc)188 if (current_crtc)
189 {189 {
190 has_cursor_ = true;
190 if (auto result = drmModeSetCursor(191 if (auto result = drmModeSetCursor(
191 drm_fd,192 drm_fd,
192 current_crtc->crtc_id,193 current_crtc->crtc_id,
@@ -194,11 +195,10 @@
194 gbm_bo_get_width(buffer),195 gbm_bo_get_width(buffer),
195 gbm_bo_get_height(buffer)))196 gbm_bo_get_height(buffer)))
196 {197 {
198 has_cursor_ = false;
197 mir::log_warning("set_cursor: drmModeSetCursor failed (%s)",199 mir::log_warning("set_cursor: drmModeSetCursor failed (%s)",
198 strerror(-result));200 strerror(-result));
199 }201 }
200
201 has_cursor_ = true;
202 }202 }
203}203}
204204
205205
=== modified file 'tests/unit-tests/graphics/mesa/kms/test_cursor.cpp'
--- tests/unit-tests/graphics/mesa/kms/test_cursor.cpp 2016-01-29 08:18:22 +0000
+++ tests/unit-tests/graphics/mesa/kms/test_cursor.cpp 2016-08-08 03:36:45 +0000
@@ -60,6 +60,13 @@
60 {11, std::make_shared<testing::NiceMock<MockKMSOutput>>()},60 {11, std::make_shared<testing::NiceMock<MockKMSOutput>>()},
61 {12, std::make_shared<testing::NiceMock<MockKMSOutput>>()}}61 {12, std::make_shared<testing::NiceMock<MockKMSOutput>>()}}
62 {62 {
63 // These need to be established before Cursor construction:
64 for (auto& entry : outputs)
65 {
66 auto& out = *entry.second;
67 ON_CALL(out, has_cursor())
68 .WillByDefault(Return(true));
69 }
63 }70 }
6471
65 std::shared_ptr<mgm::KMSOutput> get_kms_output_for(uint32_t connector_id)72 std::shared_ptr<mgm::KMSOutput> get_kms_output_for(uint32_t connector_id)
@@ -458,7 +465,7 @@
458 EXPECT_CALL(*output_container.outputs[12], clear_cursor());465 EXPECT_CALL(*output_container.outputs[12], clear_cursor());
459466
460 /* No checking of existing cursor state */467 /* No checking of existing cursor state */
461 EXPECT_CALL(*output_container.outputs[10], has_cursor()).Times(0);468 EXPECT_CALL(*output_container.outputs[10], has_cursor()).Times(1);
462 EXPECT_CALL(*output_container.outputs[11], has_cursor()).Times(0);469 EXPECT_CALL(*output_container.outputs[11], has_cursor()).Times(0);
463 EXPECT_CALL(*output_container.outputs[12], has_cursor()).Times(0);470 EXPECT_CALL(*output_container.outputs[12], has_cursor()).Times(0);
464471
@@ -469,13 +476,27 @@
469 output_container.verify_and_clear_expectations();476 output_container.verify_and_clear_expectations();
470}477}
471478
479TEST_F(MesaCursorTest, construction_fails_if_initial_set_fails)
480{
481 using namespace testing;
482
483 EXPECT_CALL(*output_container.outputs[10], has_cursor())
484 .WillOnce(Return(false));
485
486 EXPECT_THROW(
487 mgm::Cursor cursor_tmp(mock_gbm.fake_gbm.device, output_container,
488 std::make_shared<StubCurrentConfiguration>(),
489 std::make_shared<StubCursorImage>())
490 , std::runtime_error);
491}
472492
473TEST_F(MesaCursorTest, move_to_sets_clears_cursor_if_needed)493TEST_F(MesaCursorTest, move_to_sets_clears_cursor_if_needed)
474{494{
475 using namespace testing;495 using namespace testing;
476496
477 EXPECT_CALL(*output_container.outputs[10], has_cursor())497 EXPECT_CALL(*output_container.outputs[10], has_cursor())
478 .WillOnce(Return(false));498 .WillOnce(Return(false))
499 .WillOnce(Return(true));
479 EXPECT_CALL(*output_container.outputs[10], set_cursor(_));500 EXPECT_CALL(*output_container.outputs[10], set_cursor(_));
480501
481 EXPECT_CALL(*output_container.outputs[11], has_cursor())502 EXPECT_CALL(*output_container.outputs[11], has_cursor())
@@ -637,6 +658,8 @@
637 EXPECT_CALL(*output_container.outputs[11], move_cursor(geom::Point{50,25}));658 EXPECT_CALL(*output_container.outputs[11], move_cursor(geom::Point{50,25}));
638 EXPECT_CALL(*output_container.outputs[10], clear_cursor());659 EXPECT_CALL(*output_container.outputs[10], clear_cursor());
639 EXPECT_CALL(*output_container.outputs[11], clear_cursor());660 EXPECT_CALL(*output_container.outputs[11], clear_cursor());
661 EXPECT_CALL(*output_container.outputs[12], has_cursor())
662 .WillRepeatedly(Return(false));
640 EXPECT_CALL(*output_container.outputs[12], clear_cursor());663 EXPECT_CALL(*output_container.outputs[12], clear_cursor());
641664
642 cursor.move_to({150, 75});665 cursor.move_to({150, 75});
643666
=== modified file 'tests/unit-tests/graphics/mesa/kms/test_real_kms_output.cpp'
--- tests/unit-tests/graphics/mesa/kms/test_real_kms_output.cpp 2016-06-02 05:33:50 +0000
+++ tests/unit-tests/graphics/mesa/kms/test_real_kms_output.cpp 2016-08-08 03:36:45 +0000
@@ -324,6 +324,30 @@
324 });324 });
325}325}
326326
327TEST_F(RealKMSOutputTest, has_no_cursor_if_no_hardware_support)
328{ // Regression test related to LP: #1610054
329 using namespace testing;
330
331 EXPECT_CALL(mock_drm, drmModeSetCursor(_, _, _, _, _))
332 .Times(1)
333 .WillOnce(Return(-ENXIO));
334 union gbm_bo_handle some_handle;
335 some_handle.u64 = 0xbaadf00d;
336 ON_CALL(mock_gbm, gbm_bo_get_handle(_)).WillByDefault(Return(some_handle));
337 ON_CALL(mock_gbm, gbm_bo_get_width(_)).WillByDefault(Return(34));
338 ON_CALL(mock_gbm, gbm_bo_get_height(_)).WillByDefault(Return(56));
339
340 setup_outputs_connected_crtc();
341
342 mgm::RealKMSOutput output{mock_drm.fake_drm.fd(), connector_ids[0],
343 mt::fake_shared(mock_page_flipper)};
344
345 EXPECT_TRUE(output.set_crtc(987));
346 struct gbm_bo *dummy = reinterpret_cast<struct gbm_bo*>(0x1234567);
347 output.set_cursor(dummy);
348 EXPECT_FALSE(output.has_cursor());
349}
350
327TEST_F(RealKMSOutputTest, clear_crtc_throws_if_drm_call_fails)351TEST_F(RealKMSOutputTest, clear_crtc_throws_if_drm_call_fails)
328{352{
329 mir::FatalErrorStrategy on_error{mir::fatal_error_except};353 mir::FatalErrorStrategy on_error{mir::fatal_error_except};

Subscribers

People subscribed via source and target branches