Merge lp:~vanvugt/compiz/blacklist into lp:compiz/0.9.9

[Daniel van Vugt]

Add a feature for blacklisting certain graphics drivers from being able to
unredirect fullscreen windows. Right now the only known broken drivers are
nouveau and intel on precise (Mesa 8.0). But the blacklist is a regex that
can be adjusted at any time.
(LP: #1089246)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3525
http://jenkins.qa.ubuntu.com/job/compiz-ci/320/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/320/console

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/320//rebuild/?

review: Approve (continuous-integration)

[Timo Jyrinki (timo-jyrinki)]

Looks, builds and functions fine. My quantal Intel wasn't blacklisted, but I was able to blacklist it by modifying the default regexp.

review: Approve

[Michael Terry (mterry)]

Will it always be obvious what "blacklisted" means? Right now it seems it is specific to one feature. We might get a second feature in the future that needs a separate blacklist list. Just saying that maybe the function name should be something like blacklisted_for_unredirect(). But I don't feel strongly about it.

[Daniel van Vugt (vanvugt)]

Michael makes a good point and I did think about that. However it's possible and even likely to never be a problem for the lifetime of the Compiz code.

[Daniel van Vugt (vanvugt)]

Although, the "blacklisted" function is generic. It is specific to OpenGL drivers but the same function could be applied to other features; not just "unredirected fullscreen windows". So a generic name is appropriate.

=== modified file 'plugins/opengl/CMakeLists.txt'
--- plugins/opengl/CMakeLists.txt	2012-11-08 09:18:59 +0000
+++ plugins/opengl/CMakeLists.txt	2012-12-12 07:55:25 +0000
@@ -5,11 +5,13 @@
 set (INTERNAL_LIBRARIES
     compiz_opengl_double_buffer
     compiz_opengl_fsregion
+    compiz_opengl_blacklist
     compiz_opengl_glx_tfp_bind
 )
 
 add_subdirectory (src/doublebuffer)
 add_subdirectory (src/fsregion)
+add_subdirectory (src/blacklist)
 add_subdirectory (src/glxtfpbind)
 
 include_directories (src/glxtfpbind/include)
 
=== modified file 'plugins/opengl/opengl.xml.in'
--- plugins/opengl/opengl.xml.in	2012-10-15 10:31:51 +0000
+++ plugins/opengl/opengl.xml.in	2012-12-12 07:55:25 +0000
@@ -59,6 +59,11 @@
 		<_long>Use glXSwapBuffers to display every frame. This eliminates visible tearing with most drivers and dramatically improves visual smoothness. Automatically enabled when framebuffer_object is on.</_long>
 		<default>true</default>
 	    </option>
+	    <option name="unredirect_driver_blacklist" type="string">
+		<_short>Unredirect Driver Blacklist</_short>
+		<_long>If non-empty, specifies a POSIX (extended) regular expression to match against the OpenGL driver strings (newline separated): "GL_VENDOR\nGL_RENDERER\nGL_VERSION". If the regular expression matches a substring of that concatenation then no windows will ever be unredirected while using that particular graphics driver.</_long>
+		<default>(nouveau|Intel).*Mesa 8\\.0</default>
+	    </option>
 	</options>
     </plugin>
 </compiz>
 
=== added directory 'plugins/opengl/src/blacklist'
=== added file 'plugins/opengl/src/blacklist/CMakeLists.txt'
--- plugins/opengl/src/blacklist/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/blacklist/CMakeLists.txt	2012-12-12 07:55:25 +0000
@@ -0,0 +1,6 @@
+if (COMPIZ_BUILD_TESTING)
+add_subdirectory (tests)
+endif ()
+
+add_library (compiz_opengl_blacklist STATIC blacklist.cpp)
+
 
=== added file 'plugins/opengl/src/blacklist/blacklist.cpp'
--- plugins/opengl/src/blacklist/blacklist.cpp	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/blacklist/blacklist.cpp	2012-12-12 07:55:25 +0000
@@ -0,0 +1,67 @@
+/*
+ * Compiz opengl plugin, Blacklist feature
+ *
+ * Copyright (c) 2012 Canonical Ltd.
+ * Author: Daniel van Vugt <[e-mail]>
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include "blacklist.h"
+#include <cstdio>
+#include <regex.h>
+
+namespace compiz {
+namespace opengl {
+
+bool blacklisted (const char *blacklistRegex, const char *glVendor,
+                  const char *glRenderer, const char *glVersion)
+{
+    bool matches = false;
+
+    if (blacklistRegex && blacklistRegex[0])
+    {
+	regex_t re;
+
+	// Ensure the regex contains something other than spaces, or ignore.
+	const char *p = blacklistRegex;
+	while (*p == ' ')
+	    p++;
+
+	if (*p && !regcomp (&re, blacklistRegex, REG_EXTENDED))
+	{
+	    char driver[1024];
+
+	    snprintf (driver, sizeof driver, "%s\n%s\n%s",
+	              glVendor ?   glVendor   : "",
+	              glRenderer ? glRenderer : "",
+	              glVersion ?  glVersion  : "");
+
+	    if (!regexec (&re, driver, 0, NULL, 0))
+		matches = true;
+
+	    regfree (&re);
+	}
+    }
+
+    return matches;
+}
+
+} // namespace opengl
+} // namespace compiz
 
=== added file 'plugins/opengl/src/blacklist/blacklist.h'
--- plugins/opengl/src/blacklist/blacklist.h	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/blacklist/blacklist.h	2012-12-12 07:55:25 +0000
@@ -0,0 +1,38 @@
+/*
+ * Compiz opengl plugin, Blacklist function
+ *
+ * Copyright (c) 2012 Canonical Ltd.
+ * Author: Daniel van Vugt <[e-mail]>
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __COMPIZ_OPENGL_BLACKLIST_H
+#define __COMPIZ_OPENGL_BLACKLIST_H
+
+namespace compiz {
+namespace opengl {
+
+bool blacklisted (const char *blacklistRegex, const char *glVendor,
+                  const char *glRenderer, const char *glVersion);
+
+} // namespace opengl
+} // namespace compiz
+
+#endif
 
=== added directory 'plugins/opengl/src/blacklist/tests'
=== added file 'plugins/opengl/src/blacklist/tests/CMakeLists.txt'
--- plugins/opengl/src/blacklist/tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/blacklist/tests/CMakeLists.txt	2012-12-12 07:55:25 +0000
@@ -0,0 +1,9 @@
+include_directories (${GTEST_INCLUDE_DIRS} ..)
+set (exe "compiz_opengl_test_blacklist")
+add_executable (${exe} test-blacklist.cpp)
+target_link_libraries (${exe}
+    compiz_opengl_blacklist
+    ${GTEST_BOTH_LIBRARIES}
+)
+compiz_discover_tests(${exe} COVERAGE compiz_opengl_blacklist)
+
 
=== added file 'plugins/opengl/src/blacklist/tests/test-blacklist.cpp'
--- plugins/opengl/src/blacklist/tests/test-blacklist.cpp	1970-01-01 00:00:00 +0000
+++ plugins/opengl/src/blacklist/tests/test-blacklist.cpp	2012-12-12 07:55:25 +0000
@@ -0,0 +1,152 @@
+/*
+ * Compiz opengl plugin, Backlist feature
+ *
+ * Copyright (c) 2012 Canonical Ltd.
+ * Author: Daniel van Vugt <[e-mail]>
+ * 
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ * 
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ * 
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include "gtest/gtest.h"
+#include "blacklist.h"
+
+using namespace compiz::opengl;
+
+static const char *recommendedRegex = "(nouveau|Intel).*Mesa 8\\.0";
+
+TEST (DriverBlacklist, QuantalIntelIsGood)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "Intel Open Source Technology Center",
+                               "Mesa DRI Intel(R) Sandybridge Desktop",
+                               "3.0 Mesa 9.0"));
+}
+
+TEST (DriverBlacklist, PreciseIntelIsBad)
+{
+    EXPECT_TRUE  (blacklisted (recommendedRegex,
+                               "Tungsten Graphics, Inc",
+                               "Mesa DRI Intel(R) Sandybridge Desktop",
+                               "3.0 Mesa 8.0.2"));
+}
+
+TEST (DriverBlacklist, QuantalNouveauIsGood)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "nouveau",
+                               "Gallium 0.4 on NV86",
+                               "3.0 Mesa 9.0-devel"));
+}
+
+TEST (DriverBlacklist, PreciseNouveauIsBad)
+{
+    EXPECT_TRUE  (blacklisted (recommendedRegex,
+                               "nouveau",
+                               "Gallium 0.4 on NVA8",
+                               "2.1 Mesa 8.0.2"));
+}
+
+TEST (DriverBlacklist, FglrxIsGood)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "Advanced Micro Devices, Inc.",
+                               "ATI Radeon HD 5450",
+                               "4.2.11627 Compatibility Profile Context"));
+}
+
+TEST (DriverBlacklist, NvidiaIsGood)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "NVIDIA Corporation",
+                               "Quadro 1000M/PCIe/SSE2",
+                               "4.2.0 NVIDIA 304.48"));
+}
+
+TEST (DriverBlacklist, RadeonIsGood1)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "X.Org R300 Project",
+                               "Gallium 0.4 on ATI RV350",
+                               "2.1 Mesa 8.0.2"));
+}
+
+TEST (DriverBlacklist, RadeonIsGood2)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "X.Org",
+                               "Gallium 0.4 on AMD CEDAR",
+                               "2.1 Mesa 8.0.3"));
+}
+
+TEST (DriverBlacklist, RadeonIsGood3)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "X.Org",
+                               "Gallium 0.4 on AMD RS880",
+                               "2.1 Mesa 8.0.2"));
+}
+
+TEST (DriverBlacklist, LLVMpipeIsGood)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "VMware, Inc.",
+                               "Gallium 0.4 on llvmpipe (LLVM 0x300)",
+                               "2.1 Mesa 8.0.4"));
+}
+
+TEST (DriverBlacklist, UnknownIsGood)
+{
+    EXPECT_FALSE (blacklisted (recommendedRegex,
+                               "Acme",
+                               "Graphics Driver",
+                               "4.2 8.0 9.0 123.456"));
+}
+
+TEST (DriverBlacklist, NoBlacklist)
+{
+    EXPECT_FALSE (blacklisted ("",
+                               "Tungsten Graphics, Inc",
+                               "Mesa DRI Intel(R) Sandybridge Desktop",
+                               "3.0 Mesa 8.0.2"));
+    EXPECT_FALSE (blacklisted ("", "foo", "bar", "blah"));
+    EXPECT_FALSE (blacklisted ("", "", "", ""));
+}
+
+TEST (DriverBlacklist, LineContinuation)
+{
+    EXPECT_FALSE (blacklisted ("alpha",       "beta", "gamma", "delta"));
+    EXPECT_FALSE (blacklisted ("betagam",     "beta", "gamma", "delta"));
+    EXPECT_TRUE  (blacklisted ("gamma",       "beta", "gamma", "delta"));
+    EXPECT_TRUE  (blacklisted ("del",         "beta", "gamma", "delta"));
+    EXPECT_TRUE  (blacklisted ("(mag|gam)",   "beta", "gamma", "delta"));
+    EXPECT_TRUE  (blacklisted ("beta.*delt",  "beta", "gamma", "delta"));
+    EXPECT_FALSE (blacklisted ("beta.*felt",  "beta", "gamma", "delta"));
+
+    EXPECT_TRUE  (blacklisted ("beta\ngamma\ndelta", "beta", "gamma", "delta"));
+}
+
+TEST (DriverBlacklist, StraySpaces)
+{
+    EXPECT_FALSE (blacklisted (" ", "Hello world", "and", "goodbye"));
+    EXPECT_FALSE (blacklisted ("  ", " ", "  ", "    "));
+    EXPECT_FALSE (blacklisted (" ",
+                               "Tungsten Graphics, Inc",
+                               "Mesa DRI Intel(R) Sandybridge Desktop",
+                               "3.0 Mesa 8.0.2"));
+}
 
=== modified file 'plugins/opengl/src/paint.cpp'
--- plugins/opengl/src/paint.cpp	2012-12-12 04:29:49 +0000
+++ plugins/opengl/src/paint.cpp	2012-12-12 07:55:25 +0000
@@ -261,6 +261,11 @@
     const CompMatch &unredirectable = CompositeScreen::get (screen)->
 	getOption ("unredirect_match")->value ().match ();
 
+    const CompString &blacklist =
+	getOption ("unredirect_driver_blacklist")->value ().s ();
+
+    bool blacklisted = driverIsBlacklisted (blacklist.c_str ());
+
     if (mask & PAINT_SCREEN_TRANSFORMED_MASK)
     {
 	windowMask     = PAINT_WINDOW_ON_TRANSFORMED_SCREEN_MASK;
@@ -354,6 +359,7 @@
 	     * beneath them and so neither should be unredirected in that case.
 	     */
 	    if (unredirectFS &&
+		!blacklisted &&
 		unredirectable.evaluate (w) &&
 		!(mask & PAINT_SCREEN_TRANSFORMED_MASK) &&
 		!(mask & PAINT_SCREEN_WITH_TRANSFORMED_WINDOWS_MASK) &&
 
=== modified file 'plugins/opengl/src/privates.h'
--- plugins/opengl/src/privates.h	2012-12-07 08:09:56 +0000
+++ plugins/opengl/src/privates.h	2012-12-12 07:55:25 +0000
@@ -161,6 +161,8 @@
 
 	void updateView ();
 
+	bool driverIsBlacklisted (const char *regex) const;
+
     public:
 
 	GLScreen        *gScreen;
@@ -219,6 +221,8 @@
 
 	Pixmap rootPixmapCopy;
 	CompSize rootPixmapSize;
+
+	const char *glVendor, *glRenderer, *glVersion;
 };
 
 class PrivateGLWindow :
 
=== modified file 'plugins/opengl/src/screen.cpp'
--- plugins/opengl/src/screen.cpp	2012-12-10 03:28:47 +0000
+++ plugins/opengl/src/screen.cpp	2012-12-12 07:55:25 +0000
@@ -36,6 +36,7 @@
 #include <boost/make_shared.hpp>
 
 #include "privates.h"
+#include "blacklist/blacklist.h"
 
 #include <dlfcn.h>
 #include <math.h>
@@ -600,8 +601,6 @@
     GLfloat		 ambientLight[]   = { 0.0f, 0.0f,  0.0f, 0.0f };
     GLfloat		 diffuseLight[]   = { 0.9f, 0.9f,  0.9f, 0.9f };
     GLfloat		 light0Position[] = { -0.5f, 0.5f, -9.0f, 1.0f };
-    const char           *glRenderer;
-    const char           *glVendor;
     CompOption::Vector o (0);
 
     priv->ctx = glXCreateContext (dpy, visinfo, NULL, True);
@@ -640,8 +639,14 @@
 	return false;
     }
 
-    glRenderer = (const char *) glGetString (GL_RENDERER);
-    glVendor = (const char *) glGetString (GL_VENDOR);
+    const char *glVendor = (const char *) glGetString (GL_VENDOR);
+    const char *glRenderer = (const char *) glGetString (GL_RENDERER);
+    const char *glVersion = (const char *) glGetString (GL_VERSION);
+
+    priv->glVendor = glVendor;
+    priv->glRenderer = glRenderer;
+    priv->glVersion = glVersion;
+
     if (glRenderer != NULL &&
 	(strcmp (glRenderer, "Software Rasterizer") == 0 ||
 	 strcmp (glRenderer, "Mesa X11") == 0))
@@ -1240,7 +1245,10 @@
     shaderCache (),
     autoProgram (new GLScreenAutoProgram(gs)),
     rootPixmapCopy (None),
-    rootPixmapSize ()
+    rootPixmapSize (),
+    glVendor (NULL),
+    glRenderer (NULL),
+    glVersion (NULL)
 {
     ScreenInterface::setHandler (screen);
 }
@@ -2144,6 +2152,12 @@
 	CompositeScreen::get (screen)->damageScreen ();
 }
 
+bool
+PrivateGLScreen::driverIsBlacklisted (const char *regex) const
+{
+    return blacklisted (regex, glVendor, glRenderer, glVersion);
+}
+
 GLTexture::BindPixmapHandle
 GLScreen::registerBindPixmap (GLTexture::BindPixmapProc proc)
 {