Re: [patch] gl/msaa: upload image inplace instead of creating extra scratch gl surface

On Tue, Jan 29, 2013 at 05:19:53PM +0000, Henry (Yu) Song - SISA wrote:
> From 1b95f6946f028988e6905060aa5ee037ab52990f Mon Sep 17 00:00:00 2001
> From: Henry Song <[email protected]>
> Date: Tue, 29 Jan 2013 09:15:46 -0800
> Subject: [PATCH] gl/msaa: upload image inplace in paint if the source pattern
>  is an image and pattern is pixel-aligned without scale and
>  rotation. This saves GPU memory without creating extra
>  scratch surface
> 
> ---
>  src/cairo-gl-msaa-compositor.c |  117 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 116 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cairo-gl-msaa-compositor.c b/src/cairo-gl-msaa-compositor.c
> index 54772ef..b9ffd8f 100644
> --- a/src/cairo-gl-msaa-compositor.c
> +++ b/src/cairo-gl-msaa-compositor.c
> @@ -47,6 +47,8 @@
>  #include "cairo-gl-private.h"
>  #include "cairo-path-private.h"
>  #include "cairo-traps-private.h"
> +#include "cairo-image-surface-private.h"
> +#include "cairo-surface-offset-private.h"
>  
>  static cairo_bool_t
>  can_use_msaa_compositor (cairo_gl_surface_t *surface,
> @@ -213,6 +215,112 @@ _cairo_gl_msaa_compositor_draw_clip (cairo_gl_context_t *ctx,
>  }
>  
>  static cairo_bool_t
> +_pattern_is_pixel_aligned (const cairo_pattern_t *pattern)
> +{
> +    return pattern->matrix.xx == 1 &&
> +	   pattern->matrix.yy == 1 &&
> +	   pattern->matrix.xy == 0 &&
> +	   pattern->matrix.yx == 0 &&
> +	   ceil (pattern->matrix.x0) == pattern->matrix.x0 &&
> +	   ceil (pattern->matrix.y0) == pattern->matrix.y0;
> +}

This should be

  _cairo_pattern_analyze_filter(pattern, NULL) == CAIRO_FILTER_NEAREST;

> +
> +static cairo_bool_t
> +_pattern_is_image_surface (const cairo_pattern_t *pattern)
> +{
> +    cairo_surface_t *surface;
> +
> +    if (pattern->type != CAIRO_PATTERN_TYPE_SURFACE)
> +	return FALSE;
> +
> +    surface = ((cairo_surface_pattern_t *)pattern)->surface;

We should introduce
  cairo_bool_t _cairo_pattern_get_surface(pattern, &surface)
as we use that quite frequenty;

> +    return surface->type == CAIRO_SURFACE_TYPE_IMAGE;

_cairo_surface_is_image (surface)?

> +}
> +
> +static cairo_int_status_t
> +_cairo_gl_msaa_compositor_upload_image_inplace (const cairo_compositor_t *compositor,
> +						cairo_composite_rectangles_t *composite)
> +{
> +    cairo_image_surface_t *image, *rgba_image = NULL;
> +    cairo_int_status_t status = CAIRO_INT_STATUS_UNSUPPORTED;
> +    cairo_gl_context_t *ctx;
> +    cairo_gl_dispatch_t *dispatch;
> +    pixman_format_code_t pixman_format;
> +
> +    const cairo_pattern_t *pattern = composite->original_source_pattern;
> +    cairo_gl_surface_t *dst = (cairo_gl_surface_t *) composite->surface;
> +
> +    if (_pattern_is_image_surface (pattern) &&
> +	_pattern_is_pixel_aligned (pattern) &&
> +	cairo_pattern_get_extend ((cairo_pattern_t *)pattern) == CAIRO_EXTEND_NONE &&
> +	_cairo_gl_surface_is_texture (dst)) {

There is no analysis of the clip here.

> +	if ((dst->base.is_clear && 
> +	     (composite->op == CAIRO_OPERATOR_OVER ||
> +	      composite->op == CAIRO_OPERATOR_ADD)) ||
> +	    composite->op == CAIRO_OPERATOR_SOURCE) {
> +	    status = _cairo_gl_context_acquire (dst->base.device, &ctx);
> +	    if (unlikely (status))
> +		return status;
> +
> +	    dispatch = &ctx->dispatch;
> +
> +	    image = (cairo_image_surface_t *)
> +		cairo_surface_map_to_image (&dst->base, &composite->bounded);

_cairo_surface_map_to_image ... but this entire chunk would be better as
a function that could write the image directly into the
cairo_gl_surface_t.... So what is wrong with
_cairo_gl_surface_draw_image()?

But it looks like you could do with copying many more tricks from the
spans compositor to catch the trivial cases.
-Chris

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