Merge lp:~uriboni/unity/adjust-minimize-animation-speed into lp:unity

[Ugo Riboni]

Progressively adjust the speed of the minimize animation. First time it is used is slower, then speeds up the more it is used.

Progressively adjust the speed of the minimize animation. First time it is used is slower, then speeds up the more it is used.

[Sam Spilsbury (smspillaz)]

Overall looks fine, comments:

133 + /* minimize_durations is a list value, but minimize applies only to
134 + * normal windows, so there's always one value */
135 + CompOption::Value value = o.value();
136 + CompOption::Value::Vector list = value.list();

You probably do not want to make copies of these if you can avoid it.

Use a reference instead.

180 +<<<<<<< TREE
181 unity::ThumbnailGenerator thumb_generator;
182
183 +=======
184 +
185 + glib::Object<GSettings> _settings;
186 + int _minimize_count;
187 + int _minimize_speed_threshold;
188 + int _minimize_slow_duration;
189 + int _minimize_fast_duration;
190 + glib::Signal<void, GSettings*, gchar* > _minimize_count_changed;
191 + glib::Signal<void, GSettings*, gchar* > _minimize_speed_threshold_changed;
192 + glib::Signal<void, GSettings*, gchar* > _minimize_slow_duration_changed;
193 + glib::Signal<void, GSettings*, gchar* > _minimize_fast_duration_changed;
194 +
195 +>>>>>>> MERGE-SOURCE

Also conflicts.

100 + /* Perform some sanity checks on the configuration values */
101 + if (_minimize_fast_duration > _minimize_slow_duration)
102 + {
103 + LOG_WARN(logger) << "Configuration mismatch: minimize-fast-duration ("
104 + << _minimize_fast_duration
105 + << ") is longer than minimize-slow-duration ("
106 + << _minimize_slow_duration << "). Not changing speed.";
107 + return;
108 + }
109 +
110 + if (_minimize_count < 0)
111 + _minimize_count = 0;
112 + if (_minimize_count > _minimize_speed_threshold)
113 + _minimize_count = _minimize_speed_threshold;
114 +
115 + /* Adjust the speed so that it gets linearly closer to maximum speed as we
116 + approach the threshold */
117 + int speed_range = _minimize_slow_duration - _minimize_fast_duration;
118 + float position = (_minimize_speed_threshold <= 0) ? 1.0 :
119 + static_cast<float>(_minimize_count) / _minimize_speed_threshold;
120 + int speed = _minimize_slow_duration - std::ceil(position * speed_range);

You will probably want some tests to cover this code.

[Sam Spilsbury (smspillaz)]

Hey Ugo,

Were you able to make this take references?

135 + CompOption::Value value = o.value();
136 + CompOption::Value::Vector list = value.list();
137 + CompOption::Value::Vector::iterator i = list.begin();

[Sam Spilsbury (smspillaz)]

Ugo ?

[Ugo Riboni (uriboni)]

Sam, I fixed all the issues you mentioned and refactored the code some more to make it testable. I had some problems with writing the tests but hopefully they are solved now and I'll be able to add push all the changes today hopefully.

[Sam Spilsbury (smspillaz)]

272 if (!mMinimizeHandler)
273 {
274 + /* Updating the count in dconf will trigger a "changed" signal to which
275 + * the method setting the new animation speed is attached */
276 + UnityScreen* unityScreen = UnityScreen::get(screen);
277 + unityScreen->minimize_speed_controller->UpdateCount();

Its probably better to do this in UnityWindow::windowNotify under case CompWindowNotifyMinimize. This event indicates that a minimize completed successfully, ::minimize is just there for plugins to override the minimize behaviour (poorly named probably), but just a heads up.

362 + *
363 + * Authored by: Andrea Azzarone <email address hidden>

Put yourself there if you want ;-)

398 + g_setenv("GSETTINGS_SCHEMA_DIR", "", true);
399 + g_setenv("GSETTINGS_BACKEND", "", TRUE);

You can just do g_unsetenv here.

I like the tests for signal emmission.

Overall looks good. Feel free to fix those if you want - I'll put my +1 here, someone else can make the branch approved once you're fine with it :)

review: Approve

[Ugo Riboni (uriboni)]

I'll fix your comments, then ping you for final approval. Thanks !

[Sam Spilsbury (smspillaz)]

Good good :)

review: Approve

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1154/console reported an error when processing this lp:~uriboni/unity/adjust-minimize-animation-speed branch.
Not merging it.

[Unity Merger (unity-merger)]

Attempt to merge into lp:unity failed due to conflicts:

text conflict in tests/CMakeLists.txt

[Sam Spilsbury (smspillaz)]

Hey Ugo,

I can't push to your branch but there is a trivial conflict in tests/CMakeLists.txt. once its fixed, please change the branch status to approved.

review: Approve

=== modified file 'com.canonical.Unity.gschema.xml'
--- com.canonical.Unity.gschema.xml	2012-08-15 14:19:47 +0000
+++ com.canonical.Unity.gschema.xml	2012-08-31 09:17:21 +0000
@@ -21,6 +21,26 @@
       <summary>Whether the home screen should be expanded.</summary>
       <description>Whether the home screen should be expanded.</description>
     </key>
+    <key type="i" name="minimize-count">
+      <default>0</default>
+      <summary>Number of times a normal window has been minimized.</summary>
+      <description>This is used to adjust the animation speed by making it progressively faster the more it is used.</description>
+    </key>
+    <key type="i" name="minimize-fast-duration">
+      <default>300</default>
+      <summary>The duration for the minimize animation when it is at its fastest setting.</summary>
+      <description>This is the duration that the minimize animation will have when it has been used more than an number of times equal minimize-speed-threshold.</description>
+    </key>
+    <key type="i" name="minimize-slow-duration">
+      <default>800</default>
+      <summary>The duration for the minimize animation when it is at its slowest setting.</summary>
+      <description>This is the duration that the minimize animation will have when it has never been used.</description>
+    </key>
+    <key type="i" name="minimize-speed-threshold">
+      <default>100</default>
+      <summary>The number of minimizations required to reach maximum speed.</summary>
+      <description>The speed of the minimize animation will progressively get faster as minimize-count approaches this value.</description>
+    </key>
   </schema>
   <schema path="/com/canonical/unity/launcher/" id="com.canonical.Unity.Launcher" gettext-domain="unity">
     <key type="as" name="favorites">

=== added file 'plugins/unityshell/src/WindowMinimizeSpeedController.cpp'
--- plugins/unityshell/src/WindowMinimizeSpeedController.cpp	1970-01-01 00:00:00 +0000
+++ plugins/unityshell/src/WindowMinimizeSpeedController.cpp	2012-08-31 09:17:21 +0000
@@ -0,0 +1,108 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/* Compiz unity plugin
+ * unity.h
+ *
+ * Copyright (c) 2010-11 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 3
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Your own copyright notice would go above. You are free to choose whatever
+ * licence you want, just take note that some compiz code is GPL and you will
+ * not be able to re-use it if you want to use a different licence.
+ */
+
+#include <gio/gio.h>
+#include <NuxCore/Logger.h>
+
+#include "WindowMinimizeSpeedController.h"
+
+namespace
+{
+
+nux::logging::Logger logger ("unity.WindowMinimizeSpeedController");
+
+namespace local
+{
+const std::string UNITY_SCHEMA = "com.canonical.Unity";
+}
+}
+
+WindowMinimizeSpeedController::WindowMinimizeSpeedController()
+  : _settings(g_settings_new(local::UNITY_SCHEMA.c_str()))
+  , _minimize_count(g_settings_get_int(_settings, "minimize-count"))
+  , _minimize_speed_threshold(g_settings_get_int(_settings, "minimize-speed-threshold"))
+  , _minimize_slow_duration(g_settings_get_int(_settings, "minimize-slow-duration"))
+  , _minimize_fast_duration(g_settings_get_int(_settings, "minimize-fast-duration"))
+{
+  _minimize_count_changed.Connect(_settings, "changed::minimize-count",
+                                  [&] (GSettings*, gchar* name) {
+    _minimize_count = g_settings_get_int(_settings, name);
+    SetDuration();
+  });
+  _minimize_speed_threshold_changed.Connect(_settings, "changed::minimize-speed-threshold",
+                                            [&] (GSettings*, gchar* name) {
+    _minimize_speed_threshold = g_settings_get_int(_settings, name);
+    SetDuration();
+  });
+  _minimize_fast_duration_changed.Connect(_settings, "changed::minimize-fast-duration",
+                                      [&] (GSettings*, gchar* name) {
+    _minimize_fast_duration = g_settings_get_int(_settings, name);
+    SetDuration();
+  });
+  _minimize_slow_duration_changed.Connect(_settings, "changed::minimize-slow-duration",
+                                      [&] (GSettings*, gchar* name) {
+    _minimize_slow_duration = g_settings_get_int(_settings, name);
+    SetDuration();
+  });
+}
+
+void WindowMinimizeSpeedController::UpdateCount()
+{
+  if (_minimize_count < _minimize_speed_threshold) {
+    _minimize_count += 1;
+    g_settings_set_int(_settings, "minimize-count", _minimize_count);
+  }
+}
+
+int WindowMinimizeSpeedController::getDuration()
+{
+  return mDuration;
+}
+
+void WindowMinimizeSpeedController::SetDuration()
+{
+  /* Perform some sanity checks on the configuration values */
+  if (_minimize_fast_duration > _minimize_slow_duration)
+  {
+    LOG_WARN(logger) << "Configuration mismatch: minimize-fast-duration (" 
+                      << _minimize_fast_duration
+                      << ") is longer than minimize-slow-duration ("
+                      << _minimize_slow_duration << "). Not changing speed.";
+    return;
+  }
+  
+  if (_minimize_count < 0)
+    _minimize_count = 0;
+  if (_minimize_count > _minimize_speed_threshold)
+    _minimize_count = _minimize_speed_threshold;
+ 
+  /* Adjust the speed so that it gets linearly closer to maximum speed as we
+     approach the threshold */
+  int speed_range = _minimize_slow_duration - _minimize_fast_duration;
+  float position = (_minimize_speed_threshold <= 0) ? 1.0 :
+                   static_cast<float>(_minimize_count) / _minimize_speed_threshold;
+  int duration = _minimize_slow_duration - std::ceil(position * speed_range);
+  
+  if (duration != mDuration) {
+    mDuration = duration;
+    DurationChanged.emit();
+  } 
+}

=== added file 'plugins/unityshell/src/WindowMinimizeSpeedController.h'
--- plugins/unityshell/src/WindowMinimizeSpeedController.h	1970-01-01 00:00:00 +0000
+++ plugins/unityshell/src/WindowMinimizeSpeedController.h	2012-08-31 09:17:21 +0000
@@ -0,0 +1,57 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/* Compiz unity plugin
+ * unity.h
+ *
+ * Copyright (c) 2010-11 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 3
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Your own copyright notice would go above. You are free to choose whatever
+ * licence you want, just take note that some compiz code is GPL and you will
+ * not be able to re-use it if you want to use a different licence.
+ */
+
+#ifndef WINDOWMINIMIZESPEEDCONTROLLER_H
+#define WINDOWMINIMIZESPEEDCONTROLLER_H
+
+#include <core/core.h>
+#include <UnityCore/GLibWrapper.h>
+#include <UnityCore/GLibSignal.h>
+#include <sigc++/sigc++.h>
+
+typedef struct _GSettings GSettings;
+
+using namespace unity;
+
+class WindowMinimizeSpeedController
+{
+public:
+  WindowMinimizeSpeedController();
+  void UpdateCount();
+  int getDuration();
+  sigc::signal<void> DurationChanged;
+  
+private:
+  void SetDuration();
+
+  glib::Object<GSettings> _settings;
+  int _minimize_count;
+  int _minimize_speed_threshold;
+  int _minimize_slow_duration;
+  int _minimize_fast_duration;
+  glib::Signal<void, GSettings*, gchar* > _minimize_count_changed;
+  glib::Signal<void, GSettings*, gchar* > _minimize_speed_threshold_changed;
+  glib::Signal<void, GSettings*, gchar* > _minimize_slow_duration_changed;
+  glib::Signal<void, GSettings*, gchar* > _minimize_fast_duration_changed;
+  int mDuration;
+};
+
+#endif // WINDOWMINIMIZESPEEDCONTROLLER_H

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2012-08-29 08:16:08 +0000
+++ plugins/unityshell/src/unityshell.cpp	2012-08-31 09:17:21 +0000
@@ -128,6 +128,7 @@
   , panel_texture_has_changed_(true)
   , paint_panel_(false)
   , scale_just_activated_(false)
+  , minimize_speed_controller(new WindowMinimizeSpeedController())
 {
   Timer timer;
 #ifndef USE_GLES
@@ -384,6 +385,10 @@
   }
 
   panel::Style::Instance().changed.connect(sigc::mem_fun(this, &UnityScreen::OnPanelStyleChanged));
+  
+  minimize_speed_controller->DurationChanged.connect(
+      sigc::mem_fun(this, &UnityScreen::OnMinimizeDurationChanged)
+  );
 }
 
 UnityScreen::~UnityScreen()
@@ -2540,6 +2545,40 @@
 }
 
 void
+UnityScreen::OnMinimizeDurationChanged ()
+{
+  /* Update the compiz plugin setting with the new computed speed so that it
+   * will be used in the following minimizations */
+  CompPlugin *p = CompPlugin::find("animation");
+  if (p)
+  {
+    CompOption::Vector &opts = p->vTable->getOptions();
+
+    for (CompOption &o : opts)
+    {
+      if (o.name () == std::string ("minimize_durations"))
+      {
+        /* minimize_durations is a list value, but minimize applies only to
+         * normal windows, so there's always one value */
+        CompOption::Value& value = o.value();
+        CompOption::Value::Vector& list = value.list();
+        CompOption::Value::Vector::iterator i = list.begin();
+        if (i != list.end()) {
+          i->set(minimize_speed_controller->getDuration());
+        }
+        value.set(list);                
+        screen->setOptionForPlugin(p->vTable->name().c_str(),
+                                   o.name().c_str(), value);
+        break;
+      }
+    }
+  }
+  else {
+    LOG_WARN(logger) << "Animation plugin not found. Can't set minimize speed.";
+  }
+}
+
+void
 UnityWindow::minimize ()
 {
   if (!window->managed ())
@@ -2655,6 +2694,11 @@
       case CompWindowNotifyBeforeDestroy:
         being_destroyed.emit();
         break;
+      case CompWindowNotifyMinimize:
+        /* Updating the count in dconf will trigger a "changed" signal to which
+         * the method setting the new animation speed is attached */
+        UnityScreen::get(screen)->minimize_speed_controller->UpdateCount();
+        break;
       default:
         break;
   }

=== modified file 'plugins/unityshell/src/unityshell.h'
--- plugins/unityshell/src/unityshell.h	2012-08-21 05:44:45 +0000
+++ plugins/unityshell/src/unityshell.h	2012-08-31 09:17:21 +0000
@@ -65,6 +65,8 @@
 
 #include "HudController.h"
 #include "ThumbnailGenerator.h"
+#include "WindowMinimizeSpeedController.h"
+
 namespace unity
 {
 
@@ -189,6 +191,8 @@
 
   void SetUpAndShowSwitcher(switcher::ShowMode show_mode = switcher::ShowMode::CURRENT_VIEWPORT);
 
+  void OnMinimizeDurationChanged();
+
   switcher::Controller::Ptr switcher_controller();
   launcher::Controller::Ptr launcher_controller();
 
@@ -242,7 +246,7 @@
   void OnPanelStyleChanged();
 
   void InitGesturesSupport();
-
+  
   nux::animation::TickSource tick_source_;
   nux::animation::AnimationController animation_controller_;
 
@@ -339,6 +343,8 @@
   UBusManager ubus_manager_;
   glib::SourceManager sources_;
   unity::ThumbnailGenerator thumb_generator;
+
+  WindowMinimizeSpeedController* minimize_speed_controller;
   
   friend class UnityWindow;
 };

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2012-08-29 08:16:08 +0000
+++ tests/CMakeLists.txt	2012-08-31 09:17:21 +0000
@@ -227,6 +227,7 @@
                  test_switcher_model.cpp
                  test_texture_cache.cpp
                  test_thumbnail_generator.cpp
+                 test_launcher_minimize_speed.cpp
                  test_volume_imp.cpp
                  test_volume_launcher_icon.cpp
                  gmockmount.c
@@ -338,6 +339,7 @@
                  ${CMAKE_SOURCE_DIR}/unity-shared/UserThumbnailProvider.cpp
                  ${CMAKE_SOURCE_DIR}/unity-shared/WindowManager.cpp
                  ${CMAKE_SOURCE_DIR}/unity-shared/ubus-server.cpp
+                 ${CMAKE_SOURCE_DIR}/plugins/unityshell/src/WindowMinimizeSpeedController.cpp
                 )
   target_link_libraries(test-gtest gtest gmock ${LIBS})
   add_test(UnityGTest test-gtest)

=== added file 'tests/test_launcher_minimize_speed.cpp'
--- tests/test_launcher_minimize_speed.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_launcher_minimize_speed.cpp	2012-08-31 09:17:21 +0000
@@ -0,0 +1,123 @@
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the  Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 3 along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>
+ *
+ * Authored by: Ugo Riboni <[e-mail]>
+ *
+ */
+
+#include <config.h>
+
+#include <gio/gio.h>
+#include <gtest/gtest.h>
+
+#include "plugins/unityshell/src/WindowMinimizeSpeedController.h"
+
+using namespace unity;
+using namespace testing;
+
+namespace
+{
+
+const gchar* SCHEMA_DIRECTORY = BUILDDIR"/settings";
+
+class TestLauncherMinimizeSpeed : public Test
+{
+public:
+  glib::Object<GSettings> mSettings;
+  WindowMinimizeSpeedController* mController;
+  
+  /* override */ void SetUp()
+  {
+    g_setenv("GSETTINGS_SCHEMA_DIR", SCHEMA_DIRECTORY, true);
+    g_setenv("GSETTINGS_BACKEND", "memory", TRUE);
+    mSettings = g_settings_new("com.canonical.Unity");
+    mController = new WindowMinimizeSpeedController();
+  }
+
+  /* override */ void TearDown()
+  {
+    g_setenv("GSETTINGS_SCHEMA_DIR", "", true);
+    g_unsetenv("GSETTINGS_BACKEND");
+    delete mController;
+  }
+};
+
+TEST_F(TestLauncherMinimizeSpeed, TestSlowest)
+{
+    g_settings_set_int(mSettings, "minimize-count", 0);
+    g_settings_set_int(mSettings, "minimize-speed-threshold", 10);
+    g_settings_set_int(mSettings, "minimize-fast-duration", 200);
+    g_settings_set_int(mSettings, "minimize-slow-duration", 1200);
+    
+    EXPECT_TRUE(mController->getDuration() == 1200);
+}
+
+TEST_F(TestLauncherMinimizeSpeed, TestFastest)
+{
+    g_settings_set_int(mSettings, "minimize-count", 10);
+    g_settings_set_int(mSettings, "minimize-speed-threshold", 10);
+    g_settings_set_int(mSettings, "minimize-fast-duration", 200);
+    g_settings_set_int(mSettings, "minimize-slow-duration", 1200);
+    
+    EXPECT_TRUE(mController->getDuration() == 200);
+}
+
+TEST_F(TestLauncherMinimizeSpeed, TestHalfway)
+{
+    g_settings_set_int(mSettings, "minimize-count", 5);
+    g_settings_set_int(mSettings, "minimize-speed-threshold", 10);
+    g_settings_set_int(mSettings, "minimize-fast-duration", 200);
+    g_settings_set_int(mSettings, "minimize-slow-duration", 1200);
+    
+    EXPECT_TRUE(mController->getDuration() == 700);
+}
+
+TEST_F(TestLauncherMinimizeSpeed, TestOvershoot)
+{
+    g_settings_set_int(mSettings, "minimize-count", 20);
+    g_settings_set_int(mSettings, "minimize-speed-threshold", 10);
+    g_settings_set_int(mSettings, "minimize-fast-duration", 200);
+    g_settings_set_int(mSettings, "minimize-slow-duration", 1200);
+    
+    EXPECT_TRUE(mController->getDuration() == 200);
+}
+
+TEST_F(TestLauncherMinimizeSpeed, TestSignal)
+{
+
+  bool signal_emitted = false;
+  mController->DurationChanged.connect([&] () {
+    signal_emitted = true;
+  });
+  
+  g_settings_set_int(mSettings, "minimize-count", 5);
+  EXPECT_TRUE(signal_emitted);
+}
+
+TEST_F(TestLauncherMinimizeSpeed, TestInvalidFastSlow)
+{
+  g_settings_set_int(mSettings, "minimize-fast-duration", 2000);
+  g_settings_set_int(mSettings, "minimize-slow-duration", 100);
+
+  bool signal_emitted = false;
+  mController->DurationChanged.connect([&] () {
+    signal_emitted = true;
+  });
+  
+  g_settings_set_int(mSettings, "minimize-count", 5);
+  EXPECT_FALSE(signal_emitted);
+}
+}
\ No newline at end of file