Code review comment for lp:~compiz-team/compiz/compiz.fix_1016364

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Did some more investigation into this.

It seems like at least on nouveau, mipmap support is not to be found in any of the available fbconfigs:

found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0
found fbconfig with mipmap: 0

Here is an inline patch to get a similar kind of output.

=== modified file 'plugins/opengl/src/screen.cpp'
--- plugins/opengl/src/screen.cpp 2012-06-22 04:32:12 +0000
+++ plugins/opengl/src/screen.cpp 2012-06-26 12:09:05 +0000
@@ -523,6 +523,8 @@

      mipmap = value;

+ printf ("found fbconfig with mipmap: %i\n", mipmap);
+
      (*GL::getFBConfigAttrib) (dpy, fbConfigs[j],
           GLX_Y_INVERTED_EXT, &value);

=== modified file 'plugins/opengl/src/texture.cpp'
--- plugins/opengl/src/texture.cpp 2012-06-22 04:32:12 +0000
+++ plugins/opengl/src/texture.cpp 2012-06-26 12:06:09 +0000
@@ -150,6 +150,11 @@
 bool
 GLTexture::mipmap () const
 {
+ printf ("mipmap: %i support: %i npot %i fb %i\n", priv->mipmap,
+ priv->mipmapSupport,
+ GL::textureNonPowerOfTwo,
+ GL::fbo);
+
     return priv->mipmap &&
     priv->mipmapSupport &&
     GL::textureNonPowerOfTwo &&
@@ -202,6 +207,7 @@
  {
      if (mipmap ())
      {
+ printf ("use mipmaps\n");
   generateMipmap ();
   glTexParameteri (priv->target,
      GL_TEXTURE_MIN_FILTER,
@@ -327,6 +333,8 @@
  mipmap = false;
     }

+ printf ("bind regular texture: mipmap support: %i\n", mipmap);
+
     t->setData (target, matrix, mipmap);
     t->setGeometry (0, 0, width, height);

@@ -571,6 +579,8 @@
      return GLTexture::List ();
     }

+ printf ("bind tfp texture: mipmap support: %i fbconfig: %i\n", mipmap, config->mipmap);
+
     tex = new TfpTexture ();
     tex->setData (texTarget, matrix, mipmap);
     tex->setGeometry (0, 0, width, height);

If it also gives you a similar output, then the driver is lying about its support for mipmaps, saying they aren't supported when they are, in which case we will have to add a workaround to the workarounds plugin to force mipmaps even if the driver says it doesn't support them on the glxPixmap fbconfigs.

I don't have time to try it on nvidia right now (the sun is rising :( ), but I am pretty certain it worked there. In any case, we should always check the driver to see if it supports mipmaps, drivers have probably been working around every broken WM which doesn't properly check for mipmap support in the fbconfigs (KWin and Mutter both get it wrong too).

« Back to merge proposal