Merge lp:~3v1n0/unity/fix-690143 into lp:unity

[Marco Trevisan (Treviño)]

Proposal of merge resubmitted after the update of the branch against the newest unity trunk.
I never experienced the crash that Jason found on my test with this version (as I never found one with the previous one), and it also fixes a new bug introduced by the CompizMinimizedWindowHandler that caused unity not to be notified for some events like the viewport switch (and this was causing some visual issues, like the launcher hiding during the vp switch).

Old merge request message (still valid)
=======================================

Fixed bug #690143 as requested by the design team specifications.

To get this I had to get signals from Compiz related to the viewport switch, so to achive this and to keep the implementation clean, I decided to implement the new PluginAdapter signals compiz_screen_viewport_switch_{started,ended}. They are emitted using the PluginAdapter::NotifyCompizEvent function that is called directly by unityshell (UnityScreen).

So now both Launcher and BamfLauncherIcon are listening to these signals to perform their operations. In particular BamfLauncherIcon uses it to check if, after a viewport switch (or a scale terminate), all the windows of the given Icon are viewable in the current workspace and to update the icon indicator accordingly.

Plus I decided to give also another Icon backlight coloring option, and after talking with JohnLea on #ayatana I also added two more options to the "Backlight Mode":
 - Edge Illumination Toggles: illuminate only the edge of the icons linked to
                              running applications.
 - Backlight and Edge Illumination Toggles: illuminate the background of the
                                            icons linked to running
                                            applications; if the windows
                                            of this application are all in
                                            a non viewable workspace, then
                                            only the edge of the icon is
                                            illuminated.

You can see a working example of the "Backlight and Edge Illumination Toggles" option on this screenshot [1].

[1] https://bugs.launchpad.net/unity/+bug/690143/+attachment/2240766/+files/Unity-Unactive-Icons.png

[Jason Smith (jassmith)]

Crashes in current trunk

review: Needs Fixing

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

Mhmhm... Maybe I've worked with a too old version but it was working fine. However I'll retry with the current trunk.

Thank you!

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

It seems to work fine here also after merging with upstream, so I'm waiting for a crash log to fix it.

[Jason Smith (jassmith)]

Works great now

review: Approve

=== modified file 'plugins/unityshell/src/AbstractIconRenderer.h'
--- plugins/unityshell/src/AbstractIconRenderer.h	2011-08-24 22:30:37 +0000
+++ plugins/unityshell/src/AbstractIconRenderer.h	2011-08-30 19:14:24 +0000
@@ -54,6 +54,7 @@
     , running_arrow(false)
     , running_colored(false)
     , running_on_viewport(false)
+    , draw_edge_only(false)
     , active_arrow(false)
     , active_colored(false)
     , skip(false)
@@ -82,6 +83,7 @@
   bool          running_arrow;
   bool          running_colored;
   bool          running_on_viewport;
+  bool          draw_edge_only;
   bool          active_arrow;
   bool          active_colored;
   bool          skip;
 
=== modified file 'plugins/unityshell/src/BamfLauncherIcon.cpp'
--- plugins/unityshell/src/BamfLauncherIcon.cpp	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/BamfLauncherIcon.cpp	2011-08-30 19:14:24 +0000
@@ -186,6 +186,8 @@
                                                          this);
 
   WindowManager::Default()->window_minimized.connect(sigc::mem_fun(this, &BamfLauncherIcon::OnWindowMinimized));
+  WindowManager::Default()->compiz_screen_viewport_switch_ended.connect(sigc::mem_fun(this, &BamfLauncherIcon::OnViewPortSwitchEnded));
+  WindowManager::Default()->terminate_expo.connect(sigc::mem_fun(this, &BamfLauncherIcon::OnViewPortSwitchEnded));
   IconManager->hidden_changed.connect(sigc::mem_fun(this, &BamfLauncherIcon::OnLauncherHiddenChanged));
 
   // hack
@@ -290,6 +292,31 @@
   UpdateQuirkTimeDelayed(300, QUIRK_SHIMMER);
 }
 
+void BamfLauncherIcon::OnViewPortSwitchEnded()
+{
+  bool any_on_current = false;
+  GList *children = bamf_view_get_children(BAMF_VIEW(m_App));
+
+  for (GList *l = children; l; l = l->next)
+  {
+    BamfView *view = BAMF_VIEW(l->data);
+
+    if (BAMF_IS_WINDOW(view))
+    {
+      Window xid = bamf_window_get_xid(BAMF_WINDOW(view));
+      if (WindowManager::Default()->IsWindowOnCurrentDesktop(xid))
+      {
+        any_on_current = true;
+        break;
+      }
+    }
+  }
+
+  SetHasWindowOnViewport(any_on_current);
+
+  g_list_free(children);
+}
+
 bool BamfLauncherIcon::IsSticky()
 {
   return bamf_view_is_sticky(BAMF_VIEW(m_App));
@@ -358,6 +385,8 @@
   BamfView* view;
   bool owns = false;
 
+  if (!w) return owns;
+
   children = bamf_view_get_children(BAMF_VIEW(m_App));
 
   for (l = children; l; l = l->next)
@@ -546,16 +575,26 @@
 {
   GList* children, *l;
   int count = 0;
+  bool has_visible = false;
+
   children = bamf_view_get_children(BAMF_VIEW(m_App));
   for (l = children; l; l = l->next)
   {
     if (!BAMF_IS_WINDOW(l->data))
       continue;
+
+    if (!has_visible)
+    {
+      Window xid = bamf_window_get_xid(BAMF_WINDOW(l->data));
+      if (WindowManager::Default()->IsWindowOnCurrentDesktop(xid))
+        has_visible = true;
+    }
+
     count++;
   }
 
   SetRelatedWindows(count);
-  SetHasWindowOnViewport(true); // FIXME
+  SetHasWindowOnViewport(has_visible);
 
   g_list_free(children);
 }
 
=== modified file 'plugins/unityshell/src/BamfLauncherIcon.h'
--- plugins/unityshell/src/BamfLauncherIcon.h	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/BamfLauncherIcon.h	2011-08-30 19:14:24 +0000
@@ -108,6 +108,7 @@
   void EnsureMenuItemsReady();
 
   void OnWindowMinimized(guint32 xid);
+  void OnViewPortSwitchEnded();
   bool OwnsWindow(Window w);
   
   const std::set<std::string>& GetSupportedTypes();
 
=== modified file 'plugins/unityshell/src/IconRenderer.cpp'
--- plugins/unityshell/src/IconRenderer.cpp	2011-08-25 18:55:57 +0000
+++ plugins/unityshell/src/IconRenderer.cpp	2011-08-30 19:14:24 +0000
@@ -408,7 +408,7 @@
   auto tile_transform = arg.icon->GetTransform(AbstractLauncherIcon::TRANSFORM_TILE);
 
   // draw tile
-  if (backlight_intensity > 0)
+  if (backlight_intensity > 0 && !arg.draw_edge_only)
   {
     RenderElement(GfxContext,
                   arg,
@@ -778,13 +778,12 @@
     // markers are well outside screen bounds to start
     int markers [3] = {-100, -100, -100};
 
-    /*if (!arg.running_on_viewport)
+    if (!arg.running_on_viewport)
     {
-      markers.push_back (markerCenter);
-      texture = _arrow_empty_ltr;
+      markers[0] = markerCenter;
+      texture = local::arrow_empty_ltr;
     }
-    else*/
-    if (running == 1)
+    else if (running == 1)
     {
       markers[0] = markerCenter;
       texture = local::arrow_ltr;
 
=== modified file 'plugins/unityshell/src/Launcher.cpp'
--- plugins/unityshell/src/Launcher.cpp	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/Launcher.cpp	2011-08-30 19:14:24 +0000
@@ -189,6 +189,8 @@
   plugin_adapter.initiate_expo.connect(sigc::mem_fun(this, &Launcher::OnPluginStateChanged));
   plugin_adapter.terminate_spread.connect(sigc::mem_fun(this, &Launcher::OnPluginStateChanged));
   plugin_adapter.terminate_expo.connect(sigc::mem_fun(this, &Launcher::OnPluginStateChanged));
+  plugin_adapter.compiz_screen_viewport_switch_started.connect(sigc::mem_fun(this, &Launcher::OnViewPortSwitchStarted));
+  plugin_adapter.compiz_screen_viewport_switch_ended.connect(sigc::mem_fun(this, &Launcher::OnViewPortSwitchEnded));
 
   GeisAdapter& adapter = *(GeisAdapter::Default());
   adapter.drag_start.connect(sigc::mem_fun(this, &Launcher::OnDragStart));
@@ -895,7 +897,8 @@
   struct timespec starting_time = icon->GetQuirkTime(LauncherIcon::QUIRK_STARTING);
   int starting_ms = TimeDelta(&current, &starting_time);
   double starting_progress = (double) CLAMP((float) starting_ms / (float)(ANIM_DURATION_LONG * STARTING_BLINK_LAMBDA), 0.0f, 1.0f);
-  return 0.5f + (float)(std::cos(M_PI * (_backlight_mode != BACKLIGHT_NORMAL ? 4.0f : 3.0f) * starting_progress)) * 0.5f;
+  double val = IsBackLightModeToggles() ? 3.0f : 4.0f;
+  return 0.5f + (float)(std::cos(M_PI * val * starting_progress)) * 0.5f;
 }
 
 float Launcher::IconStartingPulseValue(LauncherIcon* icon, struct timespec const& current)
@@ -931,7 +934,7 @@
   float backlight_strength;
   if (_backlight_mode == BACKLIGHT_ALWAYS_ON)
     backlight_strength = BACKLIGHT_STRENGTH;
-  else if (_backlight_mode == BACKLIGHT_NORMAL)
+  else if (IsBackLightModeToggles())
     backlight_strength = BACKLIGHT_STRENGTH * running_progress;
   else
     backlight_strength = 0.0f;
@@ -956,7 +959,7 @@
       result = backlight_strength;
       if (_backlight_mode == BACKLIGHT_ALWAYS_ON)
         result *= CLAMP(running_progress + IconStartingPulseValue(icon, current), 0.0f, 1.0f);
-      else if (_backlight_mode == BACKLIGHT_NORMAL)
+      else if (IsBackLightModeToggles())
         result += (BACKLIGHT_STRENGTH - result) * (1.0f - IconStartingPulseValue(icon, current));
       else
         result = 1.0f - CLAMP(running_progress + IconStartingPulseValue(icon, current), 0.0f, 1.0f);
@@ -992,6 +995,17 @@
     return result;
 }
 
+bool Launcher::IconDrawEdgeOnly(LauncherIcon* icon)
+{
+  if (_backlight_mode == BACKLIGHT_EDGE_TOGGLE)
+    return true;
+
+  if (_backlight_mode == BACKLIGHT_NORMAL_EDGE_TOGGLE && !icon->HasWindowOnViewport())
+    return true;
+
+  return false;
+}
+
 void Launcher::SetupRenderArg(LauncherIcon* icon, struct timespec const& current, RenderArg& arg)
 {
   arg.icon                = icon;
@@ -1000,6 +1014,7 @@
   arg.running_arrow       = icon->GetQuirk(LauncherIcon::QUIRK_RUNNING);
   arg.running_colored     = icon->GetQuirk(LauncherIcon::QUIRK_URGENT);
   arg.running_on_viewport = icon->HasWindowOnViewport();
+  arg.draw_edge_only      = IconDrawEdgeOnly(icon);
   arg.active_colored      = false;
   arg.x_rotation          = 0.0f;
   arg.y_rotation          = 0.0f;
@@ -1653,6 +1668,28 @@
     return;
 }
 
+void
+Launcher::OnViewPortSwitchStarted()
+{
+  /*
+   *  don't take into account window over launcher state during
+   *  the viewport switch as we can get false positives
+   *  (like switching to an empty viewport while grabbing a fullscreen window)
+   */
+  _check_window_over_launcher = false;
+}
+
+void
+Launcher::OnViewPortSwitchEnded()
+{
+  /*
+   * compute again the list of all window on the new viewport
+   * to decide if we should or not hide the launcher
+   */
+  _check_window_over_launcher = true;
+  CheckWindowOverLauncher();
+}
+
 Launcher::LauncherHideMode Launcher::GetHideMode()
 {
   return _hidemode;
@@ -1729,6 +1766,18 @@
   return _backlight_mode;
 }
 
+bool Launcher::IsBackLightModeToggles()
+{
+  switch (_backlight_mode) {
+    case BACKLIGHT_NORMAL:
+    case BACKLIGHT_EDGE_TOGGLE:
+    case BACKLIGHT_NORMAL_EDGE_TOGGLE:
+      return true;
+    default:
+      return false;
+  }
+}
+
 void
 Launcher::SetLaunchAnimation(LaunchAnimation animation)
 {
 
=== modified file 'plugins/unityshell/src/Launcher.h'
--- plugins/unityshell/src/Launcher.h	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/Launcher.h	2011-08-30 19:14:24 +0000
@@ -99,6 +99,8 @@
     BACKLIGHT_ALWAYS_ON,
     BACKLIGHT_NORMAL,
     BACKLIGHT_ALWAYS_OFF,
+    BACKLIGHT_EDGE_TOGGLE,
+    BACKLIGHT_NORMAL_EDGE_TOGGLE
   } BacklightMode;
 
   Launcher(nux::BaseWindow* parent, NUX_FILE_LINE_PROTO);
@@ -140,6 +142,7 @@
 
   void SetBacklightMode(BacklightMode mode);
   BacklightMode GetBacklightMode();
+  bool IsBackLightModeToggles();
 
   void SetLaunchAnimation(LaunchAnimation animation);
   LaunchAnimation GetLaunchAnimation();
@@ -247,6 +250,9 @@
 
   void OnPluginStateChanged();
 
+  void OnViewPortSwitchStarted();
+  void OnViewPortSwitchEnded();
+
   static gboolean AnimationTimeout(gpointer data);
   static gboolean SuperShowLauncherTimeout(gpointer data);
   static gboolean SuperHideLauncherTimeout(gpointer data);
@@ -266,6 +272,7 @@
   void OnDragWindowAnimCompleted();
 
   bool IconNeedsAnimation(LauncherIcon* icon, struct timespec const& current);
+  bool IconDrawEdgeOnly(LauncherIcon* icon);
   bool AnimationInProgress();
 
   void SetActionState(LauncherActionState actionstate);
 
=== modified file 'plugins/unityshell/src/PluginAdapter.cpp'
--- plugins/unityshell/src/PluginAdapter.cpp	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/PluginAdapter.cpp	2011-08-30 19:14:24 +0000
@@ -63,6 +63,7 @@
 {
   _spread_state = false;
   _expo_state = false;
+  _vp_switch_started = false;
 
   _grab_show_action = 0;
   _grab_hide_action = 0;
@@ -190,6 +191,23 @@
 }
 
 void
+PluginAdapter::NotifyCompizEvent(const char* plugin, const char* event, CompOption::Vector& option)
+{
+  if (g_strcmp0(event, "start_viewport_switch") == 0)
+  {
+    _vp_switch_started = true;
+    compiz_screen_viewport_switch_started.emit();
+  }
+  else if (g_strcmp0(event, "end_viewport_switch") == 0)
+  {
+    _vp_switch_started = false;
+    compiz_screen_viewport_switch_ended.emit();
+  }
+
+  compiz_event.emit(plugin, event, option);
+}
+
+void
 MultiActionList::AddNewAction(CompAction* a, bool primary)
 {
   if (std::find(m_ActionList.begin(), m_ActionList.end(), a)  == m_ActionList.end())
@@ -785,6 +803,12 @@
   return m_Screen->grabbed();
 }
 
+bool
+PluginAdapter::IsViewPortSwitchStarted()
+{
+  return _vp_switch_started;
+}
+
 void PluginAdapter::MaximizeIfBigEnough(CompWindow* window)
 {
   XClassHint   classHint;
 
=== modified file 'plugins/unityshell/src/PluginAdapter.h'
--- plugins/unityshell/src/PluginAdapter.h	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/PluginAdapter.h	2011-08-30 19:14:24 +0000
@@ -103,6 +103,7 @@
   void NotifyMoved(CompWindow* window, int x, int y);
   void NotifyResized(CompWindow* window, int x, int y, int w, int h);
   void NotifyStateChange(CompWindow* window, unsigned int state, unsigned int last_state);
+  void NotifyCompizEvent(const char* plugin, const char* event, CompOption::Vector& option);
 
   void Decorate(guint32 xid);
   void Undecorate(guint32 xid);
@@ -126,6 +127,7 @@
   bool ScaleWindowGroup(std::vector<Window> windows, int state, bool force);
 
   bool IsScreenGrabbed();
+  bool IsViewPortSwitchStarted();
 
   unsigned int GetWindowActiveNumber (guint32 xid);
 
@@ -160,6 +162,7 @@
 
   bool _spread_state;
   bool _expo_state;
+  bool _vp_switch_started;
 
   CompAction* _grab_show_action;
   CompAction* _grab_hide_action;
 
=== modified file 'plugins/unityshell/src/WindowManager.cpp'
--- plugins/unityshell/src/WindowManager.cpp	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/WindowManager.cpp	2011-08-30 19:14:24 +0000
@@ -32,6 +32,11 @@
     return false;
   }
 
+  bool IsViewPortSwitchStarted()
+  {
+    return false;
+  }
+
   void ShowDesktop()
   {
     g_debug("%s", G_STRFUNC);
 
=== modified file 'plugins/unityshell/src/WindowManager.h'
--- plugins/unityshell/src/WindowManager.h	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/WindowManager.h	2011-08-30 19:14:24 +0000
@@ -25,6 +25,7 @@
 #include "Nux/WindowThread.h"
 #include "NuxGraphics/GLWindowManager.h"
 #include <gdk/gdkx.h>
+#include <core/core.h>
 
 class WindowManager
 {
@@ -75,6 +76,7 @@
   virtual void Undecorate(guint32 xid) {};
 
   virtual bool IsScreenGrabbed() = 0;
+  virtual bool IsViewPortSwitchStarted() = 0;
 
   void StartMove(guint32 id, int, int);
 
@@ -113,6 +115,10 @@
 
   sigc::signal<void> compiz_screen_grabbed;
   sigc::signal<void> compiz_screen_ungrabbed;
+  sigc::signal<void> compiz_screen_viewport_switch_started;
+  sigc::signal<void> compiz_screen_viewport_switch_ended;
+
+  sigc::signal<void, const char*, const char*, CompOption::Vector&> compiz_event;
 
 private:
   Atom m_MoveResizeAtom;
 
=== modified file 'plugins/unityshell/src/compizminimizedwindowhandler.h'
--- plugins/unityshell/src/compizminimizedwindowhandler.h	2011-08-24 17:01:08 +0000
+++ plugins/unityshell/src/compizminimizedwindowhandler.h	2011-08-30 19:14:24 +0000
@@ -1,3 +1,4 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
 /*
  * Copyright (C) 2011 Canonical Ltd.
  *
@@ -53,9 +54,7 @@
   void unminimize ();
 
   static void setFunctions (bool keepMinimized);
-  static void handleCompizEvent (const char *,
-                                 const char *,
-                                 CompOption::Vector &);
+  static void handleCompizEvent (const char *, const char *, CompOption::Vector &);
   static std::list<CompWindow *> minimizingWindows;
 
   typedef CompizMinimizedWindowHandler<Screen, Window> CompizMinimizedWindowHandler_complete;
@@ -67,6 +66,7 @@
 private:
 
   PrivateCompizMinimizedWindowHandler *priv;
+  static bool handleEvents;
 };
 }
 
@@ -74,6 +74,9 @@
 CompWindowList compiz::CompizMinimizedWindowHandler<Screen, Window>::minimizingWindows;
 
 template <typename Screen, typename Window>
+bool compiz::CompizMinimizedWindowHandler<Screen, Window>::handleEvents = true;
+
+template <typename Screen, typename Window>
 compiz::CompizMinimizedWindowHandler<Screen, Window>::CompizMinimizedWindowHandler(CompWindow *w) :
   MinimizedWindowHandler (screen->dpy (), w->id ())
 {
@@ -121,7 +124,7 @@
 
   std::vector<unsigned int> transients = getTransients ();
 
-  screen->handleCompizEventSetEnabled (Screen::get (screen), true);
+  handleEvents = true;
   priv->mWindow->windowNotify (CompWindowNotifyMinimize);
   priv->mWindow->changeState (priv->mWindow->state () | CompWindowStateHiddenMask);
 
@@ -205,6 +208,9 @@
                                                                          const char *eventName,
                                                                          CompOption::Vector &o)
 {
+  if (!handleEvents)
+    return;
+
   if (strncmp (pluginName, "animation", 9) == 0 &&
       strncmp (eventName, "window_animation", 16) == 0)
   {
@@ -218,13 +224,15 @@
           minimizingWindows.push_back (w);
         else
           minimizingWindows.remove (w);
-	    }
+      }
     }
   }
 
   if (!CompOption::getBoolOptionNamed (o, "active", false) &&
       minimizingWindows.empty ())
-    screen->handleCompizEventSetEnabled (Screen::get (screen), false);
+  {
+    handleEvents = false;
+  }
 }
 
 template <typename Screen, typename Window>
 
=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2011-08-29 23:36:21 +0000
+++ plugins/unityshell/src/unityshell.cpp	2011-08-30 19:14:24 +0000
@@ -848,23 +848,8 @@
                                     const char* event,
                                     CompOption::Vector& option)
 {
-  /*
-   *  don't take into account window over launcher state during
-   *  the ws switch as we can get false positives
-   *  (like switching to an empty viewport while grabbing a fullscreen window)
-   */
-  if (strcmp(event, "start_viewport_switch") == 0)
-    launcher->EnableCheckWindowOverLauncher(false);
-  else if (strcmp(event, "end_viewport_switch") == 0)
-  {
-    // compute again the list of all window on the new viewport
-    // to decide if we should or not hide the launcher
-    launcher->EnableCheckWindowOverLauncher(true);
-    launcher->CheckWindowOverLauncher();
-  }
-
+  PluginAdapter::Default()->NotifyCompizEvent(plugin, event, option);
   compiz::CompizMinimizedWindowHandler<UnityScreen, UnityWindow>::handleCompizEvent (plugin, event, option);
-
   screen->handleCompizEvent(plugin, event, option);
 }
 
 
=== modified file 'plugins/unityshell/unityshell.xml.in'
--- plugins/unityshell/unityshell.xml.in	2011-08-26 22:31:20 +0000
+++ plugins/unityshell/unityshell.xml.in	2011-08-30 19:14:24 +0000
@@ -160,7 +160,7 @@
                     <_short>Backlight Mode</_short>
                     <_long>Change how the icons are backlit</_long>
                     <min>0</min>
-                    <max>2</max>
+                    <max>4</max>
                     <default>0</default>
                     <desc>
                         <value>0</value>
@@ -174,6 +174,14 @@
                         <value>2</value>
                         <_name>Backlight Always Off</_name>
                     </desc>
+                    <desc>
+                        <value>3</value>
+                        <_name>Edge Illumination Toggles</_name>
+                    </desc>
+                    <desc>
+                        <value>4</value>
+                        <_name>Backlight and Edge Illumination Toggles</_name>
+                    </desc>
                 </option>
                 <option name="launch_animation" type="int">
                     <_short>Launch Animation</_short>