[1/3] gl: Convert images to rgba or a8 formats when uploading with GLESv2

Submitted by Bryce Harrington on Dec. 7, 2016, 11:46 p.m.

Details

Message ID 1481154412-16053-2-git-send-email-bryce@osg.samsung.com
State New
Series "Enable GLES v3 support"
Headers show

Commit Message

Bryce Harrington Dec. 7, 2016, 11:46 p.m.
From: Bryce Harrington <bryce@bryceharrington.org>

The GLESv2 backend supports only GL_RGBA and GL_ALPHA as supported
texture formats.  So, make _cairo_gl_get_image_format_and_type_gles2
force conversion of other image formats to either of these two as
appropriate when uploading images.

Patch originally from Henry Song <henry.song@samsung.com>

Bryce Harrington <bryce@osg.samsung.com>

Signed-off-by: Bryce Harrington <bryce@osg.samsung.com>
---
 src/cairo-gl-surface.c          | 45 +++++++++++++++++++++++++++++++++++++++++
 src/cairo-gl-traps-compositor.c | 30 ---------------------------
 2 files changed, 45 insertions(+), 30 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/cairo-gl-surface.c b/src/cairo-gl-surface.c
index e5e8205..8ecc3a9 100644
--- a/src/cairo-gl-surface.c
+++ b/src/cairo-gl-surface.c
@@ -868,12 +868,54 @@  _cairo_gl_surface_draw_image (cairo_gl_surface_t *dst,
     cairo_image_surface_t *clone = NULL;
     cairo_gl_context_t *ctx;
     int cpp;
+    cairo_image_surface_t *rgba_clone = NULL;
     cairo_int_status_t status = CAIRO_INT_STATUS_SUCCESS;
 
     status = _cairo_gl_context_acquire (dst->base.device, &ctx);
     if (unlikely (status))
 	return status;
 
+    if (_cairo_gl_get_flavor () == CAIRO_GL_FLAVOR_ES2) {
+	pixman_format_code_t pixman_format;
+	cairo_surface_pattern_t pattern;
+	cairo_bool_t require_conversion = FALSE;
+	pixman_format = _cairo_is_little_endian () ? PIXMAN_a8b8g8r8 : PIXMAN_r8g8b8a8;
+
+	if (src->base.content != CAIRO_CONTENT_ALPHA) {
+	    if (src->pixman_format != pixman_format)
+		require_conversion = TRUE;
+	}
+	else if (dst->base.content != CAIRO_CONTENT_ALPHA)
+	    require_conversion = TRUE;
+	else {
+	    if (src->pixman_format == PIXMAN_a1) {
+		pixman_format = PIXMAN_a8;
+		require_conversion = TRUE;
+	    }
+	}
+
+	if (require_conversion) {
+	    rgba_clone = (cairo_image_surface_t *)
+		_cairo_image_surface_create_with_pixman_format (NULL,
+								pixman_format,
+								src->width,
+								src->height,
+								0);
+	    if (unlikely (rgba_clone->base.status))
+		goto FAIL;
+
+	    _cairo_pattern_init_for_surface (&pattern, &src->base);
+	    status = _cairo_surface_paint (&rgba_clone->base,
+					   CAIRO_OPERATOR_SOURCE,
+					   &pattern.base, NULL);
+	    _cairo_pattern_fini (&pattern.base);
+	    if (unlikely (status))
+		goto FAIL;
+
+	    src = rgba_clone;
+	}
+    }
+
     if (! _cairo_gl_get_image_format_and_type (ctx->gl_flavor,
 					       src->pixman_format,
 					       &internal_format,
@@ -1008,6 +1050,9 @@  FAIL:
     if (clone)
         cairo_surface_destroy (&clone->base);
 
+    if (rgba_clone)
+	cairo_surface_destroy (&rgba_clone->base);
+
     return status;
 }
 
diff --git a/src/cairo-gl-traps-compositor.c b/src/cairo-gl-traps-compositor.c
index 664a27a..1e11006 100644
--- a/src/cairo-gl-traps-compositor.c
+++ b/src/cairo-gl-traps-compositor.c
@@ -303,36 +303,6 @@  traps_to_operand (void *_dst,
 	return image->status;
     }
 
-    /* GLES2 only supports RGB/RGBA when uploading */
-    if (_cairo_gl_get_flavor () == CAIRO_GL_FLAVOR_ES2) {
-	cairo_surface_pattern_t pattern;
-	cairo_surface_t *rgba_image;
-
-	/* XXX perform this fixup inside _cairo_gl_draw_image() */
-
-	rgba_image =
-	    _cairo_image_surface_create_with_pixman_format (NULL,
-							    _cairo_is_little_endian () ?  PIXMAN_a8b8g8r8 : PIXMAN_r8g8b8a8,
-							    extents->width,
-							    extents->height,
-							    0);
-	if (unlikely (rgba_image->status))
-	    return rgba_image->status;
-
-	_cairo_pattern_init_for_surface (&pattern, image);
-	status = _cairo_surface_paint (rgba_image, CAIRO_OPERATOR_SOURCE,
-				       &pattern.base, NULL);
-	_cairo_pattern_fini (&pattern.base);
-
-	cairo_surface_destroy (image);
-	image = rgba_image;
-
-	if (unlikely (status)) {
-	    cairo_surface_destroy (image);
-	    return status;
-	}
-    }
-
     mask = _cairo_surface_create_scratch (_dst,
 					  CAIRO_CONTENT_COLOR_ALPHA,
 					  extents->width,

Comments

Uli Schlachter Dec. 10, 2016, 3:02 p.m.
On 08.12.2016 00:46, Bryce Harrington wrote:
> From: Bryce Harrington <bryce@bryceharrington.org>
> 
> The GLESv2 backend supports only GL_RGBA and GL_ALPHA as supported
> texture formats.  So, make _cairo_gl_get_image_format_and_type_gles2
> force conversion of other image formats to either of these two as
> appropriate when uploading images.
> 
> Patch originally from Henry Song <henry.song@samsung.com>
> 
> Bryce Harrington <bryce@osg.samsung.com>
> 
> Signed-off-by: Bryce Harrington <bryce@osg.samsung.com>
> ---
>  src/cairo-gl-surface.c          | 45 +++++++++++++++++++++++++++++++++++++++++
>  src/cairo-gl-traps-compositor.c | 30 ---------------------------
>  2 files changed, 45 insertions(+), 30 deletions(-)
> 
> diff --git a/src/cairo-gl-surface.c b/src/cairo-gl-surface.c
> index e5e8205..8ecc3a9 100644
> --- a/src/cairo-gl-surface.c
> +++ b/src/cairo-gl-surface.c
> @@ -868,12 +868,54 @@ _cairo_gl_surface_draw_image (cairo_gl_surface_t *dst,
>      cairo_image_surface_t *clone = NULL;
>      cairo_gl_context_t *ctx;
>      int cpp;
> +    cairo_image_surface_t *rgba_clone = NULL;
>      cairo_int_status_t status = CAIRO_INT_STATUS_SUCCESS;
>  
>      status = _cairo_gl_context_acquire (dst->base.device, &ctx);
>      if (unlikely (status))
>  	return status;
>  
> +    if (_cairo_gl_get_flavor () == CAIRO_GL_FLAVOR_ES2) {
> +	pixman_format_code_t pixman_format;
> +	cairo_surface_pattern_t pattern;
> +	cairo_bool_t require_conversion = FALSE;
> +	pixman_format = _cairo_is_little_endian () ? PIXMAN_a8b8g8r8 : PIXMAN_r8g8b8a8;
> +
> +	if (src->base.content != CAIRO_CONTENT_ALPHA) {
> +	    if (src->pixman_format != pixman_format)
> +		require_conversion = TRUE;
> +	}
> +	else if (dst->base.content != CAIRO_CONTENT_ALPHA)
> +	    require_conversion = TRUE;
> +	else {
> +	    if (src->pixman_format == PIXMAN_a1) {

Should this perhaps better be src->pixman_format != PIXMAN_a8?
Seems safer / more general to me.

> +		pixman_format = PIXMAN_a8;
> +		require_conversion = TRUE;
> +	    }
> +	}
[...]

Besides that, I don't have enough clue about the requirements of GLES to
say anything.

Uli