[cairo] [PATCH] Revert "image: Use convolution filters for sample reconstruction when downscaling"

Uli Schlachter psychon at znc.in
Tue May 13 01:15:12 PDT 2014


This reverts commit fb57ea13e04d82866cbc8e86c83261148bb3e231.

When running cairo-test-suite with the parameter "-a", it also runs each test
with a non-zero device-offset and device-scaling. The above commit influenced
the device-scaling results badly. E.g. some test results ended up with a black
border at the top-most and left-most row that looked like there was an offset of
"0.5" in drawing the image and thus pixels outside of the image were sampled.

This can be seen by the influence that this revert has on the results from
running CAIRO_TEST_TARGET=image ./cairo-test-suite -a:

Before: 31 Passed, 489 Failed [1 crashed, 8 expected], 31 Skipped
After: 225 Passed, 295 Failed [1 crashed, 8 expected], 31 Skipped

Most of the failures that disappeared are from the device-scaling tests.

With such disastrous results on the test suite, this cannot really be usable for
real-world applications.
---
 src/cairo-image-source.c | 65 ++++++++----------------------------------------
 1 file changed, 10 insertions(+), 55 deletions(-)

The idea here is to get master into a releasable state. Also it appears to me
that the discussion around this patch stopped without any helpful results.

What do others (especially Krzysztof) think? What else blocks a new release?
Who wants to make sure the NEWS entry for this doesn't end up in a release? :)


diff --git a/src/cairo-image-source.c b/src/cairo-image-source.c
index 661bc10..c5bd228 100644
--- a/src/cairo-image-source.c
+++ b/src/cairo-image-source.c
@@ -554,42 +554,24 @@ _pixman_image_set_properties (pixman_image_t *pixman_image,
     }
     else
     {
-	double scale_x, scale_y;
-	int shrink_x, shrink_y;
 	pixman_filter_t pixman_filter;
-	pixman_kernel_t pixman_kernel_sample, pixman_kernel_reconstruct;
-
-	/* Compute scale factors as the length of basis vectors transformed by
-	 * the pattern matrix. These scale factors are from user to pattern space,
-	 * and as such they are greater than 1.0 for downscaling and less than 1.0
-	 * for upscaling.
-	 * TODO: this approach may not be completely correct if the matrix
-	 * contains a skew component. */
-	scale_x = hypot (pattern->matrix.xx, pattern->matrix.yx);
-	scale_y = hypot (pattern->matrix.yx, pattern->matrix.yy);
-
-	/* Use convolution filtering if the transformation shrinks the image
-	 * by more than half a pixel */
-	shrink_x = (extents->width / scale_x - extents->width) < -0.5;
-	shrink_y = (extents->height / scale_y - extents->height) < -0.5;
 
 	switch (pattern->filter) {
 	case CAIRO_FILTER_FAST:
+	    pixman_filter = PIXMAN_FILTER_FAST;
+	    break;
+	case CAIRO_FILTER_GOOD:
+	    pixman_filter = PIXMAN_FILTER_GOOD;
+	    break;
+	case CAIRO_FILTER_BEST:
+	    pixman_filter = PIXMAN_FILTER_BEST;
+	    break;
 	case CAIRO_FILTER_NEAREST:
 	    pixman_filter = PIXMAN_FILTER_NEAREST;
-	    pixman_kernel_sample = PIXMAN_KERNEL_IMPULSE;
-	    pixman_kernel_reconstruct = PIXMAN_KERNEL_BOX;
 	    break;
-	case CAIRO_FILTER_GOOD:
 	case CAIRO_FILTER_BILINEAR:
 	    pixman_filter = PIXMAN_FILTER_BILINEAR;
-	    pixman_kernel_sample = PIXMAN_KERNEL_BOX;
-	    pixman_kernel_reconstruct = PIXMAN_KERNEL_LINEAR;
 	    break;
-	case CAIRO_FILTER_BEST:
-	    pixman_filter = PIXMAN_FILTER_BEST;
-	    pixman_kernel_sample = PIXMAN_KERNEL_LANCZOS3;
-	    pixman_kernel_reconstruct = PIXMAN_KERNEL_LANCZOS3;
 	case CAIRO_FILTER_GAUSSIAN:
 	    /* XXX: The GAUSSIAN value has no implementation in cairo
 	     * whatsoever, so it was really a mistake to have it in the
@@ -597,37 +579,10 @@ _pixman_image_set_properties (pixman_image_t *pixman_image,
 	     * else inventing semantics and providing an actual
 	     * implementation for it. */
 	default:
-	    pixman_filter = PIXMAN_FILTER_BILINEAR;
-	    pixman_kernel_sample = PIXMAN_KERNEL_BOX;
-	    pixman_kernel_reconstruct = PIXMAN_KERNEL_LINEAR;
+	    pixman_filter = PIXMAN_FILTER_BEST;
 	}
 
-	if (pixman_filter != PIXMAN_FILTER_NEAREST && (shrink_x || shrink_y)) {
-	    pixman_kernel_t sampling_kernel_x, sampling_kernel_y;
-	    int n_params;
-	    pixman_fixed_t *params;
-
-	    sampling_kernel_x = shrink_x ? pixman_kernel_sample : PIXMAN_KERNEL_IMPULSE;
-	    sampling_kernel_y = shrink_y ? pixman_kernel_sample : PIXMAN_KERNEL_IMPULSE;
-
-	    n_params = 0;
-	    params = pixman_filter_create_separable_convolution (&n_params,
-								 scale_x * 65536.0 + 0.5,
-								 scale_y * 65536.0 + 0.5,
-								 pixman_kernel_reconstruct,
-								 pixman_kernel_reconstruct,
-								 sampling_kernel_x,
-								 sampling_kernel_y,
-								 1, 1);
-
-	    pixman_image_set_filter (pixman_image,
-				     PIXMAN_FILTER_SEPARABLE_CONVOLUTION,
-				     params, n_params);
-
-	    free (params);
-	} else {
-	    pixman_image_set_filter (pixman_image, pixman_filter, NULL, 0);
-	}
+	pixman_image_set_filter (pixman_image, pixman_filter, NULL, 0);
     }
 
     {
-- 
2.0.0.rc0



More information about the cairo mailing list