Re: [PATCH] image: V5 Use convolution filters for sample reconstruction when downscaling

On 06/18/2014 01:14 PM, Bryce W. Harrington wrote:

> First up, let me just say a big thank you for working on this.  We need
> to get better downscaling support in cairo, and this moves us forward.
>
>> This code contains an all-new filter generator to replace the one that is
>> in pixman. Results in 222 pass/298 failed image tests, which is much better
>> than the previous versions of this patch.
>
> What is the delta in passes and failures compared with trunk?

Git master:

$ CAIRO_TEST_TARGET=image ./cairo-test-suite -a
225 Passed, 295 Failed [1 crashed, 8 expected], 31 Skipped
Preamble: 1 failed - create-from-png
image (argb32): 1 crashed! - radial-outer-focus
image (argb32): 273 failed - a1-bug a1-clip-paint...
image (rgb24): 1 crashed! - radial-outer-focus
image (rgb24): 283 failed - a1-bug a1-clip-paint...

My version:

$ CAIRO_TEST_TARGET=image ./cairo-test-suite -a
221 Passed, 299 Failed [1 crashed, 8 expected], 31 Skipped
Preamble: 1 failed - create-from-png
image (argb32): 1 crashed! - radial-outer-focus
image (argb32): 283 failed - a1-bug a1-clip-paint...
image (rgb24): 1 crashed! - radial-outer-focus
image (rgb24): 280 failed - a1-bug a1-clip-paint...

I did a diff between the failed lists to see what tests were different, 
this is attached. My version seems to have fixed a few tests, I have no 
idea why.

> (I know the current downscaling tests have truth images that will show
> failures inappropriately, which iirc accounts for a hundred and some
> false failures when just changing the downscaling algorithm.)

My version has much fewer failures than the previous downscaling code 
that was removed from Cairo. I think the reason is that it uses the 
normal BILINEAR for scales greater than .75, which may make it identical 
for a lot of tests.

>> Filter generator (which should probably be in pixman):
>>
>> - Single filter, no "reconstruction" and "sample" filter
>> - Filters for derivative < 1 work
>> - Fixed IMPULSE and BOX
>> - Added TRIANGLE, CATMULL, NOTCH. Remove LANZCOS2.
>> - Renamed CUBIC to MITCHELL
>
> Thanks for including the commentary on each of them.  I don't have
> appropriate background to comment on the viability of all of these, but
> expanding the options does seem like a good idea.
>
> Regarding removal of lanzcos2, would it make sense to leave it in for
> one release, but mark it deprecated?

As the api to the filter generator has changed considerably, this 
actually is a different enum. They could be merged but since the 
functions are so close it is probably ok to make LANZCOS2 be an alias 
for CATMULL (you would also have to make CUBIC be an alias for MITCHELL, 
and perhaps GAUSSIAN an alias for NOTCH).

I have rarely seen lanzcos2 used in image processing. For some reason 
odd numbers (ie lanzcos3,5,7,9) are preferred, though I have not seen a 
reason why.

> I'm also curious about whether we should include this temporarily in
> cairo, vs. focus on getting it into pixman.  It might be handy to split
> this patch up into a few pieces to facilitate this (and to make review a
> bit easier).  Like one patch that adds filters but does not impact
> behavior, another which updates the filter quality mappings, another for
> the xlib change, etc.

I do feel it would be better to have PIXMAN_FILTER_GOOD/BEST do this and 
perhaps have no change in cairo at all.
But I am not sure what that involves. Can pixman just be updated and 
thus cairo is "fixed" if the user just installs the new pixman? What 
about the version of pixman that the XServer is calling from the XRender 
api? What about commercial (ie nVidia) x servers?

If cairo has to check the version of pixman and do this anyway, or if it 
has to do it for the x server, I'm not sure if there is any point to 
putting it in pixman.

>> NYI: This version uses the fallback for xlib always. The xlib and xcb backends
>> must be rewritten to use the fallback version if filtering is needed. Or the
>> filtering code must be moved to XRender.
>
> I may have missed the discussion on a previous patch, but could you
> explain this in more detail?

It looks like the xlib cairo backend sends commands to XRender, or 
otherwise somehow causes images to be transformed without calling 
_pixman_image_set_properties. I found that changing cairo the so the 
xlib cairo backend was the "fallback" version (which apparently just 
uses the image backend to draw into a shm buffer) made it run this 
filter code.

Assuming no changes to xrender, it looks like the solution would be for 
cairo's xlib backend to check if filtering is going to be needed, and 
switch to the fallback renderer in this case. This would require another 
function that says whether the filter will be used for the ctm. I have 
no idea where this code would be inserted into the xlib backend.

I am also a bit suspicious that it would be most expedient to just give 
up on XRender and use the fallback always. I certainly see no speed 
differences.

> Also, does this potentially affect any other backend behaviors?

Anything that uses pixman and does not call the 
_pixman_image_set_properties function in cairo will need to fix this. I 
figure the xcb backend has the same problem as xlib. Don't know about 
others.

-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo

--- master_out	2014-06-18 16:20:36.003507198 -0700
+++ myversion.out	2014-06-18 16:20:07.511506403 -0700
@@ -1,7 +1,7 @@
-225 Passed, 295 Failed [1 crashed, 8 expected], 31 Skipped
+221 Passed, 299 Failed [1 crashed, 8 expected], 31 Skipped
 Preamble: 1 failed - create-from-png
 image (argb32): 1 crashed! - radial-outer-focus
-image (argb32): 273 failed -
+image (argb32): 283 failed -
 a1-bug
 a1-clip-paint
 a1-clip-fill
@@ -37,6 +37,7 @@
 clip-fill-eo-unbounded
 clip-fill
 a1-clip-fill-rule
+clip-fill-rule-pixel-aligned
 clip-group-shapes-circles
 clip-image
 clip-intersect
@@ -51,6 +52,7 @@
 composite-integer-translate-source
 composite-integer-translate-over
 composite-integer-translate-over-repeat
+create-from-png-stream
 culled-glyphs
 dash-curve
 degenerate-arc
@@ -75,7 +77,7 @@
 fill-and-stroke-alpha-add
 fill-degenerate-sort-order
 fill-image
-fill-rule
+fill-missed-stop
 filter-nearest-offset
 filter-nearest-transformed
 finer-grained-fallbacks
@@ -121,20 +123,23 @@
 mime-data
 move-to-show-surface
 negative-stride-image
-new-sub-path
+operator
 operator-clear
 operator-source
 over-above-source
 over-around-source
 over-between-source
+overlapping-boxes
 overlapping-glyphs
 overlapping-dash-caps
 path-append
+pattern-getters
 pixman-downscale-fast-95
 pixman-downscale-good-96
 pixman-downscale-good-95
 pixman-downscale-best-96
 pixman-downscale-best-95
+pixman-downscale-best-24
 pixman-downscale-nearest-95
 pixman-downscale-bilinear-96
 pixman-downscale-bilinear-95
@@ -169,6 +174,7 @@
 record2x-fill-alpha
 record2x-select-font-face
 record2x-text-transform
+record90-paint-alpha
 record90-paint-alpha-solid-clip
 record90-paint-alpha-clip
 record90-paint-alpha-clip-mask
@@ -219,12 +225,14 @@
 smask-image-mask
 smask-stroke
 smask-text
+solid-pattern-cache-stress
 source-clip-scale
 spline-decomposition
 stride-12-image
 stroke-pattern
 subsurface
 subsurface-image-repeat
+subsurface-outside-target
 subsurface-scale
 surface-pattern-operator
 surface-pattern-scale-down
@@ -257,12 +265,14 @@
 user-font-proxy
 user-font-rescale
 world-map
-world-map-stroke
 world-map-fill
+xcb-huge-image-shm
+xcb-huge-subimage
 xcb-stress-cache
 xcb-snapshot-assert
 xcomposite-projection
 xlib-expose-event
+zero-mask
 pthread-same-source
 pthread-show-text
 bitmap-font
@@ -276,7 +286,7 @@
 xcb-surface-source
 xlib-surface-source
 image (rgb24): 1 crashed! - radial-outer-focus
-image (rgb24): 283 failed -
+image (rgb24): 280 failed -
 a1-bug
 a1-clip-paint
 a1-clip-fill
@@ -295,9 +305,7 @@
 aliasing
 arc-direction
 big-line
-big-little-triangle
 bug-spline
-bug-source-cu
 bug-extents
 bug-seams
 caps
@@ -329,12 +337,11 @@
 composite-integer-translate-source
 composite-integer-translate-over
 composite-integer-translate-over-repeat
+create-from-png-stream
 culled-glyphs
 dash-curve
-dash-zero-length
 degenerate-arc
 degenerate-dash
-degenerate-path
 device-offset
 device-offset-fractional
 device-offset-positive
@@ -357,7 +364,6 @@
 fill-and-stroke-alpha-add
 fill-degenerate-sort-order
 fill-image
-fill-missed-stop
 fill-rule
 filter-nearest-offset
 filter-nearest-transformed
@@ -405,21 +411,20 @@
 mesh-pattern-transformed
 move-to-show-surface
 negative-stride-image
-new-sub-path
 operator-clear
 operator-source
 over-above-source
-over-around-source
-over-below-source
 over-between-source
 overlapping-glyphs
 overlapping-dash-caps
 path-append
+pattern-getters
 pixman-downscale-fast-95
 pixman-downscale-good-96
 pixman-downscale-good-95
 pixman-downscale-best-96
 pixman-downscale-best-95
+pixman-downscale-best-24
 pixman-downscale-nearest-95
 pixman-downscale-bilinear-96
 pixman-downscale-bilinear-95
@@ -453,6 +458,7 @@
 record2x-fill-alpha
 record2x-select-font-face
 record2x-text-transform
+record90-paint-alpha
 record90-paint-alpha-solid-clip
 record90-paint-alpha-clip
 record90-paint-alpha-clip-mask
@@ -483,7 +489,6 @@
 rectangle-rounding-error
 a1-rectilinear-grid
 reflected-stroke
-rel-path
 rgb24-ignore-alpha
 rotate-image-surface-paint
 clip-rotate-image-surface-paint
@@ -504,6 +509,7 @@
 smask-image-mask
 smask-stroke
 smask-text
+solid-pattern-cache-stress
 source-clip-scale
 spline-decomposition
 stride-12-image
@@ -542,8 +548,9 @@
 user-font-proxy
 user-font-rescale
 world-map
-world-map-stroke
 world-map-fill
+xcb-huge-image-shm
+xcb-huge-subimage
 xcb-stress-cache
 xcb-snapshot-assert
 xcomposite-projection