Re: [PATCH] [FIXED] Minor fixes in the new downscaling code.
"Bryce W. Harrington" <[email protected]>
| Newsgroups | gmane.comp.lib.cairo |
|---|---|
| Message-ID | <[email protected]> |
Hi Krzysztof, Thanks for looking into these issues and providing a patch. I'm running it through 'make check' now. The changes look sensible, although I was not able to apply the patch to git due to formatting/line wrapping, and after that it didn't compile due to a couple typos. I've attached the fixed up patches. I'd recommend using git format-patch / git send-email to generate patches when using git. git format-patch -s HEAD~1 git send-email --to [email protected] --thread --no-chain-reply-to 00*.patch Anyway, review comments below... Bryce On Wed, Feb 05, 2014 at 01:35:41AM +0100, Krzysztof Kosiński wrote: > This adresses issues brought up on the mailing list. If we use addresses > a non-impulse samping kernel, there is no need to use anything > more sophisticated than box reconstruction. Similarly, using > a non-box reconstruction kernel is OK with impulse sampling. > > Also addressed is PIXMAN_FILTER_BEST, which previously fell through > to the default case. Split this bit out into a second patch. > This is a corrected resend of the previous patch, which contained a typo. > --- > src/cairo-image-source.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/src/cairo-image-source.c b/src/cairo-image-source.c > index 661bc10..254d6ba 100644 > --- a/src/cairo-image-source.c > +++ b/src/cairo-image-source.c > @@ -590,6 +590,7 @@ _pixman_image_set_properties (pixman_image_t *pixman_image, > pixman_filter = PIXMAN_FILTER_BEST; > pixman_kernel_sample = PIXMAN_KERNEL_LANCZOS3; > pixman_kernel_reconstruct = PIXMAN_KERNEL_LANCZOS3; > + break; > 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 > @@ -604,18 +605,31 @@ _pixman_image_set_properties (pixman_image_t > *pixman_image, > > if (pixman_filter != PIXMAN_FILTER_NEAREST && (shrink_x || shrink_y)) { > pixman_kernel_t sampling_kernel_x, sampling_kernel_y; > + pixman_kernel_t reconstruction_kernel_x, reconstruction_kernel_y; > int n_params; > pixman_fixed_t *params; > > - sampling_kernel_x = shrink_x ? pixman_kernel_sample : > PIXMAN_KERNEL_IMPULSE; Looks like lines got wrapped several places. > - sampling_kernel_y = shrink_y ? pixman_kernel_sample : > PIXMAN_KERNEL_IMPULSE; > + if (shrink_x) { > + samping_kernel_x = pixman_kernel_sample; sampling_kernel_x Also the tabbing seems off from here on down. > + reconstruction_kernel_x = PIXMAN_KERNEL_BOX; > + } else { > + sampling_kernel_x = PIXMAN_KERNEL_IMPULSE; > + reconstruction_kernel_x = pixman_kernel_reconstruct; > + } > + if (shrink_y) { > + samping_kernel_y = pixman_kernel_sample; samplling_kernel_y > + reconstruction_kernel_y = PIXMAN_KERNEL_BOX; > + } else { > + sampling_kernel_y = PIXMAN_KERNEL_IMPULSE; > + reconstruction_kernel_y = pixman_kernel_reconstruct; > + } > > 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, > + reconstruction_kernel_x, > + reconstruction_kernel_y, > sampling_kernel_x, > sampling_kernel_y, > 1, 1); I've reworked your patches with the above corrections. I've attached them here if you want to re-submit them (just replace my header with yours). -- cairo mailing list [email protected] http://lists.cairographics.org/mailman/listinfo/cairo
0001-For-PIXMAN_FILTER_BEST-Don-t-fall-through-to-the-def.patch
(text/x-diff, 1.1 KB)
From 65f78abdd5dcacfb32babbc7f81e89feef2379b3 Mon Sep 17 00:00:00 2001 From: Bryce Harrington <[email protected]> Date: Mon, 17 Mar 2014 13:56:42 -0700 Subject: [PATCH 1/2] For PIXMAN_FILTER_BEST, Don't fall-through to the default case. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch thanks to Krzysztof Kosiński <[email protected]> Signed-off-by: Bryce Harrington <[email protected]> --- src/cairo-image-source.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cairo-image-source.c b/src/cairo-image-source.c index 661bc10..880ce06 100644 --- a/src/cairo-image-source.c +++ b/src/cairo-image-source.c @@ -590,6 +590,7 @@ _pixman_image_set_properties (pixman_image_t *pixman_image, pixman_filter = PIXMAN_FILTER_BEST; pixman_kernel_sample = PIXMAN_KERNEL_LANCZOS3; pixman_kernel_reconstruct = PIXMAN_KERNEL_LANCZOS3; + break; 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 -- 1.8.1.2
0002-Optimize-downscaling-for-kernel-box-and-impulse-down.patch
(text/x-diff, 2.3 KB)
From 497ac412df67ca94bdf962eaddbafe7394b7ec9d Mon Sep 17 00:00:00 2001 From: Bryce Harrington <[email protected]> Date: Mon, 17 Mar 2014 14:00:19 -0700 Subject: [PATCH 2/2] Optimize downscaling for kernel box and impulse downscaling. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we use a non-impulse samping kernel, there is no need to use anything more sophisticated than box reconstruction. Similarly, using a non-box reconstruction kernel is OK with impulse sampling. Patch provided by Krzysztof Kosiński <[email protected]> Signed-off-by: Bryce Harrington <[email protected]> --- src/cairo-image-source.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/cairo-image-source.c b/src/cairo-image-source.c index 880ce06..254d6ba 100644 --- a/src/cairo-image-source.c +++ b/src/cairo-image-source.c @@ -605,18 +605,31 @@ _pixman_image_set_properties (pixman_image_t *pixman_image, if (pixman_filter != PIXMAN_FILTER_NEAREST && (shrink_x || shrink_y)) { pixman_kernel_t sampling_kernel_x, sampling_kernel_y; + pixman_kernel_t reconstruction_kernel_x, reconstruction_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; + if (shrink_x) { + samping_kernel_x = pixman_kernel_sample; + reconstruction_kernel_x = PIXMAN_KERNEL_BOX; + } else { + sampling_kernel_x = PIXMAN_KERNEL_IMPULSE; + reconstruction_kernel_x = pixman_kernel_reconstruct; + } + if (shrink_y) { + samping_kernel_y = pixman_kernel_sample; + reconstruction_kernel_y = PIXMAN_KERNEL_BOX; + } else { + sampling_kernel_y = PIXMAN_KERNEL_IMPULSE; + reconstruction_kernel_y = pixman_kernel_reconstruct; + } 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, + reconstruction_kernel_x, + reconstruction_kernel_y, sampling_kernel_x, sampling_kernel_y, 1, 1); -- 1.8.1.2