[Cogl] [PATCH 2/2] Fix handling of binding errors when uploading a full texture

[Robert Bragg]

This looks good to land to me:

Reviewed-by: Robert Bragg <robert at linux.intel.com>

thanks,
- Robert


On Tue, Nov 27, 2012 at 8:11 PM, Neil Roberts <neil at linux.intel.com> wrote:

> Both the texture drivers weren't handling errors correctly when a
> CoglPixelBuffer was used to set the contents of an entire texture.
> This was causing it to hit an assertion failure in the pixel buffer
> tests.
> ---
>  cogl/driver/gl/gl/cogl-texture-driver-gl.c     | 17 ++++++++++++++---
>  cogl/driver/gl/gles/cogl-texture-driver-gles.c | 13 +++++++++++--
>  tests/conform/test-conform-main.c              |  4 ++--
>  3 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/cogl/driver/gl/gl/cogl-texture-driver-gl.c
> b/cogl/driver/gl/gl/cogl-texture-driver-gl.c
> index 1f0e413..ac6ac98 100644
> --- a/cogl/driver/gl/gl/cogl-texture-driver-gl.c
> +++ b/cogl/driver/gl/gl/cogl-texture-driver-gl.c
> @@ -246,10 +246,21 @@ _cogl_texture_driver_upload_to_gl (CoglContext *ctx,
>    int bpp = _cogl_pixel_format_get_bytes_per_pixel (source_format);
>    GLenum gl_error;
>    CoglBool status = TRUE;
> +  CoglError *internal_error = NULL;
>
> -  data = _cogl_bitmap_gl_bind (source_bmp, COGL_BUFFER_ACCESS_READ, 0,
> error);
> -  if (!data)
> -    return FALSE;
> +  data = _cogl_bitmap_gl_bind (source_bmp,
> +                               COGL_BUFFER_ACCESS_READ,
> +                               0, /* hints */
> +                               &internal_error);
> +
> +  /* NB: _cogl_bitmap_gl_bind() may return NULL when successful so we
> +   * have to explicitly check the cogl error pointer to catch
> +   * problems... */
> +  if (internal_error)
> +    {
> +      _cogl_propagate_error (error, internal_error);
> +      return FALSE;
> +    }
>
>    /* Setup gl alignment to match rowstride and top-left corner */
>    prep_gl_for_pixels_upload_full (ctx,
> diff --git a/cogl/driver/gl/gles/cogl-texture-driver-gles.c
> b/cogl/driver/gl/gles/cogl-texture-driver-gles.c
> index 360d46a..5a4e0fa 100644
> --- a/cogl/driver/gl/gles/cogl-texture-driver-gles.c
> +++ b/cogl/driver/gl/gles/cogl-texture-driver-gles.c
> @@ -290,6 +290,7 @@ _cogl_texture_driver_upload_to_gl (CoglContext *ctx,
>    CoglBitmap *bmp;
>    uint8_t *data;
>    GLenum gl_error;
> +  CoglError *internal_error = NULL;
>    CoglBool status = TRUE;
>
>    bmp = prepare_bitmap_alignment_for_upload (ctx, source_bmp, error);
> @@ -303,10 +304,18 @@ _cogl_texture_driver_upload_to_gl (CoglContext *ctx,
>
>    _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign);
>
> -  data = _cogl_bitmap_gl_bind (bmp, COGL_BUFFER_ACCESS_READ, 0, error);
> -  if (!data)
> +  data = _cogl_bitmap_gl_bind (bmp,
> +                               COGL_BUFFER_ACCESS_READ,
> +                               0, /* hints */
> +                               &internal_error);
> +
> +  /* NB: _cogl_bitmap_gl_bind() may return NULL when successful so we
> +   * have to explicitly check the cogl error pointer to catch
> +   * problems... */
> +  if (internal_error)
>      {
>        cogl_object_unref (bmp);
> +      _cogl_propagate_error (error, internal_error);
>        return FALSE;
>      }
>
> diff --git a/tests/conform/test-conform-main.c
> b/tests/conform/test-conform-main.c
> index 78eefc3..ab5dd04 100644
> --- a/tests/conform/test-conform-main.c
> +++ b/tests/conform/test-conform-main.c
> @@ -67,8 +67,8 @@ main (int argc, char **argv)
>    UNPORTED_TEST (test_multitexture);
>    UNPORTED_TEST (test_texture_mipmaps);
>    ADD_TEST (test_sub_texture, 0, 0);
> -  ADD_TEST (test_pixel_buffer_map, 0, TEST_KNOWN_FAILURE);
> -  ADD_TEST (test_pixel_buffer_set_data, 0, TEST_KNOWN_FAILURE);
> +  ADD_TEST (test_pixel_buffer_map, 0, 0);
> +  ADD_TEST (test_pixel_buffer_set_data, 0, 0);
>    ADD_TEST (test_pixel_buffer_sub_region, 0, 0);
>    UNPORTED_TEST (test_texture_rectangle);
>    ADD_TEST (test_texture_3d, TEST_REQUIREMENT_TEXTURE_3D, 0);
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/cogl/attachments/20121128/f9dffe28/attachment.html>