pixman: Branch 'master' - 3 commits
Søren Sandmann Pedersen
sandmann at kemper.freedesktop.org
Mon Jul 20 23:20:42 PDT 2009
pixman/pixman-bits-image.c | 22 ++---
pixman/pixman-utils.c | 39 ++--------
pixman/pixman.c | 68 +++++++++++++++++
test/Makefile.am | 2
test/window-test.c | 173 +++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 259 insertions(+), 45 deletions(-)
New commits:
commit c7b84f8b043018368fade4ad13730cfcaaf5c8cc
Author: Søren Sandmann Pedersen <sandmann at redhat.com>
Date: Tue Jul 21 00:17:15 2009 -0400
Only apply the workaround if the clip region extends beyond the drawable.
This works because the X server always attempts to set a clip region
within the bounds of the drawable, and it only fails at it when it is
computing the wrong translation and therefore needs the workaround.
diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index be28ebc..ff29620 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -809,25 +809,18 @@ source_image_needs_out_of_bounds_workaround (bits_image_t *image)
image->common.have_clip_region &&
out_of_bounds_workaround)
{
- const pixman_box32_t *boxes;
- int n;
-
if (!image->common.client_clip)
{
/* There is no client clip, so the drawable in question
- * is a window if the clip region is different from the
- * full drawable
+ * is a window if the clip region extends beyond the
+ * drawable geometry.
*/
- boxes = pixman_region32_rectangles (&image->common.clip_region, &n);
- if (n == 1)
+ const pixman_box32_t *extents = pixman_region32_extents (&image->common.clip_region);
+
+ if (extents->x1 >= 0 && extents->x2 < image->width &&
+ extents->y1 >= 0 && extents->y2 < image->height)
{
- if (boxes[0].x1 == 0 && boxes[0].y1 == 0 &&
- boxes[0].x2 == image->width &&
- boxes[0].y2 == image->height)
- {
- /* pixmap */
- return FALSE;
- }
+ return FALSE;
}
}
commit 6bd17f1e9861693262fa88bfeff5d3279b3f6e7d
Author: Søren Sandmann Pedersen <sandmann at redhat.com>
Date: Mon Jul 20 23:46:06 2009 -0400
Rework the workaround for bogus X server images.
Bug 22844 demonstrates that it is not sufficient to play tricks with
the clip regions to work around the bogus images from the X
server. The problem there is that if the operation hits the general
path and the destination has a different format than a8r8g8b8, the
destination pixels will be fetched into a temporary array. But because
those pixels would be outside the clip region, they would be fetched
as black. The previous workaround was relying on fast paths fetching
those pixels without checking the clip region.
In the new scheme we work around the problem at the
pixman_image_composite() level. If an image is determined to need a
work around, we translate both the bits pointer, the coordinates, and
the clip region, thus effectively undoing the X server's broken
computation.
diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index b53630c..be28ebc 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -806,6 +806,7 @@ source_image_needs_out_of_bounds_workaround (bits_image_t *image)
{
if (image->common.clip_sources &&
image->common.repeat == PIXMAN_REPEAT_NONE &&
+ image->common.have_clip_region &&
out_of_bounds_workaround)
{
const pixman_box32_t *boxes;
diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c
index 3dfacde..2c34d02 100644
--- a/pixman/pixman-utils.c
+++ b/pixman/pixman-utils.c
@@ -84,20 +84,13 @@ clip_source_image (pixman_region32_t * region,
int dx,
int dy)
{
- /* The workaround lets certain fast paths run even when they
- * would normally be rejected because of out-of-bounds access.
- * We need to clip against the source geometry in that case
+ /* Source clips are ignored, unless they are explicitly turned on
+ * and the clip in question was set by an X client. (Because if
+ * the clip was not set by a client, then it is a hierarchy
+ * clip and those should always be ignored for sources).
*/
- if (!picture->common.need_workaround)
- {
- /* Source clips are ignored, unless they are explicitly turned on
- * and the clip in question was set by an X client. (Because if
- * the clip was not set by a client, then it is a hierarchy
- * clip and those should always be ignored for sources).
- */
- if (!picture->common.clip_sources || !picture->common.client_clip)
- return TRUE;
- }
+ if (!picture->common.clip_sources || !picture->common.client_clip)
+ return TRUE;
return clip_general_image (region,
&picture->common.clip_region,
@@ -133,21 +126,8 @@ pixman_compute_composite_region32 (pixman_region32_t * region,
region->extents.x1 = MAX (region->extents.x1, 0);
region->extents.y1 = MAX (region->extents.y1, 0);
-
- /* Some X servers rely on an old bug, where pixman would just believe the
- * set clip_region and not clip against the destination geometry. So,
- * since only X servers set "source clip", we don't clip against
- * destination geometry when that is set and when the workaround has
- * not been explicitly disabled by
- *
- * pixman_disable_out_of_bounds_workaround();
- *
- */
- if (!(dst_image->common.need_workaround))
- {
- region->extents.x2 = MIN (region->extents.x2, dst_image->bits.width);
- region->extents.y2 = MIN (region->extents.y2, dst_image->bits.height);
- }
+ region->extents.x2 = MIN (region->extents.x2, dst_image->bits.width);
+ region->extents.y2 = MIN (region->extents.y2, dst_image->bits.height);
region->data = 0;
@@ -746,8 +726,7 @@ _pixman_run_fast_path (const pixman_fast_path_t *paths,
if (sources_cover (
src, mask, extents,
- src_x, src_y, mask_x, mask_y, dest_x, dest_y) ||
- src->common.need_workaround)
+ src_x, src_y, mask_x, mask_y, dest_x, dest_y))
{
walk_region_internal (imp, op,
src, mask, dest,
diff --git a/pixman/pixman.c b/pixman/pixman.c
index 94675d2..12b4c5a 100644
--- a/pixman/pixman.c
+++ b/pixman/pixman.c
@@ -55,6 +55,8 @@ static const optimized_operator_info_t optimized_operators[] =
{ PIXMAN_OP_NONE }
};
+static pixman_implementation_t *imp;
+
/*
* Check if the current operator could be optimized
*/
@@ -105,7 +107,50 @@ pixman_optimize_operator (pixman_op_t op,
}
-static pixman_implementation_t *imp;
+static void
+apply_workaround (pixman_image_t *image,
+ int16_t * x,
+ int16_t * y,
+ uint32_t ** save_bits,
+ int * save_dx,
+ int * save_dy)
+{
+ /* Some X servers generate images that point to the
+ * wrong place in memory, but then set the clip region
+ * to point to the right place. Because of an old bug
+ * in pixman, this would actually work.
+ *
+ * Here we try and undo the damage
+ */
+ int bpp = PIXMAN_FORMAT_BPP (image->bits.format) / 8;
+ pixman_box32_t *extents;
+ uint8_t *t;
+ int dx, dy;
+
+ extents = pixman_region32_extents (&(image->common.clip_region));
+ dx = extents->x1;
+ dy = extents->y1;
+
+ *save_bits = image->bits.bits;
+
+ *x -= dx;
+ *y -= dy;
+ pixman_region32_translate (&(image->common.clip_region), -dx, -dy);
+
+ t = (uint8_t *)image->bits.bits;
+ t += dy * image->bits.rowstride * 4 + dx * bpp;
+ image->bits.bits = (uint32_t *)t;
+
+ *save_dx = dx;
+ *save_dy = dy;
+}
+
+static void
+unapply_workaround (pixman_image_t *image, uint32_t *bits, int dx, int dy)
+{
+ image->bits.bits = bits;
+ pixman_region32_translate (&image->common.clip_region, dx, dy);
+}
PIXMAN_EXPORT void
pixman_image_composite (pixman_op_t op,
@@ -121,6 +166,13 @@ pixman_image_composite (pixman_op_t op,
uint16_t width,
uint16_t height)
{
+ uint32_t *src_bits;
+ int src_dx, src_dy;
+ uint32_t *mask_bits;
+ int mask_dx, mask_dy;
+ uint32_t *dest_bits;
+ int dest_dx, dest_dy;
+
/*
* Check if we can replace our operator by a simpler one
* if the src or dest are opaque. The output operator should be
@@ -137,12 +189,26 @@ pixman_image_composite (pixman_op_t op,
if (!imp)
imp = _pixman_choose_implementation ();
+ if (src->common.need_workaround)
+ apply_workaround (src, &src_x, &src_y, &src_bits, &src_dx, &src_dy);
+ if (mask && mask->common.need_workaround)
+ apply_workaround (mask, &mask_x, &mask_y, &mask_bits, &mask_dx, &mask_dy);
+ if (dest->common.need_workaround)
+ apply_workaround (dest, &dest_x, &dest_y, &dest_bits, &dest_dx, &dest_dy);
+
_pixman_implementation_composite (imp, op,
src, mask, dest,
src_x, src_y,
mask_x, mask_y,
dest_x, dest_y,
width, height);
+
+ if (src->common.need_workaround)
+ unapply_workaround (src, src_bits, src_dx, src_dy);
+ if (mask && mask->common.need_workaround)
+ unapply_workaround (mask, mask_bits, mask_dx, mask_dy);
+ if (dest->common.need_workaround)
+ unapply_workaround (dest, dest_bits, dest_dx, dest_dy);
}
PIXMAN_EXPORT pixman_bool_t
diff --git a/test/window-test.c b/test/window-test.c
index b2b00b6..bbaa3e2 100644
--- a/test/window-test.c
+++ b/test/window-test.c
@@ -73,16 +73,22 @@ make_image (int width, int height, pixman_bool_t src, int *rx, int *ry)
dx = get_rand (500);
dy = get_rand (500);
- /* Now simulate the bogus X server translations */
- bits -= (dy * stride + dx) * bpp;
+ if (!src)
+ {
+ /* Now simulate the bogus X server translations */
+ bits -= (dy * stride + dx) * bpp;
+ }
image = pixman_image_create_bits (
format, width, height, (uint32_t *)bits, stride * bpp);
- /* And add the bogus clip region */
- pixman_region32_init_rect (®ion, dx, dy, dx + width, dy + height);
+ if (!src)
+ {
+ /* And add the bogus clip region */
+ pixman_region32_init_rect (®ion, dx, dy, dx + width, dy + height);
- pixman_image_set_clip_region32 (image, ®ion);
+ pixman_image_set_clip_region32 (image, ®ion);
+ }
pixman_image_set_source_clipping (image, TRUE);
@@ -112,8 +118,15 @@ make_image (int width, int height, pixman_bool_t src, int *rx, int *ry)
pixman_image_set_repeat (image, PIXMAN_REPEAT_PAD);
}
- *rx = dx;
- *ry = dy;
+ if (!src)
+ {
+ *rx = dx;
+ *ry = dy;
+ }
+ else
+ {
+ *rx = *ry = 0;
+ }
return image;
}
@@ -148,7 +161,11 @@ main ()
uint8_t *pixel =
bits + (i + dest_y) * stride + (j + dest_x) * bpp;
- assert (*(uint16_t *)pixel == 0x788f);
+ if (*(uint16_t *)pixel != 0x788f)
+ {
+ printf ("bad pixel %x\n", *(uint16_t *)pixel);
+ assert (*(uint16_t *)pixel == 0x788f);
+ }
}
}
commit dfdb8509e2160a0db7d72e775dd348090e6fb968
Author: Søren Sandmann Pedersen <sandmann at redhat.com>
Date: Mon Jul 20 22:45:47 2009 -0400
Add test case for bug 22844.
diff --git a/test/Makefile.am b/test/Makefile.am
index d44e526..324cb72 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -6,6 +6,7 @@ TESTPROGRAMS = \
scaling-test \
fetch-test \
oob-test \
+ window-test \
trap-crasher
fetch_test_LDADD = $(TEST_LDADD)
@@ -13,6 +14,7 @@ region_test_LDADD = $(TEST_LDADD)
scaling_test_LDADD = $(TEST_LDADD)
trap_crasher_LDADD = $(TEST_LDADD)
oob_test_LDADD = $(TEST_LDADD)
+window_test_LDADD = $(TEST_LDADD)
# GTK using test programs
diff --git a/test/window-test.c b/test/window-test.c
new file mode 100644
index 0000000..b2b00b6
--- /dev/null
+++ b/test/window-test.c
@@ -0,0 +1,156 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <config.h>
+#include "pixman.h"
+#include "pixman-private.h"
+
+#define FALSE 0
+#define TRUE 1
+
+/* Randomly decide between 32 and 16 bit
+ *
+ * Allocate bits with random width, stride and height
+ *
+ * Then make up some random offset (dx, dy)
+ *
+ * Then make an image with those values.
+ *
+ * Do this for both source and destination
+ *
+ * Composite them together using OVER.
+ *
+ * The bits in the source and the destination should have
+ * recognizable colors so that the result can be verified.
+ *
+ * Ie., walk the bits and verify that they have been composited.
+ */
+
+static int
+get_rand (int bound)
+{
+ return rand () % bound;
+}
+
+static pixman_image_t *
+make_image (int width, int height, pixman_bool_t src, int *rx, int *ry)
+{
+ pixman_format_code_t format;
+ pixman_image_t *image;
+ pixman_region32_t region;
+ uint8_t *bits;
+ int stride;
+ int bpp;
+ int dx, dy;
+ int i, j;
+
+ if (src)
+ format = PIXMAN_a8r8g8b8;
+ else
+ format = PIXMAN_r5g6b5;
+
+ bpp = PIXMAN_FORMAT_BPP (format) / 8;
+
+ stride = width + get_rand (width);
+ stride += (stride & 1); /* Make it an even number */
+
+ bits = malloc (height * stride * bpp);
+
+ for (j = 0; j < height; ++j)
+ {
+ for (i = 0; i < width; ++i)
+ {
+ uint8_t *pixel = bits + (stride * j + i) * bpp;
+
+ if (src)
+ *(uint32_t *)pixel = 0x7f00007f;
+ else
+ *(uint16_t *)pixel = 0xf100;
+ }
+ }
+
+ dx = dy = 0;
+
+ dx = get_rand (500);
+ dy = get_rand (500);
+
+ /* Now simulate the bogus X server translations */
+ bits -= (dy * stride + dx) * bpp;
+
+ image = pixman_image_create_bits (
+ format, width, height, (uint32_t *)bits, stride * bpp);
+
+ /* And add the bogus clip region */
+ pixman_region32_init_rect (®ion, dx, dy, dx + width, dy + height);
+
+ pixman_image_set_clip_region32 (image, ®ion);
+
+ pixman_image_set_source_clipping (image, TRUE);
+
+ if (src)
+ {
+ pixman_transform_t trans;
+
+ pixman_transform_init_identity (&trans);
+
+ pixman_transform_translate (&trans,
+ NULL,
+ - pixman_int_to_fixed (width / 2),
+ - pixman_int_to_fixed (height / 2));
+
+ pixman_transform_scale (&trans,
+ NULL,
+ pixman_double_to_fixed (0.5),
+ pixman_double_to_fixed (0.5));
+
+ pixman_transform_translate (&trans,
+ NULL,
+ pixman_int_to_fixed (width / 2),
+ pixman_int_to_fixed (height / 2));
+
+ pixman_image_set_transform (image, &trans);
+ pixman_image_set_filter (image, PIXMAN_FILTER_BILINEAR, NULL, 0);
+ pixman_image_set_repeat (image, PIXMAN_REPEAT_PAD);
+ }
+
+ *rx = dx;
+ *ry = dy;
+
+ return image;
+}
+
+int
+main ()
+{
+ pixman_image_t *src, *dest;
+ int src_x, src_y, dest_x, dest_y;
+ int i, j;
+ int width = get_rand (500);
+ int height = get_rand (500);
+
+ src = make_image (width, height, TRUE, &src_x, &src_y);
+ dest = make_image (width, height, FALSE, &dest_x, &dest_y);
+
+ pixman_image_composite (
+ PIXMAN_OP_OVER, src, NULL, dest,
+ src_x, src_y,
+ -1, -1,
+ dest_x, dest_y,
+ width, height);
+
+ for (i = 0; i < height; ++i)
+ {
+ for (j = 0; j < width; ++j)
+ {
+ uint8_t *bits = (uint8_t *)dest->bits.bits;
+ int bpp = PIXMAN_FORMAT_BPP (dest->bits.format) / 8;
+ int stride = dest->bits.rowstride * 4;
+
+ uint8_t *pixel =
+ bits + (i + dest_y) * stride + (j + dest_x) * bpp;
+
+ assert (*(uint16_t *)pixel == 0x788f);
+ }
+ }
+
+ return 0;
+}
More information about the xorg-commit
mailing list