Merge lp:~vanvugt/ubuntu/quantal/nux/fix-1039155 into lp:ubuntu/quantal/nux

[Daniel van Vugt]

Actually check the result of unity_support_test. And if it fails then fall
back to LLVMpipe, instead of leaving the user with no shell at all.
(LP: #1039155)

Test case:
  1. Simulate a machine that fails unity_support_test by replacing /usr/lib/nux/unity_support_test with a symlink to /bin/false.
  2. Log out and in again.
  3. Verify you are now using LLVMpipe:
     (a) Rendering is slower; and
     (b) glxinfo | grep OpenGL shows llvmpipe

Note: 3(b) is likely to change in future as programs spawned after Unity starts should not have LLVMpipe mode forced upon them. But that's an exercise for later.

[Didier Roche-Tolomelli (didrocks)]

8:50:34 didrocks | the checker tool isn't used anymore
8:50:44 didrocks | so the branch will do nothing and it's too late to add it
8:50:56 didrocks | last time we discussed, this checks needs to be in compiz itself
8:51:00 Mirv | aha...
8:51:21 didrocks | (so exactly the same check minus the llvmpipe one) in the opengl plugin
8:51:22 Mirv | duflu yesterday changed the bug to indicate that the problem would not be in compiz
8:51:49 didrocks | well, ok, compiz may support opengl < 1.4 and no vertex shaders
8:52:09 didrocks | but in ubuntu, we mainly use it for ubuntu and the fallback to llvmpipe need to be done before the opengl plugin
                 | load
8:52:38 didrocks | as we don't use any external tool, it would make sense to distro-patch compiz for it

review: Disapprove

[Sebastien Bacher (seb128)]

(Setting "Work In Progress" so it gets out of the sponsoring queue, it should maybe be rejected by somebody who has access to the status, doesn't seem to be my case there)

[Daniel van Vugt (vanvugt)]

Needs review again.

I know this was rejected but now it appears to be the only proper fix for bug 1066764.

[Didier Roche-Tolomelli (didrocks)]

I don't understand, we already merged a fix from you as a distro patch, where instead of using a session-wide testing, compiz is running the unity_support_test directly. We both tested that it fallbacked and force the llvmpipe code. What does this merge proposal do in addition?

review: Needs Information

[Daniel van Vugt (vanvugt)]

Yes, but the fix we landed for bug 1039155 appears to cause bug 1066764. And Colin's comments seem to suggest this will solve bug 1066764.

[Daniel van Vugt (vanvugt)]

Thought I'm happy to sit on this for a while and see if bug 1066764 actually affects more than one person in 12.10. If not then we may not need it.

[Didier Roche-Tolomelli (didrocks)]

Can you explain it a little bit more? TBH, it's really late to land as an SRU a system-wide script launched at every login without understanding why exactly it would fix this issue.
You just run it a little bit later in the process, but compiz is doing the same with the distro-patch, isn't it? it's running on its core the unity_support_test tool and getting the result. If it's =1, LIBGL_ALWAYS_SOFTWARE=1 is exported. What is different, how would some people won't be fallbacked with the current patch and will be there?

[Daniel van Vugt (vanvugt)]

It appears the problem:
  https://bugs.launchpad.net/ubuntu/+source/compiz/+bug/1066764/comments/1
is to do with running unity_support_test from _within_ compiz. Colin Law's machine has problems with that.

However Colin also points out in bug 1066764 and bug 1039155 that setting the variable before compiz starts (same as this fix) works for him.

Don't shoot the messenger. It's not my bug and I'm happy to ignore it if no one else is affected. I can't reproduce it either.

[Didier Roche-Tolomelli (didrocks)]

That's really weird that the message is appearing *after* the other plugins are loaded as the system() call is synced, and so blocking…

Let's see if other issues happen as well, I would like that we understand clearly what's happening rather than jumping on the big hammer solution.

[Daniel van Vugt (vanvugt)]

Not Work In Progress. That would imply I'm working on it when I'm not.

If this proposal is unacceptable, please reject instead.

[Didier Roche-Tolomelli (didrocks)]

@Daniel: we already used your fix in compiz itself which does something similar, right? Please, do not change that back to "Needs review".

There is a persmission issue in launchpad where reviewers for ubuntu branches can't reject the branch, only the release team can. So please, reject the branch yourself, because the branch is *again* on the sponsoring list where it shouldn't.

review: Disapprove

[Daniel van Vugt (vanvugt)]

I don't have permission to reject it either.

Unmerged revisions

369. By Daniel van Vugt on 2012-10-08

Actually check the result of unity_support_test. And if it fails then fall
back to LLVMpipe, instead of leaving the user with no shell at all.
(LP: #1039155)

=== added file 'debian/50_check_unity_support'
--- debian/50_check_unity_support	1970-01-01 00:00:00 +0000
+++ debian/50_check_unity_support	2012-10-08 06:11:21 +0000
@@ -0,0 +1,8 @@
+# This file is sourced by Xsession(5), not executed.
+# If the hardware does not pass unity_support_test, fall back to LLVMpipe
+# which does.
+
+if [ "x$DESKTOP_SESSION" = "xubuntu" ]; then
+    /usr/lib/nux/unity_support_test || export LIBGL_ALWAYS_SOFTWARE=1
+fi
+

=== modified file 'debian/nux-tools.install'
--- debian/nux-tools.install	2012-08-15 06:31:46 +0000
+++ debian/nux-tools.install	2012-10-08 06:11:21 +0000
@@ -1,2 +1,3 @@
 debian/tmp/usr/lib/nux/unity_support_test
+debian/50_check_unity_support /etc/X11/Xsession.d
 debian/source_nux.py usr/share/apport/package-hooks

=== removed file 'debian/nux-tools.maintscript'
--- debian/nux-tools.maintscript	2012-08-15 12:42:57 +0000
+++ debian/nux-tools.maintscript	1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
-rm_conffile /etc/X11/Xsession.d/50_check_unity_support 3.2.0-0ubuntu3~

=== removed file 'debian/patches/01_blacklist_llvmpipe.patch'
--- debian/patches/01_blacklist_llvmpipe.patch	2012-02-27 10:23:10 +0000
+++ debian/patches/01_blacklist_llvmpipe.patch	1970-01-01 00:00:00 +0000
@@ -1,12 +0,0 @@
-Index: ubuntu/tools/unity_support_test.c
-===================================================================
---- ubuntu.orig/tools/unity_support_test.c	2012-01-13 09:01:58.714180000 +0100
-+++ ubuntu/tools/unity_support_test.c	2012-02-27 11:16:53.435617058 +0100
-@@ -587,6 +587,7 @@
-   if (results->renderer != NULL &&
-       (strncmp (results->renderer, "Software Rasterizer", 19) == 0 ||
-        strncmp (results->renderer, "Mesa X11", 8) == 0 ||
-+       strstr (results->renderer, "llvmpipe") != NULL ||
-        strstr (results->renderer, "on softpipe") != NULL)) {
-     results->flags |= FLAG_SOFTWARE_RENDERING;
-   }

=== modified file 'debian/patches/series'
--- debian/patches/series	2012-08-01 16:09:01 +0000
+++ debian/patches/series	2012-10-08 06:11:21 +0000
@@ -1,1 +0,0 @@
-01_blacklist_llvmpipe.patch