Merge lp:~dbarth/unity-2d/envvar-jit-flag into lp:unity-2d

[David Barth]

This is a simplified version of the jit-composite feature, using an environment variable in lieu of the dconf binding.

This is a simplified version of the jit-composite feature, using an environment variable in lieu of the dconf binding. Testing showed an issue with using dconf, where the flag is not retrieved properly. Probably due to an early dbus initialization issue with dconf-qt, but since that project has been superseded, it seemed more efficient to fallback to a simpler solution for now.

[David Barth (dbarth)]

To test, use:

UNITY2D_JIT_COMPOSITE=1 ./spread/app/unity-2d-spread

There is a debug statement at the beginning to show which mode the spread runs in.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

If you're not going to use the "jit-composite" dconf key any more, you should remove it from both places in spread/app/spreadview.cpp, and from the schema.

I would also prefer reading UNITY2D_JIT_COMPOSITE in the SpreadView constructor to set a private boolean member of the class, which can be used throughout the lifetime of the spread. Env var can't be changed during runtime, so it avoids useless repeated reads.

qDebug() needed?

review: Needs Fixing

[Gerry Boland (gerboland)]

"buggy dconf binding" <- is there a bug for this?

[lp:~dbarth/unity-2d/envvar-jit-flag]

1159. By David Barth on 2013-08-08

suppress the dconf key previously added to control the jit flag

1160. By David Barth on 2013-08-08

read UNITY2D_JIT_COMPOSITE only once

[David Barth (dbarth)]

Le 07/08/2013 17:45, Gerry Boland a écrit :
> Review: Needs Fixing
>
> If you're not going to use the "jit-composite" dconf key any more, you should remove it from both places in spread/app/spreadview.cpp, and from the schema.
>
> I would also prefer reading UNITY2D_JIT_COMPOSITE in the SpreadView constructor to set a private boolean member of the class, which can be used throughout the lifetime of the spread. Env var can't be changed during runtime, so it avoids useless repeated reads.
>
> qDebug() needed?
Fixed in the last 2 revisions.

David

[David Barth (dbarth)]

Le 07/08/2013 19:30, Gerry Boland a écrit :
> "buggy dconf binding" <- is there a bug for this?
Nope, the recommendation was just to switch to the newer bindings, which
is why I resorted to using an environment variable in the end. Cost /
opportunity trade-off.

David

[PS Jenkins bot (ps-jenkins)]

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

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

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

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

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

Sorry, I forgot about this, bad mail filter meant I missed notifications about this. Please make jenkins happy, then I'll review again.

Unmerged revisions

1160. By David Barth on 2013-08-08

read UNITY2D_JIT_COMPOSITE only once

1159. By David Barth on 2013-08-08

suppress the dconf key previously added to control the jit flag

1158. By David Barth on 2013-07-18

use a simple environment variable for the jit-composite flag, in lieu of the buggy dconf binding

=== modified file 'config.h.in'
--- config.h.in	2013-05-02 18:53:53 +0000
+++ config.h.in	2013-08-08 13:19:13 +0000
@@ -13,7 +13,6 @@
 static const char* LAUNCHER2D_DCONF_SCHEMA = "com.canonical.Unity2d.Launcher";
 static const char* PANEL2D_DCONF_SCHEMA = "com.canonical.Unity2d.Panel";
 static const char* DASH2D_DCONF_SCHEMA = "com.canonical.Unity2d.Dash";
-static const char* SPREAD2D_DCONF_SCHEMA = "com.canonical.Unity2d.Spread";
 
 static const char* UNITY_DCONF_SCHEMA = "com.canonical.Unity";
 static const char* LAUNCHER_DCONF_SCHEMA = "com.canonical.Unity.Launcher";
@@ -39,11 +38,6 @@
     return configuration;
 }
 
-inline QConf& spread2dConfiguration() {
-    static QConf configuration(SPREAD2D_DCONF_SCHEMA);
-    return configuration;
-}
-
 inline QConf& unityConfiguration() {
     static QConf configuration(UNITY_DCONF_SCHEMA);
     return configuration;

=== modified file 'data/com.canonical.Unity2d.gschema.xml'
--- data/com.canonical.Unity2d.gschema.xml	2013-04-15 17:35:36 +0000
+++ data/com.canonical.Unity2d.gschema.xml	2013-08-08 13:19:13 +0000
@@ -114,13 +114,4 @@
       </description>
     </key>
   </schema>
-  <schema path="/com/canonical/unity-2d/spread/" id="com.canonical.Unity2d.Spread" gettext-domain="unity-2d">
-    <key type="b" name="jit-composite">
-      <default>false</default>
-      <summary>Use window compositing only when needed</summary>
-      <description>
-        If this option is enabled, Unity will not force all window surfaces to be redirected permanently, but only when window previews are to requested by the Spread feature. This option saves memory in the X server and helps optimizing the rendering pipeline on low-end systems.
-      </description>
-    </key>
-  </schema>
 </schemalist>

=== modified file 'spread/app/spreadview.cpp'
--- spread/app/spreadview.cpp	2013-05-02 19:19:45 +0000
+++ spread/app/spreadview.cpp	2013-08-08 13:19:13 +0000
@@ -28,13 +28,23 @@
 #include "config.h"
 #include "compositorhelper.h"
 
+static bool getenvBool(const char* name, bool defaultValue)
+{
+    QByteArray value = qgetenv(name);
+    if (value.isEmpty()) {
+        return defaultValue;
+    }
+    return value != "0";
+}
+
 SpreadView::SpreadView(int screen)
 : Unity2DDeclarativeView()
 {
     m_screenInfo = new ScreenInfo(screen, this);
     connect(m_screenInfo, SIGNAL(panelsFreeGeometryChanged(QRect)), SLOT(fitToAvailableSpace()));
 
-    bool jitComposite = spread2dConfiguration().property("jit-composite").toBool();
+    static bool jitComposite = getenvBool("UNITY2D_JIT_COMPOSITE", false);
+
     if (CompositorHelper::instance()->isCompositeSupported()
         && ! jitComposite) {
         CompositorHelper::instance()->activateComposite();