[Pixman] pixman_region_init_from_bitmap patched

Soeren Sandmann sandmann at daimi.au.dk
Tue Feb 16 11:56:10 PST 2010


Hi,

> Attached are the patches for pixman_region_init_from_bitmap, do these
> look ok to you?

I think it makes sense to have these functions. The patches are also
available as a branch here:

    http://cgit.freedesktop.org/~sandmann/pixman/log/?h=bfr

Some comments:

- Can the BM_ADDRECT macro become an inline function instead? The goto
  can be dealt with by having it return a boolean. Something like

        if (!add_rect())
                goto error;

- We need to add Keith to the copyright statement

- I think the API should take a pixman_image_t instead of pointers. I
  know I said otherwise on IRC, but we need the information in the
  image struct so that we can deal with accessors. If we add subimages
  in the future, we will probably need this function to work on those
  as well.

  The function should then return_if_fail() if the image format is not
  a1, and the name should likely be _from_image() so that we can relax
  it to take say a8 images at some point.

- A test program would be useful.

- One style comment: if one branch of an if statement has braces, the
  other should as well.


Thanks,
Soren

> =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
>  Alexander Larsson                                            Red Hat, Inc 
>        alexl at redhat.com            alexander.larsson at gmail.com 
> He's an all-American one-eyed matador searching for his wife's true killer. 
> She's a psychotic impetuous magician's assistant from a different time and 
> place. They fight crime! 
> 
> From 20b8d41db5d8c96a82cc7e856a95e5e72a2e3078 Mon Sep 17 00:00:00 2001
> From: Alexander Larsson <alexl at redhat.com>
> Date: Mon, 15 Feb 2010 09:39:59 +0100
> Subject: [PATCH 1/2] Move SCREEN_SHIFT_LEFT/RIGHT to pixman-private.h
> 
> This is needed for later use in other code.
> ---
>  pixman/pixman-edge-imp.h |    8 --------
>  pixman/pixman-private.h  |    8 ++++++++
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/pixman/pixman-edge-imp.h b/pixman/pixman-edge-imp.h
> index a30f821..74720d1 100644
> --- a/pixman/pixman-edge-imp.h
> +++ b/pixman/pixman-edge-imp.h
> @@ -79,14 +79,6 @@ RASTERIZE_EDGES (pixman_image_t  *image,
>  #if N_BITS == 1
>  	    {
>  
> -#ifdef WORDS_BIGENDIAN
> -#   define SCREEN_SHIFT_LEFT(x,n)	((x) << (n))
> -#   define SCREEN_SHIFT_RIGHT(x,n)	((x) >> (n))
> -#else
> -#   define SCREEN_SHIFT_LEFT(x,n)	((x) >> (n))
> -#   define SCREEN_SHIFT_RIGHT(x,n)	((x) << (n))
> -#endif
> -
>  #define LEFT_MASK(x)							\
>  		(((x) & 0x1f) ?						\
>  		 SCREEN_SHIFT_RIGHT (0xffffffff, (x) & 0x1f) : 0)
> diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
> index c99b101..efc690d 100644
> --- a/pixman/pixman-private.h
> +++ b/pixman/pixman-private.h
> @@ -749,6 +749,14 @@ pixman_region16_copy_from_region32 (pixman_region16_t *dst,
>       PIXMAN_FORMAT_G (f) > 8 ||						\
>       PIXMAN_FORMAT_B (f) > 8)
>  
> +#ifdef WORDS_BIGENDIAN
> +#   define SCREEN_SHIFT_LEFT(x,n)	((x) << (n))
> +#   define SCREEN_SHIFT_RIGHT(x,n)	((x) >> (n))
> +#else
> +#   define SCREEN_SHIFT_LEFT(x,n)	((x) >> (n))
> +#   define SCREEN_SHIFT_RIGHT(x,n)	((x) << (n))
> +#endif
> +
>  /*
>   * Various debugging code
>   */
> -- 
> 1.6.6
> 
> 
> From ef631cec39bed839a0791d92e4eaffd496f76ba3 Mon Sep 17 00:00:00 2001
> From: Alexander Larsson <alexl at redhat.com>
> Date: Mon, 15 Feb 2010 09:40:50 +0100
> Subject: [PATCH 2/2] Add pixman_region{32}_init_from_bitmap
> 
> This creates a region from a bitmap in PIXMAN_a1 format.
> ---
>  pixman/pixman-region.c |  215 ++++++++++++++++++++++++++++++++++++++++++++++++
>  pixman/pixman.h        |   10 ++
>  2 files changed, 225 insertions(+), 0 deletions(-)
> 
> diff --git a/pixman/pixman-region.c b/pixman/pixman-region.c
> index 9f7515c..5c73dcf 100644
> --- a/pixman/pixman-region.c
> +++ b/pixman/pixman-region.c
> @@ -2539,3 +2539,218 @@ PREFIX (_init_rects) (region_type_t *region,
>  
>      return validate (region, &i);
>  }
> +
> +#define READ(_ptr) (*(_ptr))
> +
> +#define BM_ADDRECT(reg,r,fr,rx1,ry1,rx2,ry2)			\
> +if (((rx1) < (rx2)) && ((ry1) < (ry2)) &&			\
> +    (!((reg)->data->numRects &&					\
> +       ((r-1)->y1 == (ry1)) &&					\
> +       ((r-1)->y2 == (ry2)) &&					\
> +       ((r-1)->x1 <= (rx1)) &&					\
> +       ((r-1)->x2 >= (rx2)))))					\
> +{								\
> +    if (!(reg)->data ||                                         \
> +        (reg)->data->numRects == (reg)->data->size)		\
> +    {								\
> +        if (!pixman_rect_alloc (reg, 1))			\
> +          goto error;						\
> +        fr = PIXREGION_BOXPTR(reg);				\
> +        r = fr + (reg)->data->numRects;				\
> +    }								\
> +    r->x1 = (rx1);						\
> +    r->y1 = (ry1);						\
> +    r->x2 = (rx2);						\
> +    r->y2 = (ry2);						\
> +    (reg)->data->numRects++;					\
> +    if(r->x1 < (reg)->extents.x1)				\
> +        (reg)->extents.x1 = r->x1;				\
> +    if(r->x2 > (reg)->extents.x2)				\
> +        (reg)->extents.x2 = r->x2;				\
> +    r++;							\
> +}
> +
> +/* Convert bitmap clip mask into clipping region.
> + * First, goes through each line and makes boxes by noting the transitions
> + * from 0 to 1 and 1 to 0.
> + * Then it coalesces the current line with the previous if they have boxes
> + * at the same X coordinates.
> + * Stride is in number of uint32_t per line.
> + */
> +PIXMAN_EXPORT void
> +PREFIX (_init_from_bitmap) (region_type_t *region,
> +                            uint32_t *data,
> +                            int width, int height, int stride)
> +{
> +    uint32_t mask0 = 0xffffffff & ~SCREEN_SHIFT_RIGHT(0xffffffff, 1);
> +    box_type_t *first_rect, *rects, *prect_line_start;
> +    box_type_t *old_rect, *new_rect;
> +    uint32_t *pw, w, *pw_line, *pw_line_end;
> +    int	irect_prev_start, irect_line_start;
> +    int	h, base, rx1 = 0, crects;
> +    int	ib;
> +    pixman_bool_t in_box, same;
> +
> +    PREFIX(_init) (region);
> +
> +    first_rect = PIXREGION_BOXPTR(region);
> +    rects = first_rect;
> +
> +    pw_line = data;
> +
> +    region->extents.x1 = width - 1;
> +    region->extents.x2 = 0;
> +    irect_prev_start = -1;
> +    for (h = 0; h < height; h++)
> +    {
> +        pw = pw_line;
> +        pw_line += stride;
> +        irect_line_start = rects - first_rect;
> +
> +        /* If the Screen left most bit of the word is set, we're starting in
> +         * a box */
> +        if (READ(pw) & mask0)
> +        {
> +            in_box = TRUE;
> +            rx1 = 0;
> +        }
> +        else
> +            in_box = FALSE;
> +
> +        /* Process all words which are fully in the pixmap */
> +        pw_line_end = pw + (width >> 5);
> +        for (base = 0; pw < pw_line_end; base += 32)
> +        {
> +            w = READ(pw++);
> +            if (in_box)
> +            {
> +                if (!~w)
> +                    continue;
> +            }
> +            else
> +            {
> +                if (!w)
> +                    continue;
> +            }
> +            for (ib = 0; ib < 32; ib++)
> +            {
> +                /* If the Screen left most bit of the word is set, we're
> +                 * starting a box */
> +                if (w & mask0)
> +                {
> +                    if (!in_box)
> +                    {
> +                        rx1 = base + ib;
> +                        /* start new box */
> +                        in_box = TRUE;
> +                    }
> +                }
> +                else
> +                {
> +                    if (in_box)
> +                    {
> +                        /* end box */
> +                        BM_ADDRECT(region, rects, first_rect,
> +                                   rx1, h, base + ib, h + 1);
> +                        in_box = FALSE;
> +                    }
> +                }
> +                /* Shift the word VISUALLY left one. */
> +                w = SCREEN_SHIFT_LEFT(w, 1);
> +            }
> +        }
> +
> +        if (width & 31)
> +        {
> +            /* Process final partial word on line */
> +             w = READ(pw++);
> +            for (ib = 0; ib < (width & 31); ib++)
> +            {
> +                /* If the Screen left most bit of the word is set, we're
> +                 * starting a box */
> +                if (w & mask0)
> +                {
> +                    if (!in_box)
> +                    {
> +                        rx1 = base + ib;
> +                        /* start new box */
> +                        in_box = TRUE;
> +                    }
> +                }
> +                else
> +                {
> +                    if (in_box)
> +                    {
> +                        /* end box */
> +                        BM_ADDRECT(region, rects, first_rect,
> +                                   rx1, h, base + ib, h + 1);
> +                        in_box = FALSE;
> +                    }
> +                }
> +                /* Shift the word VISUALLY left one. */
> +                w = SCREEN_SHIFT_LEFT(w, 1);
> +            }
> +        }
> +        /* If scanline ended with last bit set, end the box */
> +        if (in_box)
> +        {
> +            BM_ADDRECT(region, rects, first_rect,
> +                       rx1, h, base + (width & 31), h + 1);
> +        }
> +        /* if all rectangles on this line have the same x-coords as
> +         * those on the previous line, then add 1 to all the previous  y2s and
> +         * throw away all the rectangles from this line
> +         */
> +        same = FALSE;
> +        if (irect_prev_start != -1)
> +        {
> +            crects = irect_line_start - irect_prev_start;
> +            if (crects != 0 &&
> +                crects == ((rects - first_rect) - irect_line_start))
> +            {
> +                old_rect = first_rect + irect_prev_start;
> +                new_rect = prect_line_start = first_rect + irect_line_start;
> +                same = TRUE;
> +                while (old_rect < prect_line_start)
> +                {
> +                    if ((old_rect->x1 != new_rect->x1) ||
> +                        (old_rect->x2 != new_rect->x2))
> +                    {
> +                          same = FALSE;
> +                          break;
> +                    }
> +                    old_rect++;
> +                    new_rect++;
> +                }
> +                if (same)
> +                {
> +                    old_rect = first_rect + irect_prev_start;
> +                    while (old_rect < prect_line_start)
> +                    {
> +                        old_rect->y2 += 1;
> +                        old_rect++;
> +                    }
> +                    rects -= crects;
> +                    region->data->numRects -= crects;
> +                }
> +            }
> +        }
> +        if(!same)
> +            irect_prev_start = irect_line_start;
> +    }
> +    if (!region->data->numRects)
> +        region->extents.x1 = region->extents.x2 = 0;
> +    else
> +    {
> +        region->extents.y1 = PIXREGION_BOXPTR(region)->y1;
> +        region->extents.y2 = PIXREGION_END(region)->y2;
> +        if (region->data->numRects == 1)
> +        {
> +            free (region->data);
> +            region->data = NULL;
> +        }
> +    }
> +
> + error:
> +    return;
> +}
> diff --git a/pixman/pixman.h b/pixman/pixman.h
> index 03a233e..457df37 100644
> --- a/pixman/pixman.h
> +++ b/pixman/pixman.h
> @@ -401,6 +401,11 @@ pixman_bool_t           pixman_region_init_rects         (pixman_region16_t *reg
>  							  int                count);
>  void                    pixman_region_init_with_extents  (pixman_region16_t *region,
>  							  pixman_box16_t    *extents);
> +void                    pixman_region_init_from_bitmap   (pixman_region16_t *region,
> +							  uint32_t          *data,
> +							  int                width,
> +							  int                height,
> +							  int                stride);
>  void                    pixman_region_fini               (pixman_region16_t *region);
>  
>  
> @@ -488,6 +493,11 @@ pixman_bool_t           pixman_region32_init_rects         (pixman_region32_t *r
>  							    int                count);
>  void                    pixman_region32_init_with_extents  (pixman_region32_t *region,
>  							    pixman_box32_t    *extents);
> +void                    pixman_region32_init_from_bitmap   (pixman_region32_t *region,
> +							    uint32_t          *data,
> +							    int                width,
> +							    int                height,
> +							    int                stride);
>  void                    pixman_region32_fini               (pixman_region32_t *region);
>  
>  
> -- 
> 1.6.6


More information about the Pixman mailing list