Unity System Compositor

Merge lp:~josharenson/unity-system-compositor/handle_screenshot_button into lp:unity-system-compositor

  1. handle_screenshot_button
  2. Merge into trunk
Proposed by Josh Arenson
Status: Rejected
Rejected by: Michał Sawicz
Proposed branch: lp:~josharenson/unity-system-compositor/handle_screenshot_button
Merge into: lp:unity-system-compositor
Diff against target: 114 lines (+47/-7)
2 files modified
src/powerkey_handler.cpp (+42/-7)
src/powerkey_handler.h (+5/-0)
To merge this branch: bzr merge lp:~josharenson/unity-system-compositor/handle_screenshot_button
Reviewer Review Type Date Requested Status
Michał Sawicz Disapprove
kevin gunn (community) testing Approve
PS Jenkins bot (community) continuous-integration Approve
Kevin DuBois (community) Approve
Cemil Azizoglu (community) Approve
Review via email: mp+247085@code.launchpad.net

Commit message

Keep the screen on during screenshots

Make USC aware of the key combination necessary to take a screenshot so
that pressing Power + SCREENSHOT key doesn't turn off the screen.

Description of the change

Make USC aware of the screenshot key combo and prevent it from blanking the screen during screenshots. https://code.launchpad.net/~josharenson/unity8/physical_keys_filter is dependent on this branch.

To post a comment you must log in.
lp:~josharenson/unity-system-compositor/handle_screenshot_button updated
200. By Josh Arenson

Restore control file

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:200
http://jenkins.qa.ubuntu.com/job/unity-system-compositor-ci/175/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-vivid-amd64-ci/12
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-vivid-armhf-ci/12

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-system-compositor-ci/175/rebuild

review: Approve (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

LGTM

review: Approve
Revision history for this message
Kevin DuBois (kdub) wrote :

lgtm too

review: Approve
Revision history for this message
kevin gunn (kgunn72) wrote :

unfortunately, this in combination with
https://code.launchpad.net/~josharenson/unity8/physical_keys_filter/+merge/244244
didn't test out too well.
see (videos: most comon restul https://www.youtube.com/watch?v=WpFQHptyGe4 , then i got a couple of weird ones where the lock screen didn't engage right https://www.youtube.com/watch?v=CjkZXn-Io_8 )

review: Needs Fixing
lp:~josharenson/unity-system-compositor/handle_screenshot_button updated
201. By Josh Arenson

Make screenshot_button_pressed atomic

202. By Josh Arenson

Allow the power button to be released in any order post-screenshot

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:202
http://jenkins.qa.ubuntu.com/job/unity-system-compositor-ci/179/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-vivid-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-vivid-armhf-ci/16

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-system-compositor-ci/179/rebuild

review: Approve (continuous-integration)
lp:~josharenson/unity-system-compositor/handle_screenshot_button updated
203. By Josh Arenson

Re-enable inactivity timer when power key is released

204. By Josh Arenson

[ Ricardo Mendoza ]
Make sure the compositor is running before starting the screen state
handler.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:204
http://jenkins.qa.ubuntu.com/job/unity-system-compositor-ci/181/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-vivid-amd64-ci/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-vivid-armhf-ci/18

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-system-compositor-ci/181/rebuild

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

Still looks okay to me, but I couldn't reason through if this affects the linked bug, because I'm not familiar with how the volume notifications work.

Revision history for this message
kevin gunn (kgunn72) wrote :

looks good now

review: Approve (testing)
lp:~josharenson/unity-system-compositor/handle_screenshot_button updated
205. By Josh Arenson

[ Andreas Pokorny ]
* Port USC to event 2.0 API
[ Robert Carr ]
* Port USC to event 2.0 API
[ Alan Griffiths ]
* Port to the msh::Shell API in Mir

206. By Josh Arenson

Restore control file

Revision history for this message
Michał Sawicz (saviq) wrote :

Sorry for the noise. We managed to keep the screenshot shortcut to be volup+voldown, which means we need not have involved u-s-c.

review: Disapprove

Unmerged revisions

206. By Josh Arenson

Restore control file

205. By Josh Arenson

[ Andreas Pokorny ]
* Port USC to event 2.0 API
[ Robert Carr ]
* Port USC to event 2.0 API
[ Alan Griffiths ]
* Port to the msh::Shell API in Mir

204. By Josh Arenson

[ Ricardo Mendoza ]
Make sure the compositor is running before starting the screen state
handler.

203. By Josh Arenson

Re-enable inactivity timer when power key is released

202. By Josh Arenson

Allow the power button to be released in any order post-screenshot

201. By Josh Arenson

Make screenshot_button_pressed atomic

200. By Josh Arenson

Restore control file

199. By Josh Arenson

Remove residual comments

198. By Josh Arenson

Fix typo

197. By Josh Arenson

Keep the screen on during screenshots

Make USC aware of the key combination necessary to take a screenshot so
that pressing Power + SCREENSHOT key doesn't turn off the screen.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/powerkey_handler.cpp'
2--- src/powerkey_handler.cpp 2014-12-11 22:15:07 +0000
3+++ src/powerkey_handler.cpp 2015-02-13 17:21:57 +0000
4@@ -31,6 +31,9 @@
5 std::chrono::milliseconds shutdown_timeout,
6 ScreenStateHandler& screen_state_handler)
7 : long_press_detected{false},
8+ power_key_pressed{false},
9+ power_key_been_released_since_screenshot{true},
10+ screenshot_button_pressed{false},
11 screen_state_handler{&screen_state_handler},
12 power_key_ignore_timeout{power_key_ignore_timeout},
13 shutdown_timeout{shutdown_timeout},
14@@ -44,21 +47,31 @@
15 bool PowerKeyHandler::handle(MirEvent const& event)
16 {
17 static const int32_t POWER_KEY_CODE = 26;
18-
19+ static const int32_t SCREENSHOT_BUTTON_KEY_CODE = 25; /* Volume Decrease */
20+
21 if (mir_event_get_type(&event) != mir_event_type_input)
22 return false;
23 auto input_event = mir_event_get_input_event(&event);
24 if (mir_input_event_get_type(input_event) != mir_input_event_type_key)
25 return false;
26 auto kev = mir_input_event_get_key_input_event(input_event);
27- if (mir_key_input_event_get_key_code(kev) != POWER_KEY_CODE)
28+ auto key_code = mir_key_input_event_get_key_code(kev);
29+ if (key_code != POWER_KEY_CODE && key_code != SCREENSHOT_BUTTON_KEY_CODE)
30 return false;
31
32 auto action = mir_key_input_event_get_action(kev);
33- if (action == mir_key_input_event_action_down)
34- power_key_down();
35- else if (action == mir_key_input_event_action_up)
36- power_key_up();
37+ if (action == mir_key_input_event_action_down) {
38+ if (key_code == POWER_KEY_CODE)
39+ power_key_down();
40+ else if (key_code == SCREENSHOT_BUTTON_KEY_CODE)
41+ screenshot_key_down();
42+ }
43+ else if (action == mir_key_input_event_action_up) {
44+ if (key_code == POWER_KEY_CODE)
45+ power_key_up();
46+ else if (key_code == SCREENSHOT_BUTTON_KEY_CODE)
47+ screenshot_key_up();
48+ }
49
50 return false;
51 }
52@@ -66,6 +79,10 @@
53 void PowerKeyHandler::power_key_down()
54 {
55 std::lock_guard<std::mutex> lock{guard};
56+ power_key_pressed = true;
57+ if (screenshot_button_pressed) {
58+ power_key_been_released_since_screenshot = false;
59+ }
60 screen_state_handler->enable_inactivity_timers(false);
61 long_press_detected = false;
62 long_press_alarm->reschedule_in(power_key_ignore_timeout);
63@@ -75,12 +92,30 @@
64 void PowerKeyHandler::power_key_up()
65 {
66 std::lock_guard<std::mutex> lock{guard};
67+ power_key_pressed = false;
68 shutdown_alarm->cancel();
69 long_press_alarm->cancel();
70- if (!long_press_detected)
71+ screen_state_handler->enable_inactivity_timers(true);
72+ if (!long_press_detected && !screenshot_button_pressed && power_key_been_released_since_screenshot)
73 {
74 screen_state_handler->toggle_screen_power_mode(PowerStateChangeReason::power_key);
75 }
76+ power_key_been_released_since_screenshot = true;
77+}
78+
79+void PowerKeyHandler::screenshot_key_down()
80+{
81+ std::lock_guard<std::mutex> lock{guard};
82+ if (power_key_pressed) {
83+ power_key_been_released_since_screenshot = false;
84+ }
85+ screenshot_button_pressed = true;
86+}
87+
88+void PowerKeyHandler::screenshot_key_up()
89+{
90+ std::lock_guard<std::mutex> lock{guard};
91+ screenshot_button_pressed = false;
92 }
93
94 void PowerKeyHandler::shutdown_alarm_notification()
95
96=== modified file 'src/powerkey_handler.h'
97--- src/powerkey_handler.h 2014-11-03 17:57:10 +0000
98+++ src/powerkey_handler.h 2015-02-13 17:21:57 +0000
99@@ -50,11 +50,16 @@
100 private:
101 void power_key_up();
102 void power_key_down();
103+ void screenshot_key_down();
104+ void screenshot_key_up();
105 void shutdown_alarm_notification();
106 void long_press_notification();
107
108 std::mutex guard;
109 std::atomic<bool> long_press_detected;
110+ std::atomic<bool> power_key_pressed;
111+ std::atomic<bool> power_key_been_released_since_screenshot;
112+ std::atomic<bool> screenshot_button_pressed;
113
114 ScreenStateHandler* screen_state_handler;
115

Subscribers

People subscribed via source and target branches