Merge lp://staging/~dbarth/unity-2d/defer-composite into lp://staging/unity-2d

FAILED: Continuous integration, rev:1157
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~dbarth/unity-2d/defer-composite/+merge/157602/+edit-commit-message

http://s-jenkins:8080/job/unity-2d-ci/31/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=amd64/31
    SUCCESS: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=i386/31

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-2d-ci/31/rebuild

Functional testing underway, but I notice in your code you are mixing tabs & spaces. Please set your editor to use 4spaces instead of a tab, and apply that change to this MR.

I just pushed a new rev. to fix the indention issues

I think you forgot to push!

What concerns me about this change is you are adding the deferred-composite mode for all unity-2d users. I need to test natively, but I suspect this adds a spread activation slowdown for those users.

What I'd prefer is adding an option to unity-2d for this deferred-composite mode - an option which is usually disabled. The normal always-on-composite should remain the default. This will effectively leave an unchanged code-path for the majority of the unity-2d users, considerably reducing the risk of breakage.

Enabling the deferred-composite mode then is only done when necessary and can be tested carefully.

I'm also not a fan of activating compositing in requestImage, and disabling after a 5 second timeout. Makes more sense to activate it before spread starts to show, and disable it immediately after the window images have all been retrieved (but re-enable if window appears while spread open). Slightly easier would be this but to disable when the spread starts to hide. You should look into spread/Workspaces.qml, the show() and cancelAndExit() functions especially.

Some comments on the code:
CompositorHelper should be a singleton class.

+/* Do NOT activate the composite mode: this disables live windows previews,
+ as used in the spread and workspace switcher views
+*/
+#define DISABLE_COMPOSITE_MODE 0
The comment confuses me, and I'm not convinced why this #define is needed here. It also confuses the API of CompositorHelper if turned on, as x11supportsComposite will be true, but activateComposite() will silently do nothing. Please rethink this, at least having activateComposite returning a bool showing success/failure.

+++ libunity-2d-private/src/windowimageprovider.h
+ static bool activateComposite(Window windowId);
This function is not defined in the class.

Here is a new version with:
- a configuration option for the feature
- integration within the stricter scope of the spread feature
- adjusted at the qml level
- with a proper singleton implementation

In short, less of a hack ;)

FAILED: Continuous integration, rev:1159
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~dbarth/unity-2d/defer-composite/+merge/157602/+edit-commit-message

http://s-jenkins:8080/job/unity-2d-ci/32/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=amd64/32/console
    FAILURE: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=i386/32/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-2d-ci/32/rebuild

FAILED: Continuous integration, rev:1160
http://s-jenkins:8080/job/unity-2d-ci/33/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=amd64/33/console
    FAILURE: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=i386/33/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-2d-ci/33/rebuild

Compile error:
spread/app/spreadview.cpp: In constructor ‘SpreadView::SpreadView(int)’:
spread/app/spreadview.cpp:37:47: error: ‘spread2dConfiguration’ was not declared in this scope
spread/app/spreadview.cpp: In member function ‘void SpreadView::unloadPreviews()’:
spread/app/spreadview.cpp:70:47: error: ‘spread2dConfiguration’ was not declared in this scope

+#include <QTimer>
not needed any more.

+++ libunity-2d-private/src/compositorhelper.h
You have some indentation problems in this header file.

+++ libunity-2d-private/src/windowimageprovider.h
+ static bool activateComposite(Window windowId);
As before, this is not defined, nor needed.

+++ spread/app/spreadview.h
You're not using the previewsAvailable property anywhere, so do you need it at all?

You remove the blank line at the end of the file, please restore.

Logic looks fine, but compile error means I cannot test.

PASSED: Continuous integration, rev:1161
http://s-jenkins:8080/job/unity-2d-ci/34/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=amd64/34
    SUCCESS: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=i386/34

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-2d-ci/34/rebuild

PASSED: Continuous integration, rev:1162
http://s-jenkins:8080/job/unity-2d-ci/35/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=amd64/35
    SUCCESS: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=i386/35

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-2d-ci/35/rebuild

Functionally is working perfectly, very good.

Code is fine now too, approving