Re: [patch] gl: create BGRA texture and avoid conversion

On Wed, Jan 30, 2013 at 07:48:00PM +0000, Henry (Yu) Song - SISA wrote:
> From 73287550e6eeb68d6c9f08767d1dc4ad6d4f6905 Mon Sep 17 00:00:00 2001
> From: Henry Song <[email protected]>
> Date: Wed, 30 Jan 2013 10:43:56 -0800
> Subject: [PATCH] gl: create BGRA texture if driver supports, and consolidate
> conversion from BGRA to RGBA into one function
> 
> If GLES2 driver supports GL_EXT_read_bgra, we can create a texture with
> BGRA format.  The benefit of this is cairo does not need to do conversion
> when upload BGRA image which is the default format used by cairo.  This
> does not affect GL drivers, where we always create RGBA textures.
> 
> There are two map_to_image() function.  One is public cairo_surface_map_to_image()
> where it does conversion from CAIRO_FORMAT_INVALID TO CAIRO_FORMAT_ARGB32, the
> second one is private _cairo_surface_map_to_image () where it does not do
> conversion. This can be confusion for any compositor implementation.  This patch
> centralizes conversion of BGRA to RGBA to one single function. So regardless
> which functions to call, the conversion will always be done when unmap image
> 
> 
> ---
>  src/cairo-gl-device.c           |   28 ++++-----
>  src/cairo-gl-private.h          |    4 +-
>  src/cairo-gl-surface.c          |  126 +++++++++++++++++++++++++++++++++------
>  src/cairo-gl-traps-compositor.c |   30 ----------
>  4 files changed, 120 insertions(+), 68 deletions(-)
> 
> diff --git a/src/cairo-gl-device.c b/src/cairo-gl-device.c
> index 22297b3..2086c8f 100644
> --- a/src/cairo-gl-device.c
> +++ b/src/cairo-gl-device.c

> +static cairo_image_surface_t *
> +_cairo_gl_convert_image (cairo_gl_context_t *ctx,
> +			 cairo_gl_surface_t *dst,
> +			 cairo_image_surface_t *src,
> +			 int src_x, int src_y,
> +			 int width, int height)

I would replace the body of this function with a call to
_cairo_image_surface_coerce_to_format().  Actually, I'd rewrite that
function as _cairo_image_surface_coerce_to_pixman_format() (and make the
existing one a wrapper to it).

Other than a request to break the checks up into separate if()
statements for clarity, the refactoring looks sane.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo