this looks good to me.<div> </div><div>the only thing is, like your last patch, it adds use of _COGL_GET_CONTEXT() which could be nice to avoid if at all possible.</div><div> </div><div>kind regards,</div><div>- Robert <div class="gmail_quote">On Wed, Mar 21, 2012 at 6:13 PM, Neil Roberts <<a href="mailto:neil@linux.intel.com">neil@linux.intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> This extension lets you upload texture data from a subregion of a buffer by passing GL_UNPACK_ROW_LENGTH, GL_UNPACK_SKIP_PIXELS and GL_UNPACK_SKIP_ROWS to glPixelStore. When this extension is available the GLES texture driver will now avoid making a copy of the bitmap when a subregion is used. Note that Mesa doesn't currently advertise this extension but I've made a patch to propose it: <a href="http://lists.freedesktop.org/archives/mesa-dev/2012-March/020191.html" target="_blank">http://lists.freedesktop.org/archives/mesa-dev/2012-March/020191.html</a> --- cogl/cogl-internal.h | 3 +- cogl/driver/gles/cogl-gles.c | 3 + cogl/driver/gles/cogl-texture-driver-gles.c | 60 +++++++++++++++++++++++---- 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/cogl/cogl-internal.h b/cogl/cogl-internal.h index 4f53cc3..284fc97 100644 --- a/cogl/cogl-internal.h +++ b/cogl/cogl-internal.h @@ -101,7 +101,8 @@ typedef enum COGL_PRIVATE_FEATURE_VBOS = 1L<<6, COGL_PRIVATE_FEATURE_EXT_PACKED_DEPTH_STENCIL = 1L<<7, COGL_PRIVATE_FEATURE_OES_PACKED_DEPTH_STENCIL = 1L<<8, - COGL_PRIVATE_FEATURE_TEXTURE_FORMAT_BGRA8888 = 1L<<9 + COGL_PRIVATE_FEATURE_TEXTURE_FORMAT_BGRA8888 = 1L<<9, + COGL_PRIVATE_FEATURE_UNPACK_SUBIMAGE = 1L<<10 } CoglPrivateFeatureFlags; /* Sometimes when evaluating pipelines, either during comparisons or diff --git a/cogl/driver/gles/cogl-gles.c b/cogl/driver/gles/cogl-gles.c index e0f6500..464ddc6 100644 --- a/cogl/driver/gles/cogl-gles.c +++ b/cogl/driver/gles/cogl-gles.c @@ -169,6 +169,9 @@ _cogl_gles_update_features (CoglContext *context, if (_cogl_check_extension ("GL_EXT_texture_format_BGRA8888", gl_extensions)) private_flags |= COGL_PRIVATE_FEATURE_TEXTURE_FORMAT_BGRA8888; + if (_cogl_check_extension ("GL_EXT_unpack_subimage", gl_extensions)) + private_flags |= COGL_PRIVATE_FEATURE_UNPACK_SUBIMAGE; + /* Cache features */ context->private_feature_flags |= private_flags; context->feature_flags |= flags; diff --git a/cogl/driver/gles/cogl-texture-driver-gles.c b/cogl/driver/gles/cogl-texture-driver-gles.c index 62a65ee..fc18376 100644 --- a/cogl/driver/gles/cogl-texture-driver-gles.c +++ b/cogl/driver/gles/cogl-texture-driver-gles.c @@ -52,6 +52,18 @@ #define GL_MAX_3D_TEXTURE_SIZE_OES 0x8073 #endif +/* This extension isn't available for GLES 1.1 so these won't be + defined */ +#ifndef GL_UNPACK_ROW_LENGTH +#define GL_UNPACK_ROW_LENGTH 0x0CF2 +#endif +#ifndef GL_UNPACK_SKIP_ROWS +#define GL_UNPACK_SKIP_ROWS 0x0CF3 +#endif +#ifndef GL_UNPACK_SKIP_PIXELS +#define GL_UNPACK_SKIP_PIXELS 0x0CF4 +#endif + static void _cogl_texture_driver_gen (GLenum gl_target, GLsizei n, @@ -84,10 +96,37 @@ _cogl_texture_driver_gen (GLenum gl_target, } static void +prep_gl_for_pixels_upload_full (int pixels_rowstride, + int pixels_src_x, + int pixels_src_y, + int pixels_bpp) +{ + _COGL_GET_CONTEXT (ctx, NO_RETVAL); + + if ((ctx->private_feature_flags & COGL_PRIVATE_FEATURE_UNPACK_SUBIMAGE)) + { + GE( ctx, glPixelStorei (GL_UNPACK_ROW_LENGTH, + pixels_rowstride / pixels_bpp) ); + + GE( ctx, glPixelStorei (GL_UNPACK_SKIP_PIXELS, pixels_src_x) ); + GE( ctx, glPixelStorei (GL_UNPACK_SKIP_ROWS, pixels_src_y) ); + } + else + { + g_assert (pixels_src_x == 0); + g_assert (pixels_src_y == 0); + } + + _cogl_texture_prep_gl_alignment_for_pixels_upload (pixels_rowstride); +} + +static void _cogl_texture_driver_prep_gl_for_pixels_upload (int pixels_rowstride, int pixels_bpp) { - _cogl_texture_prep_gl_alignment_for_pixels_upload (pixels_rowstride); + prep_gl_for_pixels_upload_full (pixels_rowstride, + 0, 0, /* src_x/y */ + pixels_bpp); } static void @@ -106,7 +145,10 @@ prepare_bitmap_alignment_for_upload (CoglBitmap *src_bmp) int width = cogl_bitmap_get_width (src_bmp); int alignment = 1; - if (src_rowstride == 0) + _COGL_GET_CONTEXT (ctx, NULL); + + if ((ctx->private_feature_flags & COGL_PRIVATE_FEATURE_UNPACK_SUBIMAGE) || + src_rowstride == 0) return cogl_object_ref (src_bmp); /* Work out the alignment of the source rowstride */ @@ -145,11 +187,13 @@ _cogl_texture_driver_upload_subregion_to_gl (GLenum gl_target, _COGL_GET_CONTEXT (ctx, NO_RETVAL); - /* If we are copying a sub region of the source bitmap then we need - to copy it because GLES does not support GL_UNPACK_ROW_LENGTH */ - if (src_x != 0 || src_y != 0 || - width != cogl_bitmap_get_width (source_bmp) || - height != cogl_bitmap_get_height (source_bmp)) + /* If we have the GL_EXT_unpack_subimage extension then we can + upload from subregions directly. Otherwise we may need to copy + the bitmap */ + if (!(ctx->private_feature_flags & COGL_PRIVATE_FEATURE_UNPACK_SUBIMAGE) && + (src_x != 0 || src_y != 0 || + width != cogl_bitmap_get_width (source_bmp) || + height != cogl_bitmap_get_height (source_bmp))) { slice_bmp = _cogl_bitmap_new_with_malloc_buffer (ctx, @@ -167,7 +211,7 @@ _cogl_texture_driver_upload_subregion_to_gl (GLenum gl_target, rowstride = cogl_bitmap_get_rowstride (slice_bmp); /* Setup gl alignment to match rowstride and top-left corner */ - _cogl_texture_driver_prep_gl_for_pixels_upload (rowstride, bpp); + prep_gl_for_pixels_upload_full (rowstride, src_x, src_y, bpp); data = _cogl_bitmap_bind (slice_bmp, COGL_BUFFER_ACCESS_READ, 0); -- 1.7.3.16.g9464b _______________________________________________ Cogl mailing list <a href="mailto:Cogl@lists.freedesktop.org">Cogl@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/cogl" target="_blank">http://lists.freedesktop.org/mailman/listinfo/cogl</a> </blockquote></div> </div>