Merge lp:~didrocks/unity/fix-lens-shortcuts into lp:unity

Proposed by Didier Roche on 2011-09-07
Status: Merged
Approved by: Neil J. Patel on 2011-09-08
Approved revision: 1501
Merged at revision: 1520
Proposed branch: lp:~didrocks/unity/fix-lens-shortcuts
Merge into: lp:unity
Diff against target: 387 lines (+110/-33)
10 files modified
UnityCore/FilesystemLenses.cpp (+22/-6)
UnityCore/FilesystemLenses.h (+1/-0)
plugins/unityshell/src/DashController.cpp (+18/-0)
plugins/unityshell/src/DashController.h (+3/-0)
plugins/unityshell/src/DashView.cpp (+21/-0)
plugins/unityshell/src/DashView.h (+3/-0)
plugins/unityshell/src/Launcher.cpp (+15/-17)
plugins/unityshell/src/Launcher.h (+1/-1)
plugins/unityshell/src/unityshell.cpp (+23/-8)
plugins/unityshell/src/unityshell.h (+3/-1)
To merge this branch: bzr merge lp:~didrocks/unity/fix-lens-shortcuts
Reviewer Review Type Date Requested Status
Neil J. Patel (community) 2011-09-07 Approve on 2011-09-08
Review via email: mp+74468@code.launchpad.net

Description of the change

bring back lenses shortcut activation. Seems there is still an issue on
registering keybinding on first super invocation, but good enough for
main cases (LP: #834078).

No python code was written during this hacking period.

To post a comment you must log in.
Neil J. Patel (njpatel) wrote :

Awesome stuff! I found a couple of niggles, not sure if they are to do with your branch or not, so let me know:

- Pressing Super no longer shows the numbers on the icons :/
- If I press Super+f quickly, then the dash shows and hides, instead of just showing. I have to press super, wait around 100ms and then press f to make it show.

Any ideas?

review: Needs Fixing

The missing numbers and letters when holding down super is already in
Oneiric, so definitely not from this branch.

Tim Penhey (thumper) wrote :

On 08/09/11 04:10, Didier Roche wrote:

+Lens::Ptr FilesystemLenses::Impl::GetLensForShortcut(std::string const&
lens_shortcut) const
+{
+ Lens::Ptr p;
+
+ for (Lens::Ptr lens: lenses_)
+ {
+ if (lens->shortcut == lens_shortcut)
+ {
+ p = lens;
+ break;
+ }
+ }
+
+ return p;
+}

In a previous branch I found some weirdness with break nor exiting the
new range based for loop. What I found fixed it was to return. So
something like this:

Lens::Ptr FilesystemLenses::Impl::GetLensForShortcut(std::string const&
lens_shortcut) const
{
   for (auto lens : lenses_)
   {
     if (lens->shortcut == lens_shortcut)
       return lens
   }
   return Lens::Ptr();
}

You could use "auto" or "Lens::Ptr const&". Using just Lens::Ptr causes
an extra object to be constructed and destroyed for each time through
the loop.

Tim

1500. By Didier Roche on 2011-09-08

Fix uneeded object construction

1501. By Didier Roche on 2011-09-08

fix show/hide dash when shortcuts are pressed quickly

Didier Roche (didrocks) wrote :

I've made the change mentionned by Tim in UnityCore, I've also fixed another one in UnityCore which had a similar pattern.

The "quick typing shortcut" is now fixed as well. But that makes me thing that we should have a keybinding controller which has a reference to both models to trigger the actions in both the Launcher and Dash, being able to know about both part and so avoiding the "show/hide" in a short period of time like we had there.
I initially put everything in the Launcher as it's where we controlled all the Super keys, but it's not the case anymore with this branch, so we should think about it. I'll get to it, but maybe after this release.

Please tell me if it fixes this :)
There is still the issue in first launch (will need to check compiz registration process with Sam)
The numbers not showing and acting is an oneiric regression, indeed, not related to this branch. I'll try to have a look there as well later (some people will apparently break some libunity ABI, and that + patch piloting + zomg new Qt! needs some attention from now ;))

Neil J. Patel (njpatel) wrote :

Sweet, approved!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'UnityCore/FilesystemLenses.cpp'
2--- UnityCore/FilesystemLenses.cpp 2011-08-07 09:21:37 +0000
3+++ UnityCore/FilesystemLenses.cpp 2011-09-08 06:47:28 +0000
4@@ -114,6 +114,7 @@
5 LensList GetLenses() const;
6 Lens::Ptr GetLens(std::string const& lens_id) const;
7 Lens::Ptr GetLensAtIndex(std::size_t index) const;
8+ Lens::Ptr GetLensForShortcut(std::string const& lens_shortcut) const;
9 std::size_t count() const;
10
11 void Init();
12@@ -334,18 +335,15 @@
13
14 Lens::Ptr FilesystemLenses::Impl::GetLens(std::string const& lens_id) const
15 {
16- Lens::Ptr p;
17-
18- for (Lens::Ptr lens: lenses_)
19+ for (auto lens: lenses_)
20 {
21 if (lens->id == lens_id)
22 {
23- p = lens;
24- break;
25+ return lens;
26 }
27 }
28
29- return p;
30+ return Lens::Ptr();
31 }
32
33 Lens::Ptr FilesystemLenses::Impl::GetLensAtIndex(std::size_t index) const
34@@ -361,6 +359,19 @@
35 return Lens::Ptr();
36 }
37
38+Lens::Ptr FilesystemLenses::Impl::GetLensForShortcut(std::string const& lens_shortcut) const
39+{
40+ for (auto lens: lenses_)
41+ {
42+ if (lens->shortcut == lens_shortcut)
43+ {
44+ return lens;
45+ }
46+ }
47+
48+ return Lens::Ptr();
49+}
50+
51 std::size_t FilesystemLenses::Impl::count() const
52 {
53 return lenses_.size();
54@@ -404,5 +415,10 @@
55 return pimpl->GetLensAtIndex(index);
56 }
57
58+Lens::Ptr FilesystemLenses::GetLensForShortcut(std::string const& lens_shortcut) const
59+{
60+ return pimpl->GetLensForShortcut(lens_shortcut);
61+}
62+
63 }
64 }
65
66=== modified file 'UnityCore/FilesystemLenses.h'
67--- UnityCore/FilesystemLenses.h 2011-07-27 17:35:31 +0000
68+++ UnityCore/FilesystemLenses.h 2011-09-08 06:47:28 +0000
69@@ -48,6 +48,7 @@
70 LensList GetLenses() const;
71 Lens::Ptr GetLens(std::string const& lens_id) const;
72 Lens::Ptr GetLensAtIndex(std::size_t index) const;
73+ Lens::Ptr GetLensForShortcut(std::string const& lens_shortcut) const;
74
75 sigc::signal<void> lenses_loaded;
76
77
78=== modified file 'plugins/unityshell/src/DashController.cpp'
79--- plugins/unityshell/src/DashController.cpp 2011-09-08 04:49:34 +0000
80+++ plugins/unityshell/src/DashController.cpp 2011-09-08 06:47:28 +0000
81@@ -308,6 +308,24 @@
82 ShowDash();
83 }
84
85+gboolean DashController::CheckShortcutActivation(const char* key_string)
86+{
87+ EnsureDash();
88+ std::string lens_id = view_->GetIdForShortcutActivation(std::string(key_string));
89+ if (lens_id != "")
90+ {
91+ OnActivateRequest(g_variant_new("(sus)", lens_id.c_str(), 0, ""));
92+ return true;
93+ }
94+ return false;
95+}
96+
97+std::vector<char> DashController::GetAllShortcuts()
98+{
99+ EnsureDash();
100+ return view_->GetAllShortcuts();
101+}
102+
103 // Introspectable
104 const gchar* DashController::GetName()
105 {
106
107=== modified file 'plugins/unityshell/src/DashController.h'
108--- plugins/unityshell/src/DashController.h 2011-09-08 04:49:34 +0000
109+++ plugins/unityshell/src/DashController.h 2011-09-08 06:47:28 +0000
110@@ -48,6 +48,9 @@
111
112 nux::BaseWindow* window() const;
113
114+ gboolean CheckShortcutActivation(const char* key_string);
115+ std::vector<char> GetAllShortcuts();
116+
117 nux::Property<int> launcher_width;
118 nux::Property<int> panel_height;
119
120
121=== modified file 'plugins/unityshell/src/DashView.cpp'
122--- plugins/unityshell/src/DashView.cpp 2011-09-08 03:04:04 +0000
123+++ plugins/unityshell/src/DashView.cpp 2011-09-08 06:47:28 +0000
124@@ -768,6 +768,27 @@
125 return false;
126 }
127
128+std::string const DashView::GetIdForShortcutActivation(std::string const& shortcut) const
129+{
130+ Lens::Ptr lens = lenses_.GetLensForShortcut(shortcut);
131+ if (lens)
132+ return lens->id;
133+ return "";
134+}
135+
136+std::vector<char> DashView::GetAllShortcuts()
137+{
138+ std::vector<char> result;
139+
140+ for (Lens::Ptr lens: lenses_.GetLenses())
141+ {
142+ std::string shortcut = lens->shortcut;
143+ if(shortcut.size() > 0)
144+ result.push_back(shortcut.at(0));
145+ }
146+ return result;
147+}
148+
149 bool DashView::InspectKeyEvent(unsigned int eventType,
150 unsigned int key_sym,
151 const char* character)
152
153=== modified file 'plugins/unityshell/src/DashView.h'
154--- plugins/unityshell/src/DashView.h 2011-09-07 09:50:44 +0000
155+++ plugins/unityshell/src/DashView.h 2011-09-08 06:47:28 +0000
156@@ -56,6 +56,9 @@
157 void DisableBlur();
158 void OnActivateRequest(GVariant* args);
159
160+ std::string const GetIdForShortcutActivation(std::string const& shortcut) const;
161+ std::vector<char> GetAllShortcuts();
162+
163 nux::View* default_focus() const;
164
165 private:
166
167=== modified file 'plugins/unityshell/src/Launcher.cpp'
168--- plugins/unityshell/src/Launcher.cpp 2011-09-08 04:49:34 +0000
169+++ plugins/unityshell/src/Launcher.cpp 2011-09-08 06:47:28 +0000
170@@ -235,7 +235,6 @@
171 _ignore_repeat_shortcut_handle = 0;
172
173 _latest_shortcut = 0;
174- _super_pressed = false;
175 _shortcuts_shown = false;
176 _floating = false;
177 _hovered = false;
178@@ -1366,7 +1365,6 @@
179
180 void Launcher::StartKeyShowLauncher()
181 {
182- _super_pressed = true;
183 _hide_machine->SetQuirk(LauncherHideMachine::LAST_ACTION_ACTIVATE, false);
184
185 SetTimeStruct(&_times[TIME_TAP_SUPER]);
186@@ -1391,7 +1389,6 @@
187 clock_gettime(CLOCK_MONOTONIC, &current);
188
189 _hover_machine->SetQuirk(LauncherHoverMachine::SHORTCUT_KEYS_VISIBLE, false);
190- _super_pressed = false;
191 _shortcuts_shown = false;
192 QueueDraw();
193
194@@ -2509,9 +2506,6 @@
195 unsigned long key_state,
196 char* key_string)
197 {
198- if (!_super_pressed)
199- return false;
200-
201 LauncherModel::iterator it;
202
203 // Shortcut to start launcher icons. Only relies on Keycode, ignore modifier
204@@ -2520,16 +2514,6 @@
205 if ((XKeysymToKeycode(x_display, (*it)->GetShortcut()) == key_code) ||
206 ((gchar)((*it)->GetShortcut()) == key_string[0]))
207 {
208- /*
209- * start a timeout while repressing the same shortcut will be ignored.
210- * This is because the keypress repeat is handled by Xorg and we have no
211- * way to know if a press is an actual press or just an automated repetition
212- * because the button is hold down. (key release events are sent in both cases)
213- */
214- if (_ignore_repeat_shortcut_handle > 0)
215- g_source_remove(_ignore_repeat_shortcut_handle);
216- _ignore_repeat_shortcut_handle = g_timeout_add(IGNORE_REPEAT_SHORTCUT_DURATION, &Launcher::ResetRepeatShorcutTimeout, this);
217-
218 if (_latest_shortcut == (*it)->GetShortcut())
219 return true;
220
221@@ -2538,7 +2522,7 @@
222 else
223 (*it)->Activate(ActionArg(ActionArg::LAUNCHER, 0));
224
225- _latest_shortcut = (*it)->GetShortcut();
226+ SetLatestShortcut((*it)->GetShortcut());
227
228 // disable the "tap on super" check
229 _times[TIME_TAP_SUPER].tv_sec = 0;
230@@ -2550,6 +2534,20 @@
231 return false;
232 }
233
234+void Launcher::SetLatestShortcut(guint64 shortcut)
235+{
236+ _latest_shortcut = shortcut;
237+ /*
238+ * start a timeout while repressing the same shortcut will be ignored.
239+ * This is because the keypress repeat is handled by Xorg and we have no
240+ * way to know if a press is an actual press or just an automated repetition
241+ * because the button is hold down. (key release events are sent in both cases)
242+ */
243+ if (_ignore_repeat_shortcut_handle > 0)
244+ g_source_remove(_ignore_repeat_shortcut_handle);
245+ _ignore_repeat_shortcut_handle = g_timeout_add(IGNORE_REPEAT_SHORTCUT_DURATION, &Launcher::ResetRepeatShorcutTimeout, this);
246+}
247+
248 void
249 Launcher::EdgeRevealTriggered(int mouse_x, int mouse_y)
250 {
251
252=== modified file 'plugins/unityshell/src/Launcher.h'
253--- plugins/unityshell/src/Launcher.h 2011-09-08 04:49:34 +0000
254+++ plugins/unityshell/src/Launcher.h 2011-09-08 06:47:28 +0000
255@@ -156,6 +156,7 @@
256 void EdgeRevealTriggered(int x, int y);
257
258 gboolean CheckSuperShortcutPressed(Display *x_display, unsigned int key_sym, unsigned long key_code, unsigned long key_state, char* key_string);
259+ void SetLatestShortcut(guint64 shortcut);
260
261 nux::BaseWindow* GetParent()
262 {
263@@ -397,7 +398,6 @@
264 bool _check_window_over_launcher;
265
266 bool _shortcuts_shown;
267- bool _super_pressed;
268 bool _keynav_activated;
269 guint64 _latest_shortcut;
270
271
272=== modified file 'plugins/unityshell/src/unityshell.cpp'
273--- plugins/unityshell/src/unityshell.cpp 2011-09-08 03:04:04 +0000
274+++ plugins/unityshell/src/unityshell.cpp 2011-09-08 06:47:28 +0000
275@@ -225,6 +225,7 @@
276 this);
277
278 g_timeout_add(0, &UnityScreen::initPluginActions, this);
279+ super_keypressed_ = false;
280
281 GeisAdapter::Default()->Run();
282 gestureEngine = new GestureEngine(screen);
283@@ -237,8 +238,6 @@
284 ubus_manager_.RegisterInterest(UBUS_PLACE_VIEW_SHOWN, [&](GVariant * args) { dash_is_open_ = true; });
285 ubus_manager_.RegisterInterest(UBUS_PLACE_VIEW_HIDDEN, [&](GVariant * args) { dash_is_open_ = false; });
286
287- EnsureKeybindings();
288-
289 LOG_INFO(logger) << "UnityScreen constructed: " << timer.ElapsedSeconds() << "s";
290 }
291
292@@ -301,7 +300,7 @@
293
294 }
295
296-void UnityScreen::EnsureKeybindings()
297+void UnityScreen::EnsureSuperKeybindings()
298 {
299 for (auto action : _shortcut_actions)
300 screen->removeAction(action.get());
301@@ -313,19 +312,26 @@
302 guint64 shortcut = icon->GetShortcut();
303 if (shortcut == 0)
304 continue;
305-
306+ CreateSuperNewAction(static_cast<char>(shortcut));
307+ }
308+
309+ for (auto shortcut : dashController->GetAllShortcuts())
310+ CreateSuperNewAction(shortcut);
311+}
312+
313+void UnityScreen::CreateSuperNewAction(char shortcut)
314+{
315 CompActionPtr action(new CompAction());
316
317 CompAction::KeyBinding binding;
318 std::ostringstream sout;
319- sout << "<Super>" << static_cast<char>(shortcut);
320+ sout << "<Super>" << shortcut;
321 binding.fromString(sout.str());
322
323 action->setKey(binding);
324
325 screen->addAction(action.get());
326 _shortcut_actions.push_back(action);
327- }
328 }
329
330 void UnityScreen::nuxPrologue()
331@@ -871,7 +877,14 @@
332 if ((result = XLookupString(&(event->xkey), key_string, 2, &key_sym, 0)) > 0)
333 {
334 key_string[result] = 0;
335- skip_other_plugins = launcher->CheckSuperShortcutPressed(screen->dpy(), key_sym, event->xkey.keycode, event->xkey.state, key_string);
336+ if (super_keypressed_) {
337+ skip_other_plugins = launcher->CheckSuperShortcutPressed(screen->dpy(), key_sym, event->xkey.keycode, event->xkey.state, key_string);
338+ if (!skip_other_plugins) {
339+ skip_other_plugins = dashController->CheckShortcutActivation(key_string);
340+ if (skip_other_plugins)
341+ launcher->SetLatestShortcut(key_string[0]);
342+ }
343+ }
344 }
345 break;
346 }
347@@ -907,8 +920,9 @@
348 if (state & CompAction::StateInitKey)
349 action->setState(action->state() | CompAction::StateTermKey);
350
351+ super_keypressed_ = true;
352 launcher->StartKeyShowLauncher();
353- EnsureKeybindings ();
354+ EnsureSuperKeybindings ();
355 return false;
356 }
357
358@@ -916,6 +930,7 @@
359 CompAction::State state,
360 CompOption::Vector& options)
361 {
362+ super_keypressed_ = false;
363 launcher->EndKeyShowLauncher();
364 return false;
365 }
366
367=== modified file 'plugins/unityshell/src/unityshell.h'
368--- plugins/unityshell/src/unityshell.h 2011-09-08 03:04:04 +0000
369+++ plugins/unityshell/src/unityshell.h 2011-09-08 06:47:28 +0000
370@@ -235,7 +235,8 @@
371
372 void SendExecuteCommand();
373
374- void EnsureKeybindings ();
375+ void EnsureSuperKeybindings ();
376+ void CreateSuperNewAction(char shortcut);
377
378 static gboolean initPluginActions(gpointer data);
379 static void initLauncher(nux::NThread* thread, void* InitData);
380@@ -281,6 +282,7 @@
381 typedef std::shared_ptr<CompAction> CompActionPtr;
382 typedef std::vector<CompActionPtr> ShortcutActions;
383 ShortcutActions _shortcut_actions;
384+ bool super_keypressed_;
385
386 /* keyboard-nav mode */
387 CompWindow* newFocusedWindow;