Merge lp:~townsend/unity/fix-lp1283775 into lp:unity

[Christopher Townsend]

Fix issue where the number of Launcher icon pips are not always properly updated when a new window of an already running application is opened.

Fix issue where the number of Launcher icon pips are not always properly updated when a new window of an already running application is opened.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3807
http://jenkins.qa.ubuntu.com/job/unity-ci/932/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-amd64-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-armhf-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-i386-ci/19

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3808
http://jenkins.qa.ubuntu.com/job/unity-ci/934/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-amd64-ci/21
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-armhf-ci/21
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-utopic-i386-ci/21

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

review: Approve (continuous-integration)

[Marco Trevisan (Treviño) (3v1n0)]

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

Ouch, I still need a case where this might make no effect: if no new window is opened/closed, but there are multiple opened across monitors, and one is moved to another monitor, it's not guaranteed that the redraw is emitted.

So, while the proper solution would just be to keep around the number of windows per monitor we have, we might just emit a redraw in any case... It's still something that won't really cost much.

review: Abstain

[Christopher Townsend (townsend)]

@Marco,

Hmm, I would think the call to SetWindowVisibleOnMonitor() would take care of the redraw in the case of moving a window to a different monitor. Could you explain why this doesn't force a redraw in the case you point out?

[Christopher Townsend (townsend)]

@ Marco,

Thinking more about your scenario you point out, I think I understand what you mean. Since there are already multiple windows opened of the same app across different monitors, then moving one of those windows to another monitor will cause SetWindowVisibleOnMonitor() to just return since there are already open windows of that app on that monitor.

So yeah, either keeping a vector of app windows for each monitor (most efficient) or just calling EmitNeedsRedraw() (easiest to implement) is correct.

[Marco Trevisan (Treviño) (3v1n0)]

review: Approve

=== modified file 'launcher/ApplicationLauncherIcon.cpp'
--- launcher/ApplicationLauncherIcon.cpp	2014-04-21 15:04:09 +0000
+++ launcher/ApplicationLauncherIcon.cpp	2014-05-28 12:16:36 +0000
@@ -723,7 +723,7 @@
 
 void ApplicationLauncherIcon::EnsureWindowState()
 {
-  std::bitset<monitors::MAX> monitors;
+  std::vector<int> number_of_windows_on_monitor(monitors::MAX);
 
   for (auto& window: app_->GetWindows())
   {
@@ -735,18 +735,18 @@
       // If monitor is -1 (or negative), show on all monitors.
       if (monitor < 0)
       {
-        monitors.set();
-        break;
+        for (unsigned j; j < monitors::MAX; j++)
+            ++number_of_windows_on_monitor[j];
       }
       else
       {
-        monitors[monitor] = true;
+        ++number_of_windows_on_monitor[monitor];
       }
     }
   }
 
   for (unsigned i = 0; i < monitors::MAX; i++)
-    SetWindowVisibleOnMonitor(monitors[i], i);
+    SetNumberOfWindowsVisibleOnMonitor(number_of_windows_on_monitor[i], i);
 
   WindowsChanged.emit();
 }

=== modified file 'launcher/LauncherIcon.cpp'
--- launcher/LauncherIcon.cpp	2014-03-11 09:16:10 +0000
+++ launcher/LauncherIcon.cpp	2014-05-28 12:16:36 +0000
@@ -80,6 +80,7 @@
   , _shortcut(0)
   , _allow_quicklist_to_show(true)
   , _center(monitors::MAX)
+  , _number_of_visible_windows(monitors::MAX)
   , _quirks(monitors::MAX)
   , _quirk_animations(monitors::MAX, decltype(_quirk_animations)::value_type(unsigned(Quirk::LAST)))
   , _last_stable(monitors::MAX)
@@ -732,12 +733,15 @@
   return {-1, nux::Point3()};
 }
 
-void LauncherIcon::SetWindowVisibleOnMonitor(bool val, int monitor)
+void LauncherIcon::SetNumberOfWindowsVisibleOnMonitor(int number_of_windows, int monitor)
 {
-  if (_has_visible_window[monitor] == val)
+  if (_number_of_visible_windows[monitor] == number_of_windows)
     return;
 
-  _has_visible_window[monitor] = val;
+  _has_visible_window[monitor] = (number_of_windows > 0);
+
+  _number_of_visible_windows[monitor] = number_of_windows;
+
   EmitNeedsRedraw(monitor);
 }
 

=== modified file 'launcher/LauncherIcon.h'
--- launcher/LauncherIcon.h	2014-03-11 09:16:10 +0000
+++ launcher/LauncherIcon.h	2014-05-28 12:16:36 +0000
@@ -226,7 +226,7 @@
 
   void SetProgress(float progress);
 
-  void SetWindowVisibleOnMonitor(bool val, int monitor);
+  void SetNumberOfWindowsVisibleOnMonitor(int number_of_windows, int monitor);
 
   void Present(float urgency, int length, int monitor = -1);
 
@@ -338,6 +338,7 @@
 
   std::vector<nux::Point3> _center;
   std::bitset<monitors::MAX> _has_visible_window;
+  std::vector<int> _number_of_visible_windows;
   std::vector<std::bitset<std::size_t(Quirk::LAST)>> _quirks;
   std::vector<std::vector<std::shared_ptr<Animation>>> _quirk_animations;
   std::vector<nux::Point3> _last_stable;