Re: [Pixman] FAST_PATH_SAMPLES_COVER_CLIP flag & fast_composite_scaled_nearest_*

2010-07-29 Thread Søren Sandmann Pedersen
Siarhei Siamashka  writes:

> Overall looks like a good fix, a few comments below.

Thanks for the comments. I'll send a new patch with a long commit log
as a follow-up to this message (provided I can make it work with
git-send-email), but I'll reply to some specifics below.

The main difference in the new patch is that the 16BIT_SAFE flag is
gone entirely, and instead pixman will simply bail out in that case.

> > +if (!transform)
> > +{
> > +box->x1 = pixman_fixed_to_int (pixman_int_to_fixed (box->x1) + 
> > pixman_fixed_1 / 2 + x_off);
> > +box->y1 = pixman_fixed_to_int (pixman_int_to_fixed (box->y1) + 
> > pixman_fixed_1 / 2 + y_off);
> > +box->x2 = pixman_fixed_to_int (pixman_int_to_fixed (box->x2) - 
> > pixman_fixed_1 / 2 + x_off) + width + 1;
> > +box->y2 = pixman_fixed_to_int (pixman_int_to_fixed (box->y2) - 
> > pixman_fixed_1 / 2 + y_off) + height + 1;
> > +return TRUE;
>
> That's an interesting case. If I understand it correctly, without any 
> transform
> at all, both NEAREST and BILINEAR filters should introduce no changes in the
> bounds. This is fine for NEAREST, but not for BILINEAR filter which gets the
> bounds expanded by 1.
>
> The bilinear filter is a bit special, because the sampling of extra rightmost
> pixels may technically happen according to formulas, but they get multiplied 
> by
> zero anyway, so make no difference and are ignored by non-transformed fast
> paths.

The non-transformed fast paths certainly make this assumption, but the
general path will actually read these pixels, so we still have to
account for them. If we start seeing a lot of cases where images have
a bilinear filter, an identity transform, and we end up hitting the
general case, we could look into it, but I don't think this is a very
common case.

> > +v[0].vector[0] = pixman_int_to_fixed (box->x1) + pixman_fixed_1 / 2;
> > +v[0].vector[1] = pixman_int_to_fixed (box->y1) + pixman_fixed_1 / 2;
> > +v[0].vector[2] = pixman_int_to_fixed (1);
> > +
> > +v[1].vector[0] = pixman_int_to_fixed (box->x2) - pixman_fixed_1 / 2;
> > +v[1].vector[1] = pixman_int_to_fixed (box->y1) + pixman_fixed_1 / 2;
> > +v[1].vector[2] = pixman_int_to_fixed (1);
> > +
> > +v[2].vector[0] = pixman_int_to_fixed (box->x2) - pixman_fixed_1 / 2;
> > +v[2].vector[1] = pixman_int_to_fixed (box->y2) - pixman_fixed_1 / 2;
> > +v[2].vector[2] = pixman_int_to_fixed (1);
> > +
> > +v[3].vector[0] = pixman_int_to_fixed (box->x1) + pixman_fixed_1 / 2;
> > +v[3].vector[1] = pixman_int_to_fixed (box->y2) - pixman_fixed_1 / 2;
> > +v[3].vector[2] = pixman_int_to_fixed (1);
> > +
> > +for (i = 0; i < 4; ++i)
> > +{
> > +   if (!pixman_transform_point (transform, &v[i]))
> > +   return FALSE;
>
> Still what about the subtle differences between pixman_transform_point() and
> pixman_transform_point_3d()? They are not exactly the same. Transformed
> fetchers and fast path functions are all using pixman_transform_point_3d().

It's true that they are not exactly the same. In the new patch, I have
introduced a bit of slack in the computation of the source area (8 *
pixman_fixed_e). This is hopefully enough to account for any rounding
differences. The common special case where a NEAREST image is being
scaled so that the source area exactly matches the image will still
work because all we need in that case is for the computed bounds to be
within [0, 0.5] and 8 * pixman_fixed_1 is not even close to that.

It does mean of course that if you scale an image very slightly down,
the new code might decide to not set the FAST_PATH_SAMPLES_COVER_CLIP
flag, but I'm not that concerned about this.

I'd like to avoid relying on the two transformation functions being
exactly the same because I think it should be considered legitimate to
write compositing functions that transform in a different way if
that's more efficient, even if it means slightly different results.

> Another (minor) issue is that pixman_transform_point() has division 
> operations,
> which may be not very good for performance.

We can't really avoid the divisions if the flags are to be set
correctly when the transformation is projective. I realize we don't
actually make use of the flags in that case, but it's still unpleasent
to rely on the knowledge that there aren't any projective fast paths,
in a place that should only have knowledge about a particular image.

So basically, I think the flags should always be computed correctly,
even if we know that an incorrect computation won't have any ill
effects.

> > +
> > +   x1 = pixman_fixed_to_int (v[i].vector[0] + x_off);
> > +   y1 = pixman_fixed_to_int (v[i].vector[1] + y_off);
> > +   x2 = x1 + width + 1;
> > +   y2 = y1 + height + 1;
>
> A minor performance improvement is possible. Addition of (width + 1) and
> (height + 1) to x2/y2 is done inside of the loop on each iteration here, 8
> times total. If done after the loop just before returning, it w

[Pixman] [PATCH 1/2] Extend scaling-crash-test in various ways

2010-07-29 Thread Søren Sandmann Pedersen
From: Søren Sandmann Pedersen 

This extends scaling-crash-test to test some more things:

- All combinations of NEAREST/BILINEAR/CONVOLUTION filters and
  NORMAL/PAD/REFLECT repeat modes.

- Tests various scale factors very close to 1/7th such that the source
  area is very close to edge of the source image.

- The same things, only with scale factors very close to 1/32767th.

- Enables the commented-out tests for accessing memory outside the
  source buffer.

Also there is now a border around the source buffer which has a
different color than the source buffer itself so that if we sample
outside, it will show up.

Finally, the test now allows the destination buffer to not be changed
at all. This allows pixman to simply bail out in cases where the
transformation too strange.
---
 test/scaling-crash-test.c |  193 -
 1 files changed, 139 insertions(+), 54 deletions(-)

diff --git a/test/scaling-crash-test.c b/test/scaling-crash-test.c
index 4ab01e3..7a94115 100644
--- a/test/scaling-crash-test.c
+++ b/test/scaling-crash-test.c
@@ -8,117 +8,202 @@
  * We have a source image filled with solid color, set NORMAL or PAD repeat,
  * and some transform which results in nearest neighbour scaling.
  *
- * The expected result is the destination image filled with this solid
- * color.
+ * The expected result is either that the destination image filled with this 
solid
+ * color or, if the transformation is such that we can't composite anything at
+ * all, that nothing has changed in the destination.
+ *
+ * The surrounding memory of the source image is a different solid color so 
that
+ * we are sure to get failures if we access it.
  */
 static int
-do_test (int32_t   dst_size,
-int32_tsrc_size,
-int32_tsrc_offs,
-int32_tscale_factor,
-pixman_repeat_trepeat)
+run_test (int32_t  dst_width,
+ int32_t   dst_height,
+ int32_t   src_width,
+ int32_t   src_height,
+ int32_t   src_x,
+ int32_t   src_y,
+ int32_t   scale_x,
+ int32_t   scale_y,
+ pixman_filter_t   filter,
+ pixman_repeat_t   repeat)
 {
-int i;
 pixman_image_t *   src_img;
 pixman_image_t *   dst_img;
 pixman_transform_t transform;
 uint32_t * srcbuf;
 uint32_t * dstbuf;
+pixman_box32_t box = { 0, 0, src_width, src_height };
+pixman_color_t color_cc = { 0x, 0x, 0x, 0x };
+int result;
+int i;
 
-srcbuf = (uint32_t *)malloc (src_size * 4);
-dstbuf = (uint32_t *)malloc (dst_size * 4);
+static const pixman_fixed_t kernel[] =
+{
+#define D(f)   (pixman_double_to_fixed (f) + 0x0001)
+
+   pixman_int_to_fixed (5),
+   pixman_int_to_fixed (5),
+   D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0),
+   D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0),
+   D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0),
+   D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0),
+   D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0), D(1/25.0)
+};
+
+result = 0;
 
-/* horizontal test */
-memset (srcbuf, 0xCC, src_size * 4);
-memset (dstbuf, 0x33, dst_size * 4);
+srcbuf = (uint32_t *)malloc ((src_width + 10) * (src_height + 10) * 4);
+dstbuf = (uint32_t *)malloc (dst_width * dst_height * 4);
+
+memset (srcbuf, 0x88, src_width * src_height * 4);
+memset (dstbuf, 0x33, dst_width * dst_height * 4);
 
 src_img = pixman_image_create_bits (
-PIXMAN_a8r8g8b8, src_size, 1, srcbuf, src_size * 4);
+PIXMAN_a8r8g8b8, src_width, src_height,
+   srcbuf + (src_width + 10) * 5 + 5, (src_width + 10) * 4);
+
+pixman_image_fill_boxes (PIXMAN_OP_SRC, src_img, &color_cc, 1, &box);
+
 dst_img = pixman_image_create_bits (
-PIXMAN_a8r8g8b8, dst_size, 1, dstbuf, dst_size * 4);
+PIXMAN_a8r8g8b8, dst_width, dst_height, dstbuf, dst_width * 4);
 
-pixman_transform_init_scale (&transform, scale_factor, 65536);
+pixman_transform_init_scale (&transform, scale_x, scale_y);
 pixman_image_set_transform (src_img, &transform);
 pixman_image_set_repeat (src_img, repeat);
-pixman_image_set_filter (src_img, PIXMAN_FILTER_NEAREST, NULL, 0);
+if (filter == PIXMAN_FILTER_CONVOLUTION)
+   pixman_image_set_filter (src_img, filter, kernel, 27);
+else
+   pixman_image_set_filter (src_img, filter, NULL, 0);
 
 pixman_image_composite (PIXMAN_OP_SRC, src_img, NULL, dst_img,
-src_offs, 0, 0, 0, 0, 0, dst_size, 1);
+src_x, src_y, 0, 0, 0, 0, dst_width, dst_height);
 
 pixman_image_unref (src_img);
 pixman_image_unref (dst_img);
 
-for (i = 0; i < dst_size; i++)
+for (i = 0; i < dst_width * dst_height; i

[Pixman] [PATCH 2/2] Replace compute_src_extent_flags() with analyze_extents()

2010-07-29 Thread Søren Sandmann Pedersen
From: Søren Sandmann Pedersen 

This commit fixes two separate problems: 1. Incorrect computation of
the FAST_PATH_SAMPLES_COVER_CLIP flag, and 2. FAST_PATH_16BIT_SAFE is
a nonsensical thing to compute.

== 1. Incorrect computation of SAMPLES_COVER_CLIP:

Previously we were using pixman_transform_bounds() to compute which
source samples would be used for a composite operation. This is
incorrect for several reasons:

(a) pixman_transform_bounds() is transforming the integer bounding box
of the destination samples, where it should be transforming the
bounding box of the samples themselves. In other words, it is too
pessimistic in some cases.

(b) pixman_transform_bounds() is not rounding the same way as we do
during sampling. For example, for a NEAREST filter we subtract
pixman_fixed_e before rounding off to the nearest sample so that a
transformed value of 1 will round to the sample at 0.5 and not to the
one at 1.5. However, pixman_transform_bounds() would simply truncate
to 1 which would imply that the first sample to be used was the one at
1.5. In other words, it is too optimistic in some cases.

(c) The result of pixman_transform_bounds() does not account for the
interpolation filter applied to the source.

== 2. FAST_PATH_16BIT_SAFE is nonsensical

The FAST_PATH_16BIT_SAFE is a flag that indicates that various
computations can be safely done within a 16.16 fixed-point
variable. It was used by certain fast paths who relied on those
computations succeeding. The problem is that many other compositing
functions were making similar assumptions but not actually requiring
the flag to be set. Notably, all the general compositing functions
simply walk the source region using 16.16 variables. If the
transformation happens to overflow, strange things will happen.

So instead of computing this flag in certain cases, it is better to
simply detect that overflows will happen and not try to composite at
all in that case. This has the advantage that most compositing
functions can be written naturally way.

It does have the disadvantage that we are giving up on some cases that
previously worked, but those are all corner cases where the areas
involved were very close to the limits of the coordinate
system. Relying on these working reliably was always a somewhat
dubious proposition. The most important case that might have worked
previously was untransformed compositing involving images larger than
32 bits. But even in those cases, if you had REPEAT_PAD or
REPEAT_REFLECT turned on, you would hit bits_image_fetch_transformed()
which has the 16 bit limitations.

== Fixes

This patch fixes both problems by introducing a new function called
analyze_extents() that has the responsibility to reject corner cases,
and to compute flags based on the extents.

It does this through a new compute_sample_extents() function that will
compute a conservative (but tight) approximation to the bounding box
of the samples that will actually be needed. By basing the computation
on the positions of the _sample_ locations in the destination, and by
taking the interpolation filter into account, it fixes problem one.

The same function is also used with a one-pixel expanded version of
the destination extents. By checking if the transformed bounding box
will overflow 16.16 fixed point, it fixes problem two.
---
 pixman/pixman-fast-path.c |2 +-
 pixman/pixman-private.h   |3 +-
 pixman/pixman.c   |  288 +
 3 files changed, 212 insertions(+), 81 deletions(-)

diff --git a/pixman/pixman-fast-path.c b/pixman/pixman-fast-path.c
index 6ed1580..7858a6d 100644
--- a/pixman/pixman-fast-path.c
+++ b/pixman/pixman-fast-path.c
@@ -1881,7 +1881,7 @@ static const pixman_fast_path_t c_fast_paths[] =
 #define SIMPLE_NEAREST_FAST_PATH(op,s,d,func)  \
 {   PIXMAN_OP_ ## op,  \
PIXMAN_ ## s,   \
-   SCALED_NEAREST_FLAGS | HAS_NORMAL_REPEAT_FLAGS | FAST_PATH_16BIT_SAFE | 
FAST_PATH_X_UNIT_POSITIVE, \
+   SCALED_NEAREST_FLAGS | HAS_NORMAL_REPEAT_FLAGS | 
FAST_PATH_X_UNIT_POSITIVE, \
PIXMAN_null, 0, \
PIXMAN_ ## d, FAST_PATH_STD_DEST_FLAGS, \
fast_composite_scaled_nearest_ ## func ## _normal ## _ ## op,   \
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index 8718fcb..f6424e7 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -577,8 +577,7 @@ _pixman_choose_implementation (void);
 #define FAST_PATH_NEEDS_WORKAROUND (1 << 14)
 #define FAST_PATH_NO_NONE_REPEAT   (1 << 15)
 #define FAST_PATH_SAMPLES_COVER_CLIP   (1 << 16)
-#define FAST_PATH_16BIT_SAFE   (1 << 17)
-#define FAST_PATH_X_UNIT_POSITIVE  (1 << 18)
+#define FAST_PATH_X_UNIT_POSITIVE  (1 << 17)
 
 #define _FAST_PATH_STANDARD_F

[Pixman] [bits] Some cleanups of fetchers, plus flags for bits-image

2010-07-29 Thread Søren Sandmann
The following patches contain some cleanups of the way the scanline
fetching works for bits images. By removing the support for alpha map
recursion, there is no longer any need to distinguish between 'raw'
and non-raw scanline fetchers.

Instead there is only one type of scanline fetcher, and only two
function pointers in the image struct: one for 32 bit fetching and one
for 64 bits.

This then allows the various types of fetchers to be stored in an
array indexed by flags, similar to how the fast paths are stored.

Soren

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman


[Pixman] [PATCH 1/7] Eliminate recursion from alpha map code

2010-07-29 Thread Søren Sandmann
From: Søren Sandmann Pedersen 

Alpha maps with alpha maps are no longer supported. It's not a useful
feature and it could could lead to infinite recursion.
---
 pixman/pixman-bits-image.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index 36ea0af..81722c2 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -51,7 +51,7 @@ bits_image_store_scanline_32 (bits_image_t *  image,
x -= image->common.alpha_origin_x;
y -= image->common.alpha_origin_y;
 
-   bits_image_store_scanline_32 (image->common.alpha_map, x, y, width, 
buffer);
+   image->common.alpha_map->store_scanline_raw_32 
(image->common.alpha_map, x, y, width, buffer);
 }
 }
 
@@ -69,7 +69,7 @@ bits_image_store_scanline_64 (bits_image_t *  image,
x -= image->common.alpha_origin_x;
y -= image->common.alpha_origin_y;
 
-   bits_image_store_scanline_64 (image->common.alpha_map, x, y, width, 
buffer);
+   image->common.alpha_map->store_scanline_raw_64 
(image->common.alpha_map, x, y, width, buffer);
 }
 }
 
-- 
1.7.1.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman


[Pixman] [PATCH 2/7] Eliminate get_pixel_32() and get_pixel_64() from bits_image.

2010-07-29 Thread Søren Sandmann
From: Søren Sandmann Pedersen 

These functions can simply be passed as arguments to the various pixel
fetchers. We don't need to store them. Since they are known at compile
time and the pixel fetchers are force_inline, this is not a
performance issue.

Also temporarily make all pixel access go through the alpha path.
---
 pixman/pixman-bits-image.c |   78 +--
 pixman/pixman-private.h|4 --
 2 files changed, 45 insertions(+), 37 deletions(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index 81722c2..0372217 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -96,38 +96,47 @@ _pixman_image_store_scanline_64 (bits_image_t *  image,
 /* Fetch functions */
 
 static uint32_t
-bits_image_fetch_pixel_alpha (bits_image_t *image, int x, int y)
+fetch_pixel_general (bits_image_t *image, int x, int y, pixman_bool_t 
check_bounds)
 {
 uint32_t pixel;
-uint32_t pixel_a;
+
+if (check_bounds &&
+   (x < 0 || x >= image->width || y < 0 || y >= image->height))
+{
+   return 0;
+}
 
 pixel = image->fetch_pixel_raw_32 (image, x, y);
 
-assert (image->common.alpha_map);
+if (image->common.alpha_map)
+{
+   uint32_t pixel_a;
 
-x -= image->common.alpha_origin_x;
-y -= image->common.alpha_origin_y;
+   x -= image->common.alpha_origin_x;
+   y -= image->common.alpha_origin_y;
 
-if (x < 0 || x >= image->common.alpha_map->width ||
-   y < 0 || y >= image->common.alpha_map->height)
-{
-   pixel_a = 0;
-}
-else
-{
-   pixel_a = image->common.alpha_map->fetch_pixel_raw_32 (
-   image->common.alpha_map, x, y);
-   pixel_a = ALPHA_8 (pixel_a);
-}
+   if (x < 0 || x >= image->common.alpha_map->width ||
+   y < 0 || y >= image->common.alpha_map->height)
+   {
+   pixel_a = 0;
+   }
+   else
+   {
+   pixel_a = image->common.alpha_map->fetch_pixel_raw_32 (
+   image->common.alpha_map, x, y);
 
-pixel &= 0x00ff;
-pixel |= (pixel_a << 24);
+   pixel_a = ALPHA_8 (pixel_a);
+   }
+
+   pixel &= 0x00ff;
+   pixel |= (pixel_a << 24);
+}
 
 return pixel;
 }
 
 static force_inline uint32_t
-get_pixel (bits_image_t *image, int x, int y, pixman_bool_t check_bounds)
+fetch_pixel_no_alpha (bits_image_t *image, int x, int y, pixman_bool_t 
check_bounds)
 {
 if (check_bounds &&
(x < 0 || x >= image->width || y < 0 || y >= image->height))
@@ -135,9 +144,12 @@ get_pixel (bits_image_t *image, int x, int y, 
pixman_bool_t check_bounds)
return 0;
 }
 
-return image->fetch_pixel_32 (image, x, y);
+return image->fetch_pixel_raw_32 (image, x, y);
 }
 
+typedef uint32_t (* get_pixel_t) (bits_image_t *image,
+ int x, int y, pixman_bool_t check_bounds);
+
 static force_inline void
 repeat (pixman_repeat_t repeat, int size, int *coord)
 {
@@ -169,7 +181,8 @@ repeat (pixman_repeat_t repeat, int size, int *coord)
 static force_inline uint32_t
 bits_image_fetch_pixel_nearest (bits_image_t   *image,
pixman_fixed_t  x,
-   pixman_fixed_t  y)
+   pixman_fixed_t  y,
+   get_pixel_t get_pixel)
 {
 int x0 = pixman_fixed_to_int (x - pixman_fixed_e);
 int y0 = pixman_fixed_to_int (y - pixman_fixed_e);
@@ -281,7 +294,8 @@ bilinear_interpolation (uint32_t tl, uint32_t tr,
 static force_inline uint32_t
 bits_image_fetch_pixel_bilinear (bits_image_t   *image,
 pixman_fixed_t  x,
-pixman_fixed_t  y)
+pixman_fixed_t  y,
+get_pixel_t get_pixel)
 {
 pixman_repeat_t repeat_mode = image->common.repeat;
 int width = image->width;
@@ -538,7 +552,8 @@ bits_image_fetch_bilinear_no_repeat_ (pixman_image_t * 
ima,
 static force_inline uint32_t
 bits_image_fetch_pixel_convolution (bits_image_t   *image,
pixman_fixed_t  x,
-   pixman_fixed_t  y)
+   pixman_fixed_t  y,
+   get_pixel_t get_pixel)
 {
 pixman_fixed_t *params = image->common.filter_params;
 int x_off = (params[0] - pixman_fixed_1) >> 1;
@@ -611,23 +626,24 @@ bits_image_fetch_pixel_convolution (bits_image_t   *image,
 static force_inline uint32_t
 bits_image_fetch_pixel_filtered (bits_image_t *image,
 pixman_fixed_t x,
-pixman_fixed_t y)
+pixman_fixed_t y,
+get_pixel_tget_pixel)
 {
 switch (image->common.filter)
 {
 case PIXMAN_FILTER_NEAREST:
 case PIXMAN_FILTER_FAST:
-   return bits_image_fetch_pixel_near

[Pixman] [PATCH 3/7] Split bits_image_fetch_transformed() into two functions.

2010-07-29 Thread Søren Sandmann
From: Søren Sandmann Pedersen 

One function deals with the common affine, no-alpha-map case. The
other deals with perspective transformations and alpha maps.
---
 pixman/pixman-bits-image.c |  222 +---
 1 files changed, 128 insertions(+), 94 deletions(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index 0372217..09b69df 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -95,48 +95,9 @@ _pixman_image_store_scanline_64 (bits_image_t *  image,
 
 /* Fetch functions */
 
-static uint32_t
-fetch_pixel_general (bits_image_t *image, int x, int y, pixman_bool_t 
check_bounds)
-{
-uint32_t pixel;
-
-if (check_bounds &&
-   (x < 0 || x >= image->width || y < 0 || y >= image->height))
-{
-   return 0;
-}
-
-pixel = image->fetch_pixel_raw_32 (image, x, y);
-
-if (image->common.alpha_map)
-{
-   uint32_t pixel_a;
-
-   x -= image->common.alpha_origin_x;
-   y -= image->common.alpha_origin_y;
-
-   if (x < 0 || x >= image->common.alpha_map->width ||
-   y < 0 || y >= image->common.alpha_map->height)
-   {
-   pixel_a = 0;
-   }
-   else
-   {
-   pixel_a = image->common.alpha_map->fetch_pixel_raw_32 (
-   image->common.alpha_map, x, y);
-
-   pixel_a = ALPHA_8 (pixel_a);
-   }
-
-   pixel &= 0x00ff;
-   pixel |= (pixel_a << 24);
-}
-
-return pixel;
-}
-
 static force_inline uint32_t
-fetch_pixel_no_alpha (bits_image_t *image, int x, int y, pixman_bool_t 
check_bounds)
+fetch_pixel_no_alpha (bits_image_t *image,
+ int x, int y, pixman_bool_t check_bounds)
 {
 if (check_bounds &&
(x < 0 || x >= image->width || y < 0 || y >= image->height))
@@ -654,12 +615,101 @@ bits_image_fetch_pixel_filtered (bits_image_t *image,
 }
 
 static void
-bits_image_fetch_transformed (pixman_image_t * image,
-  int  offset,
-  int  line,
-  int  width,
-  uint32_t *   buffer,
-  const uint32_t * mask)
+bits_image_fetch_affine_no_alpha (pixman_image_t * image,
+ int  offset,
+ int  line,
+ int  width,
+ uint32_t *   buffer,
+ const uint32_t * mask)
+{
+pixman_fixed_t x, y;
+pixman_fixed_t ux, uy;
+pixman_vector_t v;
+int i;
+
+/* reference point is the center of the pixel */
+v.vector[0] = pixman_int_to_fixed (offset) + pixman_fixed_1 / 2;
+v.vector[1] = pixman_int_to_fixed (line) + pixman_fixed_1 / 2;
+v.vector[2] = pixman_fixed_1;
+
+if (image->common.transform)
+{
+   if (!pixman_transform_point_3d (image->common.transform, &v))
+   return;
+
+   ux = image->common.transform->matrix[0][0];
+   uy = image->common.transform->matrix[1][0];
+}
+else
+{
+   ux = pixman_fixed_1;
+   uy = 0;
+}
+
+x = v.vector[0];
+y = v.vector[1];
+
+for (i = 0; i < width; ++i)
+{
+   if (!mask || mask[i])
+   {
+   buffer[i] = bits_image_fetch_pixel_filtered (
+   &image->bits, x, y, fetch_pixel_no_alpha);
+   }
+   
+   x += ux;
+   y += uy;
+}
+}
+
+/* General fetcher */
+static force_inline uint32_t
+fetch_pixel_general (bits_image_t *image, int x, int y, pixman_bool_t 
check_bounds)
+{
+uint32_t pixel;
+
+if (check_bounds &&
+   (x < 0 || x >= image->width || y < 0 || y >= image->height))
+{
+   return 0;
+}
+
+pixel = image->fetch_pixel_raw_32 (image, x, y);
+
+if (image->common.alpha_map)
+{
+   uint32_t pixel_a;
+
+   x -= image->common.alpha_origin_x;
+   y -= image->common.alpha_origin_y;
+
+   if (x < 0 || x >= image->common.alpha_map->width ||
+   y < 0 || y >= image->common.alpha_map->height)
+   {
+   pixel_a = 0;
+   }
+   else
+   {
+   pixel_a = image->common.alpha_map->fetch_pixel_raw_32 (
+   image->common.alpha_map, x, y);
+
+   pixel_a = ALPHA_8 (pixel_a);
+   }
+
+   pixel &= 0x00ff;
+   pixel |= (pixel_a << 24);
+}
+
+return pixel;
+}
+
+static void
+bits_image_fetch_general (pixman_image_t * image,
+ int  offset,
+ int  line,
+ int  width,
+ uint32_t *   buffer,
+ const uint32_t * mask)
 {
 pixman_fixed_t x, y, w;
 pixman_fixed_t ux, uy, uw;
@@ -671,8 +721,6 @@ bits_image_fetch_transformed (pixman_image_t * image,
 v.vector[1] = pixman_int_to_fixed (line) + pixman_f

[Pixman] [PATCH 4/7] Eliminate the store_scanline_{32, 64} function pointers.

2010-07-29 Thread Søren Sandmann
From: Søren Sandmann Pedersen 

Now that we can't recurse on alpha maps, they are not needed anymore.
---
 pixman/pixman-bits-image.c |   54 +---
 pixman/pixman-private.h|4 ---
 2 files changed, 16 insertions(+), 42 deletions(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index 09b69df..f8ddcbc 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -36,13 +36,12 @@
 #include "pixman-combine32.h"
 
 /* Store functions */
-
-static void
-bits_image_store_scanline_32 (bits_image_t *  image,
-  int x,
-  int y,
-  int width,
-  const uint32_t *buffer)
+void
+_pixman_image_store_scanline_32 (bits_image_t *  image,
+ int x,
+ int y,
+ int width,
+ const uint32_t *buffer)
 {
 image->store_scanline_raw_32 (image, x, y, width, buffer);
 
@@ -51,16 +50,17 @@ bits_image_store_scanline_32 (bits_image_t *  image,
x -= image->common.alpha_origin_x;
y -= image->common.alpha_origin_y;
 
-   image->common.alpha_map->store_scanline_raw_32 
(image->common.alpha_map, x, y, width, buffer);
+   image->common.alpha_map->store_scanline_raw_32 (
+   image->common.alpha_map, x, y, width, buffer);
 }
 }
 
-static void
-bits_image_store_scanline_64 (bits_image_t *  image,
-  int x,
-  int y,
-  int width,
-  const uint32_t *buffer)
+void
+_pixman_image_store_scanline_64 (bits_image_t *  image,
+ int x,
+ int y,
+ int width,
+ const uint32_t *buffer)
 {
 image->store_scanline_raw_64 (image, x, y, width, buffer);
 
@@ -69,30 +69,11 @@ bits_image_store_scanline_64 (bits_image_t *  image,
x -= image->common.alpha_origin_x;
y -= image->common.alpha_origin_y;
 
-   image->common.alpha_map->store_scanline_raw_64 
(image->common.alpha_map, x, y, width, buffer);
+   image->common.alpha_map->store_scanline_raw_64 (
+   image->common.alpha_map, x, y, width, buffer);
 }
 }
 
-void
-_pixman_image_store_scanline_32 (bits_image_t *  image,
- int x,
- int y,
- int width,
- const uint32_t *buffer)
-{
-image->store_scanline_32 (image, x, y, width, buffer);
-}
-
-void
-_pixman_image_store_scanline_64 (bits_image_t *  image,
- int x,
- int y,
- int width,
- const uint32_t *buffer)
-{
-image->store_scanline_64 (image, x, y, width, buffer);
-}
-
 /* Fetch functions */
 
 static force_inline uint32_t
@@ -983,9 +964,6 @@ bits_image_property_changed (pixman_image_t *image)
image->common.get_scanline_64 = _pixman_image_get_scanline_generic_64;
image->common.get_scanline_32 = bits_image_fetch_general;
 }
-
-bits->store_scanline_64 = bits_image_store_scanline_64;
-bits->store_scanline_32 = bits_image_store_scanline_32;
 }
 
 static uint32_t *
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index 06f6d11..15a5bc2 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -187,10 +187,6 @@ struct bits_image
 store_scanline_t   store_scanline_raw_32;
 store_scanline_t   store_scanline_raw_64;
 
-/* Store a scanline, taking alpha maps into account */
-store_scanline_t   store_scanline_32;
-store_scanline_t   store_scanline_64;
-
 /* Used for indirect access to the bits */
 pixman_read_memory_func_t  read_func;
 pixman_write_memory_func_t write_func;
-- 
1.7.1.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman


[Pixman] [PATCH 5/7] Remove "_raw_" from all the accessors.

2010-07-29 Thread Søren Sandmann
From: Søren Sandmann Pedersen 

There are no non-raw accessors anymore.
---
 pixman/pixman-access.c |   40 
 pixman/pixman-bits-image.c |   28 ++--
 pixman/pixman-private.h|   19 +++
 3 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/pixman/pixman-access.c b/pixman/pixman-access.c
index 80fa9e8..a786947 100644
--- a/pixman/pixman-access.c
+++ b/pixman/pixman-access.c
@@ -2667,7 +2667,7 @@ store_scanline_generic_64 (bits_image_t *  image,
  */
 pixman_contract (argb8_pixels, (uint64_t *)values, width);
 
-image->store_scanline_raw_32 (image, x, y, width, argb8_pixels);
+image->store_scanline_32 (image, x, y, width, argb8_pixels);
 
 free (argb8_pixels);
 }
@@ -2688,7 +2688,7 @@ fetch_scanline_generic_64 (pixman_image_t *image,
 /* Fetch the pixels into the first half of buffer and then expand them in
  * place.
  */
-image->bits.fetch_scanline_raw_32 (image, x, y, width, buffer, NULL);
+image->bits.fetch_scanline_32 (image, x, y, width, buffer, NULL);
 
 format = image->bits.format;
 if (PIXMAN_FORMAT_TYPE (format) == PIXMAN_TYPE_COLOR   ||
@@ -2711,7 +2711,7 @@ fetch_pixel_generic_64 (bits_image_t *image,
int   offset,
int   line)
 {
-uint32_t pixel32 = image->fetch_pixel_raw_32 (image, offset, line);
+uint32_t pixel32 = image->fetch_pixel_32 (image, offset, line);
 uint64_t result;
 pixman_format_code_t format;
 
@@ -2743,7 +2743,7 @@ fetch_pixel_generic_lossy_32 (bits_image_t *image,
  int   offset,
  int   line)
 {
-uint64_t pixel64 = image->fetch_pixel_raw_64 (image, offset, line);
+uint64_t pixel64 = image->fetch_pixel_64 (image, offset, line);
 uint32_t result;
 
 pixman_contract (&result, &pixel64, 1);
@@ -2754,12 +2754,12 @@ fetch_pixel_generic_lossy_32 (bits_image_t *image,
 typedef struct
 {
 pixman_format_code_t   format;
-fetch_scanline_t   fetch_scanline_raw_32;
-fetch_scanline_t   fetch_scanline_raw_64;
-fetch_pixel_32_t   fetch_pixel_raw_32;
-fetch_pixel_64_t   fetch_pixel_raw_64;
-store_scanline_t   store_scanline_raw_32;
-store_scanline_t   store_scanline_raw_64;
+fetch_scanline_t   fetch_scanline_32;
+fetch_scanline_t   fetch_scanline_64;
+fetch_pixel_32_t   fetch_pixel_32;
+fetch_pixel_64_t   fetch_pixel_64;
+store_scanline_t   store_scanline_32;
+store_scanline_t   store_scanline_64;
 } format_info_t;
 
 #define FORMAT_INFO(format)\
@@ -2885,12 +2885,12 @@ setup_accessors (bits_image_t *image)
 {
if (info->format == image->format)
{
-   image->fetch_scanline_raw_32 = info->fetch_scanline_raw_32;
-   image->fetch_scanline_raw_64 = info->fetch_scanline_raw_64;
-   image->fetch_pixel_raw_32 = info->fetch_pixel_raw_32;
-   image->fetch_pixel_raw_64 = info->fetch_pixel_raw_64;
-   image->store_scanline_raw_32 = info->store_scanline_raw_32;
-   image->store_scanline_raw_64 = info->store_scanline_raw_64;
+   image->fetch_scanline_32 = info->fetch_scanline_32;
+   image->fetch_scanline_64 = info->fetch_scanline_64;
+   image->fetch_pixel_32 = info->fetch_pixel_32;
+   image->fetch_pixel_64 = info->fetch_pixel_64;
+   image->store_scanline_32 = info->store_scanline_32;
+   image->store_scanline_64 = info->store_scanline_64;

return;
}
@@ -2901,13 +2901,13 @@ setup_accessors (bits_image_t *image)
 
 #ifndef PIXMAN_FB_ACCESSORS
 void
-_pixman_bits_image_setup_raw_accessors_accessors (bits_image_t *image);
+_pixman_bits_image_setup_accessors_accessors (bits_image_t *image);
 
 void
-_pixman_bits_image_setup_raw_accessors (bits_image_t *image)
+_pixman_bits_image_setup_accessors (bits_image_t *image)
 {
 if (image->read_func || image->write_func)
-   _pixman_bits_image_setup_raw_accessors_accessors (image);
+   _pixman_bits_image_setup_accessors_accessors (image);
 else
setup_accessors (image);
 }
@@ -2915,7 +2915,7 @@ _pixman_bits_image_setup_raw_accessors (bits_image_t 
*image)
 #else
 
 void
-_pixman_bits_image_setup_raw_accessors_accessors (bits_image_t *image)
+_pixman_bits_image_setup_accessors_accessors (bits_image_t *image)
 {
 setup_accessors (image);
 }
diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index f8ddcbc..d27256d 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -43,14 +43,14 @@ _pixman_image_store_scanline_32 (bits_image_t *  image,
  int width,
  const uint32_t *b

[Pixman] [PATCH 6/7] Add some new FAST_PATH flags

2010-07-29 Thread Søren Sandmann
From: Søren Sandmann Pedersen 

The flags are:

 *  AFFINE_TRANSFORM, for affine transforms

 *  Y_UNIT_ZERO, for when the 10 entry in the transformation is zero

 *  FILTER_BILINEAR, for when the image has a bilinear filter

 *  NO_NORMAL_REPEAT, for when the repeat mode is not NORMAL

 *  HAS_TRANSFORM, for when the transform is not NULL

Also add some new FAST_PATH_REPEAT_* macros. These are just shorthands
for the image not having any of the other repeat modes. For example
REPEAT_NORMAL is (NO_NONE | NO_PAD | NO_REFLECT).
---
 pixman/pixman-bits-image.c |1 -
 pixman/pixman-fast-path.c  |9 +++--
 pixman/pixman-image.c  |   36 +++-
 pixman/pixman-private.h|   25 +
 4 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index d27256d..7129e86 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -954,7 +954,6 @@ bits_image_property_changed (pixman_image_t *image)
 bits->common.transform->matrix[2][0] == 0  &&
 bits->common.transform->matrix[2][1] == 0  &&
 bits->common.transform->matrix[2][2] == pixman_fixed_1)
-
 {
image->common.get_scanline_64 = _pixman_image_get_scanline_generic_64;
image->common.get_scanline_32 = bits_image_fetch_affine_no_alpha;
diff --git a/pixman/pixman-fast-path.c b/pixman/pixman-fast-path.c
index 9cc8529..f03752f 100644
--- a/pixman/pixman-fast-path.c
+++ b/pixman/pixman-fast-path.c
@@ -1866,15 +1866,12 @@ static const pixman_fast_path_t c_fast_paths[] =
  FAST_PATH_NO_ACCESSORS|   \
  FAST_PATH_NO_WIDE_FORMAT)
 
-#define HAS_NORMAL_REPEAT_FLAGS
\
-(FAST_PATH_NO_REFLECT_REPEAT | \
- FAST_PATH_NO_PAD_REPEAT | \
- FAST_PATH_NO_NONE_REPEAT)
-
 #define SIMPLE_NEAREST_FAST_PATH(op,s,d,func)  \
 {   PIXMAN_OP_ ## op,  \
PIXMAN_ ## s,   \
-   SCALED_NEAREST_FLAGS | HAS_NORMAL_REPEAT_FLAGS | 
FAST_PATH_X_UNIT_POSITIVE, \
+   (SCALED_NEAREST_FLAGS   |   \
+FAST_PATH_NORMAL_REPEAT|   \
+FAST_PATH_X_UNIT_POSITIVE),\
PIXMAN_null, 0, \
PIXMAN_ ## d, FAST_PATH_STD_DEST_FLAGS, \
fast_composite_scaled_nearest_ ## func ## _normal ## _ ## op,   \
diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 5c6d415..269c3c1 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -297,21 +297,33 @@ compute_image_info (pixman_image_t *image)
 /* Transform */
 if (!image->common.transform)
 {
-   flags |= (FAST_PATH_ID_TRANSFORM | FAST_PATH_X_UNIT_POSITIVE);
+   flags |= (FAST_PATH_ID_TRANSFORM|
+ FAST_PATH_X_UNIT_POSITIVE |
+ FAST_PATH_Y_UNIT_ZERO |
+ FAST_PATH_AFFINE_TRANSFORM);
 }
 else
 {
-   if (image->common.transform->matrix[0][1] == 0 &&
-   image->common.transform->matrix[1][0] == 0 &&
-   image->common.transform->matrix[2][0] == 0 &&
-   image->common.transform->matrix[2][1] == 0 &&
+   flags |= FAST_PATH_HAS_TRANSFORM;
+
+   if (image->common.transform->matrix[2][0] == 0  &&
+   image->common.transform->matrix[2][1] == 0  &&
image->common.transform->matrix[2][2] == pixman_fixed_1)
{
-   flags |= FAST_PATH_SCALE_TRANSFORM;
+   flags |= FAST_PATH_AFFINE_TRANSFORM;
+
+   if (image->common.transform->matrix[0][1] == 0 &&
+   image->common.transform->matrix[1][0] == 0)
+   {
+   flags |= FAST_PATH_SCALE_TRANSFORM;
+   }
}
 
if (image->common.transform->matrix[0][0] > 0)
flags |= FAST_PATH_X_UNIT_POSITIVE;
+
+   if (image->common.transform->matrix[1][0] == 0)
+   flags |= FAST_PATH_Y_UNIT_ZERO;
 }
 
 /* Alpha map */
@@ -326,6 +338,12 @@ compute_image_info (pixman_image_t *image)
flags |= (FAST_PATH_NEAREST_FILTER | FAST_PATH_NO_CONVOLUTION_FILTER);
break;
 
+case PIXMAN_FILTER_BILINEAR:
+case PIXMAN_FILTER_GOOD:
+case PIXMAN_FILTER_BEST:
+   flags |= (FAST_PATH_BILINEAR_FILTER | FAST_PATH_NO_CONVOLUTION_FILTER);
+   break;
+
 case PIXMAN_FILTER_CONVOLUTION:
break;
 
@@ -338,15 +356,15 @@ compute_image_info (pixman_image_t *image)
 switch (image->common.repeat)
 {
 case PIXMAN_REPEAT_NONE:
-   flags |= FAST_PATH_NO_REFLECT_REPEAT 

[Pixman] [PATCH 7/7] Store the various bits image fetchers in a table with formats and flags.

2010-07-29 Thread Søren Sandmann
From: Søren Sandmann Pedersen 

Similarly to how the fast paths are done, put the various bits_image
fetchers in a table, so that we can quickly find the best one based on
the image's flags and format.
---
 pixman/pixman-bits-image.c |  126 ++-
 1 files changed, 76 insertions(+), 50 deletions(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index 7129e86..a32ebcc 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -906,62 +906,88 @@ bits_image_fetch_untransformed_64 (pixman_image_t * image,
 }
 }
 
+typedef struct
+{
+pixman_format_code_t   format;
+uint32_t   flags;
+fetch_scanline_t   fetch_32;
+fetch_scanline_t   fetch_64;
+} fetcher_info_t;
+
+static const fetcher_info_t fetcher_info[] =
+{
+{ PIXMAN_solid,
+  FAST_PATH_NO_ALPHA_MAP,
+  bits_image_fetch_solid_32,
+  bits_image_fetch_solid_64
+},
+
+{ PIXMAN_any,
+  (FAST_PATH_NO_ALPHA_MAP  |
+   FAST_PATH_ID_TRANSFORM  |
+   FAST_PATH_NO_CONVOLUTION_FILTER |
+   FAST_PATH_NO_PAD_REPEAT |
+   FAST_PATH_NO_REFLECT_REPEAT),
+  bits_image_fetch_untransformed_32,
+  bits_image_fetch_untransformed_64
+},
+
+#define FAST_BILINEAR_FLAGS\
+(FAST_PATH_NO_ALPHA_MAP|   \
+ FAST_PATH_NO_ACCESSORS|   \
+ FAST_PATH_HAS_TRANSFORM   |   \
+ FAST_PATH_AFFINE_TRANSFORM|   
\
+ FAST_PATH_X_UNIT_POSITIVE |   \
+ FAST_PATH_Y_UNIT_ZERO |   \
+ FAST_PATH_NONE_REPEAT |   \
+ FAST_PATH_BILINEAR_FILTER)
+
+{ PIXMAN_a8r8g8b8,
+  FAST_BILINEAR_FLAGS,
+  bits_image_fetch_bilinear_no_repeat_,
+  _pixman_image_get_scanline_generic_64
+},
+
+{ PIXMAN_x8r8g8b8,
+  FAST_BILINEAR_FLAGS,
+  bits_image_fetch_bilinear_no_repeat_,
+  _pixman_image_get_scanline_generic_64
+},
+
+{ PIXMAN_any,
+  (FAST_PATH_NO_ALPHA_MAP |
+   FAST_PATH_HAS_TRANSFORM |
+   FAST_PATH_AFFINE_TRANSFORM),
+  bits_image_fetch_affine_no_alpha,
+  _pixman_image_get_scanline_generic_64
+},
+
+{ PIXMAN_any, 0, bits_image_fetch_general, 
_pixman_image_get_scanline_generic_64 },
+
+{ PIXMAN_null },
+};
+
 static void
 bits_image_property_changed (pixman_image_t *image)
 {
-bits_image_t *bits = (bits_image_t *)image;
+uint32_t flags = image->common.flags;
+pixman_format_code_t format = image->common.extended_format_code;
+const fetcher_info_t *info;
 
-_pixman_bits_image_setup_accessors (bits);
+_pixman_bits_image_setup_accessors (&image->bits);
 
-if (bits->common.alpha_map)
-{
-   image->common.get_scanline_64 = _pixman_image_get_scanline_generic_64;
-   image->common.get_scanline_32 = bits_image_fetch_general;
-}
-else if ((bits->common.repeat != PIXMAN_REPEAT_NONE) &&
- bits->width == 1 &&
- bits->height == 1)
-{
-   image->common.get_scanline_64 = bits_image_fetch_solid_64;
-   image->common.get_scanline_32 = bits_image_fetch_solid_32;
-}
-else if (!bits->common.transform &&
- bits->common.filter != PIXMAN_FILTER_CONVOLUTION &&
- (bits->common.repeat == PIXMAN_REPEAT_NONE ||
-  bits->common.repeat == PIXMAN_REPEAT_NORMAL))
-{
-   image->common.get_scanline_64 = bits_image_fetch_untransformed_64;
-   image->common.get_scanline_32 = bits_image_fetch_untransformed_32;
-}
-else if (bits->common.transform&&
-bits->common.transform->matrix[2][0] == 0  &&
-bits->common.transform->matrix[2][1] == 0  &&
-bits->common.transform->matrix[2][2] == pixman_fixed_1 &&
-bits->common.transform->matrix[0][0] > 0   &&
-bits->common.transform->matrix[1][0] == 0  &&
-!bits->read_func   &&
-(bits->common.filter == PIXMAN_FILTER_BILINEAR ||
- bits->common.filter == PIXMAN_FILTER_GOOD ||
- bits->common.filter == PIXMAN_FILTER_BEST)&&
-bits->common.repeat == PIXMAN_REPEAT_NONE  &&
-(bits->format == PIXMAN_a8r8g8b8   ||
- bits->format == PIXMAN_x8r8g8b8))
-{
-   image->common.get_scanline_64 = _pixman_image_get_scanline_generic_64;
-   image->common.get_scanline_32 = 
bits_image_fetch_bilinear_no_repeat_;
-}
-else if (bits->common.transform&&
-