Merge lp:~sil2100/unity/hud_icon_bfb_fix into lp:unity

Proposed by Łukasz Zemczak on 2013-01-04
Status: Merged
Approved by: Marco Trevisan (Treviño) on 2013-01-08
Approved revision: 3015
Merged at revision: 3017
Proposed branch: lp:~sil2100/unity/hud_icon_bfb_fix
Merge into: lp:unity
Diff against target: 41 lines (+11/-2)
1 file modified
unity-shared/BamfApplicationManager.cpp (+11/-2)
To merge this branch: bzr merge lp:~sil2100/unity/hud_icon_bfb_fix
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve on 2013-01-08
Łukasz Zemczak Needs Information on 2013-01-07
PS Jenkins bot continuous-integration Pending
Review via email: mp+141977@code.launchpad.net

Commit message

When opening the HUD when the Dash is opened when there are no applications, a '?' icon is shown instead of the BFB icon. To fix this, we ignore all Unity-related windows in the GetActiveWindow() function.

Description of the change

- Problem:

When there are no focused applications on the desktop, the dash is opened and then instantly the HUD, the HUD icon is wrong (showing '?'). It's because the dash is the currently active application then, so it tries to fetch the icon of the dash instead.

- Fix:

A quick workaround - when we show the HUD, we also make sure the active application is not the dash, basing on the application title.

- Test:

Already handled by existing test (test_dash_hud_only_uses_icon_from_current_desktop).

To post a comment you must log in.
Marco Trevisan (Treviño) (3v1n0) wrote :

More than doing this in the HUD itself, probably it's just better to avoid to get an unity window as the active one in the Application manager (as we did before).

To do that just filter-out the xid's that are in nux::XInputWindow::NativeHandleList() in bamf::Manager::GetActiveWindow()

Łukasz Zemczak (sil2100) wrote :

Thanks, will try doing it this way. Actually this way was the easiest and fastest ;) So a better solution like this is a good idea.

Łukasz Zemczak (sil2100) wrote :

On the other hand, I also remembered why I preferred not to look into the GetActiveWindow() code - since I didn't want to break anything in the meantime. Since I didn't want to assume that the dash will be never fetched by GetActiveWindow() to actually do anything practical. Is it really safe to filter out the dash in this code? Or is there even a small chance that it should be fetch-able through GetActiveWindow()?

review: Needs Information
Marco Trevisan (Treviño) (3v1n0) wrote :

I think it's safe. Really no part of unity needs to get one of its window as active.

Since we're a shell, when it comes to window management, we should always act as we'd not exist.

lp:~sil2100/unity/hud_icon_bfb_fix updated on 2013-01-07
3013. By Łukasz Zemczak on 2013-01-07

Revert last fix

3014. By Łukasz Zemczak on 2013-01-07

Black-list all Unity-related windows in the GetActiveWindow() code as advised by Trevinho.

Łukasz Zemczak (sil2100) wrote :

Modified!

Marco Trevisan (Treviño) (3v1n0) wrote :

32 + std::find(our_xids.begin(), our_xids.end(), active_xid) == our_xids.end())

I think you should check "xid" here instead of "active_xid" or you'll repeat the same check.

Otherwise only define "Window xid" at at above level, and always use it to avoid confusion.

lp:~sil2100/unity/hud_icon_bfb_fix updated on 2013-01-08
3015. By Łukasz Zemczak on 2013-01-08

Fix typo, we were using the wrong xid in one place

Marco Trevisan (Treviño) (3v1n0) wrote :

Looks good now, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'unity-shared/BamfApplicationManager.cpp'
2--- unity-shared/BamfApplicationManager.cpp 2012-12-18 17:57:48 +0000
3+++ unity-shared/BamfApplicationManager.cpp 2013-01-08 12:21:22 +0000
4@@ -21,6 +21,7 @@
5 #include "unity-shared/WindowManager.h"
6
7 #include <NuxCore/Logger.h>
8+#include <NuxGraphics/XInputWindow.h>
9
10
11 DECLARE_LOGGER(logger, "unity.appmanager.bamf");
12@@ -545,6 +546,13 @@
13 // No transfer of ownership for bamf_matcher_get_active_window.
14 BamfWindow* active_win = bamf_matcher_get_active_window(matcher_);
15
16+ std::vector<Window> const& our_xids = nux::XInputWindow::NativeHandleList();
17+ Window xid = bamf_window_get_xid(active_win);
18+
19+ // First check if the active window is not an Unity window
20+ if (std::find(our_xids.begin(), our_xids.end(), xid) != our_xids.end())
21+ active_win = nullptr;
22+
23 // If the active window is a dock type, then we want the first visible, non-dock type.
24 if (active_win &&
25 bamf_window_get_window_type(active_win) == BAMF_WINDOW_DOCK)
26@@ -562,12 +570,13 @@
27
28 auto win = static_cast<BamfWindow*>(l->data);
29 auto view = static_cast<BamfView*>(l->data);
30- Window xid = bamf_window_get_xid(win);
31+ xid = bamf_window_get_xid(win);
32
33 if (bamf_view_is_user_visible(view) &&
34 bamf_window_get_window_type(win) != BAMF_WINDOW_DOCK &&
35 wm.IsWindowOnCurrentDesktop(xid) &&
36- wm.IsWindowVisible(xid))
37+ wm.IsWindowVisible(xid) &&
38+ std::find(our_xids.begin(), our_xids.end(), xid) == our_xids.end())
39 {
40 active_win = win;
41 }