From 24ca92951f739278697990fb25f351045d6ece9c Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Wed, 17 Jun 2009 14:30:44 +0100 Subject: [PATCH] [cogl] Adds cogl_read_pixels to replace direct use of glReadPixels To allow for flushing of batched geometry within Cogl we can't support users directly calling glReadPixels. glReadPixels is also awkward, not least because it returns upside down image data. All the unit tests have been swithed over and clutter_stage_read_pixels now sits on top of this too. --- clutter/clutter-stage.c | 44 ++------------- clutter/cogl/cogl.h.in | 33 +++++++++++ clutter/cogl/common/cogl.c | 56 +++++++++++++++++++ doc/reference/cogl/cogl-sections.txt | 4 ++ tests/conform/test-blend-strings.c | 16 ++++-- tests/conform/test-premult.c | 14 +++-- tests/conform/test-vertex-buffer-contiguous.c | 28 +++++++--- tests/conform/test-vertex-buffer-interleved.c | 9 +-- tests/conform/test-vertex-buffer-mutability.c | 13 +++-- 9 files changed, 151 insertions(+), 66 deletions(-) diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c index e9ef7f1aa..ec2976694 100644 --- a/clutter/clutter-stage.c +++ b/clutter/clutter-stage.c @@ -1366,10 +1366,9 @@ clutter_stage_read_pixels (ClutterStage *stage, gint height) { guchar *pixels; - guchar *temprow; GLint viewport[4]; gint rowstride; - gint stage_x, stage_y, stage_width, stage_height; + gint stage_width, stage_height; g_return_val_if_fail (CLUTTER_IS_STAGE (stage), NULL); @@ -1383,8 +1382,6 @@ clutter_stage_read_pixels (ClutterStage *stage, clutter_actor_paint (CLUTTER_ACTOR (stage)); glGetIntegerv (GL_VIEWPORT, viewport); - stage_x = viewport[0]; - stage_y = viewport[1]; stage_width = viewport[2]; stage_height = viewport[3]; @@ -1397,42 +1394,11 @@ clutter_stage_read_pixels (ClutterStage *stage, rowstride = width * 4; pixels = g_malloc (height * rowstride); - temprow = g_malloc (rowstride); - /* Setup the pixel store parameters that may have been changed by - Cogl */ - glPixelStorei (GL_PACK_ALIGNMENT, 4); -#ifdef HAVE_COGL_GL - glPixelStorei (GL_PACK_ROW_LENGTH, 0); - glPixelStorei (GL_PACK_SKIP_PIXELS, 0); - glPixelStorei (GL_PACK_SKIP_ROWS, 0); -#endif /* HAVE_COGL_GL */ - - /* The y co-ordinate should be given in OpenGL's coordinate system - so 0 is the bottom row */ - y = stage_height - y - height; - - glFinish (); - - /* check whether we need to read into a smaller temporary buffer */ - glReadPixels (x, y, width, height, GL_RGBA, GL_UNSIGNED_BYTE, pixels); - - /* vertically flip the buffer in-place */ - for (y = 0; y < height / 2; y++) - { - if (y != height - y - 1) /* skip center row */ - { - memcpy (temprow, - pixels + y * rowstride, rowstride); - memcpy (pixels + y * rowstride, - pixels + (height - y - 1) * rowstride, rowstride); - memcpy (pixels + (height - y - 1) * rowstride, - temprow, - rowstride); - } - } - - g_free (temprow); + cogl_read_pixels (x, y, width, height, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixels); return pixels; } diff --git a/clutter/cogl/cogl.h.in b/clutter/cogl/cogl.h.in index c1cc10f29..adc329288 100644 --- a/clutter/cogl/cogl.h.in +++ b/clutter/cogl/cogl.h.in @@ -691,6 +691,39 @@ void cogl_push_draw_buffer (void); */ void cogl_pop_draw_buffer (void); +/** + * CoglReadPixelsFlags: + * @COGL_READ_PIXELS_COLOR_BUFFER: Read from the color buffer + */ +typedef enum _CoglReadPixelsFlags +{ + COGL_READ_PIXELS_COLOR_BUFFER = 1L<<0 +} CoglReadPixelsFlags; + +/** + * cogl_read_pixels: + * @x: The window x position to start reading from + * @y: The window y position to start reading from + * @width: The width of the rectangle you want to read + * @height: The height of the rectangle you want to read + * @source: Identifies which auxillary buffer you want to read + * (only COGL_READ_PIXELS_COLOR_BUFFER supported currently) + * @format: The pixel format you want the result in + * (only COGL_PIXEL_FORMAT_RGBA_8888 supported currently) + * @pixels: The location to write the pixel data. + * + * This reads a rectangle of pixels from the current draw buffer 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) + */ +void cogl_read_pixels (int x, + int y, + int width, + int height, + CoglReadPixelsFlags source, + CoglPixelFormat format, + guint8 *pixels); + /* * Internal API available only to Clutter. diff --git a/clutter/cogl/common/cogl.c b/clutter/cogl/common/cogl.c index 5857db2d3..d06d8ddd9 100644 --- a/clutter/cogl/common/cogl.c +++ b/clutter/cogl/common/cogl.c @@ -572,6 +572,8 @@ void cogl_get_viewport (float v[4]) { /* FIXME: cogl_get_viewport should return a gint vec */ + /* FIXME: cogl_get_viewport should only return a width + height + * (I don't think we need to support offset viewports) */ #if defined (HAVE_COGL_GLES2) || defined (HAVE_COGL_GLES) GLint viewport[4]; int i; @@ -676,3 +678,57 @@ cogl_flush_gl_state (int flags) } #endif +void +cogl_read_pixels (int x, + int y, + int width, + int height, + CoglReadPixelsFlags source, + CoglPixelFormat format, + guint8 *pixels) +{ + GLint viewport[4]; + GLint viewport_height; + int rowstride = width * 4; + guint8 temprow[rowstride]; + + g_return_if_fail (format == COGL_PIXEL_FORMAT_RGBA_8888); + g_return_if_fail (source == COGL_READ_PIXELS_COLOR_BUFFER); + + glGetIntegerv (GL_VIEWPORT, viewport); + viewport_height = viewport[3]; + + /* The y co-ordinate should be given in OpenGL's coordinate system + so 0 is the bottom row */ + y = viewport_height - y - height; + + /* Setup the pixel store parameters that may have been changed by + Cogl */ + glPixelStorei (GL_PACK_ALIGNMENT, 4); +#ifdef HAVE_COGL_GL + glPixelStorei (GL_PACK_ROW_LENGTH, 0); + glPixelStorei (GL_PACK_SKIP_PIXELS, 0); + glPixelStorei (GL_PACK_SKIP_ROWS, 0); +#endif /* HAVE_COGL_GL */ + + glReadPixels (x, y, width, height, GL_RGBA, GL_UNSIGNED_BYTE, pixels); + + /* 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 */ + { + memcpy (temprow, + pixels + y * rowstride, rowstride); + memcpy (pixels + y * rowstride, + pixels + (height - y - 1) * rowstride, rowstride); + memcpy (pixels + (height - y - 1) * rowstride, + temprow, + rowstride); + } + } +} + diff --git a/doc/reference/cogl/cogl-sections.txt b/doc/reference/cogl/cogl-sections.txt index 623aa3e92..c28ab52f6 100644 --- a/doc/reference/cogl/cogl-sections.txt +++ b/doc/reference/cogl/cogl-sections.txt @@ -68,6 +68,10 @@ cogl_set_source_color4ub cogl_set_source_color4f cogl_set_source_texture + +CoglReadPixelsFlags +cogl_read_pixels + COGL_TYPE_ATTRIBUTE_TYPE COGL_TYPE_BUFFER_BIT diff --git a/tests/conform/test-blend-strings.c b/tests/conform/test-blend-strings.c index 14b7fbcc0..c8f5a081b 100644 --- a/tests/conform/test-blend-strings.c +++ b/tests/conform/test-blend-strings.c @@ -124,8 +124,7 @@ test_blend (TestState *state, /* See what we got... */ - /* NB: glReadPixels is done in GL screen space so y = 0 is at the bottom */ - y_off = state->stage_geom.height - y * QUAD_WIDTH - (QUAD_WIDTH / 2); + y_off = y * QUAD_WIDTH + (QUAD_WIDTH / 2); x_off = x * QUAD_WIDTH + (QUAD_WIDTH / 2); /* XXX: @@ -134,7 +133,10 @@ test_blend (TestState *state, if (state->frame <= 2) return; - glReadPixels (x_off, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (x_off, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) { g_print ("test_blend (%d, %d):\n%s\n", x, y, blend_string); @@ -252,8 +254,7 @@ test_tex_combine (TestState *state, /* See what we got... */ - /* NB: glReadPixels is done in GL screen space so y = 0 is at the bottom */ - y_off = state->stage_geom.height - y * QUAD_WIDTH - (QUAD_WIDTH / 2); + y_off = y * QUAD_WIDTH + (QUAD_WIDTH / 2); x_off = x * QUAD_WIDTH + (QUAD_WIDTH / 2); /* XXX: @@ -262,7 +263,10 @@ test_tex_combine (TestState *state, if (state->frame <= 2) return; - glReadPixels (x_off, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (x_off, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) { g_print ("test_tex_combine (%d, %d):\n%s\n", x, y, combine_string); diff --git a/tests/conform/test-premult.c b/tests/conform/test-premult.c index 93b28784a..2c1a77884 100644 --- a/tests/conform/test-premult.c +++ b/tests/conform/test-premult.c @@ -101,9 +101,9 @@ check_texture (TestState *state, CoglHandle tex, guint32 expected_result) { - GLubyte pixel[4]; - GLint y_off; - GLint x_off; + guchar pixel[4]; + int y_off; + int x_off; cogl_material_set_layer (state->passthrough_material, 0, tex); @@ -115,8 +115,7 @@ check_texture (TestState *state, /* See what we got... */ - /* NB: glReadPixels is done in GL screen space so y = 0 is at the bottom */ - y_off = state->stage_geom.height - y * QUAD_WIDTH - (QUAD_WIDTH / 2); + y_off = y * QUAD_WIDTH + (QUAD_WIDTH / 2); x_off = x * QUAD_WIDTH + (QUAD_WIDTH / 2); /* XXX: @@ -125,7 +124,10 @@ check_texture (TestState *state, if (state->frame <= SKIP_FRAMES) return; - glReadPixels (x_off, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (x_off, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) { g_print ("check texture (%d, %d):\n", x, y); diff --git a/tests/conform/test-vertex-buffer-contiguous.c b/tests/conform/test-vertex-buffer-contiguous.c index 29558bcbd..f7a714e96 100644 --- a/tests/conform/test-vertex-buffer-contiguous.c +++ b/tests/conform/test-vertex-buffer-contiguous.c @@ -28,9 +28,8 @@ static void validate_result (TestState *state) { GLubyte pixel[4]; - GLint y_off = state->stage_geom.height - 90; + GLint y_off = 90; - /* NB: glReadPixels is done in GL screen space so y = 0 is at the bottom */ if (g_test_verbose ()) g_print ("y_off = %d\n", y_off); @@ -42,31 +41,46 @@ validate_result (TestState *state) #define BLUE 2 /* Should see a blue pixel */ - glReadPixels (10, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (10, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) g_print ("pixel 0 = %x, %x, %x\n", pixel[RED], pixel[GREEN], pixel[BLUE]); g_assert (pixel[RED] == 0 && pixel[GREEN] == 0 && pixel[BLUE] != 0); /* Should see a red pixel */ - glReadPixels (110, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (110, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) g_print ("pixel 1 = %x, %x, %x\n", pixel[RED], pixel[GREEN], pixel[BLUE]); g_assert (pixel[RED] != 0 && pixel[GREEN] == 0 && pixel[BLUE] == 0); /* Should see a blue pixel */ - glReadPixels (210, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (210, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) g_print ("pixel 2 = %x, %x, %x\n", pixel[RED], pixel[GREEN], pixel[BLUE]); g_assert (pixel[RED] == 0 && pixel[GREEN] == 0 && pixel[BLUE] != 0); /* Should see a green pixel, at bottom of 4th triangle */ - glReadPixels (310, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (310, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) g_print ("pixel 3 = %x, %x, %x\n", pixel[RED], pixel[GREEN], pixel[BLUE]); g_assert (pixel[GREEN] > pixel[RED] && pixel[GREEN] > pixel[BLUE]); /* Should see a red pixel, at top of 4th triangle */ - glReadPixels (310, y_off + 70 , 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (310, y_off - 70, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) g_print ("pixel 4 = %x, %x, %x\n", pixel[RED], pixel[GREEN], pixel[BLUE]); g_assert (pixel[RED] > pixel[GREEN] && pixel[RED] > pixel[BLUE]); diff --git a/tests/conform/test-vertex-buffer-interleved.c b/tests/conform/test-vertex-buffer-interleved.c index f2c2a1d0f..c91c57117 100644 --- a/tests/conform/test-vertex-buffer-interleved.c +++ b/tests/conform/test-vertex-buffer-interleved.c @@ -35,9 +35,7 @@ static void validate_result (TestState *state) { GLubyte pixel[4]; - GLint y_off = state->stage_geom.height - 90; - - /* NB: glReadPixels is done in GL screen space so y = 0 is at the bottom */ + GLint y_off = 90; /* NB: We ignore the alpha, since we don't know if our render target is * RGB or RGBA */ @@ -47,7 +45,10 @@ validate_result (TestState *state) #define BLUE 2 /* Should see a blue pixel */ - glReadPixels (10, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (10, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) g_print ("pixel 0 = %x, %x, %x\n", pixel[RED], pixel[GREEN], pixel[BLUE]); g_assert (pixel[RED] == 0 && pixel[GREEN] == 0 && pixel[BLUE] != 0); diff --git a/tests/conform/test-vertex-buffer-mutability.c b/tests/conform/test-vertex-buffer-mutability.c index ae25f083b..5c4e2eb2a 100644 --- a/tests/conform/test-vertex-buffer-mutability.c +++ b/tests/conform/test-vertex-buffer-mutability.c @@ -22,8 +22,7 @@ static void validate_result (TestState *state) { GLubyte pixel[4]; - GLint y_off = state->stage_geom.height - 90; - /* NB: glReadPixels is done in GL screen space so y = 0 is at the bottom */ + GLint y_off = 90; /* NB: We ignore the alpha, since we don't know if our render target is * RGB or RGBA */ @@ -33,13 +32,19 @@ validate_result (TestState *state) #define BLUE 2 /* Should see a red pixel */ - glReadPixels (110, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (110, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) g_print ("pixel 0 = %x, %x, %x\n", pixel[RED], pixel[GREEN], pixel[BLUE]); g_assert (pixel[RED] != 0 && pixel[GREEN] == 0 && pixel[BLUE] == 0); /* Should see a green pixel */ - glReadPixels (210, y_off, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, &pixel); + cogl_read_pixels (210, y_off, 1, 1, + COGL_READ_PIXELS_COLOR_BUFFER, + COGL_PIXEL_FORMAT_RGBA_8888, + pixel); if (g_test_verbose ()) g_print ("pixel 1 = %x, %x, %x\n", pixel[RED], pixel[GREEN], pixel[BLUE]); g_assert (pixel[RED] == 0 && pixel[GREEN] != 0 && pixel[BLUE] == 0);