move the "copy pixels" code into its own function (it's used twice)

glxbackend.cpp

@@ -391,54 +391,13 @@ void GlxBackend::present()
             // convert to OpenGL coordinates
             int y = displayHeight() - r.y() - r.height();
             glXCopySubBuffer(display(), glxWindow, r.x(), y, r.width(), r.height());
-         }
-    } else { // Copy Pixels
-        // if a shader is bound or the texture unit is enabled, copy pixels results in a black screen
-        // therefore unbind the shader and restore after copying the pixels
-        GLint shader = 0;
-        if (ShaderManager::instance()->isShaderBound()) {
-            glGetIntegerv(GL_CURRENT_PROGRAM, &shader);
-            glUseProgram(0);
         }
-        bool reenableTexUnit = false;
+    } else { // Copy Pixels (horribly slow on Mesa)
-        if (glIsEnabled(GL_TEXTURE_2D)) {
-            glDisable(GL_TEXTURE_2D);
-            reenableTexUnit = true;
-        }
-        // no idea why glScissor() is used, but Compiz has it and it doesn't seem to hurt
-        glEnable(GL_SCISSOR_TEST);
         glDrawBuffer(GL_FRONT);
         waitSync();
-        int xpos = 0;
+        SceneOpenGL::copyPixels(lastDamage());
-        int ypos = 0;
-        foreach (const QRect & r, lastDamage().rects()) {
-            // convert to OpenGL coordinates
-            int y = displayHeight() - r.y() - r.height();
-            // Move raster position relatively using glBitmap() rather
-            // than using glRasterPos2f() - the latter causes drawing
-            // artefacts at the bottom screen edge with some gfx cards
-            //glRasterPos2f( r.x(), r.y() + r.height());
-            glBitmap(0, 0, 0, 0, r.x() - xpos, y - ypos, NULL);
-            xpos = r.x();
-            ypos = y;
-            glScissor(r.x(), y, r.width(), r.height());
-            glCopyPixels(r.x(), y, r.width(), r.height(), GL_COLOR);
-        }
-        glBitmap(0, 0, 0, 0, -xpos, -ypos, NULL);   // move position back to 0,0
         glDrawBuffer(GL_BACK);
-        glDisable(GL_SCISSOR_TEST);
-        if (reenableTexUnit) {
-            glEnable(GL_TEXTURE_2D);
-        }
-        // rebind previously bound shader
-        if (ShaderManager::instance()->isShaderBound()) {
-            glUseProgram(shader);
-        }
     }
-
-    glXWaitGL();
-    setLastDamage(QRegion());
-    XFlush(display());
 }
 
 void GlxBackend::screenGeometryChanged(const QSize &size)

scene_opengl.cpp

@@ -254,6 +254,46 @@ bool SceneOpenGL::initFailed() const
     return !init_ok;
 }
 
+#ifndef KWIN_HAVE_OPENGLES
+void SceneOpenGL::copyPixels(const QRegion &region)
+{
+    GLint shader = 0;
+    if (ShaderManager::instance()->isShaderBound()) {
+        glGetIntegerv(GL_CURRENT_PROGRAM, &shader);
+        glUseProgram(0);
+    }
+    bool reenableTexUnit = false;
+    if (glIsEnabled(GL_TEXTURE_2D)) {
+        glDisable(GL_TEXTURE_2D);
+        reenableTexUnit = true;
+    }
+    // no idea why glScissor() is used, but Compiz has it and it doesn't seem to hurt
+    glEnable(GL_SCISSOR_TEST);
+
+    int xpos = 0;
+    int ypos = 0;
+    foreach (const QRect &r, region.rects()) {
+        // convert to OpenGL coordinates
+        int y = displayHeight() - r.y() - r.height();
+        glBitmap(0, 0, 0, 0, r.x() - xpos, y - ypos, NULL); // not glRasterPos2f, see glxbackend.cpp
+        xpos = r.x();
+        ypos = y;
+        glScissor(r.x(), y, r.width(), r.height());
+        glCopyPixels(r.x(), y, r.width(), r.height(), GL_COLOR);
+    }
+
+    glBitmap(0, 0, 0, 0, -xpos, -ypos, NULL); // move position back to 0,0
+    glDisable(GL_SCISSOR_TEST);
+    if (reenableTexUnit) {
+        glEnable(GL_TEXTURE_2D);
+    }
+    // rebind previously bound shader
+    if (ShaderManager::instance()->isShaderBound()) {
+        glUseProgram(shader);
+    }
+}
+#endif
+
 int SceneOpenGL::paint(QRegion damage, ToplevelList toplevels)
 {
     // actually paint the frame, flushed with the NEXT frame
@@ -269,48 +309,14 @@ int SceneOpenGL::paint(QRegion damage, ToplevelList toplevels)
     checkGLError("Paint1");
 #endif
 
-    const QRegion displayRegion(0, 0, displayWidth(), displayHeight());
     paintScreen(&mask, &damage);   // call generic implementation
 #ifndef KWIN_HAVE_OPENGLES
+    const QRegion displayRegion(0, 0, displayWidth(), displayHeight());
     // copy dirty parts from front to backbuffer
     if (options->glPreferBufferSwap() == Options::CopyFrontBuffer && damage != displayRegion) {
-        GLint shader = 0;
-        if (ShaderManager::instance()->isShaderBound()) {
-            glGetIntegerv(GL_CURRENT_PROGRAM, &shader);
-            glUseProgram(0);
-        }
-        bool reenableTexUnit = false;
-        if (glIsEnabled(GL_TEXTURE_2D)) {
-            glDisable(GL_TEXTURE_2D);
-            reenableTexUnit = true;
-        }
-        // no idea why glScissor() is used, but Compiz has it and it doesn't seem to hurt
-        glEnable(GL_SCISSOR_TEST);
         glReadBuffer(GL_FRONT);
-
+        copyPixels(displayRegion - damage);
-        int xpos = 0;
-        int ypos = 0;
-        const QRegion dirty = displayRegion - damage;
-        foreach (const QRect &r, dirty.rects()) {
-            // convert to OpenGL coordinates
-            int y = displayHeight() - r.y() - r.height();
-            glBitmap(0, 0, 0, 0, r.x() - xpos, y - ypos, NULL); // not glRasterPos2f, see glxbackend.cpp
-            xpos = r.x();
-            ypos = y;
-            glScissor(r.x(), y, r.width(), r.height());
-            glCopyPixels(r.x(), y, r.width(), r.height(), GL_COLOR);
-        }
-
-        glBitmap(0, 0, 0, 0, -xpos, -ypos, NULL); // move position back to 0,0
         glReadBuffer(GL_BACK);
-        glDisable(GL_SCISSOR_TEST);
-        if (reenableTexUnit) {
-            glEnable(GL_TEXTURE_2D);
-        }
-        // rebind previously bound shader
-        if (ShaderManager::instance()->isShaderBound()) {
-            glUseProgram(shader);
-        }
         damage = displayRegion;
     }
 #endif

scene_opengl.h

@@ -63,6 +63,13 @@ public:
     Texture *createTexture();
     Texture *createTexture(const QPixmap& pix, GLenum target = GL_TEXTURE_2D);
 
+#ifndef KWIN_HAVE_OPENGLES
+    /**
+     * Copy a region of pixels from the current read to the current draw buffer
+     */
+    static void copyPixels(const QRegion &region);
+#endif
+
     static SceneOpenGL *createScene();
 
 protected: