Code review comment for lp://staging/~dbarth/unity-2d/defer-composite

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.