From 519b6fe6f259762f05a638716691f626b3b54f12 Mon Sep 17 00:00:00 2001
From: Neil Roberts <neil@linux.intel.com>
Date: Wed, 17 Nov 2010 17:57:17 +0000
Subject: [PATCH] cogl-framebuffer: Try to track format of the framebuffer

Previously in cogl_read_pixels we assume the format of the framebuffer
is always premultiplied because that is the most likely format with
the default Cogl blend mode. However when the framebuffer is bound to
a texture we should be able to make a better guess at the format
because we know the texture keeps track of the premult status. This
patch adds an internal format member to CoglFramebuffer. For onscreen
framebuffers we still assume it is RGBA_8888_PRE but for offscreen to
textures we copy the texture format. cogl_read_pixels uses this to
determine whether the data returned by glReadPixels will be
premultiplied.

http://bugzilla.clutter-project.org/show_bug.cgi?id=2414
---
 clutter/cogl/cogl/cogl-framebuffer-private.h |  3 +++
 clutter/cogl/cogl/cogl-framebuffer.c         | 15 +++++++++++++
 clutter/cogl/cogl/cogl.c                     | 22 +++++++++-----------
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/clutter/cogl/cogl/cogl-framebuffer-private.h b/clutter/cogl/cogl/cogl-framebuffer-private.h
index 480c61bae..ad6c7ce62 100644
--- a/clutter/cogl/cogl/cogl-framebuffer-private.h
+++ b/clutter/cogl/cogl/cogl-framebuffer-private.h
@@ -39,6 +39,9 @@ struct _CoglFramebuffer
   CoglFramebufferType  type;
   int                 width;
   int                 height;
+  /* Format of the pixels in the framebuffer (including the expected
+     premult state) */
+  CoglPixelFormat     format;
 
   CoglMatrixStack    *modelview_stack;
   CoglMatrixStack    *projection_stack;
diff --git a/clutter/cogl/cogl/cogl-framebuffer.c b/clutter/cogl/cogl/cogl-framebuffer.c
index c8bf9cc70..b65dae730 100644
--- a/clutter/cogl/cogl/cogl-framebuffer.c
+++ b/clutter/cogl/cogl/cogl-framebuffer.c
@@ -139,12 +139,14 @@ _cogl_is_framebuffer (void *object)
 static void
 _cogl_framebuffer_init (CoglFramebuffer *framebuffer,
                         CoglFramebufferType type,
+                        CoglPixelFormat format,
                         int width,
                         int height)
 {
   framebuffer->type             = type;
   framebuffer->width            = width;
   framebuffer->height           = height;
+  framebuffer->format           = format;
   framebuffer->viewport_x       = 0;
   framebuffer->viewport_y       = 0;
   framebuffer->viewport_width   = width;
@@ -540,6 +542,7 @@ _cogl_offscreen_new_to_texture_full (CoglHandle texhandle,
     {
       _cogl_framebuffer_init (COGL_FRAMEBUFFER (offscreen),
                               COGL_FRAMEBUFFER_TYPE_OFFSCREEN,
+                              cogl_texture_get_format (texhandle),
                               data.level_width,
                               data.level_height);
 
@@ -594,9 +597,21 @@ _cogl_onscreen_new (void)
    * implement CoglOnscreen framebuffers, since we can't, e.g. keep track of
    * the window size. */
 
+  /* FIXME: We are assuming onscreen buffers will always be
+     premultiplied 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
+     CoglPipeline 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. */
+
   onscreen = g_new0 (CoglOnscreen, 1);
   _cogl_framebuffer_init (COGL_FRAMEBUFFER (onscreen),
                           COGL_FRAMEBUFFER_TYPE_ONSCREEN,
+                          COGL_PIXEL_FORMAT_RGBA_8888_PRE,
                           0xdeadbeef, /* width */
                           0xdeadbeef); /* height */
 
diff --git a/clutter/cogl/cogl/cogl.c b/clutter/cogl/cogl/cogl.c
index 7cc2d66d7..ce8326955 100644
--- a/clutter/cogl/cogl/cogl.c
+++ b/clutter/cogl/cogl/cogl.c
@@ -603,17 +603,14 @@ _cogl_read_pixels_with_rowstride (int x,
 
   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 CoglPipeline 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;
+      /* We match the premultiplied state of the target buffer to the
+       * premultiplied state of the framebuffer so that it will get
+       * converted to the right format below */
+
+      if ((framebuffer->format & COGL_PREMULT_BIT))
+        bmp_format |= COGL_PREMULT_BIT;
+      else
+        bmp_format &= ~COGL_PREMULT_BIT;
     }
 
   bmp = _cogl_bitmap_new_from_data (pixels,
@@ -640,7 +637,8 @@ _cogl_read_pixels_with_rowstride (int x,
       guint8 *tmp_data = g_malloc (width * height * 4);
 
       tmp_bmp = _cogl_bitmap_new_from_data (tmp_data,
-                                            COGL_PIXEL_FORMAT_RGBA_8888_PRE,
+                                            COGL_PIXEL_FORMAT_RGBA_8888 |
+                                            (bmp_format & COGL_PREMULT_BIT),
                                             width, height, 4 * width,
                                             (CoglBitmapDestroyNotify) g_free,
                                             NULL);