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

[Henry (Yu) Song - SISA]

Hi, Chris

You are right, I forget the clip case.  Regarding to unmap_image() - I was using cairo_gl_surface_draw_image(), but thought you prefer using unmap_image()? :). I should re-work on that, and re-submit.

Thanks

Henry
________________________________________
From: Chris Wilson [chris at chris-wilson.co.uk]
Sent: Tuesday, January 29, 2013 1:30 PM
To: Henry (Yu) Song - SISA
Cc: cairo at cairographics.org
Subject: Re: [cairo] [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 <henry.song at samsung.com>
> 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