cogl: Support any format in cogl_read_pixels

cogl_read_pixels() no longer asserts that the format passed in is
RGBA_8888 but instead accepts any format. The appropriate GL enums for
the format are passed to glReadPixels so OpenGL should be perform a
conversion if neccessary.

It currently assumes glReadPixels will always give us premultiplied
data. This will usually be correct because the result of the default
blending operations for Cogl ends up with premultiplied data in the
framebuffer. However it is possible for the framebuffer to be in
whatever format depending on what CoglMaterial is used to render to
it. Eventually we may want to add a way for an application to inform
Cogl that the framebuffer is not premultiplied in case it is being
used for some special purpose.

If the requested format is not premultiplied then Cogl will convert
it. The tests have been changed to read the data as premultiplied so
that they won't be affected by the conversion. Picking in Clutter has
been changed to use COGL_PIXEL_FORMAT_RGB_888 because it doesn't need
the alpha component. clutter_stage_read_pixels is left unchanged
because the application can't specify a format for that so it seems to
make most sense to store unpremultiplied values.

http://bugzilla.openedhand.com/show_bug.cgi?id=1959

cogl/cogl.c

@@ -41,6 +41,9 @@
 #include "cogl-framebuffer-private.h"
 #include "cogl-matrix-private.h"
 #include "cogl-journal-private.h"
+#include "cogl-bitmap-private.h"
+#include "cogl-texture-private.h"
+#include "cogl-texture-driver.h"
 
 #if defined (HAVE_COGL_GLES2) || defined (HAVE_COGL_GLES)
 #include "cogl-gles2-wrapper.h"
@@ -630,20 +633,20 @@ cogl_read_pixels (int x,
 {
   CoglHandle framebuffer;
   int        framebuffer_height;
-  int        rowstride = width * 4;
+  int        bpp;
-  guint8    *temprow;
+  CoglBitmap bmp;
+  GLenum     gl_intformat;
+  GLenum     gl_format;
+  GLenum     gl_type;
 
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
-  g_return_if_fail (format == COGL_PIXEL_FORMAT_RGBA_8888);
   g_return_if_fail (source == COGL_READ_PIXELS_COLOR_BUFFER);
 
   /* make sure any batched primitives get emitted to the GL driver before
    * issuing our read pixels... */
   cogl_flush ();
 
-  temprow = g_alloca (rowstride * sizeof (guint8));
-
   framebuffer = _cogl_get_framebuffer ();
 
   _cogl_framebuffer_flush_state (framebuffer, 0);
@@ -659,37 +662,64 @@ cogl_read_pixels (int x,
   if (!cogl_is_offscreen (framebuffer))
     y = framebuffer_height - y - height;
 
+  /* Initialise the CoglBitmap */
+  bpp = _cogl_get_format_bpp (format);
+  bmp.format = format;
+  bmp.data = pixels;
+  bmp.width = width;
+  bmp.height = height;
+  bmp.rowstride = bpp * width;
+
+  if ((format & COGL_A_BIT))
+    {
+      /* FIXME: We are assuming glReadPixels will always give us
+         premultiplied data so we'll set the premult flag on the
+         bitmap format. This will usually be correct because the
+         result of the default blending operations for Cogl ends up
+         with premultiplied data in the framebuffer. However it is
+         possible for the framebuffer to be in whatever format
+         depending on what CoglMaterial is used to render to
+         it. Eventually we may want to add a way for an application to
+         inform Cogl that the framebuffer is not premultiplied in case
+         it is being used for some special purpose. */
+      bmp.format |= COGL_PREMULT_BIT;
+    }
+
   /* Setup the pixel store parameters that may have been changed by
      Cogl */
-  GE (glPixelStorei (GL_PACK_ALIGNMENT, 1));
+  _cogl_texture_driver_prep_gl_for_pixels_download (bmp.rowstride, bpp);
-#ifdef HAVE_COGL_GL
-  GE (glPixelStorei (GL_PACK_ROW_LENGTH, 0));
-  GE (glPixelStorei (GL_PACK_SKIP_PIXELS, 0));
-  GE (glPixelStorei (GL_PACK_SKIP_ROWS, 0));
-#endif /* HAVE_COGL_GL */
 
-  GE (glReadPixels (x, y, width, height, GL_RGBA, GL_UNSIGNED_BYTE, pixels));
+  _cogl_pixel_format_to_gl (format, &gl_intformat, &gl_format, &gl_type);
+
+  GE (glReadPixels (x, y, width, height, gl_format, gl_type, pixels));
+
+  /* Convert to the premult format specified by the caller
+     in-place. This will do nothing if the premult status is already
+     correct. */
+  _cogl_bitmap_convert_premult_status (&bmp, format);
 
   /* NB: All offscreen rendering is done upside down so there is no need
    * to flip in this case... */
-  if (cogl_is_offscreen (framebuffer))
+  if (!cogl_is_offscreen (framebuffer))
-    return;
-
-  /* TODO: consider using the GL_MESA_pack_invert extension in the future
-   * to avoid this flip... */
-
-  /* vertically flip the buffer in-place */
-  for (y = 0; y < height / 2; y++)
     {
-      if (y != height - y - 1) /* skip center row */
+      guint8 *temprow = g_alloca (bmp.rowstride * sizeof (guint8));
+
+      /* TODO: consider using the GL_MESA_pack_invert extension in the future
+       * to avoid this flip... */
+
+      /* vertically flip the buffer in-place */
+      for (y = 0; y < height / 2; y++)
         {
-          memcpy (temprow,
+          if (y != height - y - 1) /* skip center row */
-                  pixels + y * rowstride, rowstride);
+            {
-          memcpy (pixels + y * rowstride,
+              memcpy (temprow,
-                  pixels + (height - y - 1) * rowstride, rowstride);
+                      pixels + y * bmp.rowstride, bmp.rowstride);
-          memcpy (pixels + (height - y - 1) * rowstride,
+              memcpy (pixels + y * bmp.rowstride,
-                  temprow,
+                      pixels + (height - y - 1) * bmp.rowstride, bmp.rowstride);
-                  rowstride);
+              memcpy (pixels + (height - y - 1) * bmp.rowstride,
+                      temprow,
+                      bmp.rowstride);
+            }
         }
     }
 }

cogl/cogl.h

@@ -936,7 +936,13 @@ typedef enum { /*< prefix=COGL_READ_PIXELS >*/
  *
  * This reads a rectangle of pixels from the current framebuffer where
  * position (0, 0) is the top left. The pixel at (x, y) is the first
- * read, and the data is returned with a rowstride of (width * 4)
+ * read, and the data is returned with a rowstride of (width * 4).
+ *
+ * Currently Cogl assumes that the framebuffer is in a premultiplied
+ * format so if @format is non-premultiplied it will convert it. To
+ * read the pixel values without any conversion you should either
+ * specify a format that doesn't use an alpha channel or use one of
+ * the formats ending in PRE.
  */
 void
 cogl_read_pixels (int x,