Merge lp:~macslow/unity/unity.fix-863230 into lp:unity

[Mirco Müller]

Trying to understand how and where changes to an icons alpha- and saturation-value are taking place is material to get insane. By now I've spent many many many hours trying to get behind what's going on. I can't say I fully grasp
the intricate ways of icons opacity and saturation in class Launcher.

As far as I can tell, there are four valid "cases" for the launcher icons in terms of alpha and saturation. To make it easier for you to follow, have a look at this table:

                                            alpha | saturation
-------------------------------------------------------------------
| dash open | 0.5 | 0.0 |
| dash closed | 1.0 | 1.0 |
| drop-target (dragging & dash open) | 1.0 | 1.0 |
| no drop-target (dragging & dash open) | 0.5 | 0.0 |
-------------------------------------------------------------------

So alpha should never be outside [0.5 .. 1.0] and saturation should never be outside [0.0 .. 1.0]. From my investigation it is sufficient to protect only against the lower threshold, where alpha needs to be 0.5 and saturation 0.0

without fix:
http://people.canonical.com/~mmueller/863230-issue-1.ogv

with fix:
http://people.canonical.com/~mmueller/after-fix-863230.ogv

If anybody ever causes this to regress, I'll ... !

[Mirco Müller (macslow)]

Screw LP text-formatting of merge-proposal messages!

[Tim Penhey (thumper)]

This sounds like a classic use-case for a Clamp.

See tests/test_properties.cpp in nux.

If we changed alpha and saturation to be properties with a clamp, you wouldn't have to manually check them.

However that would change all uses of RenderArg, and I'm not sure if that is desirable.

Due to the visual nature of this, I accept that we can't have automated tests for it.

We do however want design signoff, so getting John to look.

review: Approve

[Alex Launi (alexlauni)]

Actually Tim, we should be able to export the alpha and saturation values via introspection, and check them at various interaction states. However they're just values, a test like that doesn't seem appropriate.

[Mirco Müller (macslow)]

Setting it to approved after catch up with Tim, yesterday.

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/435/console reported an error when processing this lp:~macslow/unity/unity.fix-863230 branch.
Not merging it.

[John Lea (johnlea)]

If you watch the Chromium icon in the video, it's tinting is changing at a time when it should remain static. Look at the Chromium icon at 0:06 in the video, this is the correct state. Now if you scrub back to 0:04, you will see the tinting on the Chromium icon has changed slightly, even though it is not a valid drop target.

Requested fix in order for this change to be complete and ready to land: When a user starts dragging a icon in the Dash, the tinting of Launcher icons that are not valid drop targets should not change in *any* way, even temporally.

review: Needs Fixing (design)

=== added file 'manual-tests/DragDropDashLauncher.txt'
--- manual-tests/DragDropDashLauncher.txt	1970-01-01 00:00:00 +0000
+++ manual-tests/DragDropDashLauncher.txt	2012-02-29 21:18:19 +0000
@@ -0,0 +1,6 @@
+- open Dash
+- go to Files-lens
+- select and drag a document-file (e.g. text or image) to the launcher
+- as you drag the document-file watch the drag-targets (drop-recepticals) saturate
+- this saturation-process should be smooth and without flicker
+- the tinting of the non-drag-targets should not change

=== modified file 'plugins/unityshell/src/Launcher.cpp'
--- plugins/unityshell/src/Launcher.cpp	2012-02-27 22:53:53 +0000
+++ plugins/unityshell/src/Launcher.cpp	2012-02-29 21:18:19 +0000
@@ -899,6 +899,13 @@
                             icon->GetIconType() == AbstractLauncherIcon::TYPE_DEVICE  ||
                             icon->GetIconType() == AbstractLauncherIcon::TYPE_EXPO;
 
+  // trying to protect against flickering when icon is dragged from dash LP: #863230
+  if (arg.alpha < 0.5)
+  {
+    arg.alpha = 0.5;
+    arg.saturation = 0.0;
+  }
+
   if (_dash_is_open)
     arg.active_arrow = icon->GetIconType() == AbstractLauncherIcon::TYPE_HOME;
   else
@@ -978,6 +985,13 @@
   if (drop_dim_value < 1.0f)
     arg.alpha *= drop_dim_value;
 
+  // trying to protect against flickering when icon is dragged from dash LP: #863230
+  if (arg.alpha < 0.5)
+  {
+    arg.alpha = 0.5;
+    arg.saturation = 0.0;
+  }
+
   if (icon == _drag_icon)
   {
     if (MouseBeyondDragThreshold())