Soeren Sandmann <sandm...@cs.au.dk> writes: > The new code only computes the transformation once, and then it add the > absolute value of the 00 and 10 components to get a conservative > estimate of the transformed destination boundary. I'm especially > interested in review of this part, both of the code and of whether it's > actually correct mathematically.
It is in fact not at all correct. It's completely bogus in fact. For affine matrices the idea may not be entirely wrong, but the correct thing to add to the X coordinates would be |m00| + |m01| + |m11| and not just |m00|. Similar for Y coordinates. But that fails to take projective matrices into account, so I just went back to computing the transformed bounding box twice. New patch below. Soren >From e02d17612531fb8556d30fc2dde3ad2680f2b739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Sandmann=20Pedersen?= <s...@redhat.com> Date: Mon, 5 Sep 2011 00:19:51 -0400 Subject: [PATCH] Eliminate compute_sample_extents() function In analyze_extents(), instead of calling compute_sample_extents() call compute_transformed_extents() and inline the remaining part of compute_sample_extents(). The upcoming bilinear->nearest optimization will do something different with these two pieces of code. --- pixman/pixman.c | 100 +++++++++++++++++++++++-------------------------------- 1 files changed, 42 insertions(+), 58 deletions(-) diff --git a/pixman/pixman.c b/pixman/pixman.c index 264a56b..19eda08 100644 --- a/pixman/pixman.c +++ b/pixman/pixman.c @@ -514,45 +514,9 @@ compute_transformed_extents (pixman_transform_t *transform, return TRUE; } -static pixman_bool_t -compute_sample_extents (pixman_transform_t *transform, - pixman_box32_t *extents, - pixman_fixed_t x_off, pixman_fixed_t y_off, - pixman_fixed_t width, pixman_fixed_t height) -{ - box_48_16_t transformed; - - if (!compute_transformed_extents (transform, extents, &transformed)) - return FALSE; - - /* Expand the source area by a tiny bit so account of different rounding that - * may happen during sampling. Note that (8 * pixman_fixed_e) is very far from - * 0.5 so this won't cause the area computed to be overly pessimistic. - */ - transformed.x1 += x_off - 8 * pixman_fixed_e; - transformed.y1 += y_off - 8 * pixman_fixed_e; - transformed.x2 += x_off + width + 8 * pixman_fixed_e; - transformed.y2 += y_off + height + 8 * pixman_fixed_e; - - if (transformed.x1 < pixman_min_fixed_48_16 || transformed.x1 > pixman_max_fixed_48_16 || - transformed.y1 < pixman_min_fixed_48_16 || transformed.y1 > pixman_max_fixed_48_16 || - transformed.x2 < pixman_min_fixed_48_16 || transformed.x2 > pixman_max_fixed_48_16 || - transformed.y2 < pixman_min_fixed_48_16 || transformed.y2 > pixman_max_fixed_48_16) - { - return FALSE; - } - else - { - extents->x1 = pixman_fixed_to_int (transformed.x1); - extents->y1 = pixman_fixed_to_int (transformed.y1); - extents->x2 = pixman_fixed_to_int (transformed.x2) + 1; - extents->y2 = pixman_fixed_to_int (transformed.y2) + 1; - - return TRUE; - } -} - #define IS_16BIT(x) (((x) >= INT16_MIN) && ((x) <= INT16_MAX)) +#define ABS(f) (((f) < 0)? (-(f)) : (f)) +#define IS_16_16(f) (((f) >= pixman_min_fixed_48_16 && ((f) <= pixman_max_fixed_48_16))) static pixman_bool_t analyze_extent (pixman_image_t *image, @@ -563,7 +527,8 @@ analyze_extent (pixman_image_t *image, pixman_fixed_t *params; pixman_fixed_t x_off, y_off; pixman_fixed_t width, height; - pixman_box32_t ex; + box_48_16_t transformed; + pixman_box32_t exp_extents; if (!image) return TRUE; @@ -633,17 +598,6 @@ analyze_extent (pixman_image_t *image, default: return FALSE; } - - /* Check whether the non-expanded, transformed extent is entirely within - * the source image, and set the FAST_PATH_SAMPLES_COVER_CLIP if it is. - */ - ex = *extents; - if (compute_sample_extents (transform, &ex, x_off, y_off, width, height) && - ex.x1 >= 0 && ex.y1 >= 0 && - ex.x2 <= image->bits.width && ex.y2 <= image->bits.height) - { - *flags |= FAST_PATH_SAMPLES_COVER_CLIP; - } } else { @@ -653,17 +607,47 @@ analyze_extent (pixman_image_t *image, height = 0; } - /* Check that the extents expanded by one don't overflow. This ensures that - * compositing functions can simply walk the source space using 16.16 - * variables without worrying about overflow. + if (!compute_transformed_extents (transform, extents, &transformed)) + return FALSE; + + /* Expand the source area by a tiny bit so account of different rounding that + * may happen during sampling. Note that (8 * pixman_fixed_e) is very far from + * 0.5 so this won't cause the area computed to be overly pessimistic. */ - ex.x1 = extents->x1 - 1; - ex.y1 = extents->y1 - 1; - ex.x2 = extents->x2 + 1; - ex.y2 = extents->y2 + 1; + transformed.x1 += x_off - 8 * pixman_fixed_e; + transformed.y1 += y_off - 8 * pixman_fixed_e; + transformed.x2 += x_off + width + 8 * pixman_fixed_e; + transformed.y2 += y_off + height + 8 * pixman_fixed_e; + + if (image->common.type == BITS && + pixman_fixed_to_int (transformed.x1) >= 0 && + pixman_fixed_to_int (transformed.y1) >= 0 && + pixman_fixed_to_int (transformed.x2) < image->bits.width && + pixman_fixed_to_int (transformed.y2) < image->bits.height) + { + *flags |= FAST_PATH_SAMPLES_COVER_CLIP; + } - if (!compute_sample_extents (transform, &ex, x_off, y_off, width, height)) + /* Check we don't overflow when the destination extents are expanded by one. + * This ensures that compositing functions can simply walk the source space + * using 16.16 variables without worrying about overflow. + */ + exp_extents = *extents; + exp_extents.x1 -= 1; + exp_extents.y1 -= 1; + exp_extents.x2 += 1; + exp_extents.y2 += 1; + + if (!compute_transformed_extents (transform, &exp_extents, &transformed)) + return FALSE; + + if (!IS_16_16 (transformed.x1 + x_off - 8 * pixman_fixed_e) || + !IS_16_16 (transformed.y1 + y_off - 8 * pixman_fixed_e) || + !IS_16_16 (transformed.x2 + x_off + 8 * pixman_fixed_e + width) || + !IS_16_16 (transformed.y2 + y_off + 8 * pixman_fixed_e + height)) + { return FALSE; + } return TRUE; } -- 1.7.4 _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman