[Pixman] [PATCH 15/18] Add direct-write optimization back

Andrea Canciani ranma42 at gmail.com
Fri Jan 7 07:57:44 PST 2011


On Thu, Jan 6, 2011 at 3:27 AM, Søren Sandmann <sandmann at cs.au.dk> wrote:
> From: Søren Sandmann Pedersen <ssp at redhat.com>
>
> Introduce a new ITER_LOCALIZED_ALPHA flag that indicates that the
> alpha value computed is used only for the alpha channel of the output;
> it doesn't affect the RGB channels.
>
> Then in pixman-bits-image.c, if a destination is either a8r8g8b8 or
> x8r8g8b8 with localized alpha, the iterator will return a pointer
> directly into the image.
> ---
>  pixman/pixman-bits-image.c |   31 +++++++++++++++++++++++++++++--
>  pixman/pixman-general.c    |   19 +++++++++++++++++--
>  pixman/pixman-private.h    |   19 +++++++++++++++++--
>  3 files changed, 63 insertions(+), 6 deletions(-)
>
> diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
> index 9cd8b5a..df8d56e 100644
> --- a/pixman/pixman-bits-image.c
> +++ b/pixman/pixman-bits-image.c
> @@ -1410,6 +1410,18 @@ next_line_write_wide (pixman_iter_t *iter)
>     iter->y++;
>  }
>
> +static uint32_t *
> +get_scanline_direct_write (pixman_iter_t *iter, const uint32_t *mask)
> +{
> +    return iter->buffer;
> +}
> +
> +static void
> +next_line_direct_write (pixman_iter_t *iter)
> +{
> +    iter->buffer += iter->image->bits.rowstride;
> +}
> +
>  void
>  _pixman_bits_image_iter_init (pixman_image_t *image,
>                              pixman_iter_t *iter,
> @@ -1418,8 +1430,23 @@ _pixman_bits_image_iter_init (pixman_image_t *image,
>  {
>     if ((flags & (ITER_NARROW | ITER_WRITE)) == (ITER_NARROW | ITER_WRITE))
>     {
> -       iter->get_scanline = get_scanline_narrow;
> -       iter->next_line = next_line_write_narrow;
> +       if (((image->common.flags &
> +             (FAST_PATH_NO_ALPHA_MAP | FAST_PATH_NO_ACCESSORS)) ==
> +            (FAST_PATH_NO_ALPHA_MAP | FAST_PATH_NO_ACCESSORS)) &&
> +           (image->bits.format == PIXMAN_a8r8g8b8      ||
> +            (image->bits.format == PIXMAN_x8r8g8b8     &&
> +             (flags & ITER_LOCALIZED_ALPHA))))
> +       {
> +           iter->buffer = image->bits.bits + y * image->bits.rowstride + x;
> +
> +           iter->get_scanline = get_scanline_direct_write;
> +           iter->next_line = next_line_direct_write;
> +       }
> +       else
> +       {
> +           iter->get_scanline = get_scanline_narrow;
> +           iter->next_line = next_line_write_narrow;
> +       }
>     }
>     else if (flags & ITER_WRITE)
>     {
> diff --git a/pixman/pixman-general.c b/pixman/pixman-general.c
> index 4f73331..84b52c2 100644
> --- a/pixman/pixman-general.c
> +++ b/pixman/pixman-general.c
> @@ -106,7 +106,7 @@ general_composite_rect  (pixman_implementation_t *imp,
>     pixman_iter_t src_iter, mask_iter, dest_iter;
>     pixman_combine_32_func_t compose;
>     pixman_bool_t component_alpha;
> -    iter_flags_t narrow;
> +    iter_flags_t narrow, dest_flags;
>     int Bpp;
>     int i;
>
> @@ -143,9 +143,24 @@ general_composite_rect  (pixman_implementation_t *imp,
>                                      mask_x, mask_y, width, height,
>                                      mask_buffer, narrow);
>
> +    if (op == PIXMAN_OP_CLEAR          ||
> +       op == PIXMAN_OP_SRC             ||
> +       op == PIXMAN_OP_DST             ||
> +       op == PIXMAN_OP_OVER            ||
> +       op == PIXMAN_OP_IN_REVERSE      ||
> +       op == PIXMAN_OP_OUT_REVERSE     ||
> +       op == PIXMAN_OP_ADD)
> +    {
> +       dest_flags = narrow | ITER_WRITE | ITER_LOCALIZED_ALPHA;
> +    }
> +    else
> +    {
> +       dest_flags = narrow | ITER_WRITE;
> +    }
> +
>     _pixman_implementation_iter_init (imp->toplevel, &dest_iter, dest,
>                                      dest_x, dest_y, width, height,
> -                                     dest_buffer, narrow | ITER_WRITE);
> +                                     dest_buffer, dest_flags);
>
>     component_alpha =
>         mask                            &&
> diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
> index 8d3a604..8532b98 100644
> --- a/pixman/pixman-private.h
> +++ b/pixman/pixman-private.h
> @@ -185,8 +185,23 @@ union pixman_image
>  typedef struct pixman_iter_t pixman_iter_t;
>  typedef enum
>  {
> -    ITER_NARROW        = (1 << 0),
> -    ITER_WRITE =  (1 << 1)
> +    ITER_NARROW =              (1 << 0),
> +    ITER_WRITE =               (1 << 1),
> +
> +    /* "Localized alpha" is when the alpha channel is used only to compute the
> +     * alpha value of the destination and doesn't influence the RGB part.
> +     *
> +     * For example, the OVER operator has localized alpha for the destination,
> +     * because the destination alpha doesn't affect the results in the RGB
> +     * channels. Similarly, ADD has localized alpha for both source and
> +     * destination because the alpha values never 'leak' into the resulting
> +     * RGB values.
> +     *

I thought I understood the definition after the first two lines, then
the OVER/ADD
explanation confused me. After reading again everything, I was convinced again
that I had understood it correctly the first time.

I don't know if this is actually better (I still think that it needs a
better wording),
but I'd suggest something like:

+    /* "Separable alpha" is when the alpha channel is used only to compute the
+     * alpha value of the destination. This means that the computation of the
+     * RGB values of the result is independent of the alpha value.
+     *
+     * For example, the OVER operator has separable alpha for the destination,
+     * because the RGB values of the result can be computed without knowing
+     * the destination alpha. Similarly, ADD has separable alpha for
both source
+     * and destination because the RGB values of the result can be
+     * computed without knowing the alpha value of source or destination.

I'm proposing the "separable" term because it is consistent with
"separable blend modes"
(compositing operators in which each channel can be computed
individually), whereas
"localized" didn't tell me anything until I read the explanation.

> +     * When he destination is xRGB, this is useful knowledge, because then
> +     * we can treat it as ARGB, which means in some cases we can avoid copying
> +     * it to a temporary buffer.
> +     */
> +    ITER_LOCALIZED_ALPHA =     (1 << 2)
>  } iter_flags_t;
>
>  struct pixman_iter_t
> --
> 1.6.0.6
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman
>


More information about the Pixman mailing list