Mir

Merge lp:~raof/mir/cursoring into lp:mir

  1. cursoring
  2. Merge into development-branch
Proposed by Chris Halse Rogers
Status: Merged
Merged at revision: 1664
Proposed branch: lp:~raof/mir/cursoring
Merge into: lp:mir
Diff against target: 153 lines (+60/-25)
4 files modified
src/platform/graphics/mesa/cursor.cpp (+10/-2)
src/platform/graphics/mesa/cursor.h (+2/-1)
src/platform/graphics/mesa/display.cpp (+3/-3)
tests/unit-tests/graphics/mesa/test_cursor.cpp (+45/-19)
To merge this branch: bzr merge lp:~raof/mir/cursoring
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Alexandros Frantzis (community) Approve
Alan Griffiths Approve
Daniel van Vugt Approve
Brandon Schaefer (community) Approve
Review via email: mp+220951@code.launchpad.net

Commit message

Add suspend()/resume() to Mesa::Cursor, replacing .show_at_last_position().

This is conceptually what show_at_last_position() was trying to do, and
with an explicit suspend/resume interface we can save whether or not the
cursor was hidden across VT switches.

Fixes: LP: #1323225

Description of the change

Make the cursor retain its shown/hidden state over VT switches

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1656
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1728/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/324
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/325
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/324
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/247
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/247/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/246
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/246/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/907
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/907/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1568
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7729

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1728/rebuild

review: Approve (continuous-integration)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Confirmed fixes the issue.

review: Approve
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

108 +TEST_F(MesaCursorTest, cursor_is_shown_at_correct_locaiton_after_suspend_resume)

typo 'location'

18 +void mir::graphics::mesa::Cursor::suspend()
19 +{
20 + output_container.for_each_output(
21 + [&](KMSOutput& output) { output.clear_cursor(); });
22 +}

Lock needed (although with the current code I don't think we can hit a race here, but of course the situation can easily change).

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Works for me.

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

LGTM

review: Approve
Revision history for this message
Alexandros Frantzis (afrantzis) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1658
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1740/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/341
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/342
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/339/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/259
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/259/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/258
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/258/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/948/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1740/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/701/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/352
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/353/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/350
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/85
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/85/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/85
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/85/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/981
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/981/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1585
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7792

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/709/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/363
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/364/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/361/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/93
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-autolanding/93/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/93
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-autolanding/93/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1013
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1013/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1594/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7828

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/platform/graphics/mesa/cursor.cpp'
--- src/platform/graphics/mesa/cursor.cpp 2014-05-22 11:31:21 +0000
+++ src/platform/graphics/mesa/cursor.cpp 2014-05-27 08:08:21 +0000
@@ -83,7 +83,7 @@
83{83{
84 show(*initial_image);84 show(*initial_image);
8585
86 show_at_last_known_position();86 resume();
87}87}
8888
89mgm::Cursor::~Cursor() noexcept89mgm::Cursor::~Cursor() noexcept
@@ -116,7 +116,15 @@
116 place_cursor_at(position, UpdateState);116 place_cursor_at(position, UpdateState);
117}117}
118118
119void mgm::Cursor::show_at_last_known_position()119void mir::graphics::mesa::Cursor::suspend()
120{
121 std::lock_guard<std::mutex> lg(guard);
122
123 output_container.for_each_output(
124 [&](KMSOutput& output) { output.clear_cursor(); });
125}
126
127void mgm::Cursor::resume()
120{128{
121 place_cursor_at(current_position, ForceState);129 place_cursor_at(current_position, ForceState);
122}130}
123131
=== modified file 'src/platform/graphics/mesa/cursor.h'
--- src/platform/graphics/mesa/cursor.h 2014-05-22 11:31:21 +0000
+++ src/platform/graphics/mesa/cursor.h 2014-05-27 08:08:21 +0000
@@ -75,7 +75,8 @@
7575
76 void move_to(geometry::Point position);76 void move_to(geometry::Point position);
7777
78 void show_at_last_known_position();78 void suspend();
79 void resume();
7980
80private:81private:
81 enum ForceCursorState { UpdateState, ForceState };82 enum ForceCursorState { UpdateState, ForceState };
8283
=== modified file 'src/platform/graphics/mesa/display.cpp'
--- src/platform/graphics/mesa/display.cpp 2014-05-07 02:56:33 +0000
+++ src/platform/graphics/mesa/display.cpp 2014-05-27 08:08:21 +0000
@@ -224,7 +224,7 @@
224 clear_connected_unused_outputs();224 clear_connected_unused_outputs();
225 }225 }
226226
227 if (auto c = cursor.lock()) c->show_at_last_known_position();227 if (auto c = cursor.lock()) c->resume();
228}228}
229229
230void mgm::Display::register_configuration_change_handler(230void mgm::Display::register_configuration_change_handler(
@@ -255,7 +255,7 @@
255{255{
256 try256 try
257 {257 {
258 if (auto c = cursor.lock()) c->hide();258 if (auto c = cursor.lock()) c->suspend();
259 platform->drm.drop_master();259 platform->drm.drop_master();
260 }260 }
261 catch(std::runtime_error const& e)261 catch(std::runtime_error const& e)
@@ -291,7 +291,7 @@
291 clear_connected_unused_outputs();291 clear_connected_unused_outputs();
292 }292 }
293293
294 if (auto c = cursor.lock()) c->show_at_last_known_position();294 if (auto c = cursor.lock()) c->resume();
295}295}
296296
297auto mgm::Display::create_hardware_cursor(std::shared_ptr<mg::CursorImage> const& initial_image) -> std::shared_ptr<graphics::Cursor>297auto mgm::Display::create_hardware_cursor(std::shared_ptr<mg::CursorImage> const& initial_image) -> std::shared_ptr<graphics::Cursor>
298298
=== modified file 'tests/unit-tests/graphics/mesa/test_cursor.cpp'
--- tests/unit-tests/graphics/mesa/test_cursor.cpp 2014-05-22 11:31:21 +0000
+++ tests/unit-tests/graphics/mesa/test_cursor.cpp 2014-05-27 08:08:21 +0000
@@ -374,25 +374,6 @@
374 output_container.verify_and_clear_expectations();374 output_container.verify_and_clear_expectations();
375}375}
376376
377TEST_F(MesaCursorTest, shows_at_last_known_position)
378{
379 using namespace testing;
380
381 EXPECT_CALL(*output_container.outputs[10], move_cursor(geom::Point{150,75}));
382 EXPECT_CALL(*output_container.outputs[11], move_cursor(geom::Point{50,25}));
383
384 cursor.move_to({150, 75});
385
386 output_container.verify_and_clear_expectations();
387
388 EXPECT_CALL(*output_container.outputs[10], move_cursor(geom::Point{150,75}));
389 EXPECT_CALL(*output_container.outputs[11], move_cursor(geom::Point{50,25}));
390
391 cursor.show_at_last_known_position();
392
393 output_container.verify_and_clear_expectations();
394}
395
396TEST_F(MesaCursorTest, moves_properly_to_and_inside_left_rotated_output)377TEST_F(MesaCursorTest, moves_properly_to_and_inside_left_rotated_output)
397{378{
398 using namespace testing;379 using namespace testing;
@@ -478,3 +459,48 @@
478 EXPECT_CALL(*output_container.outputs[11], clear_cursor());459 EXPECT_CALL(*output_container.outputs[11], clear_cursor());
479 EXPECT_CALL(*output_container.outputs[12], clear_cursor());460 EXPECT_CALL(*output_container.outputs[12], clear_cursor());
480}461}
462
463TEST_F(MesaCursorTest, cursor_is_shown_at_correct_location_after_suspend_resume)
464{
465 using namespace testing;
466
467 EXPECT_CALL(*output_container.outputs[10], move_cursor(geom::Point{150,75}));
468 EXPECT_CALL(*output_container.outputs[11], move_cursor(geom::Point{50,25}));
469 EXPECT_CALL(*output_container.outputs[10], clear_cursor());
470 EXPECT_CALL(*output_container.outputs[11], clear_cursor());
471 EXPECT_CALL(*output_container.outputs[12], clear_cursor());
472
473 cursor.move_to({150, 75});
474 cursor.suspend();
475
476 output_container.verify_and_clear_expectations();
477
478 EXPECT_CALL(*output_container.outputs[10], set_cursor(_));
479 EXPECT_CALL(*output_container.outputs[11], set_cursor(_));
480 EXPECT_CALL(*output_container.outputs[10], move_cursor(geom::Point{150,75}));
481 EXPECT_CALL(*output_container.outputs[11], move_cursor(geom::Point{50,25}));
482
483 cursor.resume();
484 output_container.verify_and_clear_expectations();
485}
486
487TEST_F(MesaCursorTest, hidden_cursor_is_not_shown_after_suspend_resume)
488{
489 using namespace testing;
490
491 EXPECT_CALL(*output_container.outputs[10], clear_cursor()).Times(AnyNumber());
492 EXPECT_CALL(*output_container.outputs[11], clear_cursor()).Times(AnyNumber());
493 EXPECT_CALL(*output_container.outputs[12], clear_cursor()).Times(AnyNumber());
494
495 cursor.hide();
496 cursor.suspend();
497
498 output_container.verify_and_clear_expectations();
499
500 EXPECT_CALL(*output_container.outputs[10], set_cursor(_)).Times(0);
501 EXPECT_CALL(*output_container.outputs[11], set_cursor(_)).Times(0);
502 EXPECT_CALL(*output_container.outputs[12], set_cursor(_)).Times(0);
503
504 cursor.resume();
505 output_container.verify_and_clear_expectations();
506}

Subscribers

People subscribed via source and target branches