Merge lp:~sil2100/compiz-core/scale_all-0.9.7 into lp:compiz-core

[Łukasz Zemczak]

Add support for initiating window picker in other than nomal mode. For now added only the additional 'All windows' picker, essentially fixing LP: #933776.
The code from the all mode comes from a patch made by Doug McMahon. Thanks!

Submitting the fix first to 0.9.7 because I had the branch prepared and I want to go to sleep ASAP. But still, I would like some comment on this way of fixing the bug.

The scale plugin has many actions defined: initiate for current workspace, for all windows, for groups and for current output device. But the scale plugin only supports one handler for all actions (the type of the action is not considered at all). For now I added a distinction between ScaleTypeAll and ScaleTypeNormal. I took the code proposed by Doug in the bug report as the code for ScaleTypeAll.

Would be nice to have this SRUed.

I browsed through the commits, but I never really saw any type-distinct action code. But I might have missed that, since it's REALLY busy here in London!lp:~sil2100/compiz-core/scale_all-0.9.7

[Łukasz Zemczak (sil2100)]

Will add the trunk version in the morning, if this one is alright.

[Sam Spilsbury (smspillaz)]

Looks fine to me. Are you willing to write tests for it?

review: Approve

[Daniel van Vugt (vanvugt)]

I'm going to say Needs fixing, just because there is no fix upstream yet :)

Also, I strongly recommend investigating the other scale regressions and starting by simply reverting the code that caused the regressions from oneiric to precise. We will fix more bugs, faster and more reliably. Then we can re-implement whatever was reverted more safely.

If we accept bespoke fixes like this, then fixing the other scale regressions in precise becomes harder.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

> I'm going to say Needs fixing, just because there is no fix upstream yet :)
>
> Also, I strongly recommend investigating the other scale regressions and
> starting by simply reverting the code that caused the regressions

This will inherently "cause" regressions by the fact that you just reverted important bugfixes. It does not take into account the waste of developer time as they are chasing up old code that they probably don't even remember writing.

If you want to fix things safely, analyse how the code works, analyse what its meant to do, write unit tests to ensure that it matches the required behaviour and implement your fixes. I don't revert things simply because I don't understand them - I learn the code by making it better and writing tests to ensure that it does what I think it does.

Lucasz - can you propose this fix to lp:compiz and then resubmit it here? Thanks for taking the time to look into this.

[Daniel van Vugt (vanvugt)]

Approved for upstream today. So 0.9.7 is OK too.

This seems to solve 2 of the 3 scale regressions. And the other one, we found last week, was actually a unity bug.

review: Approve

=== modified file 'plugins/scale/src/privates.h'
--- plugins/scale/src/privates.h	2012-02-01 17:49:07 +0000
+++ plugins/scale/src/privates.h	2012-06-21 23:21:19 +0000
@@ -63,6 +63,8 @@
 	void findBestSlots ();
 	bool fillInWindows ();
 	bool layoutThumbs ();
+	bool layoutThumbsAll ();
+	bool layoutThumbsSingle ();
 
 	SlotArea::vector getSlotAreas ();
 
 
=== modified file 'plugins/scale/src/scale.cpp'
--- plugins/scale/src/scale.cpp	2012-04-01 07:34:59 +0000
+++ plugins/scale/src/scale.cpp	2012-06-21 23:21:19 +0000
@@ -684,6 +684,47 @@
 bool
 PrivateScaleScreen::layoutThumbs ()
 {
+    switch (type) {
+	case ScaleTypeAll:
+	    return layoutThumbsAll ();
+	case ScaleTypeNormal:
+	default:
+	    return layoutThumbsSingle ();
+    }
+}
+
+bool
+PrivateScaleScreen::layoutThumbsAll ()
+{
+    windows.clear ();
+
+    /* add windows scale list, top most window first */
+    foreach (CompWindow *w, screen->windows ())
+    {
+	SCALE_WINDOW (w);
+
+	if (sw->priv->slot)
+	    sw->priv->adjust = true;
+
+	sw->priv->slot = NULL;
+
+	if (!sw->priv->isScaleWin ())
+            continue;
+
+	windows.push_back (sw);
+    }
+
+    if (windows.empty ())
+	return false;
+
+    slots.resize (windows.size ());
+
+    return ScaleScreen::get (screen)->layoutSlotsAndAssignWindows ();
+}
+
+bool
+PrivateScaleScreen::layoutThumbsSingle ()
+{
     bool ret = false;
     std::map <ScaleWindow *, ScaleSlot> slotWindows;
     CompWindowList          allWindows;