Re: [Pixman] Dithering patches, v2

[Bill Spitzak]

Is the "blue noise" version actually slower than the Bayer? Seems to be
doing a bit less calculation but it reading the array of weights. The
removal of the need for the pattern offset seems like a win.

On Mon, Apr 22, 2019 at 9:27 AM Matt Turner  wrote:

> On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
>  wrote:
> > Inkscape would love to see Basile's dithering patches included.  Our
> > testing shows that they make a huge quality difference for our users;
> > this solves a critical need.
> >
> > Mc and I have done some preliminary investigation into how to plumb this
> > into Cairo, and would love to hear your review of Basile's approach to
> > the problem.
>
> I don't feel like I'm experienced enough with that side of pixman to
> offer meaningful comments. I've Cc'd Søren in the hopes that he
> remains interested enough in the project to review the patches that
> Basile says implement the approach Søren described.
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Dithering patches, v2

[Bill Spitzak]

How difficult is it to just make the gradients use dithering, so they
produce 8-bit (or whatever) results directly but with the dithering pattern
in them?

What other operations would need dithering (a large blur comes to mind,
also zooming way in on an image in bilinear)?

I think the blue noise large pattern is a great idea. Could have used that
years ago in Nuke (it uses error diffusion but people always complained
that the pattern did not reproduce when tiny changes were made to the
image).


On Tue, Apr 9, 2019 at 3:01 PM Basile Clement 
wrote:

> I forgot to mention two things:
>
>  - Enabling dithering makes processing much slower, since it forces the
> wide pipeline to be used (and some optimisations are disabled).  This
> could be solved through the use of fast paths where needed (e.g. when
> blitting images from an equal or lower bits-per-pixels format, or with
> fused gradient+dithering paths).  In any case, the design of the
> implementation allows setting the `dither` property of a destination
> image in between calls to `pixman_image_composite` where dithering is
> required, allowing users to pay the cost of the slower wide pipeline
> only for those calls where dithering is actually required.
>
>  - The dithering is not gamma corrected, which doesn't matter much for
> 8bpp formats -- but makes a noticeable difference for lower bpp formats,
> producing images which are way too luminous (this is particularily
> visible in the dithering demos when using 2bpp and 1bpp formats).  Since
> the main use case for now is a8r8g8b8 gradient export in Inkscape (and
> since the series is starting to be a bit large), I did not include gamma
> correction in this series.  If needed, it can be added in subsequent
> patches, and should probably be configurable since the proper curve to
> use depend on the characteristic of the target device, which pixman may
> not know about.
>
> - Basile
>
> On 4/9/19 11:29 PM, basile-pix...@clement.pm wrote:
> > I am resubmitting for review and inclusion my series of patches on
> > dithering from October, which I ended up not having the time to finish
> > then.  There are only a couple changes to make the patches compatible
> > with the recently added floating point formats, and I have also added a
> > dithering matrix based on blue noise which provides more pleasant
> > results than the Bayer matrix from the first series.
> >
> > Maarten, I would appreciate if you can take a second look (although as
> > noted above not much has changed since your first review -- I don't
> > think you had comments on the dithering part?).
> >
> > - Basile
> >
> >
> > ___
> > Pixman mailing list
> > Pixman@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/pixman
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Is circle rendering possible?

[Bill Spitzak]

If you are rendering a solid color (so the result is either that color or
transparent black) then you can just fill the entire RGB with that color to
turn the premulitplied output into unpremultiplied.
However I am unsure what your precision problem is. You would have to
composite several dozen mostly-transparent pixels for it to make any
visible difference.


On Wed, Feb 6, 2019 at 2:28 AM Indi Kernick  wrote:

> I thought pixman allowed you to choose between premultiplied and not
> premultiplied but I checked and apparently not. I think I'm getting my
> libraries mixed up. The problem I have with premultiplied alpha is that you
> lose precision when you have to un-premultiply. That's something I need in
> my application. I want to get the color of a pixel without losing precision
> with semi-transparent pixels.
>
> So if I don't want to lose precision, I'll have to store images in regular
> alpha and then multiply the alpha every time I want to composite. That's
> kind of annoying.
>
> To render a polygon, I could divide it into triangles and render those
> with pixman.
>
> Thanks for your help. I think I'll explore other libraries.
>
>
> On Wed, 6 Feb 2019 at 20:24 Pekka Paalanen  wrote:
>
>> On Wed, 6 Feb 2019 19:25:48 +1030
>> Indi Kernick  wrote:
>>
>> > The only problem I can find with Cairo is that it uses premultiplied
>> alpha.
>>
>> Pixman uses premultiplied alpha. Also Wayland uses premultiplied alpha
>> FWIW. I think you will find it quite common in general.
>>
>> > That's so annoying! Also, I'm not sure if the Cairo renderer is GPU or
>> CPU
>> > based.
>>
>> You choose through the API which one you use. Use the image API, and
>> you get CPU rendering.
>>
>> > I'm using Qt to render the images. Qt can render surfaces so I can use
>> > pixman to manipulate a surface and then Qt to upload it to the GPU when
>> > it's time to render. Cairo can do the same thing but then I have to
>> worry
>> > about premultiplied alpha. I could use pixman to do the compositing and
>> > Cairo to do other stuff but that's just a mess.
>> >
>> > I'm guessing that using a GPU backend for Cairo and then making it play
>> > nice with the Qt layout engine will be a nightmare. If I have to go back
>> > and forth between CPU and GPU, it might actually be faster to use CPU
>> > rendering. The only thing I'm worried about is compositing. Compositing
>> is
>> > much faster on the GPU (I presume) but if I use Cairo, I'll have to
>> fiddle
>> > around with the alpha channel. That means a round trip between CPU and
>> GPU
>> > memory.
>>
>> You don't need to fiddle with alpha. Just write your fragment shaders
>> and blending state to expect it.
>>
>> The point of using premultiplied alpha is to pre-compute values you
>> would be computing in any case during blending.
>>
>> > I only really need non-antialiased circle rendering (antialiasing is a
>> > nice-to-have). I can figure out to do that myself (Bresenham comes to
>> > mind). I think I'll use pixman and do everything on the CPU.
>> >
>> > Another question: can pixman render a filled polygon without
>> antialiasing?
>> > If not, I might have to do that myself too. 
>>
>> That I don't know. I have never used Pixman beyond
>> pixman_image_composite32(). I am guessing the trapezoids or triangles
>> API could be used, but there does not seem to be anything about polygons
>> per se. You can see the header yourself, that is all the documentation
>> there is, which is practically none. Cairo at least has documentation.
>>
>> I believe there are also other CPU rendering libraries, but I wouldn't
>> know to point to them.
>>
>>
>> Thanks,
>> pq
>>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] pixman: Add support for argb/xrgb float formats, v5.

[Bill Spitzak]

I don't like the fact that the design which allows the rgba to be different
sizes cannot make them vary between float and decimal. The main reason is
so that an integer for Alpha can be supported. I think maybe there should
be a bit that changes the interpretation of the bit widths to a 16-valued
type enumeration. For n-bit integers use n-1 as now, but remove the unusual
ones and replace them with floating point and signed types. Use a lookup
table to get type + width. I really don't think there are more than 16 of
them:

8, 16, 32, 64 bit unsigned
8, 16, 32, 64 bit float
this set leaves 8 unused values, which maybe could be allocated in the
future for signed types or 128-bit types. You could also provide a few more
unsigned widths (1,2,4,10,12?)
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 1/2] pixman: Add support for argb/xrgb float formats, v3.

[Bill Spitzak]

I think an indicator for float can double as indicating the size is
multiplied by 8. All float formats I am familiar with take a mulitple of 8
bits, including an 8-bit format that is sometimes used in CG.


On Mon, Oct 29, 2018 at 2:18 AM Maarten Lankhorst <
maarten.lankho...@linux.intel.com> wrote:

> Op 29-10-18 om 09:16 schreef Siarhei Siamashka:
> > On Wed, 03 Oct 2018 10:11:46 +0100
> > Chris Wilson  wrote:
> >
> >> Quoting Maarten Lankhorst (2018-08-01 13:41:33)
> >>> diff --git a/pixman/pixman.h b/pixman/pixman.h
> >>> index 509ba5e534a8..c9bf4faa80d4 100644
> >>> --- a/pixman/pixman.h
> >>> +++ b/pixman/pixman.h
> >>> @@ -647,19 +647,24 @@ struct pixman_indexed
> >>>   * sample implementation allows only packed RGB and GBR
> >>>   * representations for data to simplify software rendering,
> >>>   */
> >>> -#define PIXMAN_FORMAT(bpp,type,a,r,g,b)(((bpp) << 24) |  \
> >>> +#define PIXMAN_FORMAT_PACKC(val) ((val) <= 10 ? (val) : \
> >>> +(val) == 32 ? 11 : 0)
> >>> +#define PIXMAN_FORMAT_UNPACKC(val) ((val) <= 10 ? (val) : \
> >>> +   (val) == 11 ? 32 : 0)
> >> We have 4bits, why not reserve 0xf for float32? Certainly you
> >> want space for
> >>
> >> #define PIXMAN_FORMAT_PACKED_FLOAT16 0xb
> >> #define PIXMAN_FORMAT_PACKED_FLOAT32 0xc
> >> #define PIXMAN_FORMAT_PACKED_FLOAT64 0xd
> >>
> >> I just wonder if we may have 12bpc one day as well, so leaving 0xc clear
> >> has some some appeal.
> > We could probably also try to do something like this:
> >
> > /*
> >  * While the protocol is generous in format support, the
> >  * sample implementation allows only packed RGB and GBR
> >  * representations for data to simplify software rendering,
> >  *
> >  * Bit 23 selects the granularity of channel color depth
> >  *  0: 1-bit granularity (allows up to 15 bits per channel)
> >  *  1: 1-byte granularity (allows up to 120 bits per channel)
> >  */
> > #define PIXMAN_FORMAT(bpp,type,a,r,g,b)   \
> >   (((bpp) << 24) |  \
> >   ((type) << 16) | \
> >   (((a) < 16) ? ((a) << 12) : (((a / 8) << 12) | (1 << 23))) | \
> >   (((r) < 16) ? ((r) << 8)  : (((r / 8) << 8)  | (1 << 23))) | \
> >   (((g) < 16) ? ((g) << 4)  : (((g / 8) << 4)  | (1 << 23))) | \
> >   (((b) < 16) ? ((b))   : (((b / 8))   | (1 << 23
> >
> > /* 0 for 1-bit granularity and 3 for 1-byte granularity */
> > #define PIXMAN_FORMAT_CHANDEPTH_SHIFT(f) \
> >   f) >> 23) & 1) | f) >> 23) & 1) << 1))
> I would use 2 bits then, 6 is still plenty for the rest of the type.
>
> Perhaps make a separate PIXMAN_FORMAT_TYPE_RGBA_FLOAT and
> PIXMAN_FORMAT_TYPE_RGB_FLOAT?
>
> > #define PIXMAN_FORMAT_A(f) \
> >   f) >> 12) & 0x0f) << PIXMAN_FORMAT_CHANDEPTH_SHIFT(f))
> > #define PIXMAN_FORMAT_R(f) \
> >   f) >>  8) & 0x0f) << PIXMAN_FORMAT_CHANDEPTH_SHIFT(f))
> > #define PIXMAN_FORMAT_G(f) \
> >   f) >>  4) & 0x0f) << PIXMAN_FORMAT_CHANDEPTH_SHIFT(f))
> > #define PIXMAN_FORMAT_B(f) \
> >   f)  ) & 0x0f) << PIXMAN_FORMAT_CHANDEPTH_SHIFT(f))
> >
> > #define PIXMAN_FORMAT_BPP(f)  (((uint32_t)(f)) >> 24)
> > #define PIXMAN_FORMAT_TYPE(f) (((f) >> 16) & 0x7f)
> > #define PIXMAN_FORMAT_RGB(f)  (((f)  ) & 0xfff)
> > #define PIXMAN_FORMAT_VIS(f)  (((f)  ) & 0x)
> > #define PIXMAN_FORMAT_DEPTH(f)(PIXMAN_FORMAT_A(f) +   \
> >PIXMAN_FORMAT_R(f) +   \
> >PIXMAN_FORMAT_G(f) +   \
> >PIXMAN_FORMAT_B(f))
> >
> >
> > Right now the format type occupies 8 bits. The highest bit could be
> > repurposed as a flag for storing channel depth as bytes instead
> > of bits. This way 16-bit and 64-bit per color component will be
> > also supported. The limitation of this approach is that the depth
> > of every color component has to be a multiple of 8 bits if any of
> > them is 16 bits or larger.
> >
> > I don't feel very comfortable about the fact that some applications
> > are using the PIXMAN_FORMAT_A/R/G/B macros and this code gets compiled
> > as part of these application binaries.
> >
> > Are there any other interesting >32bpp formats that we may need
> > to support in the future?
> >
> Doubt it, pixman doesn't have that accuracy currently, but F16 might be
> interesting at some point..
>
> ~Maarten
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [patch] Gradient dithering into pixman

[Bill Spitzak]

I get it, the dithering is turned on/off with a Cairo control. I thought
you were suggesting that it is turned off if the destination is an image.

Quick look at the patch and it seems like this will work, though it is a
random dither. You might get much better compression of .png with a
patterned dither. I've also had good luck with pseudo-error-diffusion. You
keep in static memory an accumulated error (per color, but not really
depending on the location of the pixel) and that is the threshold for the
random number. This produces a bit more patterning than a straight random
generator. You might also try it with no random number generator at all,
but you have to preserve the accumulated error so that solid areas that are
an integer don't reset it so each adjacent line gets the same pattern.

May want to call the "on" setting PIXMAN_DITHERING_GOOD. On the assumption
that anybody who wants it on is happy with "good" dithering, and that they
may not want to pay for the slowness of "best" dithering.


On Tue, Mar 27, 2018 at 2:20 AM, Marc Jeanmougin <m...@jeanmougin.fr> wrote:

> Hi,
>
> Le 27/03/2018 à 02:04, Søren Sandmann a écrit :
> > A long time ago I wrote this:
> >
> > https://lists.freedesktop.org/archives/pixman/2012-July/002175.html
> >
> > about how dithering could be added to pixman. The basic idea is that
> > "dithering" is a property of the destination image, not of the
> > gradient.  I still think this is the right way to do it.
>
> Thank you for your input. Would it be possible to use your preferred way
> of doing it to my patch ?
>
> Le 27/03/2018 à 03:47, Bill Spitzak a écrit :
> > I don't understand why you would want to disable it when writing .png
> > files. There will be banding in the .png file, which I would think is
> > worse than the increased size.
> Depends. In many cases there is no visible banding, and some people may
> not want a x100 filesize increase for it (for files that are entirely
> made of gradients).
>
> > Also kind of fools the user if they did not look at the .png file and
> > only at InkScape's display.
> We already have such differences for filter quality, and of course it
> would be possible to switch it in the display windows for previewing.
>
> --
> Marc
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [patch] Gradient dithering into pixman

[Bill Spitzak]

I don't understand why you would want to disable it when writing .png
files. There will be banding in the .png file, which I would think is worse
than the increased size. Also kind of fools the user if they did not look
at the .png file and only at InkScape's display.


On Mon, Mar 26, 2018 at 5:04 PM, Søren Sandmann 
wrote:

> Hi,
>
> A long time ago I wrote this:
>
> https://lists.freedesktop.org/archives/pixman/2012-July/002175.html
>
> about how dithering could be added to pixman. The basic idea is that
> "dithering" is a property of the destination image, not of the gradient.  I
> still think this is the right way to do it.
>
>
> Søren
>
> On Mon, Mar 26, 2018 at 6:37 PM, Marc Jeanmougin 
> wrote:
>
>> Hi,
>>
>> I'm Marc, I'm an Inkscape contributor, and I would like to improve
>> pixman by providing a patch to allow gradient dithering, to eliminate
>> all gradient banding. This "banding" problem is hugely visible in many
>> Inkscape files, and I hope you could help me putting it into pixman,
>> then in exposing the functionality into cairo.
>>
>> I tried to very closely follow the existing code style, and I came up
>> with the proof of concept (attached Proof_of_concept.patch ), which
>> applies to pixman/master to enable the feature directly.
>>
>> Since Inkscape also uses pixman to render files when exporting to png,
>> and that kind of dithering affects very significantly the file size (PNG
>> compression algorithm and randomness don't mix well) we also need a
>> switch to enable/disable it "at will".
>>
>> I tried to understand how it could be done by looking at
>> pixman_image_set_* functions and finally came up with the other patch
>> attached 0001-Adds-a-gradient-dithering-function-to-pixman.patch (which
>> cannot directly be tested, since it would require changes in cairo to
>> call pixman_image_set_dithering, but changing "gradient->dithering =
>> PIXMAN_DITHERING_NONE;" to _BEST in pixman-image.c works)
>>
>> Thanks for any help in merging this, or any pointer to how to improve it,
>>
>> --
>> Marc
>>
>> ___
>> Pixman mailing list
>> Pixman@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/pixman
>>
>>
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 11/15] pixman-filter: integral splitting is only needed for triangle filter

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Only the triangle is discontinuous at 0. The other filters resemble a
cubic closely enough that Simpsons integration works without
splitting.

Changes by Søren: Rebase without the changes to the integral function,
update comment to match the new code.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Signed-off-by: Søren Sandmann <soren.sandm...@gmail.com>
Reviewed-by: Søren Sandmann <soren.sandm...@gmail.com>
---
 pixman/pixman-filter.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index ecff852..878cf9d 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -160,18 +160,17 @@ integral (pixman_kernel_t kernel1, double x1,
  pixman_kernel_t kernel2, double scale, double x2,
  double width)
 {
-/* If the integration interval crosses zero, break it into
- * two separate integrals. This ensures that filters such
- * as LINEAR that are not differentiable at 0 will still
- * integrate properly.
+/* The LINEAR filter is not differentiable at 0, so if the
+ * integration interval crosses zero, break it into two
+ * separate integrals.
  */
-if (x1 < 0 && x1 + width > 0)
+if (kernel1 == PIXMAN_KERNEL_LINEAR && x1 < 0 && x1 + width > 0)
 {
return
integral (kernel1, x1, kernel2, scale, x2, - x1) +
integral (kernel1, 0, kernel2, scale, x2 - x1, width + x1);
 }
-else if (x2 < 0 && x2 + width > 0)
+else if (kernel2 == PIXMAN_KERNEL_LINEAR && x2 < 0 && x2 + width > 0)
 {
return
integral (kernel1, x1, kernel2, scale, x2, - x2) +
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 06/15] demos/scale: Default to locked axis

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Søren Sandmann <soren.sandm...@gmail.com>
---
 demos/scale.ui | 1 +
 1 file changed, 1 insertion(+)

diff --git a/demos/scale.ui b/demos/scale.ui
index f6f6e89..d498d26 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -177,6 +177,7 @@
  id="lock_checkbutton">
Lock X and Y 
Dimensions
0.0
+   True
  
   
 False
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH] Fixes to pixman filtering

[spitzak]

If anybody is maintaining pixman now, these patches have been reviewed
several times and should be pushed. They are primarily written by
Søren based on versions I wrote. They fix several defects in the
seperable filter generation.

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 15/15] pixman-filter: Made Gaussian a bit wider

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Expanded the size slightly (from ~4.25 to 5) to make the cutoff less
noticable.  Previouly the value at the cutoff was
gaussian_filter(sqrt(2)*3/2) = 0.00626 which is larger than the
difference between 8-bit pixels (1/255 = 0.003921). New cutoff is
gaussian_filter(2.5) = 0.001089 which is smaller.

v11: added some math to commit message
v14: left SIGMA in there
Signed-off-by: Bill Spitzak <spit...@gmail.com>
Acked-by: Oded Gabbay <oded.gab...@gmail.com>
Reviewed-by: Søren Sandmann <soren.sandm...@gmail.com>
---
 pixman/pixman-filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 35ebf02..f149ebc 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -143,7 +143,7 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_BOX,   box_kernel,   1.0 },
 { PIXMAN_KERNEL_LINEAR,linear_kernel,2.0 },
 { PIXMAN_KERNEL_CUBIC, cubic_kernel, 4.0 },
-{ PIXMAN_KERNEL_GAUSSIAN,  gaussian_kernel,  6 * SIGMA },
+{ PIXMAN_KERNEL_GAUSSIAN,  gaussian_kernel,  5.0 },
 { PIXMAN_KERNEL_LANCZOS2,  lanczos2_kernel,  4.0 },
 { PIXMAN_KERNEL_LANCZOS3,  lanczos3_kernel,  6.0 },
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 02/15] Add new test of filter reduction from BILINEAR to NEAREST

[spitzak]

From: Søren Sandmann Pedersen <soren.sandm...@gmail.com>

This new test tests a bunch of bilinear downscalings, where many have
a transformation such that the BILINEAR filter can be reduced to
NEAREST (and many don't).

A CRC32 is computed for all the resulting images and compared to a
known-good value for both 4-bit and 7-bit interpolation.

V2: Remove leftover comment, some minor formatting fixes, use a
timestamp as the PRNG seed.

Signed-off-by: Søren Sandmann <soren.sandm...@gmail.com>
Reviewed-by: Bill Spitzak <spit...@gmail.com>
---
 test/Makefile.sources|   1 +
 test/filter-reduction-test.c | 112 +++
 2 files changed, 113 insertions(+)
 create mode 100644 test/filter-reduction-test.c

diff --git a/test/Makefile.sources b/test/Makefile.sources
index 5d55e67..0a56231 100644
--- a/test/Makefile.sources
+++ b/test/Makefile.sources
@@ -21,6 +21,7 @@ TESTPROGRAMS =  \
gradient-crash-test   \
pixel-test\
matrix-test   \
+   filter-reduction-test \
composite-traps-test  \
region-contains-test  \
glyph-test\
diff --git a/test/filter-reduction-test.c b/test/filter-reduction-test.c
new file mode 100644
index 000..705fa4b
--- /dev/null
+++ b/test/filter-reduction-test.c
@@ -0,0 +1,112 @@
+#include 
+#include 
+#include "utils.h"
+
+static const pixman_fixed_t entries[] =
+{
+pixman_double_to_fixed (-1.0),
+pixman_double_to_fixed (-0.5),
+pixman_double_to_fixed (-1/3.0),
+pixman_double_to_fixed (0.0),
+pixman_double_to_fixed (0.5),
+pixman_double_to_fixed (1.0),
+pixman_double_to_fixed (1.5),
+pixman_double_to_fixed (2.0),
+pixman_double_to_fixed (3.0),
+};
+
+#define SIZE 12
+
+static uint32_t
+test_scale (const pixman_transform_t *xform, uint32_t crc)
+{
+uint32_t *srcbuf, *dstbuf;
+pixman_image_t *src, *dest;
+
+srcbuf = malloc (SIZE * SIZE * 4);
+prng_randmemset (srcbuf, SIZE * SIZE * 4, 0);
+src = pixman_image_create_bits (
+   PIXMAN_a8r8g8b8, SIZE, SIZE, srcbuf, SIZE * 4);
+
+dstbuf = malloc (SIZE * SIZE * 4);
+prng_randmemset (dstbuf, SIZE * SIZE * 4, 0);
+dest = pixman_image_create_bits (
+   PIXMAN_a8r8g8b8, SIZE, SIZE, dstbuf, SIZE * 4);
+
+pixman_image_set_transform (src, xform);
+pixman_image_set_repeat (src, PIXMAN_REPEAT_NORMAL);
+pixman_image_set_filter (src, PIXMAN_FILTER_BILINEAR, NULL, 0);
+
+image_endian_swap (src);
+image_endian_swap (dest);
+
+pixman_image_composite (PIXMAN_OP_SRC,
+   src, NULL, dest,
+   0, 0, 0, 0, 0, 0,
+   SIZE, SIZE);
+
+crc = compute_crc32_for_image (crc, dest);
+
+pixman_image_unref (src);
+pixman_image_unref (dest);
+
+free (srcbuf);
+free (dstbuf);
+
+return crc;
+}
+
+#if BILINEAR_INTERPOLATION_BITS == 7
+#define CHECKSUM 0x02169677
+#elif BILINEAR_INTERPOLATION_BITS == 4
+#define CHECKSUM 0xE44B29AC
+#else
+#define CHECKSUM 0x
+#endif
+
+int
+main (int argc, const char *argv[])
+{
+const pixman_fixed_t *end = entries + ARRAY_LENGTH (entries);
+const pixman_fixed_t *t0, *t1, *t2, *t3, *t4, *t5;
+uint32_t crc = 0;
+
+prng_srand (0x56EA1DBD);
+
+for (t0 = entries; t0 < end; ++t0)
+{
+   for (t1 = entries; t1 < end; ++t1)
+   {
+   for (t2 = entries; t2 < end; ++t2)
+   {
+   for (t3 = entries; t3 < end; ++t3)
+   {
+   for (t4 = entries; t4 < end; ++t4)
+   {
+   for (t5 = entries; t5 < end; ++t5)
+   {
+   pixman_transform_t xform = {
+   { { *t0, *t1, *t2 },
+ { *t3, *t4, *t5 },
+ { 0, 0, pixman_fixed_1 } }
+   };
+
+   crc = test_scale (, crc);
+   }
+   }
+   }
+   }
+   }
+}
+
+if (crc != CHECKSUM)
+{
+   printf ("filter-reduction-test failed! (checksum=0x%08X, expected 
0x%08X)\n", crc, CHECKSUM);
+   return 1;
+}
+else
+{
+   printf ("filter-reduction-test passed (checksum=0x%08X)\n", crc);
+   return 0;
+}
+}
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 14/15] pixman-filter: Nested polynomial for cubic

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

v11: Restored range checks

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index b3c36b8..35ebf02 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -109,14 +109,16 @@ general_cubic (double x, double B, double C)
 
 if (ax < 1)
 {
-   return ((12 - 9 * B - 6 * C) * ax * ax * ax +
-   (-18 + 12 * B + 6 * C) * ax * ax + (6 - 2 * B)) / 6;
+   return (((12 - 9 * B - 6 * C) * ax +
+(-18 + 12 * B + 6 * C)) * ax * ax +
+   (6 - 2 * B)) / 6;
 }
-else if (ax >= 1 && ax < 2)
+else if (ax < 2)
 {
-   return ((-B - 6 * C) * ax * ax * ax +
-   (6 * B + 30 * C) * ax * ax + (-12 * B - 48 * C) *
-   ax + (8 * B + 24 * C)) / 6;
+   return -B - 6 * C) * ax +
+ (6 * B + 30 * C)) * ax +
+(-12 * B - 48 * C)) * ax +
+   (8 * B + 24 * C)) / 6;
 }
 else
 {
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 10/15] pixman-filter: Correct Simpsons integration

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Simpsons uses cubic curve fitting, with 3 samples defining each
cubic. This makes the weights of the samples be in a pattern of
1,4,2,4,2...4,1, and then dividing the result by 3.

The previous code was using weights of 1,2,0,6,0,6...,2,1.

With this fix the integration is accurate enough that the number of
samples could be reduced a lot. Multiples of 12 seem to work best.

v7: Merged with patch to reduce from 128 samples to 16
v9: Changed samples from 16 to 12
v10: Fixed rebase error that made it not compile
v11: minor whitespace change
v14: more whitespace changes

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
Reviewed-by: Søren Sandmann <soren.sandm...@gmail.com>
---
 pixman/pixman-filter.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index c018a67..ecff852 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -189,13 +189,19 @@ integral (pixman_kernel_t kernel1, double x1,
 }
 else
 {
-   /* Integration via Simpson's rule */
-#define N_SEGMENTS 128
+   /* Integration via Simpson's rule
+* See http://www.intmath.com/integration/6-simpsons-rule.php
+* 12 segments (6 cubic approximations) seems to produce best
+* result for lanczos3.linear, which was the combination that
+* showed the most errors.  This makes sense as the lanczos3
+* filter is 6 wide.
+*/
+#define N_SEGMENTS 12
 #define SAMPLE(a1, a2) \
(filters[kernel1].func ((a1)) * filters[kernel2].func ((a2) * scale))

double s = 0.0;
-   double h = width / (double)N_SEGMENTS;
+   double h = width / N_SEGMENTS;
int i;
 
s = SAMPLE (x1, x2);
@@ -204,11 +210,14 @@ integral (pixman_kernel_t kernel1, double x1,
{
double a1 = x1 + h * i;
double a2 = x2 + h * i;
+   s += 4 * SAMPLE (a1, a2);
+   }
 
+   for (i = 2; i < N_SEGMENTS; i += 2)
+   {
+   double a1 = x1 + h * i;
+   double a2 = x2 + h * i;
s += 2 * SAMPLE (a1, a2);
-
-   if (i >= 2 && i < N_SEGMENTS - 1)
-   s += 4 * SAMPLE (a1, a2);
}
 
s += SAMPLE (x1 + width, x2 + width);
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 05/15] demos/scale: fix blank subsamples spin box

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

It now shows the initial value of 4 when the demo is started

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Søren Sandmann <soren.sandm...@gmail.com>
---
 demos/scale.ui | 1 +
 1 file changed, 1 insertion(+)

diff --git a/demos/scale.ui b/demos/scale.ui
index ee985dd..f6f6e89 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -301,6 +301,7 @@
   
 True
subsample_adjustment
+   4
   
   
 1
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 12/15] pixman-filter: Speed up BOX/BOX filter

[spitzak]

From: Søren Sandmann Pedersen <soren.sandm...@gmail.com>

The convolution of two BOX filters is simply the length of the
interval where both are non-zero, so we can simply return width from
the integral() function because the integration region has already
been restricted to be such that both functions are non-zero on it.

This is both faster and more accurate than doing numerical integration.

This patch is based on one by Bill Spitzak

https://lists.freedesktop.org/archives/pixman/2016-March/004446.html

with these changes:

- Rebased to not assume any changes in the arguments to integral().

- Dropped the multiplication by scale

- Added more details in the commit message.

Signed-off-by: Søren Sandmann <soren.sandm...@gmail.com>
Reviewed-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-filter.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 878cf9d..11e7d0e 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -160,11 +160,15 @@ integral (pixman_kernel_t kernel1, double x1,
  pixman_kernel_t kernel2, double scale, double x2,
  double width)
 {
+if (kernel1 == PIXMAN_KERNEL_BOX && kernel2 == PIXMAN_KERNEL_BOX)
+{
+   return width;
+}
 /* The LINEAR filter is not differentiable at 0, so if the
  * integration interval crosses zero, break it into two
  * separate integrals.
  */
-if (kernel1 == PIXMAN_KERNEL_LINEAR && x1 < 0 && x1 + width > 0)
+else if (kernel1 == PIXMAN_KERNEL_LINEAR && x1 < 0 && x1 + width > 0)
 {
return
integral (kernel1, x1, kernel2, scale, x2, - x1) +
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH 07/15] demos/scale: Added pulldown to choose PIXMAN_FILTER_* value

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This is very useful for comparing the results of SEPARABLE_CONVOLUTION
with BILINEAR and NEAREST.

v14: Removed good/best items
v15: Skip filter generation so gnuplot output continues showing previous value

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 demos/scale.c  | 41 ++---
 demos/scale.ui | 40 ++--
 2 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index 0995ad0..0c6b533 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -127,6 +127,13 @@ typedef struct
 intvalue;
 } named_int_t;
 
+static const named_int_t filter_types[] =
+{
+{ "Separable", PIXMAN_FILTER_SEPARABLE_CONVOLUTION },
+{ "Nearest",   PIXMAN_FILTER_NEAREST },
+{ "Bilinear",  PIXMAN_FILTER_BILINEAR },
+};
+
 static const named_int_t filters[] =
 {
 { "Box",   PIXMAN_KERNEL_BOX },
@@ -249,18 +256,29 @@ rescale (GtkWidget *may_be_null, app_t *app)
 pixman_transform_from_pixman_f_transform (, );
 pixman_image_set_transform (app->original, );
 
-params = pixman_filter_create_separable_convolution (
-_params,
-sx * 65536.0 + 0.5,
-   sy * 65536.0 + 0.5,
-   get_value (app, filters, "reconstruct_x_combo_box"),
-   get_value (app, filters, "reconstruct_y_combo_box"),
-   get_value (app, filters, "sample_x_combo_box"),
-   get_value (app, filters, "sample_y_combo_box"),
-   gtk_adjustment_get_value (app->subsample_adjustment),
-   gtk_adjustment_get_value (app->subsample_adjustment));
+if (get_value (app, filter_types, "filter_combo_box") ==
+   PIXMAN_FILTER_SEPARABLE_CONVOLUTION)
+{
+   params = pixman_filter_create_separable_convolution (
+   _params,
+   sx * 65536.0 + 0.5,
+   sy * 65536.0 + 0.5,
+   get_value (app, filters, "reconstruct_x_combo_box"),
+   get_value (app, filters, "reconstruct_y_combo_box"),
+   get_value (app, filters, "sample_x_combo_box"),
+   get_value (app, filters, "sample_y_combo_box"),
+   gtk_adjustment_get_value (app->subsample_adjustment),
+   gtk_adjustment_get_value (app->subsample_adjustment));
+}
+else
+{
+   params = 0;
+   n_params = 0;
+}
 
-pixman_image_set_filter (app->original, 
PIXMAN_FILTER_SEPARABLE_CONVOLUTION, params, n_params);
+pixman_image_set_filter (app->original,
+   get_value (app, filter_types, "filter_combo_box"),
+   params, n_params);
 
 pixman_image_set_repeat (
 app->original, get_value (app, repeats, "repeat_combo_box"));
@@ -402,6 +420,7 @@ app_new (pixman_image_t *original)
 widget = get_widget (app, "drawing_area");
 g_signal_connect (widget, "expose_event", G_CALLBACK (on_expose), app);
 
+set_up_combo_box (app, "filter_combo_box", G_N_ELEMENTS (filter_types), 
filter_types);
 set_up_filter_box (app, "reconstruct_x_combo_box");
 set_up_filter_box (app, "reconstruct_y_combo_box");
 set_up_filter_box (app, "sample_x_combo_box");
diff --git a/demos/scale.ui b/demos/scale.ui
index d498d26..7e999c1 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -192,12 +192,23 @@
 8
 6
 
+  
+True
+1
+bFilter:/b
+True
+  
+
+
   
 True
 1
 bReconstruct X:/b
 True
   
+  
+1
+  
 
 
   
@@ -207,7 +218,7 @@
 True
   
   
-1
+2
   
 
 
@@ -218,7 +229,7 @@
 True
   
   
-2
+3
   
 
 
@@ -229,7 +240,7 @@
 True
   
   
-3
+4
   
 
 
@@ -240,7 +251,7 @@
 True
   
   
- 

[Pixman] [PATCH 08/15] pixman-image: Added enable-gnuplot config to view filters in gnuplot

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

If enable-gnuplot is configured, then you can pipe the output of a
pixman-using program to gnuplot and get a continuously-updated plot of
the horizontal filter. This works well with demos/scale to test the
filter generation.

The plot is all the different subposition filters shuffled
together. This is misleading in a few cases:

  IMPULSE.BOX - goes up and down as the subfilters have different
numbers of non-zero samples

  IMPULSE.TRIANGLE - somewhat crooked for the same reason

  1-wide filters - looks triangular, but a 1-wide box would be more
   accurate

Changes by Søren: Rewrote the pixman-filter.c part to
 - make it generate correct coordinates
 - add a comment on how coordinates are generated
 - in rounding.txt, add a ceil() variant of the first-sample
   formula
 - make the gnuplot output slightly prettier

v7: First time this ability was included

v8: Use config option
Moved code to the filter generator
Modified scale demo to not call filter generator a second time.

v10: Only print if successful generation of plots
 Use #ifdef, not #if

v11: small whitespace fixes
v12: output range from -width/2 to width/2 and include y==0, to avoid 
misleading plots
 for subsample_bits==0 and for box filters which may have no small values.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 configure.ac   |  13 ++
 pixman/pixman-filter.c | 117 +
 pixman/rounding.txt|   1 +
 3 files changed, 131 insertions(+)

diff --git a/configure.ac b/configure.ac
index 6b2134e..e833e45 100644
--- a/configure.ac
+++ b/configure.ac
@@ -834,6 +834,19 @@ fi
 AC_SUBST(PIXMAN_TIMERS)
 
 dnl ===
+dnl gnuplot
+
+AC_ARG_ENABLE(gnuplot,
+   [AC_HELP_STRING([--enable-gnuplot],
+   [enable output of filters that can be piped to gnuplot 
[default=no]])],
+   [enable_gnuplot=$enableval], [enable_gnuplot=no])
+
+if test $enable_gnuplot = yes ; then
+   AC_DEFINE(PIXMAN_GNUPLOT, 1, [enable output that can be piped to gnuplot])
+fi
+AC_SUBST(PIXMAN_GNUPLOT)
+
+dnl ===
 dnl GTK+
 
 AC_ARG_ENABLE(gtk,
diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index b2bf53f..aa7bb80 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -297,6 +297,119 @@ create_1d_filter (int *width,
 return params;
 }
 
+#ifdef PIXMAN_GNUPLOT
+
+/* If enable-gnuplot is configured, then you can pipe the output of a
+ * pixman-using program to gnuplot and get a continuously-updated plot
+ * of the horizontal filter. This works well with demos/scale to test
+ * the filter generation.
+ *
+ * The plot is all the different subposition filters shuffled
+ * together. This is misleading in a few cases:
+ *
+ *  IMPULSE.BOX - goes up and down as the subfilters have different
+ *   numbers of non-zero samples
+ *  IMPULSE.TRIANGLE - somewhat crooked for the same reason
+ *  1-wide filters - looks triangular, but a 1-wide box would be more
+ *  accurate
+ */
+static void
+gnuplot_filter (int width, int n_phases, const pixman_fixed_t* p)
+{
+double step;
+int i, j;
+int first;
+
+step = 1.0 / n_phases;
+
+printf ("set style line 1 lc rgb '#0060ad' lt 1 lw 0.5 pt 7 pi 1 ps 
0.5\n");
+printf ("plot [x=%g:%g] '-' with linespoints ls 1\n", -width*0.5, 
width*0.5);
+/* Print a point at the origin so that y==0 line is included: */
+printf ("0 0\n\n");
+
+/* The position of the first sample of the phase corresponding to
+ * frac is given by:
+ * 
+ * ceil (frac - width / 2.0 - 0.5) + 0.5 - frac
+ * 
+ * We have to find the frac that minimizes this expression.
+ * 
+ * For odd widths, we have
+ * 
+ * ceil (frac - width / 2.0 - 0.5) + 0.5 - frac
+ *   = ceil (frac) + K - frac
+ *   = 1 + K - frac
+ * 
+ * for some K, so this is minimized when frac is maximized and
+ * strictly growing with frac. So for odd widths, we can simply
+ * start at the last phase and go backwards.
+ * 
+ * For even widths, we have
+ * 
+ * ceil (frac - width / 2.0 - 0.5) + 0.5 - frac
+ *   = ceil (frac - 0.5) + K - frac
+ * 
+ * The graph for this function (ignoring K) looks like this:
+ * 
+ *0.5
+ *   ||\ 
+ *   || \ 
+ *   ||  \ 
+ * 0 ||   \ 
+ *   |\   |
+ *   | \  |
+ *   |  \ |
+ *  -0.5 |   \|
+ *   -
+ *   00.5   1
+ * 
+ * So in this case we need to start with the phase whose frac is
+ * less than, but as close as possible to 0.5, then go backwards
+ * until we hit the first phase, then wrap around to the

[Pixman] [PATCH 03/15] More general BILINEAR=>NEAREST reduction

[spitzak]

From: Søren Sandmann Pedersen <soren.sandm...@gmail.com>

Generalize and simplify the code that reduces BILINEAR to NEAREST so
that the reduction happens for all affine transformations where
t00...t12 are integers and (t00 + t01) and (t10 + t11) are both
odd. This is a sufficient condition for the resulting transformed
coordinates to be exactly at the center of a pixel so that BILINEAR
becomes identical to NEAREST.

V2: Address some comments by Bill Spitzak

Signed-off-by: Søren Sandmann <soren.sandm...@gmail.com>
Reviewed-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-image.c | 66 +--
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 1ff1a49..681864e 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -335,37 +335,47 @@ compute_image_info (pixman_image_t *image)
{
flags |= FAST_PATH_NEAREST_FILTER;
}
-   else if (
-   /* affine and integer translation components in matrix ... */
-   ((flags & FAST_PATH_AFFINE_TRANSFORM) &&
-!pixman_fixed_frac (image->common.transform->matrix[0][2] |
-image->common.transform->matrix[1][2])) &&
-   (
-   /* ... combined with a simple rotation */
-   (flags & (FAST_PATH_ROTATE_90_TRANSFORM |
- FAST_PATH_ROTATE_180_TRANSFORM |
- FAST_PATH_ROTATE_270_TRANSFORM)) ||
-   /* ... or combined with a simple non-rotated translation */
-   (image->common.transform->matrix[0][0] == pixman_fixed_1 &&
-image->common.transform->matrix[1][1] == pixman_fixed_1 &&
-image->common.transform->matrix[0][1] == 0 &&
-image->common.transform->matrix[1][0] == 0)
-   )
-   )
+   else if (flags & FAST_PATH_AFFINE_TRANSFORM)
{
-   /* FIXME: there are some affine-test failures, showing that
-* handling of BILINEAR and NEAREST filter is not quite
-* equivalent when getting close to 32K for the translation
-* components of the matrix. That's likely some bug, but for
-* now just skip BILINEAR->NEAREST optimization in this case.
+   /* Suppose the transform is
+*
+*[ t00, t01, t02 ]
+*[ t10, t11, t12 ]
+*[   0,   0,   1 ]
+*
+* and the destination coordinates are (n + 0.5, m + 0.5). Then
+* the transformed x coordinate is:
+*
+* tx = t00 * (n + 0.5) + t01 * (m + 0.5) + t02
+*= t00 * n + t01 * m + t02 + (t00 + t01) * 0.5
+*
+* which implies that if t00, t01 and t02 are all integers
+* and (t00 + t01) is odd, then tx will be an integer plus 0.5,
+* which means a BILINEAR filter will reduce to NEAREST. The same
+* applies in the y direction
 */
-   pixman_fixed_t magic_limit = pixman_int_to_fixed (3);
-   if (image->common.transform->matrix[0][2] <= magic_limit  &&
-   image->common.transform->matrix[1][2] <= magic_limit  &&
-   image->common.transform->matrix[0][2] >= -magic_limit &&
-   image->common.transform->matrix[1][2] >= -magic_limit)
+   pixman_fixed_t (*t)[3] = image->common.transform->matrix;
+
+   if ((pixman_fixed_frac (
+t[0][0] | t[0][1] | t[0][2] |
+t[1][0] | t[1][1] | t[1][2]) == 0) &&
+   (pixman_fixed_to_int (
+   (t[0][0] + t[0][1]) & (t[1][0] + t[1][1])) % 2) == 1)
{
-   flags |= FAST_PATH_NEAREST_FILTER;
+   /* FIXME: there are some affine-test failures, showing that
+* handling of BILINEAR and NEAREST filter is not quite
+* equivalent when getting close to 32K for the translation
+* components of the matrix. That's likely some bug, but for
+* now just skip BILINEAR->NEAREST optimization in this case.
+*/
+   pixman_fixed_t magic_limit = pixman_int_to_fixed (3);
+   if (image->common.transform->matrix[0][2] <= magic_limit  &&
+   image->common.transform->matrix[1][2] <= magic_limit  &&
+   image->common.transform->matrix[0][2] >= -magic_limit &&
+   image->common.transform->matrix[1][2] >= -magic_limit)
+   {
+   flags |= FAST_PATH_NEAREST_FILTER;
+   }
}
}
break;
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 02/14] Add new test of filter reduction from BILINEAR to NEAREST

[Bill Spitzak]

On Fri, Apr 29, 2016 at 4:55 AM, Oded Gabbay  wrote:

> On Tue, Apr 12, 2016 at 5:36 AM, Søren Sandmann Pedersen
>  wrote:
> > This new test tests a bunch of bilinear downscalings, where many have
> > a transformation such that the BILINEAR filter can be reduced to
> > NEAREST (and many don't).
> >
> > A CRC32 is computed for all the resulting images and compared to a
> > known-good value for both 4-bit and 7-bit interpolation.
> >
> > V2: Remove leftover comment, some minor formatting fixes, use a
> > timestamp as the PRNG seed.
> >
> > Signed-off-by: Søren Sandmann 
> > ---
> >  test/Makefile.sources|   1 +
> >  test/filter-reduction-test.c | 112
> +++
> >  2 files changed, 113 insertions(+)
> >  create mode 100644 test/filter-reduction-test.c
> >
> > diff --git a/test/Makefile.sources b/test/Makefile.sources
> > index 5d55e67..0a56231 100644
> > --- a/test/Makefile.sources
> > +++ b/test/Makefile.sources
> > @@ -21,6 +21,7 @@ TESTPROGRAMS =  \
> > gradient-crash-test   \
> > pixel-test\
> > matrix-test   \
> > +   filter-reduction-test \
> > composite-traps-test  \
> > region-contains-test  \
> > glyph-test\
> > diff --git a/test/filter-reduction-test.c b/test/filter-reduction-test.c
> > new file mode 100644
> > index 000..705fa4b
> > --- /dev/null
> > +++ b/test/filter-reduction-test.c
> > @@ -0,0 +1,112 @@
> > +#include 
> > +#include 
> > +#include "utils.h"
> > +
> > +static const pixman_fixed_t entries[] =
> > +{
> > +pixman_double_to_fixed (-1.0),
> > +pixman_double_to_fixed (-0.5),
> > +pixman_double_to_fixed (-1/3.0),
> > +pixman_double_to_fixed (0.0),
> > +pixman_double_to_fixed (0.5),
> > +pixman_double_to_fixed (1.0),
> > +pixman_double_to_fixed (1.5),
> > +pixman_double_to_fixed (2.0),
> > +pixman_double_to_fixed (3.0),
> > +};
> > +
> > +#define SIZE 12
> > +
> > +static uint32_t
> > +test_scale (const pixman_transform_t *xform, uint32_t crc)
> > +{
> > +uint32_t *srcbuf, *dstbuf;
> > +pixman_image_t *src, *dest;
> > +
> > +srcbuf = malloc (SIZE * SIZE * 4);
> I admit I didn't look at other tests, but it really caught my eye we
> don't check for memory allocation failure here.
>
> > +prng_randmemset (srcbuf, SIZE * SIZE * 4, 0);
> > +src = pixman_image_create_bits (
> > +   PIXMAN_a8r8g8b8, SIZE, SIZE, srcbuf, SIZE * 4);
> > +
> > +dstbuf = malloc (SIZE * SIZE * 4);
> same comment
>

I think it is ok to not look for that in unit tests. They will either crash
or fail.


>
> > +prng_randmemset (dstbuf, SIZE * SIZE * 4, 0);
> > +dest = pixman_image_create_bits (
> > +   PIXMAN_a8r8g8b8, SIZE, SIZE, dstbuf, SIZE * 4);
> > +
> > +pixman_image_set_transform (src, xform);
> > +pixman_image_set_repeat (src, PIXMAN_REPEAT_NORMAL);
> > +pixman_image_set_filter (src, PIXMAN_FILTER_BILINEAR, NULL, 0);
> > +
> > +image_endian_swap (src);
> > +image_endian_swap (dest);
> > +
> > +pixman_image_composite (PIXMAN_OP_SRC,
> > +   src, NULL, dest,
> > +   0, 0, 0, 0, 0, 0,
> > +   SIZE, SIZE);
> > +
> > +crc = compute_crc32_for_image (crc, dest);
> > +
> > +pixman_image_unref (src);
> > +pixman_image_unref (dest);
> > +
> > +free (srcbuf);
> > +free (dstbuf);
> > +
> > +return crc;
> > +}
> > +
> > +#if BILINEAR_INTERPOLATION_BITS == 7
> > +#define CHECKSUM 0x02169677
> > +#elif BILINEAR_INTERPOLATION_BITS == 4
> > +#define CHECKSUM 0xE44B29AC
> > +#else
> > +#define CHECKSUM 0x
> > +#endif
> > +
> > +int
> > +main (int argc, const char *argv[])
> > +{
> > +const pixman_fixed_t *end = entries + ARRAY_LENGTH (entries);
> > +const pixman_fixed_t *t0, *t1, *t2, *t3, *t4, *t5;
> > +uint32_t crc = 0;
> > +
> > +prng_srand (0x56EA1DBD);
> > +
> > +for (t0 = entries; t0 < end; ++t0)
> > +{
> > +   for (t1 = entries; t1 < end; ++t1)
> > +   {
> > +   for (t2 = entries; t2 < end; ++t2)
> > +   {
> > +   for (t3 = entries; t3 < end; ++t3)
> > +   {
> > +   for (t4 = entries; t4 < end; ++t4)
> > +   {
> > +   for (t5 = entries; t5 < end; ++t5)
> > +   {
> > +   pixman_transform_t xform = {
> > +   { { *t0, *t1, *t2 },
> > + { *t3, *t4, *t5 },
> > + { 0, 0, pixman_fixed_1 } }
> > +   };
> > +
> > +   crc = test_scale (, crc);
> > +   }
> > +   }
> > +   }
> > +   }
> > +

Re: [Pixman] [RFC 1/2] Remove seemingly unneeded comparison(s)

[Bill Spitzak]

If the comparison fails, the returned values are modulus fixed_16_16,
rather than clamped. I think that gives a lot less possibilities for the
calling code to recover from or simulate the results of the out-of-range
value.


On Fri, Apr 29, 2016 at 6:33 AM, Emil Velikov <emil.l.veli...@gmail.com>
wrote:

> On 29 April 2016 at 11:35, Pekka Paalanen <ppaala...@gmail.com> wrote:
> > On Fri, 29 Apr 2016 10:15:37 +0100
> > Emil Velikov <emil.l.veli...@gmail.com> wrote:
> >
> >> On 27 April 2016 at 18:46, Bill Spitzak <spit...@gmail.com> wrote:
> >> > On Wed, Apr 27, 2016 at 2:03 AM, Pekka Paalanen <ppaala...@gmail.com>
> wrote:
> >> >>
> >> >> On Wed, 27 Apr 2016 09:56:44 +0100
> >> >> Emil Velikov <emil.l.veli...@gmail.com> wrote:
> >> >>
> >> >> > On 26 April 2016 at 19:12, Bill Spitzak <spit...@gmail.com> wrote:
> >> >> > > The old code is comparing pixman_fixed_48_16_t values to
> >> >> > > pixman_fixed_16_16_t values, thus it is checking for truncation
> of
> >> >> > > overflow
> >> >> > > values.
> >> >> > >
> >> >> > Indeed it does. I'll grep more before asking silly questions ;-)
> >> >> >
> >> >> > > It would probably be better to clamp these overflowed values,
> like
> >> >> > > pixman_transform_point_31_16 is doing to clamp to the
> >> >> > > pixman_fixed_48_16
> >> >> > > result. Right now the result is an odd mix of clamping and
> modulus. A
> >> >> > > rewrite to go directly to clamped pixman_fixed_16_16 values
> would be
> >> >> > > even
> >> >> > > better.
> >> >> > >
> >> >> > Sounds like a plan. Sadly I doubt I'll get to it any time soon.
> >> >>
> >> >> Wasn't the point of the boolean return from these functions to tell
> the
> >> >> caller to drop what it is doing because it cannot be done properly?
> >> >
> >> >
> >> > Dropping a fill is a lot worse than trying to simulate it using the
> clamped
> >> > path. It will produce a wrong result if one of the edges connected to
> a
> >> > clamped point passes through the clip, but this often does not
> happen, and
> >> > the transition to the wrong result is gradual as the point moves
> outside the
> >> > clamped region.
> >> >
> >> > More importantly the caller cannot do anything with the return values
> right
> >> > now, as they are modulus MAX_16_16+1. Even the direction they are in
> is
> >> > lost.
> >> >
> >> I think that keeping the user provided memory as-is when the function
> >> does not succeed is a good idea.
> >> Afaics currently the contents get overwritten regardless of the result.
> >>
> >> This is what you guys were on about, right ? Or perhaps you're
> >> thinking about spinning v2 of the function with different
> >> signature/behaviour ?
> >
> > Hi Emil,
> >
> > I think the conclusion was that the comparisons are not useless, and
> > this patch should be dropped. You noted it yourself that this patch
> > causes a regression in the test suite.
> >
> Fully agree on both points. Just trying to understand what you and
> Bill are talking about and suggest that if one changes things, would
> be nice to avoid "feeding garbage" back to the user [on error].
>
> Thanks
> Emil
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [RFC 1/2] Remove seemingly unneeded comparison(s)

[Bill Spitzak]

On Wed, Apr 27, 2016 at 2:03 AM, Pekka Paalanen <ppaala...@gmail.com> wrote:

> On Wed, 27 Apr 2016 09:56:44 +0100
> Emil Velikov <emil.l.veli...@gmail.com> wrote:
>
> > On 26 April 2016 at 19:12, Bill Spitzak <spit...@gmail.com> wrote:
> > > The old code is comparing pixman_fixed_48_16_t values to
> > > pixman_fixed_16_16_t values, thus it is checking for truncation of
> overflow
> > > values.
> > >
> > Indeed it does. I'll grep more before asking silly questions ;-)
> >
> > > It would probably be better to clamp these overflowed values, like
> > > pixman_transform_point_31_16 is doing to clamp to the
> pixman_fixed_48_16
> > > result. Right now the result is an odd mix of clamping and modulus. A
> > > rewrite to go directly to clamped pixman_fixed_16_16 values would be
> even
> > > better.
> > >
> > Sounds like a plan. Sadly I doubt I'll get to it any time soon.
>
> Wasn't the point of the boolean return from these functions to tell the
> caller to drop what it is doing because it cannot be done properly?
>

Dropping a fill is a lot worse than trying to simulate it using the clamped
path. It will produce a wrong result if one of the edges connected to a
clamped point passes through the clip, but this often does not happen, and
the transition to the wrong result is gradual as the point moves outside
the clamped region.

More importantly the caller cannot do anything with the return values right
now, as they are modulus MAX_16_16+1. Even the direction they are in is
lost.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [RFC 1/2] Remove seemingly unneeded comparison(s)

[Bill Spitzak]

The old code is comparing pixman_fixed_48_16_t values to
pixman_fixed_16_16_t values, thus it is checking for truncation of overflow
values.

It would probably be better to clamp these overflowed values, like
pixman_transform_point_31_16 is doing to clamp to the pixman_fixed_48_16
result. Right now the result is an odd mix of clamping and modulus. A
rewrite to go directly to clamped pixman_fixed_16_16 values would be even
better.



On Tue, Apr 26, 2016 at 10:59 AM, Petr Kobalíček 
wrote:

> Doesn't this check for NaNs?
>
> On Sun, Apr 24, 2016 at 8:22 PM, Emil Velikov 
> wrote:
>
>> With commit ed39992564b "Use pixman_transform_point_31_16() from
>> pixman_transform_point()" we added some strange hunks.
>>
>> Namely: we copy the data from the internal storage to the user vector
>> only to compare them immediately after.
>>
>> Cc: Siarhei Siamashka 
>> ---
>>
>> Siarhei, what is the intent with the original commit ? Any ideas why
>> things crash ?
>>
>> Seemingly this can be dropped/replaced with TRUE, yet it causes one of
>> the tests () to segfault in the optimised SSE2 codepath -
>> scaled_bilinear_scanline_sse2___SRC.
>>
>> BILINEAR_INTERPOLATE_ONE_PIXEL -> BILINEAR_INTERPOLATE_ONE_PIXEL_HELPER
>>
>> Regards,
>> Emil
>> ---
>>
>>  pixman/pixman-matrix.c | 8 ++--
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/pixman/pixman-matrix.c b/pixman/pixman-matrix.c
>> index 65b3d32..117015b 100644
>> --- a/pixman/pixman-matrix.c
>> +++ b/pixman/pixman-matrix.c
>> @@ -393,9 +393,7 @@ pixman_transform_point_3d (const struct
>> pixman_transform *transform,
>>  vector->vector[1] = tmp.v[1];
>>  vector->vector[2] = tmp.v[2];
>>
>> -return vector->vector[0] == tmp.v[0] &&
>> -   vector->vector[1] == tmp.v[1] &&
>> -   vector->vector[2] == tmp.v[2];
>> +return TRUE;
>>  }
>>
>>  PIXMAN_EXPORT pixman_bool_t
>> @@ -414,9 +412,7 @@ pixman_transform_point (const struct pixman_transform
>> *transform,
>>  vector->vector[1] = tmp.v[1];
>>  vector->vector[2] = tmp.v[2];
>>
>> -return vector->vector[0] == tmp.v[0] &&
>> -   vector->vector[1] == tmp.v[1] &&
>> -   vector->vector[2] == tmp.v[2];
>> +return TRUE;
>>  }
>>
>>  PIXMAN_EXPORT pixman_bool_t
>> --
>> 2.8.0
>>
>> ___
>> Pixman mailing list
>> Pixman@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/pixman
>>
>
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 12/14] pixman-filter: Fix several issues related to normalization

[Bill Spitzak]

The attached patch changes the normalization to the version I was
suggesting. I added some print statements and discovered that the "residual
error" was always zero, after some analysis I figured out that this version
will always produce a set of values that sum to pixman_fixed_1 (unless the
filter is unrealistically wide, far larger than 2^20, to get double
precision errors to sum large enough to change this). So I removed the
new_total and residual error and left a comment in there.

I am having trouble with IMPULSE.BOX producing values that are all equal or
very close together. Do you know of a way to get gnuplot to include y==0 as
otherwise the plots are very misleading unless you read the axis values.
Also this produces "Warning: empty y range [1:1], adjusting to [0.99:1.01]"
messages, possibly these are not an actual error.


patch
Description: Binary data
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 07/14] pixman-image: Added enable-gnuplot config to view filters in gnuplot

[Bill Spitzak]

On 04/12/2016 01:55 PM, Søren Sandmann wrote:


This series is available as a git repository here:

https://cgit.freedesktop.org/~sandmann/pixman/log/?h=spitzak-for-master


Not having much luck with that url:

fatal: repository 
'https://cgit.freedesktop.org/~sandmann/pixman/log/?h=spitzak-for-master/' 
not found


___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 13/14] pixman-filter: Nested polynomial for cubic

[Bill Spitzak]

Because the integral function is relying on the range being clamped to the
non-zero portion for impulse filters and for box, and range checks are
missing for some of the other filter functions, I would be tempted to
remove the range check from here, and also from the impulse function (it
would return 1 for all x). Oded did not like that idea but I wonder if you
have any opinion.

On Mon, Apr 11, 2016 at 7:36 PM, Søren Sandmann Pedersen <
soren.sandm...@gmail.com> wrote:

> From: Bill Spitzak <spit...@gmail.com>
>
> v11: Restored range checks
>
> Signed-off-by: Bill Spitzak <spit...@gmail.com>
> Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
> ---
>  pixman/pixman-filter.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index 4abd05f..db4ab6e 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -109,14 +109,16 @@ general_cubic (double x, double B, double C)
>
>  if (ax < 1)
>  {
> -   return ((12 - 9 * B - 6 * C) * ax * ax * ax +
> -   (-18 + 12 * B + 6 * C) * ax * ax + (6 - 2 * B)) / 6;
> +   return (((12 - 9 * B - 6 * C) * ax +
> +(-18 + 12 * B + 6 * C)) * ax * ax +
> +   (6 - 2 * B)) / 6;
>  }
> -else if (ax >= 1 && ax < 2)
> +else if (ax < 2)
>  {
> -   return ((-B - 6 * C) * ax * ax * ax +
> -   (6 * B + 30 * C) * ax * ax + (-12 * B - 48 * C) *
> -   ax + (8 * B + 24 * C)) / 6;
> +   return -B - 6 * C) * ax +
> + (6 * B + 30 * C)) * ax +
> +(-12 * B - 48 * C)) * ax +
> +   (8 * B + 24 * C)) / 6;
>  }
>  else
>  {
> --
> 1.7.11.7
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 12/14] pixman-filter: Fix several issues related to normalization

[Bill Spitzak]

Excellent idea to put the error diffusion into the division. Just a bit of
cleanup and changes for some suspected bugs (that are probably invisible
but might as well get them):

On Mon, Apr 11, 2016 at 7:36 PM, Søren Sandmann Pedersen <
soren.sandm...@gmail.com> wrote:

> There are a few bugs in the current normalization code
>
> (1) The normalization is based on the sum of the *floating point*
> values generated by integral(). But in order to get the sum to be
> close to pixman_fixed_1, the sum of the rounded fixed point values
> should be used.
>
> (2) The multiplications in the normalization loops often round the
> same way, so the residual error can fairly large.
>
> (3) The residual error is added to the sample located at index
> (width - width / 2), which is not the midpoint for odd widths (and
> for width 1 is in fact outside the array).
>
> This patch fixes these issues by (1) using the sum of the fixed point
> values as the total to divide by, (2) doing error diffusion in the
> normalization loop, and (3) putting any residual error (which is now
> guaranteed to be less than pixman_fixed_e) at the first sample, which
> is the only one that didn't get any error diffused into it.
>
> Signed-off-by: Søren Sandmann 
> ---
>  pixman/pixman-filter.c | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index 32aaa9a..4abd05f 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -247,7 +247,7 @@ create_1d_filter (int  width,
>  double frac = step / 2.0 + i * step;
> pixman_fixed_t new_total;
>  int x, x1, x2;
> -   double total;
> +   double total, e;
>

I think total should be a pixman_fixed_t as that is what it is summing.


> /* Sample convolution of reconstruction and sampling
>  * filter. See rounding.txt regarding the rounding
> @@ -278,24 +278,31 @@ create_1d_filter (int  width,
>   ihigh - ilow);
> }
>
> -   total += c;
> -*p++ = (pixman_fixed_t)(c * 65536.0 + 0.5);
> +*p = (pixman_fixed_t)floor (c * 65536.0 + 0.5);
>

floor probably is a good idea to make negative filter entries work. Is
there a macro that does this conversion?

+   total += *p;
> +   p++;
>  }
>

Might want to skip the normalize if total==pixman_fixed_1, though perhaps a
test to see how often that happens would be informative.


>
> -   /* Normalize */
> +   /* Normalize, with error diffusion */
> p -= width;
> -total = 1 / total;
> +total = 65536.0 / total;
>

Remove the division so total can be a pixman_fixed_t.


>  new_total = 0;
> +   e = 0.0;
>

Change this to 0.5


> for (x = x1; x < x2; ++x)
> {
> -   pixman_fixed_t t = (*p) * total + 0.5;
> +   double v = (*p) * total + e;


Change to this so total can be a pixman_fixed_t:
   double v = (*p) * (double)(pixman_fixed_1) / total + e;

+   pixman_fixed_t t = floor (v + 0.5);
>

Change this to just floor(v). The 0.5 factor is incorporated into e. This
version is in effect adding .5 to all the samples, though that is
compensated for somewhat by floor in effect subtracting .5, but imagine
what happens if e > 0.5. This bug was in Nuke for many years btw.


> +   e = v - t;
> new_total += t;
> *p++ = t;
> }
>
> -   if (new_total != pixman_fixed_1)
> -   *(p - width / 2) += (pixman_fixed_1 - new_total);
> +   /* pixman_fixed_e's worth of error may remain; put it
> +* at the first sample, since that is the only one that
> +* hasn't had any error diffused into it.
> +*/
> +   *(p - width) += pixman_fixed_1 - new_total;
>

I'm not absolutely convinced that the first sample is best. Dumping this on
the central pixel may be better because that value is larger so the
relative change is smaller:

*(p - (width+1)/2) += pixman_fixed_1 - new_total;


>  }
>  }
>
> --
> 1.7.11.7
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 07/14] pixman-image: Added enable-gnuplot config to view filters in gnuplot

[Bill Spitzak]

On Tue, Apr 12, 2016 at 1:55 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

>
>   1-wide filters - looks triangular, but a 1-wide box would be more
>>>accurate
>>>
>>
>> Because you are not plotting the two dummy points at (0,±width/2), a
>> 1-wide filter is actually just a single point.
>>
>> You may be right that leaving the dummy points off the plot may make it
>> easier to figure out what is going on.
>>
>
> Okay, I will update the comment.
>
> I don't like to make up fake data points, but maybe I should add
> [x=-width/2:width/2] to the gnuplot command line.
>

Yes that sounds like a very good idea.


>
> diff --git a/pixman/rounding.txt b/pixman/rounding.txt
>>
>>> index b52b084..1c00019 100644
>>> --- a/pixman/rounding.txt
>>> +++ b/pixman/rounding.txt
>>> @@ -160,6 +160,7 @@ which means the contents of the matrix corresponding
>>> to (frac) should
>>>  contain width samplings of the function, with the first sample at:
>>>
>>> floor (frac - (width - 1) / 2.0 - e) + 0.5 - frac
>>> + = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac
>>>
>>
>> Unfortunately this produces numbers that don't agree with the filter
>> generator or filtering code.
>>
>> For a width==4 filter with n_phases==1, the generator seems to produce
>> values at -1, 0, 1, 2, so the first sample is at -1. It also appears the
>> filtering sampler is using the same rule, otherwise these filters would
>> produce an obvious shift in the image.
>>
>
> If you add printf's on top of this series
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index 176dfae..4a8743e 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -129,12 +129,15 @@ general_cubic (double x, double B, double C)
>  static double
>  cubic_kernel (double x)
>  {
> +  double v = general_cubic (x, 1/3.0, 1/3.0);
> +
> +  printf ("cubic(%f) => %f\n", x, v);
>  /* This is the Mitchell-Netravali filter.
>   *
>   * (0.0, 0.5) would give us the Catmull-Rom spline,
>   * but that one seems to be indistinguishable from Lanczos2.
>   */
> -return general_cubic (x, 1/3.0, 1/3.0);
> +  return v;
>  }
>
> and run scale with Reconstruction=Cubic, Sample=Impulse, and Subsample=0,
> it prints
>
> cubic(-2.00) => 0.00
> cubic(-1.00) => 0.06
> cubic(0.00) => 0.89
> cubic(1.00) => 0.06
>
> so the filter generator *does* generate values at [ -2, -1, 0, 1 ]. And as
> mentioned, the sampling code, if given the value 17.5 will convolve with
> the pixels at 15.5, 16.5, 17.5, 18.5, so I'm pretty sure this matrix is
> correct.
>
> (I suspect your changes to the integral() arguments caused it to generate
> different values, so you should check without them included.
>

Looks like you are correct, toggling between this and nearest shows that
the filtering code is reading the arrays this way.


>
> This series is available as a git repository here:
>
>
> https://cgit.freedesktop.org/~sandmann/pixman/log/?h=spitzak-for-master
> )
>
>
> Søren
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 07/14] pixman-image: Added enable-gnuplot config to view filters in gnuplot

[Bill Spitzak]

On Mon, Apr 11, 2016 at 7:36 PM, Søren Sandmann Pedersen <
soren.sandm...@gmail.com> wrote:

> From: Bill Spitzak <spit...@gmail.com>
>
> If enable-gnuplot is configured, then you can pipe the output of a
> pixman-using program to gnuplot and get a continuously-updated plot of
> the horizontal filter. This works well with demos/scale to test the
> filter generation.
>
> The plot is all the different subposition filters shuffled
> together. This is misleading in a few cases:
>
>   IMPULSE.BOX - goes up and down as the subfilters have different
> numbers of non-zero samples
>
>   IMPULSE.TRIANGLE - somewhat crooked for the same reason
>
>   1-wide filters - looks triangular, but a 1-wide box would be more
>accurate
>

Because you are not plotting the two dummy points at (0,±width/2), a 1-wide
filter is actually just a single point.

You may be right that leaving the dummy points off the plot may make it
easier to figure out what is going on.


> Changes by Søren: Rewrote the pixman-filter.c part to
>  - make it generate correct coordinates
>  - add a comment on how coordinates are generated
>  - in rounding.txt, add a ceil() variant of the first-sample
>formula
>  - make the gnuplot output slightly prettier
>
> v7: First time this ability was included
>
> v8: Use config option
> Moved code to the filter generator
> Modified scale demo to not call filter generator a second time.
>
> v10: Only print if successful generation of plots
>  Use #ifdef, not #if
>
> v11: small whitespace fixes
>
> Signed-off-by: Bill Spitzak <spit...@gmail.com>
> Signed-off-by: Søren Sandmann <soren.sandm...@gmail.com>
> ---
>  configure.ac   |  13 ++
>  pixman/pixman-filter.c | 115
> +
>  pixman/rounding.txt|   1 +
>  3 files changed, 129 insertions(+)
>
> diff --git a/configure.ac b/configure.ac
> index 6b2134e..e833e45 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -834,6 +834,19 @@ fi
>  AC_SUBST(PIXMAN_TIMERS)
>
>  dnl ===
> +dnl gnuplot
> +
> +AC_ARG_ENABLE(gnuplot,
> +   [AC_HELP_STRING([--enable-gnuplot],
> +   [enable output of filters that can be piped to gnuplot
> [default=no]])],
> +   [enable_gnuplot=$enableval], [enable_gnuplot=no])
> +
> +if test $enable_gnuplot = yes ; then
> +   AC_DEFINE(PIXMAN_GNUPLOT, 1, [enable output that can be piped to
> gnuplot])
> +fi
> +AC_SUBST(PIXMAN_GNUPLOT)
> +
> +dnl ===
>  dnl GTK+
>
>  AC_ARG_ENABLE(gtk,
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index b2bf53f..af46a43 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -297,6 +297,117 @@ create_1d_filter (int *width,
>  return params;
>  }
>
> +#ifdef PIXMAN_GNUPLOT
> +
> +/* If enable-gnuplot is configured, then you can pipe the output of a
> + * pixman-using program to gnuplot and get a continuously-updated plot
> + * of the horizontal filter. This works well with demos/scale to test
> + * the filter generation.
> + *
> + * The plot is all the different subposition filters shuffled
> + * together. This is misleading in a few cases:
> + *
> + *  IMPULSE.BOX - goes up and down as the subfilters have different
> + *   numbers of non-zero samples
> + *  IMPULSE.TRIANGLE - somewhat crooked for the same reason
> + *  1-wide filters - looks triangular, but a 1-wide box would be more
> + *  accurate
> + */
> +static void
> +gnuplot_filter (int width, int n_phases, const pixman_fixed_t* p)
> +{
> +double step;
> +int i, j;
> +int first;
> +
> +step = 1.0 / n_phases;
> +
> +printf ("set style line 1 lc rgb '#0060ad' lt 1 lw 0.5 pt 7 pi 1 ps
> 0.5\n");
> +printf ("plot '-' with linespoints ls 1\n");
> +
> +/* The position of the first sample of the phase corresponding to
> + * frac is given by:
> + *
> + * ceil (frac - width / 2.0 - 0.5) + 0.5 - frac
> + *
> + * We have to find the frac that minimizes this expression.
> + *
> + * For odd widths, we have
> + *
> + * ceil (frac - width / 2.0 - 0.5) + 0.5 - frac
> + *   = ceil (frac) + K - frac
> + *   = 1 + K - frac
> + *
> + * for some K, so this is minimized when frac is maximized and
> + * strictly growing with frac. So for odd widths, we can simply
> + * start at the last phase and go backwards.
> + *
> + * For even widths, we have
> + *
>

Re: [Pixman] [PATCH 03/14] More general BILINEAR=>NEAREST reduction

[Bill Spitzak]

Only minor comments on the large nearest patch at the bottom

On 04/11/2016 07:36 PM, Søren Sandmann Pedersen wrote:

Generalize and simplify the code that reduces BILINEAR to NEAREST so
that the reduction happens for all affine transformations where
t00...t12 are integers and (t00 + t01) and (t10 + t11) are both
odd. This is a sufficient condition for the resulting transformed
coordinates to be exactly at the center of a pixel so that BILINEAR
becomes identical to NEAREST.

V2: Address some comments by Bill Spitzak

Signed-off-by: Søren Sandmann <soren.sandm...@gmail.com>
---
  pixman/pixman-image.c | 66 +--
  1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 1ff1a49..681864e 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -335,37 +335,47 @@ compute_image_info (pixman_image_t *image)
{
flags |= FAST_PATH_NEAREST_FILTER;
}
-   else if (
-   /* affine and integer translation components in matrix ... */
-   ((flags & FAST_PATH_AFFINE_TRANSFORM) &&
-!pixman_fixed_frac (image->common.transform->matrix[0][2] |
-image->common.transform->matrix[1][2])) &&
-   (
-   /* ... combined with a simple rotation */
-   (flags & (FAST_PATH_ROTATE_90_TRANSFORM |
- FAST_PATH_ROTATE_180_TRANSFORM |
- FAST_PATH_ROTATE_270_TRANSFORM)) ||
-   /* ... or combined with a simple non-rotated translation */
-   (image->common.transform->matrix[0][0] == pixman_fixed_1 &&
-image->common.transform->matrix[1][1] == pixman_fixed_1 &&
-image->common.transform->matrix[0][1] == 0 &&
-image->common.transform->matrix[1][0] == 0)
-   )
-   )
+   else if (flags & FAST_PATH_AFFINE_TRANSFORM)
{
-   /* FIXME: there are some affine-test failures, showing that
-* handling of BILINEAR and NEAREST filter is not quite
-* equivalent when getting close to 32K for the translation
-* components of the matrix. That's likely some bug, but for
-* now just skip BILINEAR->NEAREST optimization in this case.
+   /* Suppose the transform is
+*
+*[ t00, t01, t02 ]
+*[ t10, t11, t12 ]
+*[   0,   0,   1 ]
+*
+* and the destination coordinates are (n + 0.5, m + 0.5). Then
+* the transformed x coordinate is:
+*
+* tx = t00 * (n + 0.5) + t01 * (m + 0.5) + t02
+*= t00 * n + t01 * m + t02 + (t00 + t01) * 0.5
+*
+* which implies that if t00, t01 and t02 are all integers
+* and (t00 + t01) is odd, then tx will be an integer plus 0.5,
+* which means a BILINEAR filter will reduce to NEAREST. The same
+* applies in the y direction
 */
-   pixman_fixed_t magic_limit = pixman_int_to_fixed (3);
-   if (image->common.transform->matrix[0][2] <= magic_limit  &&
-   image->common.transform->matrix[1][2] <= magic_limit  &&
-   image->common.transform->matrix[0][2] >= -magic_limit &&
-   image->common.transform->matrix[1][2] >= -magic_limit)
+   pixman_fixed_t (*t)[3] = image->common.transform->matrix;
+
+   if ((pixman_fixed_frac (
+t[0][0] | t[0][1] | t[0][2] |
+t[1][0] | t[1][1] | t[1][2]) == 0) &&
+   (pixman_fixed_to_int (
+   (t[0][0] + t[0][1]) & (t[1][0] + t[1][1])) % 2) == 1)
{
-   flags |= FAST_PATH_NEAREST_FILTER;
+   /* FIXME: there are some affine-test failures, showing that
+* handling of BILINEAR and NEAREST filter is not quite
+* equivalent when getting close to 32K for the translation
+* components of the matrix. That's likely some bug, but for
+* now just skip BILINEAR->NEAREST optimization in this case.
+*/


May be a good idea to point out that the bug is (probably) in BILINEAR, 
not in NEAREST. Also you think this may have been fixed so this could be 
removed, but that would be a different patch.



+   pixman_fixed_t magic_limit = pixman_int_to_fixed (3);
+   if (image->common.transform->matrix[0][2] <= magic_limit  &&
+   image->common.transform->matrix[1][2] <= magic_limit  &&
+   image->common.transform-&

Re: [Pixman] [PATCH 02/14] Add new test of filter reduction from BILINEAR to NEAREST

[Bill Spitzak]

Reviewed-by: Bill Spitzak <spit...@gmail.com>

On 04/11/2016 07:36 PM, Søren Sandmann Pedersen wrote:

This new test tests a bunch of bilinear downscalings, where many have
a transformation such that the BILINEAR filter can be reduced to
NEAREST (and many don't).

A CRC32 is computed for all the resulting images and compared to a
known-good value for both 4-bit and 7-bit interpolation.

V2: Remove leftover comment, some minor formatting fixes, use a
timestamp as the PRNG seed.

Signed-off-by: Søren Sandmann <soren.sandm...@gmail.com>
---
  test/Makefile.sources|   1 +
  test/filter-reduction-test.c | 112 +++
  2 files changed, 113 insertions(+)
  create mode 100644 test/filter-reduction-test.c

diff --git a/test/Makefile.sources b/test/Makefile.sources
index 5d55e67..0a56231 100644
--- a/test/Makefile.sources
+++ b/test/Makefile.sources
@@ -21,6 +21,7 @@ TESTPROGRAMS =  \
gradient-crash-test   \
pixel-test\
matrix-test   \
+   filter-reduction-test \
composite-traps-test  \
region-contains-test  \
glyph-test\
diff --git a/test/filter-reduction-test.c b/test/filter-reduction-test.c
new file mode 100644
index 000..705fa4b
--- /dev/null
+++ b/test/filter-reduction-test.c
@@ -0,0 +1,112 @@
+#include 
+#include 
+#include "utils.h"
+
+static const pixman_fixed_t entries[] =
+{
+pixman_double_to_fixed (-1.0),
+pixman_double_to_fixed (-0.5),
+pixman_double_to_fixed (-1/3.0),
+pixman_double_to_fixed (0.0),
+pixman_double_to_fixed (0.5),
+pixman_double_to_fixed (1.0),
+pixman_double_to_fixed (1.5),
+pixman_double_to_fixed (2.0),
+pixman_double_to_fixed (3.0),
+};
+
+#define SIZE 12
+
+static uint32_t
+test_scale (const pixman_transform_t *xform, uint32_t crc)
+{
+uint32_t *srcbuf, *dstbuf;
+pixman_image_t *src, *dest;
+
+srcbuf = malloc (SIZE * SIZE * 4);
+prng_randmemset (srcbuf, SIZE * SIZE * 4, 0);
+src = pixman_image_create_bits (
+   PIXMAN_a8r8g8b8, SIZE, SIZE, srcbuf, SIZE * 4);
+
+dstbuf = malloc (SIZE * SIZE * 4);
+prng_randmemset (dstbuf, SIZE * SIZE * 4, 0);
+dest = pixman_image_create_bits (
+   PIXMAN_a8r8g8b8, SIZE, SIZE, dstbuf, SIZE * 4);
+
+pixman_image_set_transform (src, xform);
+pixman_image_set_repeat (src, PIXMAN_REPEAT_NORMAL);
+pixman_image_set_filter (src, PIXMAN_FILTER_BILINEAR, NULL, 0);
+
+image_endian_swap (src);
+image_endian_swap (dest);
+
+pixman_image_composite (PIXMAN_OP_SRC,
+   src, NULL, dest,
+   0, 0, 0, 0, 0, 0,
+   SIZE, SIZE);
+
+crc = compute_crc32_for_image (crc, dest);
+
+pixman_image_unref (src);
+pixman_image_unref (dest);
+
+free (srcbuf);
+free (dstbuf);
+
+return crc;
+}
+
+#if BILINEAR_INTERPOLATION_BITS == 7
+#define CHECKSUM 0x02169677
+#elif BILINEAR_INTERPOLATION_BITS == 4
+#define CHECKSUM 0xE44B29AC
+#else
+#define CHECKSUM 0x
+#endif
+
+int
+main (int argc, const char *argv[])
+{
+const pixman_fixed_t *end = entries + ARRAY_LENGTH (entries);
+const pixman_fixed_t *t0, *t1, *t2, *t3, *t4, *t5;
+uint32_t crc = 0;
+
+prng_srand (0x56EA1DBD);
+
+for (t0 = entries; t0 < end; ++t0)
+{
+   for (t1 = entries; t1 < end; ++t1)
+   {
+   for (t2 = entries; t2 < end; ++t2)
+   {
+   for (t3 = entries; t3 < end; ++t3)
+   {
+   for (t4 = entries; t4 < end; ++t4)
+   {
+   for (t5 = entries; t5 < end; ++t5)
+   {
+   pixman_transform_t xform = {
+   { { *t0, *t1, *t2 },
+ { *t3, *t4, *t5 },
+ { 0, 0, pixman_fixed_1 } }
+   };
+
+   crc = test_scale (, crc);
+   }
+   }
+   }
+   }
+   }
+}
+
+if (crc != CHECKSUM)
+{
+   printf ("filter-reduction-test failed! (checksum=0x%08X, expected 
0x%08X)\n", crc, CHECKSUM);
+   return 1;
+}
+else
+{
+   printf ("filter-reduction-test passed (checksum=0x%08X)\n", crc);
+   return 0;
+}
+}



___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 01/14] pixman-fast-path.c: Pick NEAREST affine fast paths before BILINEAR ones

[Bill Spitzak]

I can confirm this fixes the problem and allows multiple fast path flags 
to be set.


Reviewed-by: Bill Spitzak <spit...@gmail.com>

On 04/11/2016 07:36 PM, Søren Sandmann Pedersen wrote:

When a BILINEAR filter is reduced to NEAREST, it is possible for both
types of fast paths to run; in this case, the NEAREST ones should be
preferred as that is the simpler filter.

Signed-off-by: Soren Sandmann <soren.sandm...@gmail.com>
---
  pixman/pixman-fast-path.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pixman/pixman-fast-path.c b/pixman/pixman-fast-path.c
index 53d4a1f..b4daa26 100644
--- a/pixman/pixman-fast-path.c
+++ b/pixman/pixman-fast-path.c
@@ -3258,9 +3258,9 @@ static const pixman_iter_info_t fast_iters[] =
  },

  #define AFFINE_FAST_PATHS(name, format, repeat)   
\
-SEPARABLE_CONVOLUTION_AFFINE_FAST_PATH(name, format, repeat)   \
+NEAREST_AFFINE_FAST_PATH(name, format, repeat) \
  BILINEAR_AFFINE_FAST_PATH(name, format, repeat)   \
-NEAREST_AFFINE_FAST_PATH(name, format, repeat)
+SEPARABLE_CONVOLUTION_AFFINE_FAST_PATH(name, format, repeat)

  AFFINE_FAST_PATHS (pad_a8r8g8b8, a8r8g8b8, PAD)
  AFFINE_FAST_PATHS (none_a8r8g8b8, a8r8g8b8, NONE)



___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0

[Bill Spitzak]

On Mon, Apr 11, 2016 at 12:35 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Mon, Apr 11, 2016 at 2:43 PM, Bill Spitzak <spit...@gmail.com> wrote:
>
>
>> I feel this can be fixed. It is already correct for subsample_bits==0.
>> Since both the filter generator and filtering code would be changed in the
>> same version, only programs that generate their own filters would actually
>> be incompatible. And as you point out the error is very tiny for large
>> numbers of subsamples, and Cairo is already using an excessively large
>> number of subsamples (likely because I was trying to remove the difference
>> between identity filters and nearest filtering, and did not realize this
>> was the underlying problem).
>>
>
> It is technically an ABI break, and cairo 1.14 did ship with a
> copied-and-pasted filter generator that assumes the current subpixel
> positioning.
>
> But yeah, maybe we can just ignore that, if we make sure a there is a
> cairo 1.14.x update that uses pixman_filter_create_separate_convolution()
> instead of the copied-and-pasted filter generator.
>
> Other than that, the fix should be straight-forward enough.
>

As I wrote that Cairo patch I can state that I think it would be perfectly
fine to make this change. The subsamples are set unnaturally high in that
patch and hide the problem. Also it is possible my filter generator is
producing centered samples so this could be an improvement, need to check.
In addition, other than the (abandoned) idea of getting good/best into
pixman, this patch series is also designed so that Cairo can use the pixman
filter generator rather than it's own.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0

[Bill Spitzak]

Okay the actual bug is that the gnuplot output is wrong for
subsample_bits==0. It apparently is correct for other values of
subsampling. I will try to get an updated version posted soon.

But I very much agree with Owen (and you?) that the current behavior is
incorrect and should be fixed exactly as stated. The current version
produces different images from fallbacks to impulse, bilinear, or
integer-sized box filters, when the filter math says the fallback should
produce an identical result. Scaling down by an integer produces a slightly
blurry and offset result, when a perfect result could be achieved at the
same speed. Increasing the subsamples moves the existing samples, thus may
increase artifacts, rather than always reducing them. All of this is not a
good situation.

I feel this can be fixed. It is already correct for subsample_bits==0.
Since both the filter generator and filtering code would be changed in the
same version, only programs that generate their own filters would actually
be incompatible. And as you point out the error is very tiny for large
numbers of subsamples, and Cairo is already using an excessively large
number of subsamples (likely because I was trying to remove the difference
between identity filters and nearest filtering, and did not realize this
was the underlying problem).

On Sun, Apr 10, 2016 at 10:01 PM, Søren Sandmann 
wrote:

>
> It does look like there is something really wrong. I compared and (except
>> for the subsample_bits==0 case) my version produces the same output as the
>> current git head.
>>
>> I think your intention is that there is a sample at offset=0 whether the
>> filter width is even or odd. However (except when subsample_bits==0) the
>> filter generator makes a symmetric filter for even sizes, with two equal
>> samples around the maximum center value. If a sample was at offset==0 then
>> it would be unique and larger than all the other samples.
>>
>
> The root of this confusion is probably that when subsample_bits = k, the
> subpixel positions used are:
>
> 0.5 / 2^k, 1.5 / 2^k, ..., (2^k-0.5)/2^k
>
> For example, for subsample_bits = 2:
>
> 0.125, 0.375, 0.625, 0.875
>
> and for subsample_bits = 0:
>
> 0.5
>
> That is, they are regularly spaced, but centered within the pixel. When
> there is an even number of them, this means there will not be a filter
> position at 0.5, and therefore no sample at offset 0. And the only case
> where number of subpixel locations is odd, is when subsample_bits = 0.
>
> I'm pretty sure that the existing code gets the filter generation right
> for these subpixel positions.
>
>
> [ You can argue that it would be better to use the sampling positions
>
> 0, 0.25, 0.5, 0.75
>
> for subsample_bits = 2, as Owen did here:
>
> https://lists.cairographics.org/archives/cairo/2014-March/025105.html
>
> and I agree that that would have been better. ]
>
>
> Søren
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0

[Bill Spitzak]

On 04/09/2016 01:13 PM, Søren Sandmann wrote:

On Sat, Apr 9, 2016 at 3:45 PM, Bill Spitzak <spit...@gmail.com
<mailto:spit...@gmail.com>> wrote:

On 04/03/2016 11:17 AM, Søren Sandmann wrote:

I don't believe this patch is correct.

As an example, consider an image with an identity transformation
and a
cubic filter (which has width 4) with subsample_bits = 0. The
current
code will compute a matrix

  [ cubic(-2), cubic(-1), cubic(0), cubic(1) ]

Now suppose we are filtering a pixel located at x = 17.5. The
code in
bits_image_fetch_pixel_separable_convolution() will eventually
end up
convolving with the pixels at locations 15.5, 16.5, 17.5 and
18.5, which
is the correct result since cubic(-2) = 0 [1].

With your code, the matrix computed would be

  [ cubic(-1.5), cubic(-0.5), cubic(0.5), cubic(1) ]

which would not be correct no matter what: With an identity
transformation, the pixel at 17.5 should be multiplied with
cubic(0).

There is a detailed explanation of these issues in the file
pixman/rounding.txt.


I have finally checked this and it is producing the correct value in
the above example. pos is not the left edge of the range to
integrate, it is the center of it, due to the changes I made to the
integrate function.


No, it really doesn't produce the correct values. If you apply the patch
below on top of your series and run demos/scale with

 subsample set to 0,
 reconstruct set to 'Cubic',
 sample set to 'Impulse'
 scale factor set to 1.0,

it prints out this:

 cubic(1.50) = -0.034722
 cubic(0.50) = 0.534722
 cubic(-0.50) = 0.534722
 cubic(-1.50) = -0.034722
 [ -0.035, 0.535, 0.535, -0.035, ]

which is not the correct result. The first value should be cubic(-2) = 0.

And if you toggle back and forth between subsamples = 0 and 1, you can
see the image shift slightly.

(The *gnuplot* graph looks correct, but that's because the gnuplot
function is also generating the wrong coordinates).


It does look like there is something really wrong. I compared and 
(except for the subsample_bits==0 case) my version produces the same 
output as the current git head.


I think your intention is that there is a sample at offset=0 whether the 
filter width is even or odd. However (except when subsample_bits==0) the 
filter generator makes a symmetric filter for even sizes, with two equal 
samples around the maximum center value. If a sample was at offset==0 
then it would be unique and larger than all the other samples.


Comparing box+box with subsample_bits==1 with bilinear, it does appear 
there is an offset and the filter is wrong. It matches exactly (in your 
version) when subsample_bits==0.


The gnuplot output is wrong because I adjusted it to make this output 
look correct.


I then further broke the only case that actually worked (when 
subsample_bits==0) with this patch, since it did not match the others.


I need to look at the actual filter sampling code and at your paper I 
guess. I think that is doing the right thing, and the generator and 
gnuplot have to be adjusted to match.




___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0

[Bill Spitzak]

On 04/03/2016 11:17 AM, Søren Sandmann wrote:

I don't believe this patch is correct.

As an example, consider an image with an identity transformation and a
cubic filter (which has width 4) with subsample_bits = 0. The current
code will compute a matrix

 [ cubic(-2), cubic(-1), cubic(0), cubic(1) ]

Now suppose we are filtering a pixel located at x = 17.5. The code in
bits_image_fetch_pixel_separable_convolution() will eventually end up
convolving with the pixels at locations 15.5, 16.5, 17.5 and 18.5, which
is the correct result since cubic(-2) = 0 [1].

With your code, the matrix computed would be

 [ cubic(-1.5), cubic(-0.5), cubic(0.5), cubic(1) ]

which would not be correct no matter what: With an identity
transformation, the pixel at 17.5 should be multiplied with cubic(0).

There is a detailed explanation of these issues in the file
pixman/rounding.txt.


I have finally checked this and it is producing the correct value in the 
above example. pos is not the left edge of the range to integrate, it is 
the center of it, due to the changes I made to the integrate function.


However the code is incorrect and only works because n_phases&1 is true 
only when subsample_bits == 0. The intention was to treat all odd widths 
the same.


___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0

[Bill Spitzak]

I believe there may have been a rebasing error. Will look into it.

I certainly intended this to change behavior when the filter size is odd,
not when the number of subsamples is odd. Not sure if this is truly rebase
screwing up, or I just mistyped an attempt to fix a rebase error.


On Mon, Apr 4, 2016 at 12:21 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> No, it changes behavior when the number of *phases* is odd, which it is in
> the example I gave.
>
>
> Søren
>
> On Mon, Apr 4, 2016 at 2:51 PM, Bill Spitzak <spit...@gmail.com> wrote:
>
>> That's why this only changes the behavior when the number of samples is
>> odd.
>>
>>
>> On Sun, Apr 3, 2016 at 11:17 AM, Søren Sandmann <soren.sandm...@gmail.com
>> > wrote:
>>
>>> I don't believe this patch is correct.
>>>
>>> As an example, consider an image with an identity transformation and a
>>> cubic filter (which has width 4) with subsample_bits = 0. The current code
>>> will compute a matrix
>>>
>>> [ cubic(-2), cubic(-1), cubic(0), cubic(1) ]
>>>
>>> Now suppose we are filtering a pixel located at x = 17.5. The code in
>>> bits_image_fetch_pixel_separable_convolution() will eventually end up
>>> convolving with the pixels at locations 15.5, 16.5, 17.5 and 18.5, which is
>>> the correct result since cubic(-2) = 0 [1].
>>>
>>> With your code, the matrix computed would be
>>>
>>> [ cubic(-1.5), cubic(-0.5), cubic(0.5), cubic(1) ]
>>>
>>> which would not be correct no matter what: With an identity
>>> transformation, the pixel at 17.5 should be multiplied with cubic(0).
>>>
>>> There is a detailed explanation of these issues in the file
>>> pixman/rounding.txt.
>>>
>>>
>>> Søren
>>>
>>> [1] You might consider optimizing this case away and generate a matrix
>>> with width 3, but since this would only work with subsample_bits=0, it's
>>> not worth the special-casing.
>>>
>>>
>>> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>>>
>>>> From: Bill Spitzak <spit...@gmail.com>
>>>>
>>>> The position of only one subsample was wrong as ceil() was done on an
>>>> integer.
>>>> Use a different function for all odd numbers of subsamples that gets
>>>> this right.
>>>>
>>>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>>>> ---
>>>>  pixman/pixman-filter.c | 5 -
>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>>>> index ac89dda..f9ad45f 100644
>>>> --- a/pixman/pixman-filter.c
>>>> +++ b/pixman/pixman-filter.c
>>>> @@ -252,7 +252,10 @@ create_1d_filter (int  width,
>>>>  * and sample positions.
>>>>  */
>>>>
>>>> -   pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
>>>> +   if (n_phases & 1)
>>>> +   pos = frac - width / 2.0;
>>>> +   else
>>>> +   pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
>>>>
>>>> total = 0;
>>>> for (x = 0; x < width; ++x, ++pos)
>>>> --
>>>> 1.9.1
>>>>
>>>> ___
>>>> Pixman mailing list
>>>> Pixman@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/pixman
>>>>
>>>
>>>
>>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 08/22] pixman-filter: rename "scale" to "size" when it is 1/scale

[Bill Spitzak]

Okay that sounds pretty good. Also 'r' is better than 'i' since 'i' can be
mis-read as "input" or "image". 'r' I think is mostly going to be read as
"reverse" which actually makes sense.

Sorry to be a pain about this but I really find it confusing to use the
same term for different numbers.

On Sun, Apr 3, 2016 at 9:29 AM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> I guess I can live with 'rscale' (for reciprocal scale).
>
>
> Søren
>
> On Thu, Mar 24, 2016 at 9:30 PM, Bill Spitzak <spit...@gmail.com> wrote:
>
>> Would using "iscale" (for inverse scale) work? Another is "tscale"
>> because it is the scale from the transform matrix.
>>
>> I really would like to use two different names for these variables as it
>> is really confusing using the same one.
>>
>>
>> On Fri, Mar 18, 2016 at 8:24 AM, Bill Spitzak <spit...@gmail.com> wrote:
>>
>>>
>>>
>>> On Thu, Mar 17, 2016 at 10:06 PM, Søren Sandmann <
>>> soren.sandm...@gmail.com> wrote:
>>>
>>>> I suppose it's a little illogical that scale_x and scale_y really are
>>>> the reciprocal values of how much the source image should be scaled. I
>>>> don't remember exactly what I was thinking, but it might have something
>>>> like "this allows you to just pass t[0][0] and t[1][1] if you have a pure
>>>> scaling, which avoids a division". There is also kind of precedence in the
>>>> Pixman API since the transformation given as in the dst->source direction.
>>>>
>>>> I don't really like "size" either though. It's not really the size of
>>>> the filter that we are specifying; it just happens to be proportional to 
>>>> it.
>>>>
>>>> If it comes down to "size" and "scale", I prefer "scale".
>>>>
>>>
>>> I tried "width" but it is not actually the filter width. How about "dx"
>>> and "dy" since it is almost always the derivative of the x and y in the
>>> input (or du,dv or dtx dtx)
>>>
>>>>
>>>>
>>>> Søren
>>>>
>>>>
>>>> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>>>>
>>>>> From: Bill Spitzak <spit...@gmail.com>
>>>>>
>>>>> This is to remove some confusion when reading the code. "scale" gets
>>>>> larger
>>>>> as the picture gets larger, while "size" (ie the size of the filter)
>>>>> gets
>>>>> smaller.
>>>>>
>>>>> v14: Removed changes to integral function
>>>>>
>>>>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>>>>> Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
>>>>> ---
>>>>>  pixman/pixman-filter.c | 18 +-
>>>>>  pixman/pixman.h|  6 +++---
>>>>>  2 files changed, 12 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>>>>> index a29116a..c03a7f6 100644
>>>>> --- a/pixman/pixman-filter.c
>>>>> +++ b/pixman/pixman-filter.c
>>>>> @@ -221,7 +221,7 @@ static void
>>>>>  create_1d_filter (int  width,
>>>>>   pixman_kernel_t  reconstruct,
>>>>>   pixman_kernel_t  sample,
>>>>> - double   scale,
>>>>> + double   size,
>>>>>   int  n_phases,
>>>>>   pixman_fixed_t *p)
>>>>>  {
>>>>> @@ -251,8 +251,8 @@ create_1d_filter (int  width,
>>>>> double pos = x + 0.5 - frac;
>>>>> double rlow = - filters[reconstruct].width / 2.0;
>>>>> double rhigh = rlow + filters[reconstruct].width;
>>>>> -   double slow = pos - scale * filters[sample].width / 2.0;
>>>>> -   double shigh = slow + scale * filters[sample].width;
>>>>> +   double slow = pos - size * filters[sample].width / 2.0;
>>>>> +   double shigh = slow + size * filters[sample].width;
>>>>> double c = 0.0;
>>>>> double ilow, ihigh;
>>>&g

Re: [Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0

[Bill Spitzak]

That's why this only changes the behavior when the number of samples is odd.


On Sun, Apr 3, 2016 at 11:17 AM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> I don't believe this patch is correct.
>
> As an example, consider an image with an identity transformation and a
> cubic filter (which has width 4) with subsample_bits = 0. The current code
> will compute a matrix
>
> [ cubic(-2), cubic(-1), cubic(0), cubic(1) ]
>
> Now suppose we are filtering a pixel located at x = 17.5. The code in
> bits_image_fetch_pixel_separable_convolution() will eventually end up
> convolving with the pixels at locations 15.5, 16.5, 17.5 and 18.5, which is
> the correct result since cubic(-2) = 0 [1].
>
> With your code, the matrix computed would be
>
> [ cubic(-1.5), cubic(-0.5), cubic(0.5), cubic(1) ]
>
> which would not be correct no matter what: With an identity
> transformation, the pixel at 17.5 should be multiplied with cubic(0).
>
> There is a detailed explanation of these issues in the file
> pixman/rounding.txt.
>
>
> Søren
>
> [1] You might consider optimizing this case away and generate a matrix
> with width 3, but since this would only work with subsample_bits=0, it's
> not worth the special-casing.
>
>
> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> The position of only one subsample was wrong as ceil() was done on an
>> integer.
>> Use a different function for all odd numbers of subsamples that gets this
>> right.
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>> ---
>>  pixman/pixman-filter.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> index ac89dda..f9ad45f 100644
>> --- a/pixman/pixman-filter.c
>> +++ b/pixman/pixman-filter.c
>> @@ -252,7 +252,10 @@ create_1d_filter (int  width,
>>  * and sample positions.
>>  */
>>
>> -   pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
>> +   if (n_phases & 1)
>> +   pos = frac - width / 2.0;
>> +   else
>> +   pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
>>
>> total = 0;
>> for (x = 0; x < width; ++x, ++pos)
>> --
>> 1.9.1
>>
>> ___
>> Pixman mailing list
>> Pixman@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/pixman
>>
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 08/22] pixman-filter: rename "scale" to "size" when it is 1/scale

[Bill Spitzak]

Would using "iscale" (for inverse scale) work? Another is "tscale" because
it is the scale from the transform matrix.

I really would like to use two different names for these variables as it is
really confusing using the same one.


On Fri, Mar 18, 2016 at 8:24 AM, Bill Spitzak <spit...@gmail.com> wrote:

>
>
> On Thu, Mar 17, 2016 at 10:06 PM, Søren Sandmann <soren.sandm...@gmail.com
> > wrote:
>
>> I suppose it's a little illogical that scale_x and scale_y really are the
>> reciprocal values of how much the source image should be scaled. I don't
>> remember exactly what I was thinking, but it might have something like
>> "this allows you to just pass t[0][0] and t[1][1] if you have a pure
>> scaling, which avoids a division". There is also kind of precedence in the
>> Pixman API since the transformation given as in the dst->source direction.
>>
>> I don't really like "size" either though. It's not really the size of the
>> filter that we are specifying; it just happens to be proportional to it.
>>
>> If it comes down to "size" and "scale", I prefer "scale".
>>
>
> I tried "width" but it is not actually the filter width. How about "dx"
> and "dy" since it is almost always the derivative of the x and y in the
> input (or du,dv or dtx dtx)
>
>>
>>
>> Søren
>>
>>
>> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>>
>>> From: Bill Spitzak <spit...@gmail.com>
>>>
>>> This is to remove some confusion when reading the code. "scale" gets
>>> larger
>>> as the picture gets larger, while "size" (ie the size of the filter) gets
>>> smaller.
>>>
>>> v14: Removed changes to integral function
>>>
>>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>>> Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
>>> ---
>>>  pixman/pixman-filter.c | 18 +-
>>>  pixman/pixman.h|  6 +++---
>>>  2 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>>> index a29116a..c03a7f6 100644
>>> --- a/pixman/pixman-filter.c
>>> +++ b/pixman/pixman-filter.c
>>> @@ -221,7 +221,7 @@ static void
>>>  create_1d_filter (int  width,
>>>   pixman_kernel_t  reconstruct,
>>>   pixman_kernel_t  sample,
>>> - double   scale,
>>> + double   size,
>>>   int  n_phases,
>>>   pixman_fixed_t *p)
>>>  {
>>> @@ -251,8 +251,8 @@ create_1d_filter (int  width,
>>> double pos = x + 0.5 - frac;
>>> double rlow = - filters[reconstruct].width / 2.0;
>>> double rhigh = rlow + filters[reconstruct].width;
>>> -   double slow = pos - scale * filters[sample].width / 2.0;
>>> -   double shigh = slow + scale * filters[sample].width;
>>> +   double slow = pos - size * filters[sample].width / 2.0;
>>> +   double shigh = slow + size * filters[sample].width;
>>> double c = 0.0;
>>> double ilow, ihigh;
>>>
>>> @@ -262,7 +262,7 @@ create_1d_filter (int  width,
>>> ihigh = MIN (shigh, rhigh);
>>>
>>> c = integral (reconstruct, ilow,
>>> - sample, 1.0 / scale, ilow - pos,
>>> + sample, 1.0 / size, ilow - pos,
>>>   ihigh - ilow);
>>> }
>>>
>>> @@ -335,12 +335,12 @@ filter_width (pixman_kernel_t reconstruct,
>>> pixman_kernel_t sample, double size)
>>>  }
>>>
>>>  /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
>>> - * with the given kernels and scale parameters
>>> + * with the given kernels and size parameters
>>>   */
>>>  PIXMAN_EXPORT pixman_fixed_t *
>>>  pixman_filter_create_separable_convolution (int *n_values,
>>> -   pixman_fixed_t   scale_x,
>>> -   pixman_fixed_t   scale_y,
>>> +   pixman_fixed_t   size_x,
>>> +   pixman_fixed_t   size_y,
>>>

Re: [Pixman] [PATCH v14 10/22] pixman-filter: Correct integration with impulse filters

[Bill Spitzak]

On Mon, Mar 21, 2016 at 7:41 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

>
>
> On Mon, Mar 21, 2016 at 12:29 PM, Bill Spitzak <spit...@gmail.com> wrote:
>
>> On Sun, Mar 20, 2016 at 11:36 PM, Søren Sandmann <
>> soren.sandm...@gmail.com> wrote:
>>
>>> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>>>
>>>> From: Bill Spitzak <spit...@gmail.com>
>>>>
>>>> The IMPULSE special-cases did not sample the center of the of the
>>>> region. This
>>>> caused it to sample the filters outside their range, and produce
>>>> assymetric
>>>> filters and other errors. Fixing this required changing the arguments to
>>>> integral() so the correct point could be determined.
>>>>
>>>
>>> I don't understand what is wrong and why this patch fixes it. Which
>>> region precisely did not have its center sampled? When IMPULSE filters are
>>> involved the width of the integral is 0 so there isn't really any "region"
>>> to sample.
>>>
>>> Can you give a concrete example where the previous code produced
>>> asymmetric filters? Also, what "other errors" was produced? I think these
>>> examples should be added to the commit log.
>>>
>>
>> It sampled the *other* filter (the one that is not impulse) at the left
>> edge of the region being passed, rather than at the location of the center
>> of the impulse filter. This was detected by putting asserts in the filter
>> functions to see if they were being called outside their width.
>>
>
> I tried adding such asserts and I couldn't make them trigger with the
> scale demo. It would be helpful if you could give a specific pair of
> filters and scale factor where a filter is sampled outside its width.
>
> And it really doesn't make sense to talk about the "region being passed"
> when one of the filters is IMPULSE. In that case, the width parameter is
> always 0 so there is no "region".
>

Your version relies on width==0 when either filter is impulse. This is how
it is being called right now, but I think it a very good idea to avoid
this. This patch series does not include fixes I attempted to make the
picture not vanish when impulse+impulse was selected. Those changes made
the impulse filter act like it had a width of 1 and easily triggered the
asserts.

I think my version is a lot clearer. The center of the sampling filter is
at pos, an argument to the function, and the range for the integral is
separated from the alignment and scale. I think it is obvious that this
reduces the size of both the implementation and the calling code, in
particular the sampling only needs a single argument rather than two.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 16/22] pixman-filter: distribute normalization error over filter

[Bill Spitzak]

On Fri, Mar 18, 2016 at 6:54 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> This removes a high-frequency spike in the middle of some filters that is
>> caused by math errors all being in the same direction.
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>> ---
>>  pixman/pixman-filter.c | 12 +++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> index 36dd811..ab62e0a 100644
>> --- a/pixman/pixman-filter.c
>> +++ b/pixman/pixman-filter.c
>> @@ -282,8 +282,18 @@ create_1d_filter (int  width,
>> p[x] = t;
>> }
>>
>> +   /* Distribute any remaining error over all samples */
>> if (new_total != pixman_fixed_1)
>> -   p[width / 2] += (pixman_fixed_1 - new_total);
>> +   {
>> +   pixman_fixed_t delta = new_total - pixman_fixed_1;
>> +   pixman_fixed_t t = 0;
>> +   for (x = 0; x < width; ++x)
>> +   {
>> +   pixman_fixed_t new_t = delta * (x + 1) / width;
>> +   p[x] += new_t - t;
>> +   t = new_t;
>> +   }
>> +   }
>>
>
> I think there is a sign error in this code: delta is new_total - 1, which
> is positive when new_total is bigger than 1. But this positive delta is
> then added to the samples, making the total even bigger.
>

Yes I believe you are right. Looks like my mistake there, and you can see
the other line that this replaces is in the correct direction so I'm not
sure how I managed to do this. This incorrect version still hid the
artifact in the filter (a small dip/spike in the middle of large ones for
sync filters), so I guess I concluded I got it right.

Also, I would write the code like this:
>
> pixman_fixed_t error = pixman_fixed_1 - new_total;
> for (x = 0; x < width; ++x)
> {
> pixman_fixed_t d = error * (x + 1) / width;
> p[x] += d;
> error -= d;
> }
>
> to get rid of the temporary and to make it more obvious that there is an
> error that is being distributed.
>

That will not distribute the error evenly. I could just add
error*(x+1)/width-error*x/width and avoid the temporary entirely, though I
guess there has to be a warning comment that doing the obvious-looking
optimization will make it not work.

>
> Another possibility is to do error diffusion in the /* Normalize */ loop.
>

I don't think that will work because it needs to take into account the
rounding to pixman_fixed_t after the division.

>
>
> Søren
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v14 10/22] pixman-filter: Correct integration with impulse filters

[Bill Spitzak]

On Sun, Mar 20, 2016 at 11:36 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> The IMPULSE special-cases did not sample the center of the of the region.
>> This
>> caused it to sample the filters outside their range, and produce
>> assymetric
>> filters and other errors. Fixing this required changing the arguments to
>> integral() so the correct point could be determined.
>>
>
> I don't understand what is wrong and why this patch fixes it. Which region
> precisely did not have its center sampled? When IMPULSE filters are
> involved the width of the integral is 0 so there isn't really any "region"
> to sample.
>
> Can you give a concrete example where the previous code produced
> asymmetric filters? Also, what "other errors" was produced? I think these
> examples should be added to the commit log.
>

It sampled the *other* filter (the one that is not impulse) at the left
edge of the region being passed, rather than at the location of the center
of the impulse filter. This was detected by putting asserts in the filter
functions to see if they were being called outside their width.


>
>
>
> Søren
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCHv2 3/3] More general BILINEAR=>NEAREST reduction

[Bill Spitzak]

Looks good to me

On Wed, Mar 16, 2016 at 9:14 PM, Søren Sandmann Pedersen <
soren.sandm...@gmail.com> wrote:

> Generalize and simplify the code that reduces BILINEAR to NEAREST so
> that all the reduction happens for all affine transformations where
> t00..t12 are integers and (t00 + t01) and (t10 + t11) are both
> odd. This is a sufficient condition for the resulting transformed
> coordinates to be exactly at the center of a pixel so that BILINEAR
> becomes identical to NEAREST.
>
> V2: Address some comments by Bill Spitzak
>
> Signed-off-by: Søren Sandmann <soren.sandm...@gmail.com>
> ---
>  pixman/pixman-image.c | 66
> +--
>  1 file changed, 38 insertions(+), 28 deletions(-)
>
> diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
> index 1ff1a49..681864e 100644
> --- a/pixman/pixman-image.c
> +++ b/pixman/pixman-image.c
> @@ -335,37 +335,47 @@ compute_image_info (pixman_image_t *image)
> {
> flags |= FAST_PATH_NEAREST_FILTER;
> }
> -   else if (
> -   /* affine and integer translation components in matrix ... */
> -   ((flags & FAST_PATH_AFFINE_TRANSFORM) &&
> -!pixman_fixed_frac (image->common.transform->matrix[0][2] |
> -image->common.transform->matrix[1][2])) &&
> -   (
> -   /* ... combined with a simple rotation */
> -   (flags & (FAST_PATH_ROTATE_90_TRANSFORM |
> - FAST_PATH_ROTATE_180_TRANSFORM |
> - FAST_PATH_ROTATE_270_TRANSFORM)) ||
> -   /* ... or combined with a simple non-rotated translation */
> -   (image->common.transform->matrix[0][0] == pixman_fixed_1 &&
> -image->common.transform->matrix[1][1] == pixman_fixed_1 &&
> -image->common.transform->matrix[0][1] == 0 &&
> -image->common.transform->matrix[1][0] == 0)
> -   )
> -   )
> +   else if (flags & FAST_PATH_AFFINE_TRANSFORM)
> {
> -   /* FIXME: there are some affine-test failures, showing that
> -* handling of BILINEAR and NEAREST filter is not quite
> -* equivalent when getting close to 32K for the translation
> -* components of the matrix. That's likely some bug, but for
> -* now just skip BILINEAR->NEAREST optimization in this case.
> +   /* Suppose the transform is
> +*
> +*[ t00, t01, t02 ]
> +*[ t10, t11, t12 ]
> +*[   0,   0,   1 ]
> +*
> +* and the destination coordinates are (n + 0.5, m + 0.5). Then
> +* the transformed x coordinate is:
> +*
> +* tx = t00 * (n + 0.5) + t01 * (m + 0.5) + t02
> +*= t00 * n + t01 * m + t02 + (t00 + t01) * 0.5
> +*
> +* which implies that if t00, t01 and t02 are all integers
> +* and (t00 + t01) is odd, then tx will be an integer plus 0.5,
> +* which means a BILINEAR filter will reduce to NEAREST. The
> same
> +* applies in the y direction
>  */
> -   pixman_fixed_t magic_limit = pixman_int_to_fixed (3);
> -   if (image->common.transform->matrix[0][2] <= magic_limit  &&
> -   image->common.transform->matrix[1][2] <= magic_limit  &&
> -   image->common.transform->matrix[0][2] >= -magic_limit &&
> -   image->common.transform->matrix[1][2] >= -magic_limit)
> +   pixman_fixed_t (*t)[3] = image->common.transform->matrix;
> +
> +   if ((pixman_fixed_frac (
> +t[0][0] | t[0][1] | t[0][2] |
> +t[1][0] | t[1][1] | t[1][2]) == 0) &&
> +   (pixman_fixed_to_int (
> +   (t[0][0] + t[0][1]) & (t[1][0] + t[1][1])) % 2) == 1)
>

Very clever! This optimization looks correct.


> {
> -   flags |= FAST_PATH_NEAREST_FILTER;
> +   /* FIXME: there are some affine-test failures, showing that
> +* handling of BILINEAR and NEAREST filter is not quite
> +* equivalent when getting close to 32K for the translation
> +* components of the matrix. That's likely some bug, but
> for
> +* now just skip BILINEAR->NEAREST optimization in this
> case.
> +*/
> +   

Re: [Pixman] [PATCH v14 08/22] pixman-filter: rename "scale" to "size" when it is 1/scale

[Bill Spitzak]

On Thu, Mar 17, 2016 at 10:06 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> I suppose it's a little illogical that scale_x and scale_y really are the
> reciprocal values of how much the source image should be scaled. I don't
> remember exactly what I was thinking, but it might have something like
> "this allows you to just pass t[0][0] and t[1][1] if you have a pure
> scaling, which avoids a division". There is also kind of precedence in the
> Pixman API since the transformation given as in the dst->source direction.
>
> I don't really like "size" either though. It's not really the size of the
> filter that we are specifying; it just happens to be proportional to it.
>
> If it comes down to "size" and "scale", I prefer "scale".
>

I tried "width" but it is not actually the filter width. How about "dx" and
"dy" since it is almost always the derivative of the x and y in the input
(or du,dv or dtx dtx)

>
>
> Søren
>
>
> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> This is to remove some confusion when reading the code. "scale" gets
>> larger
>> as the picture gets larger, while "size" (ie the size of the filter) gets
>> smaller.
>>
>> v14: Removed changes to integral function
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>> Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
>> ---
>>  pixman/pixman-filter.c | 18 +-
>>  pixman/pixman.h|  6 +++---
>>  2 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> index a29116a..c03a7f6 100644
>> --- a/pixman/pixman-filter.c
>> +++ b/pixman/pixman-filter.c
>> @@ -221,7 +221,7 @@ static void
>>  create_1d_filter (int  width,
>>   pixman_kernel_t  reconstruct,
>>   pixman_kernel_t  sample,
>> - double   scale,
>> + double   size,
>>   int  n_phases,
>>   pixman_fixed_t *p)
>>  {
>> @@ -251,8 +251,8 @@ create_1d_filter (int  width,
>> double pos = x + 0.5 - frac;
>> double rlow = - filters[reconstruct].width / 2.0;
>> double rhigh = rlow + filters[reconstruct].width;
>> -   double slow = pos - scale * filters[sample].width / 2.0;
>> -   double shigh = slow + scale * filters[sample].width;
>> +   double slow = pos - size * filters[sample].width / 2.0;
>> +   double shigh = slow + size * filters[sample].width;
>> double c = 0.0;
>> double ilow, ihigh;
>>
>> @@ -262,7 +262,7 @@ create_1d_filter (int  width,
>> ihigh = MIN (shigh, rhigh);
>>
>> c = integral (reconstruct, ilow,
>> - sample, 1.0 / scale, ilow - pos,
>> + sample, 1.0 / size, ilow - pos,
>>   ihigh - ilow);
>> }
>>
>> @@ -335,12 +335,12 @@ filter_width (pixman_kernel_t reconstruct,
>> pixman_kernel_t sample, double size)
>>  }
>>
>>  /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
>> - * with the given kernels and scale parameters
>> + * with the given kernels and size parameters
>>   */
>>  PIXMAN_EXPORT pixman_fixed_t *
>>  pixman_filter_create_separable_convolution (int *n_values,
>> -   pixman_fixed_t   scale_x,
>> -   pixman_fixed_t   scale_y,
>> +   pixman_fixed_t   size_x,
>> +   pixman_fixed_t   size_y,
>> pixman_kernel_t
>> reconstruct_x,
>> pixman_kernel_t
>> reconstruct_y,
>> pixman_kernel_t  sample_x,
>> @@ -348,8 +348,8 @@ pixman_filter_create_separable_convolution (int
>>*n_values,
>> int
>> subsample_bits_x,
>> int
>> subsample_bits_y)
>>  {
>> -double sx = fabs (pixman_fixed_to_double (scale_x));
>> -double sy = fabs (pixman_fixed_to_double (scale_y));
>> +double sx = fabs (pix

Re: [Pixman] [PATCHv2 2/3] Add new test of filter reduction from BILINEAR to NEAREST

[Bill Spitzak]

Looks like a good idea to me. It is unfortunate that if it fails there is
not much indication which image failed but that would require a lot more
checksums.

I would guess the checksums are computed by running this with the fast
paths disabled?
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 2/2] More general BILINEAR=>NEAREST reduction

[Bill Spitzak]

Seems correct and gets more cases than my code did (I was assuming nobody
would use bilinear for scales less than 1/2 so I did not check for it).
Minor comments:

On Mon, Mar 14, 2016 at 11:19 PM, Søren Sandmann Pedersen <
soren.sandm...@gmail.com> wrote:

> Generalize and simplify the code that reduces BILINEAR to NEAREST so
> that all the reduction happens for all affine transformations where
> t00..t22 are integers and (t00 + t01) and (t10 + t11) are both
> odd. This is a sufficient condition for the resulting transformed
> coordinates being exactly at the center of a pixel so that BILINEAR
> becomes identical to NEAREST.
>

Actually the last row (t20, t21, t22) has to be 0,0,1 (or 0,0,w where you
then make a new matrix that divides everything by w and then it is 0,0,1).
This is already tested for by the test for AFFINE, but this comment is a
bit incorrect.


>
> Signed-off-by: Søren Sandmann 
> ---
>  pixman/pixman-image.c | 69
> ++-
>  1 file changed, 41 insertions(+), 28 deletions(-)
>
> diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
> index 1ff1a49..cff8a30 100644
> --- a/pixman/pixman-image.c
> +++ b/pixman/pixman-image.c
> @@ -335,37 +335,50 @@ compute_image_info (pixman_image_t *image)
> {
> flags |= FAST_PATH_NEAREST_FILTER;
> }
> -   else if (
> -   /* affine and integer translation components in matrix ... */
> -   ((flags & FAST_PATH_AFFINE_TRANSFORM) &&
> -!pixman_fixed_frac (image->common.transform->matrix[0][2] |
> -image->common.transform->matrix[1][2])) &&
> -   (
> -   /* ... combined with a simple rotation */
> -   (flags & (FAST_PATH_ROTATE_90_TRANSFORM |
> - FAST_PATH_ROTATE_180_TRANSFORM |
> - FAST_PATH_ROTATE_270_TRANSFORM)) ||
> -   /* ... or combined with a simple non-rotated translation */
> -   (image->common.transform->matrix[0][0] == pixman_fixed_1 &&
> -image->common.transform->matrix[1][1] == pixman_fixed_1 &&
> -image->common.transform->matrix[0][1] == 0 &&
> -image->common.transform->matrix[1][0] == 0)
> -   )
> -   )
> +   else if (flags & FAST_PATH_AFFINE_TRANSFORM)
> {
> -   /* FIXME: there are some affine-test failures, showing that
> -* handling of BILINEAR and NEAREST filter is not quite
> -* equivalent when getting close to 32K for the translation
> -* components of the matrix. That's likely some bug, but for
> -* now just skip BILINEAR->NEAREST optimization in this case.
> +   /* Suppose the transform is
> +*
> +*[ t00, t01, t02 ]
> +*[ t10, t11, t12 ]
> +*[ 120, t21, t22 ]
>

typo 120->t20. I would change the last row to just read [0,0,1] as you are
assuming this and not using these values in the following equation.


> +*
> +* and the destination coordinates are (n + 0.5, m + 0.5). Then
> +* the transformed x coordinate is:
> +*
> +* tx = t00 * (n + 0.5) + t01 * (m + 0.5) + t02
> +*= t00 * n + t01 * m + t02 + (t00 + t01) * 0.5
>

I would use u,v rather than tx,ty for the source coordinates. More complex
analysis of transforms gets very hard to read unless the variables are kept
to one letter.

+* which implies that if t00, t01 and t02 are all integers
> +* and (t00 + t01) is odd, then tx will be an integer plus 0.5,
> +* which means a BILINEAR filter will reduce to NEAREST. The
> same
> +* applies in the y direction
>  */
> -   pixman_fixed_t magic_limit = pixman_int_to_fixed (3);
> -   if (image->common.transform->matrix[0][2] <= magic_limit  &&
> -   image->common.transform->matrix[1][2] <= magic_limit  &&
> -   image->common.transform->matrix[0][2] >= -magic_limit &&
> -   image->common.transform->matrix[1][2] >= -magic_limit)
> +   pixman_fixed_t (*t)[3] = image->common.transform->matrix;
> +
> +   if (pixman_fixed_frac (t[0][0]) == 0
>  &&
> +   pixman_fixed_frac (t[0][1]) == 0
>  &&
> +   pixman_fixed_frac (t[0][2]) == 0
>  &&
> +   ((t[0][0] + t[0][1]) & pixman_fixed_1) == pixman_fixed_1
>  &&
> +   pixman_fixed_frac (t[1][0]) == 0
>  &&
> +   pixman_fixed_frac (t[1][1]) == 0
>  &&
> +   pixman_fixed_frac (t[1][2]) == 0
>  &&
> +   ((t[1][0] + t[1][1]) & pixman_fixed_1) == pixman_fixed_1)
>

You can or all the numbers together to test they are all integers in one
step.


> {
> -   flags |= FAST_PATH_NEAREST_FILTER;
> +   /* FIXME: there are some 

Re: [Pixman] [PATCH 1/2] Add new test of filter reduction from BILINEAR to NEAREST

[Bill Spitzak]

Great idea to test this.

On Mon, Mar 14, 2016 at 11:24 PM, Søren Sandmann 
wrote:

> diff --git a/test/filter-reduction-test.c b/test/filter-reduction-test.c
>
>> new file mode 100644
>> index 000..72b3142
>> --- /dev/null
>> +++ b/test/filter-reduction-test.c
>> @@ -0,0 +1,119 @@
>> +/*
>> + * Test program, which can detect some problems with nearest neighbour
>> + * and bilinear scaling in pixman. Testing is done by running lots
>> + * of random SRC and OVER compositing operations a8r8g8b8, x8a8r8g8b8
>> + * and r5g6b5 color formats.
>> + *
>> + * Script 'fuzzer-find-diff.pl' can be used to narrow down the problem
>> in
>> + * the case of test failure.
>> + */
>>
>
> This comment is of course a leftover due to starting from a copy of
> scaling-test.c.
>
>
> Søren
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 13/14] pixman-image: Implement PIXMAN_FILTER_GOOD/BEST as separable convolutions

[Bill Spitzak]

On Mon, Mar 7, 2016 at 1:15 AM, Pekka Paalanen <ppaala...@gmail.com> wrote:

> On Fri, 4 Mar 2016 19:12:36 -0800
> Bill Spitzak <spit...@gmail.com> wrote:
>
> > On Fri, Mar 4, 2016 at 4:17 PM, Søren Sandmann <soren.sandm...@gmail.com
> >
> > wrote:
> >
> > > On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
> > >
> > >> From: Bill Spitzak <spit...@gmail.com>
> > >>
> > >> Detects and uses PIXMAN_FILTER_NEAREST for all 8 90-degree rotations
> and
> > >> reflections when the scale is 1.0 and integer translation.
> > >>
> > >> GOOD uses:
> > >>
> > >>  scale < 1/16 : BOX.BOX at size 16
> > >>  scale < 3/4 : BOX.BOX at size 1/scale
> > >>  larger : BOX.BOX at size 1
> > >>
> > >>  If both directions have a scale >= 3/4 or a scale of 1/2 and an
> integer
> > >>  translation, the faster PIXMAN_FILTER_BILINEAR code is used. This is
> > >>  compatable at these scales with older versions of pixman where
> bilinear
> > >>  was always used for GOOD.
> > >>
> > >> BEST uses:
> > >>
> > >>  scale < 1/24 : BOX.BOX at size 24
> > >>  scale < 1/16 : BOX.BOX at size 1/scale
> > >>  scale < 1 : IMPULSE.LANCZOS2 at size 1/scale
> > >>  scale < 2.333 : IMPULSE.LANCZOS2 at size 1
> > >>  scale < 128 : BOX.LANCZOS2 at size 1/(scale-1) (antialiased square
> > >> pixels)
> > >>  larger : BOX.LANCZOS2 at size 1/127 (antialias blur gets thicker)
> > >>
> > >> v8: Cutoff in BEST between IMPULSE.LANCZOS2 and BOX.LANCZOS2 adjusted
> for
> > >> a better match between the filters.
> > >>
> > >> v9: Use the new negative subsample controls to scale the subsamples.
> These
> > >> were chosen by finding the lowest number that did not add visible
> > >> artifacts to the zone plate image.
> > >>
> > >> Scale demo altered to default to GOOD and locked-together x+y
> scale
> > >>
> > >> Fixed divide-by-zero from all-zero matrix found by stress-test
> > >>
> > >> v11: Whitespace and formatting fixes
> > >>  Moved demo changes to a later patch
> > >>
> > >> v12: Whitespace and formatting fixes
> > >>
> > >> Signed-off-by: Bill Spitzak <spit...@gmail.com>
> > >>
> > >
> > > The short answer to this patch is NACK - it doesn't make sense to
> change
> > > the meaning of GOOD and BEST.
> > >
> >
> > Okay that is going to need a big WTF?
>
> No, it's perfectly understandable.
>
> > > There are at least three reasons:
> > >
> > > 1. Pixman is a low-level API that is in the camp of "do what I say" not
> > > "do what I mean". When users such as cairo specify GOOD and BEST, they
> are
> > > essentially asking Pixman to make a tradeoff that it doesn't have the
> > > knowledge to do. As such, I think it was a mistake to have these
> values in
> > > the first place (and indeed also a mistake to have them in the Render
> > > extension since X11 is also supposed to be a do-what-I-say API). As
> such, I
> > > think they should remain aliases to BILINEAR as they are now.
> > >
> >
> > I have no idea why having three ways to say BILINEAR is necessary to "do
> > what I say". One seems sufficient.
>
> You must forget any resemblance the strings "GOOD" and "BEST" have
> with English, and see them as abstract names for particular
> mathematical functions.
>
> That is what do-what-I-say means.
>
> Therefore you cannot change what mathematical function they correspond
> to.
>
> There are three ways of saying the same only because of a mistake in
> the past which became evident only much later. It happens. Get over it.
>
> > > The way to go is to add a new SeparateConvolution filter to Render and
> make
> > > fb call into Pixman to implement it. Once that is in place, the
> hardware
> > > drivers can then implement it.
> > >
> >
> > You seem to think the hardware drivers will implement this version of
> > SeparateConvolution? Really???
>
> That is implied by do-what-I-say API policy.
>
> > The purpose of this was so that the hardware drivers could implement
> > something they call "good" and something they call "best" using wha

[Pixman] [PATCH v14 18/22] pixman-filter: Made Gaussian a bit wider

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Expanded the size slightly (from ~4.25 to 5) to make the cutoff less noticable.
Previouly the value at the cutoff was gaussian_filter(sqrt(2)*3/2) = 0.00626
which is larger than the difference between 8-bit pixels (1/255 = 0.003921).
New cutoff is gaussian_filter(2.5) = 0.001089 which is smaller.

v11: added some math to commit message
v14: left SIGMA in there
Signed-off-by: Bill Spitzak <spit...@gmail.com>
Acked-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 4b7..f582187 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -143,7 +143,7 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_BOX,   box_kernel,   1.0 },
 { PIXMAN_KERNEL_LINEAR,linear_kernel,2.0 },
 { PIXMAN_KERNEL_CUBIC, cubic_kernel, 4.0 },
-{ PIXMAN_KERNEL_GAUSSIAN,  gaussian_kernel,  6 * SIGMA },
+{ PIXMAN_KERNEL_GAUSSIAN,  gaussian_kernel,  5.0 },
 { PIXMAN_KERNEL_LANCZOS2,  lanczos2_kernel,  4.0 },
 { PIXMAN_KERNEL_LANCZOS3,  lanczos3_kernel,  6.0 },
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 07/22] pixman-filter: reduce amount of malloc/free/memcpy to generate filter

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Rearranged so that the entire block of memory for the filter pair
is allocated first, and then filled in. Previous version allocated
and freed two temporary buffers for each filter and did an extra
memcpy.

v8: small refactor to remove the filter_width function

v10: Restored filter_width function but with arguments changed to
 match later patches

v11: Removed unused arg and pointer from filter_width function
 Whitespace fixes.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 57 --
 1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index d6c0f74..a29116a 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -217,25 +217,17 @@ integral (pixman_kernel_t kernel1, double x1,
 }
 }
 
-static pixman_fixed_t *
-create_1d_filter (int *width,
+static void
+create_1d_filter (int  width,
  pixman_kernel_t  reconstruct,
  pixman_kernel_t  sample,
  double   scale,
- int  n_phases)
+ int  n_phases,
+ pixman_fixed_t *p)
 {
-pixman_fixed_t *params, *p;
 double step;
-double size;
 int i;
 
-size = scale * filters[sample].width + filters[reconstruct].width;
-*width = ceil (size);
-
-p = params = malloc (*width * n_phases * sizeof (pixman_fixed_t));
-if (!params)
-return NULL;
-
 step = 1.0 / n_phases;
 
 for (i = 0; i < n_phases; ++i)
@@ -250,8 +242,8 @@ create_1d_filter (int *width,
 * and sample positions.
 */
 
-   x1 = ceil (frac - *width / 2.0 - 0.5);
-x2 = x1 + *width;
+   x1 = ceil (frac - width / 2.0 - 0.5);
+   x2 = x1 + width;
 
total = 0;
 for (x = x1; x < x2; ++x)
@@ -279,7 +271,7 @@ create_1d_filter (int *width,
 }
 
/* Normalize */
-   p -= *width;
+   p -= width;
 total = 1 / total;
 new_total = 0;
for (x = x1; x < x2; ++x)
@@ -291,10 +283,8 @@ create_1d_filter (int *width,
}
 
if (new_total != pixman_fixed_1)
-   *(p - *width / 2) += (pixman_fixed_1 - new_total);
+   *(p - width / 2) += (pixman_fixed_1 - new_total);
 }
-
-return params;
 }
 
 #ifdef PIXMAN_GNUPLOT
@@ -338,6 +328,12 @@ gnuplot_filter (int width, int samples, const 
pixman_fixed_t* p)
 }
 #endif
 
+static int
+filter_width (pixman_kernel_t reconstruct, pixman_kernel_t sample, double size)
+{
+return ceil (filters[reconstruct].width + size * filters[sample].width);
+}
+
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and scale parameters
  */
@@ -354,42 +350,35 @@ pixman_filter_create_separable_convolution (int   
  *n_values,
 {
 double sx = fabs (pixman_fixed_to_double (scale_x));
 double sy = fabs (pixman_fixed_to_double (scale_y));
-pixman_fixed_t *horz = NULL, *vert = NULL, *params = NULL;
+pixman_fixed_t *params;
 int subsample_x, subsample_y;
 int width, height;
 
+width = filter_width (reconstruct_x, sample_x, sx);
 subsample_x = (1 << subsample_bits_x);
-subsample_y = (1 << subsample_bits_y);
 
-horz = create_1d_filter (, reconstruct_x, sample_x, sx, subsample_x);
-vert = create_1d_filter (, reconstruct_y, sample_y, sy, 
subsample_y);
+height = filter_width (reconstruct_y, sample_y, sy);
+subsample_y = (1 << subsample_bits_y);
 
-if (!horz || !vert)
-goto out;
-
 *n_values = 4 + width * subsample_x + height * subsample_y;
 
 params = malloc (*n_values * sizeof (pixman_fixed_t));
 if (!params)
-goto out;
+   return NULL;
 
 params[0] = pixman_int_to_fixed (width);
 params[1] = pixman_int_to_fixed (height);
 params[2] = pixman_int_to_fixed (subsample_bits_x);
 params[3] = pixman_int_to_fixed (subsample_bits_y);
 
-memcpy (params + 4, horz,
-   width * subsample_x * sizeof (pixman_fixed_t));
-memcpy (params + 4 + width * subsample_x, vert,
-   height * subsample_y * sizeof (pixman_fixed_t));
+create_1d_filter (width, reconstruct_x, sample_x, sx, subsample_x,
+ params + 4);
+create_1d_filter (height, reconstruct_y, sample_y, sy, subsample_y,
+ params + 4 + width * subsample_x);
 
 #ifdef PIXMAN_GNUPLOT
 gnuplot_filter(width, subsample_x, params+4);
 #endif
 
-out:
-free (horz);
-free (vert);
-
 return params;
 }
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 04/22] demos/scale: fix blank subsamples spin box

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

It now shows the initial value of 4 when the demo is started

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 demos/scale.ui | 1 +
 1 file changed, 1 insertion(+)

diff --git a/demos/scale.ui b/demos/scale.ui
index b62cbfb..6028ab7 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -321,6 +321,7 @@
   
 True
subsample_adjustment
+   4
   
   
 1
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 02/22] demos/scale: Added pulldown to choose PIXMAN_FILTER_* value

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This is very useful for comparing the results of SEPARABLE_CONVOLUTION
with BILINEAR and NEAREST.

v14: Removed good/best items

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 demos/scale.c  | 12 +++-
 demos/scale.ui | 40 ++--
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index 0995ad0..85c12e4 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -127,6 +127,13 @@ typedef struct
 intvalue;
 } named_int_t;
 
+static const named_int_t filter_types[] =
+{
+{ "Separable", PIXMAN_FILTER_SEPARABLE_CONVOLUTION },
+{ "Nearest",   PIXMAN_FILTER_NEAREST },
+{ "Bilinear",  PIXMAN_FILTER_BILINEAR },
+};
+
 static const named_int_t filters[] =
 {
 { "Box",   PIXMAN_KERNEL_BOX },
@@ -260,7 +267,9 @@ rescale (GtkWidget *may_be_null, app_t *app)
gtk_adjustment_get_value (app->subsample_adjustment),
gtk_adjustment_get_value (app->subsample_adjustment));
 
-pixman_image_set_filter (app->original, 
PIXMAN_FILTER_SEPARABLE_CONVOLUTION, params, n_params);
+pixman_image_set_filter (app->original,
+   get_value (app, filter_types, "filter_combo_box"),
+   params, n_params);
 
 pixman_image_set_repeat (
 app->original, get_value (app, repeats, "repeat_combo_box"));
@@ -402,6 +411,7 @@ app_new (pixman_image_t *original)
 widget = get_widget (app, "drawing_area");
 g_signal_connect (widget, "expose_event", G_CALLBACK (on_expose), app);
 
+set_up_combo_box (app, "filter_combo_box", G_N_ELEMENTS (filter_types), 
filter_types);
 set_up_filter_box (app, "reconstruct_x_combo_box");
 set_up_filter_box (app, "reconstruct_y_combo_box");
 set_up_filter_box (app, "sample_x_combo_box");
diff --git a/demos/scale.ui b/demos/scale.ui
index ee985dd..b62cbfb 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -191,12 +191,23 @@
 8
 6
 
+  
+True
+1
+bFilter:/b
+True
+  
+
+
   
 True
 1
 bReconstruct X:/b
 True
   
+  
+1
+  
 
 
   
@@ -206,7 +217,7 @@
 True
   
   
-1
+2
   
 
 
@@ -217,7 +228,7 @@
 True
   
   
-2
+3
   
 
 
@@ -228,7 +239,7 @@
 True
   
   
-3
+4
   
 
 
@@ -239,7 +250,7 @@
 True
   
   
-4
+5
   
 
 
@@ -250,7 +261,15 @@
 True
   
   
-5
+6
+  
+
+
+  
+True
+  
+  
+1
   
 
 
@@ -259,6 +278,7 @@
   
   
 1
+1
   
 
 
@@ -267,7 +287,7 @@
   
   
 1
-1
+2
   
 
 
@@ -276,7 +296,7 @@
   
   
 1
-2
+3
   
 
 
@@ -285,7 +305,7 @@
   
   
 1
-3
+4
   
 
 
@@ -294,7 +314,7 @@
   
   
  

[Pixman] [PATCH v14 14/22] pixman-filter: Do BOX.BOX much faster

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

The desired result from the integration is directly available, as the range has 
been
clipped to the intersection of the two boxes. As this filter is probably the 
most
common one, this optimization looks very useful.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-filter.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 6d589c2..8dfb49b 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -171,6 +171,11 @@ integral (pixman_kernel_t kernel1,
 {
return filters[kernel2].func (-pos * scale) * scale;
 }
+else if (kernel1 == PIXMAN_KERNEL_BOX && kernel2 == PIXMAN_KERNEL_BOX)
+{
+   assert (high <= low + 1.0);
+   return (high - low) * scale;
+}
 /* If the integration interval crosses zero, break it into
  * two separate integrals. This ensures that filters such
  * as LINEAR that are not differentiable at 0 will still
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 22/22] demos/scale: Add good/best filter types

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Allows testing them. Good is the default to match default behavior of
pixman/cairo at startup.

v14: Locked axis put in it's own commit

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 demos/scale.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index 0c6b533..d1fce5d 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -132,6 +132,8 @@ static const named_int_t filter_types[] =
 { "Separable", PIXMAN_FILTER_SEPARABLE_CONVOLUTION },
 { "Nearest",   PIXMAN_FILTER_NEAREST },
 { "Bilinear",  PIXMAN_FILTER_BILINEAR },
+{ "Good",  PIXMAN_FILTER_GOOD },
+{ "Best",  PIXMAN_FILTER_BEST },
 };
 
 static const named_int_t filters[] =
@@ -338,7 +340,7 @@ on_expose (GtkWidget *da, GdkEvent *event, gpointer data)
 
 static void
 set_up_combo_box (app_t *app, const char *box_name,
-  int n_entries, const named_int_t table[])
+  int n_entries, const named_int_t table[], int active)
 {
 GtkWidget *widget = get_widget (app, box_name);
 GtkListStore *model;
@@ -364,7 +366,7 @@ set_up_combo_box (app_t *app, const char *box_name,
gtk_list_store_set (model, , 0, info->name, -1);
 }
 
-gtk_combo_box_set_active (GTK_COMBO_BOX (widget), 0);
+gtk_combo_box_set_active (GTK_COMBO_BOX (widget), active);
 
 g_signal_connect (widget, "changed", G_CALLBACK (rescale), app);
 }
@@ -372,7 +374,7 @@ set_up_combo_box (app_t *app, const char *box_name,
 static void
 set_up_filter_box (app_t *app, const char *box_name)
 {
-set_up_combo_box (app, box_name, G_N_ELEMENTS (filters), filters);
+set_up_combo_box (app, box_name, G_N_ELEMENTS (filters), filters, 0);
 }
 
 static char *
@@ -420,14 +422,14 @@ app_new (pixman_image_t *original)
 widget = get_widget (app, "drawing_area");
 g_signal_connect (widget, "expose_event", G_CALLBACK (on_expose), app);
 
-set_up_combo_box (app, "filter_combo_box", G_N_ELEMENTS (filter_types), 
filter_types);
+set_up_combo_box (app, "filter_combo_box", G_N_ELEMENTS (filter_types), 
filter_types, 3);
 set_up_filter_box (app, "reconstruct_x_combo_box");
 set_up_filter_box (app, "reconstruct_y_combo_box");
 set_up_filter_box (app, "sample_x_combo_box");
 set_up_filter_box (app, "sample_y_combo_box");
 
 set_up_combo_box (
-app, "repeat_combo_box", G_N_ELEMENTS (repeats), repeats);
+   app, "repeat_combo_box", G_N_ELEMENTS (repeats), repeats, 0);
 
 g_signal_connect (
gtk_builder_get_object (app->builder, "lock_checkbutton"),
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 06/22] pixman-image: Added enable-gnuplot config to view filters in gnuplot

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

If enable-gnuplot is configured, then you can pipe the output of a pixman-using 
program
to gnuplot and get a continuously-updated plot of the horizontal filter. This
works well with demos/scale to test the filter generation.

The plot is all the different subposition filters shuffled together. This is
misleading in a few cases:

  IMPULSE.BOX - goes up and down as the subfilters have different numbers of 
non-zero samples
  IMPULSE.TRIANGLE - somewhat crooked for the same reason
  1-wide filters - looks triangular, but a 1-wide box would be more accurate

v7: First time this ability was included

v8: Use config option
Moved code to the filter generator
Modified scale demo to not call filter generator a second time.

v10: Only print if successful generation of plots
 Use #ifdef, not #if

v11: small whitespace fixes

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 configure.ac   | 13 +
 pixman/pixman-filter.c | 45 +
 2 files changed, 58 insertions(+)

diff --git a/configure.ac b/configure.ac
index 6b2134e..e833e45 100644
--- a/configure.ac
+++ b/configure.ac
@@ -834,6 +834,19 @@ fi
 AC_SUBST(PIXMAN_TIMERS)
 
 dnl ===
+dnl gnuplot
+
+AC_ARG_ENABLE(gnuplot,
+   [AC_HELP_STRING([--enable-gnuplot],
+   [enable output of filters that can be piped to gnuplot 
[default=no]])],
+   [enable_gnuplot=$enableval], [enable_gnuplot=no])
+
+if test $enable_gnuplot = yes ; then
+   AC_DEFINE(PIXMAN_GNUPLOT, 1, [enable output that can be piped to gnuplot])
+fi
+AC_SUBST(PIXMAN_GNUPLOT)
+
+dnl ===
 dnl GTK+
 
 AC_ARG_ENABLE(gtk,
diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index b2bf53f..d6c0f74 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -297,6 +297,47 @@ create_1d_filter (int *width,
 return params;
 }
 
+#ifdef PIXMAN_GNUPLOT
+/* If enable-gnuplot is configured, then you can pipe the output of a
+ * pixman-using program to gnuplot and get a continuously-updated plot
+ * of the horizontal filter. This works well with demos/scale to test
+ * the filter generation.
+ *
+ * The plot is all the different subposition filters shuffled
+ * together. This is misleading in a few cases:
+ *
+ *  IMPULSE.BOX - goes up and down as the subfilters have different
+ *   numbers of non-zero samples
+ *  IMPULSE.TRIANGLE - somewhat crooked for the same reason
+ *  1-wide filters - looks triangular, but a 1-wide box would be more
+ *  accurate
+ */
+static void
+gnuplot_filter (int width, int samples, const pixman_fixed_t* p)
+{
+int x,y;
+printf ("plot '-' with linespoints\n");
+printf ("%g 0\n", - width * .5);
+for (x = 0; x < width; ++x) {
+   for (y = 0; y < samples; ++y) {
+   int yy;
+   if (width & 1)
+   yy = y;
+   else if (y >= samples / 2)
+   yy = y - samples / 2;
+   else
+   yy = samples / 2 + y;
+   printf ("%g %g\n",
+   x - width * 0.5 + (y + 0.5) * (1.0 / samples),
+   pixman_fixed_to_double(p[(yy + 1) * width - x - 1]));
+   }
+}
+printf ("%g 0\n", width * .5);
+printf ("e\n");
+fflush (stdout);
+}
+#endif
+
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and scale parameters
  */
@@ -342,6 +383,10 @@ pixman_filter_create_separable_convolution (int
 *n_values,
 memcpy (params + 4 + width * subsample_x, vert,
height * subsample_y * sizeof (pixman_fixed_t));
 
+#ifdef PIXMAN_GNUPLOT
+gnuplot_filter(width, subsample_x, params+4);
+#endif
+
 out:
 free (horz);
 free (vert);
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 09/22] pixman-filter: Correct Simpsons integration

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Simpsons uses cubic curve fitting, with 3 samples defining each cubic. This
makes the weights of the samples be in a pattern of 1,4,2,4,2...4,1, and then
dividing the result by 3.

The previous code was using weights of 1,2,0,6,0,6...,2,1.

With this fix the integration is accurate enough that the number of samples
could be reduced a lot. Multiples of 12 seem to work best.

v7: Merged with patch to reduce from 128 samples to 16
v9: Changed samples from 16 to 12
v10: Fixed rebase error that made it not compile
v11: minor whitespace change
v14: more whitespace changes

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index c03a7f6..bd5ac4c 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -189,13 +189,19 @@ integral (pixman_kernel_t kernel1, double x1,
 }
 else
 {
-   /* Integration via Simpson's rule */
-#define N_SEGMENTS 128
+   /* Integration via Simpson's rule
+* See http://www.intmath.com/integration/6-simpsons-rule.php
+* 12 segments (6 cubic approximations) seems to produce best
+* result for lanczos3.linear, which was the combination that
+* showed the most errors.  This makes sense as the lanczos3
+* filter is 6 wide.
+*/
+#define N_SEGMENTS 12
 #define SAMPLE(a1, a2) \
(filters[kernel1].func ((a1)) * filters[kernel2].func ((a2) * scale))

double s = 0.0;
-   double h = width / (double)N_SEGMENTS;
+   double h = width / N_SEGMENTS;
int i;
 
s = SAMPLE (x1, x2);
@@ -204,11 +210,14 @@ integral (pixman_kernel_t kernel1, double x1,
{
double a1 = x1 + h * i;
double a2 = x2 + h * i;
+   s += 4 * SAMPLE (a1, a2);
+   }
 
+   for (i = 2; i < N_SEGMENTS; i += 2)
+   {
+   double a1 = x1 + h * i;
+   double a2 = x2 + h * i;
s += 2 * SAMPLE (a1, a2);
-
-   if (i >= 2 && i < N_SEGMENTS - 1)
-   s += 4 * SAMPLE (a1, a2);
}
 
s += SAMPLE (x1 + width, x2 + width);
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 10/22] pixman-filter: Correct integration with impulse filters

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

The IMPULSE special-cases did not sample the center of the of the region. This
caused it to sample the filters outside their range, and produce assymetric
filters and other errors. Fixing this required changing the arguments to
integral() so the correct point could be determined.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-filter.c | 111 ++---
 1 file changed, 49 insertions(+), 62 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index bd5ac4c..fef2189 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -147,45 +147,46 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
 };
 
-/* This function scales @kernel2 by @scale, then
- * aligns @x1 in @kernel1 with @x2 in @kernel2 and
- * and integrates the product of the kernels across @width.
+/* This function scales @kernel2 by @scale, shifts it by @pos,
+ * and then integrates the product of the kernels across low..high
  *
  * This function assumes that the intervals are within
  * the kernels in question. E.g., the caller must not
  * try to integrate a linear kernel ouside of [-1:1]
  */
 static double
-integral (pixman_kernel_t kernel1, double x1,
- pixman_kernel_t kernel2, double scale, double x2,
- double width)
+integral (pixman_kernel_t kernel1,
+ pixman_kernel_t kernel2, double scale, double pos,
+ double low, double high)
 {
+if (high < low)
+{
+   return 0.0;
+}
+else if (kernel2 == PIXMAN_KERNEL_IMPULSE)
+{
+   return filters[kernel1].func (-pos);
+}
+else if (kernel1 == PIXMAN_KERNEL_IMPULSE)
+{
+   return filters[kernel2].func (-pos * scale);
+}
 /* If the integration interval crosses zero, break it into
  * two separate integrals. This ensures that filters such
  * as LINEAR that are not differentiable at 0 will still
  * integrate properly.
  */
-if (x1 < 0 && x1 + width > 0)
+else if (low < 0 && high > 0)
 {
return
-   integral (kernel1, x1, kernel2, scale, x2, - x1) +
-   integral (kernel1, 0, kernel2, scale, x2 - x1, width + x1);
+   integral (kernel1, kernel2, scale, pos, low, 0) +
+   integral (kernel1, kernel2, scale, pos, 0, high);
 }
-else if (x2 < 0 && x2 + width > 0)
+else if (low < pos && high > pos)
 {
return
-   integral (kernel1, x1, kernel2, scale, x2, - x2) +
-   integral (kernel1, x1 - x2, kernel2, scale, 0, width + x2);
-}
-else if (kernel1 == PIXMAN_KERNEL_IMPULSE)
-{
-   assert (width == 0.0);
-   return filters[kernel2].func (x2 * scale);
-}
-else if (kernel2 == PIXMAN_KERNEL_IMPULSE)
-{
-   assert (width == 0.0);
-   return filters[kernel1].func (x1);
+   integral (kernel1, kernel2, scale, pos, low, pos) +
+   integral (kernel1, kernel2, scale, pos, pos, high);
 }
 else
 {
@@ -197,30 +198,28 @@ integral (pixman_kernel_t kernel1, double x1,
 * filter is 6 wide.
 */
 #define N_SEGMENTS 12
-#define SAMPLE(a1, a2) \
-   (filters[kernel1].func ((a1)) * filters[kernel2].func ((a2) * scale))
-   
+#define SAMPLE(a)  \
+   (filters[kernel1].func ((a)) * filters[kernel2].func (((a) - pos)))
+
double s = 0.0;
-   double h = width / N_SEGMENTS;
+   double h = (high - low) / N_SEGMENTS;
int i;
 
-   s = SAMPLE (x1, x2);
+   s = SAMPLE (low);
 
for (i = 1; i < N_SEGMENTS; i += 2)
{
-   double a1 = x1 + h * i;
-   double a2 = x2 + h * i;
-   s += 4 * SAMPLE (a1, a2);
+   double a1 = low + h * i;
+   s += 4 * SAMPLE (a1);
}
 
for (i = 2; i < N_SEGMENTS; i += 2)
{
-   double a1 = x1 + h * i;
-   double a2 = x2 + h * i;
-   s += 2 * SAMPLE (a1, a2);
+   double a1 = low + h * i;
+   s += 2 * SAMPLE (a1);
}
 
-   s += SAMPLE (x1 + width, x2 + width);
+   s += SAMPLE (high);

return h * s * (1.0 / 3.0);
 }
@@ -235,55 +234,43 @@ create_1d_filter (int  width,
  pixman_fixed_t *p)
 {
 double step;
+double rwidth2 = filters[reconstruct].width / 2.0;
+double swidth2 = size * filters[sample].width / 2.0;
 int i;
 
 step = 1.0 / n_phases;
 
 for (i = 0; i < n_phases; ++i)
 {
-double frac = step / 2.0 + i * step;
+   double frac = step / 2.0 + i * step;
pixman_fixed_t new_total;
-int x, x1, x2;
-   double total;
+   int x;
+   double pos, total;
 
/* Sample convolution of reconstruction and sampling

[Pixman] [PATCH v14 17/22] pixman-filter: Nested polynomial for cubic

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

v11: Restored range checks

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index ab62e0a..4b7 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -109,14 +109,16 @@ general_cubic (double x, double B, double C)
 
 if (ax < 1)
 {
-   return ((12 - 9 * B - 6 * C) * ax * ax * ax +
-   (-18 + 12 * B + 6 * C) * ax * ax + (6 - 2 * B)) / 6;
+   return (((12 - 9 * B - 6 * C) * ax +
+(-18 + 12 * B + 6 * C)) * ax * ax +
+   (6 - 2 * B)) / 6;
 }
-else if (ax >= 1 && ax < 2)
+else if (ax < 2)
 {
-   return ((-B - 6 * C) * ax * ax * ax +
-   (6 * B + 30 * C) * ax * ax + (-12 * B - 48 * C) *
-   ax + (8 * B + 24 * C)) / 6;
+   return -B - 6 * C) * ax +
+ (6 * B + 30 * C)) * ax +
+(-12 * B - 48 * C)) * ax +
+   (8 * B + 24 * C)) / 6;
 }
 else
 {
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 20/22] pixman-image: Detect all 8 transforms that can do nearest filter

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This patch consolidates the examination of the matrix into one place, and 
detects
the reflected versions of the transforms that can be done with nearest filter.

v14: Split this code from the GOOD/BEST as I think it will be accepted.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-image.c | 119 --
 1 file changed, 57 insertions(+), 62 deletions(-)

diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 1ff1a49..8ad2891 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -266,52 +266,82 @@ compute_image_info (pixman_image_t *image)
 {
 pixman_format_code_t code;
 uint32_t flags = 0;
+int nearest_ok = FALSE;
+pixman_fixed_t (*m)[3];
 
 /* Transform */
 if (!image->common.transform)
 {
+   nearest_ok = TRUE;
flags |= (FAST_PATH_ID_TRANSFORM|
  FAST_PATH_X_UNIT_POSITIVE |
  FAST_PATH_Y_UNIT_ZERO |
  FAST_PATH_AFFINE_TRANSFORM);
+   m = 0;
 }
 else
 {
+   m = image->common.transform->matrix;
+
flags |= FAST_PATH_HAS_TRANSFORM;
 
-   if (image->common.transform->matrix[2][0] == 0  &&
-   image->common.transform->matrix[2][1] == 0  &&
-   image->common.transform->matrix[2][2] == pixman_fixed_1)
+   if (m[2][0] == 0&&
+   m[2][1] == 0&&
+   m[2][2] == pixman_fixed_1)
{
flags |= FAST_PATH_AFFINE_TRANSFORM;
 
-   if (image->common.transform->matrix[0][1] == 0 &&
-   image->common.transform->matrix[1][0] == 0)
+   if (m[0][1] == 0 &&
+   m[1][0] == 0)
{
-   if (image->common.transform->matrix[0][0] == -pixman_fixed_1 &&
-   image->common.transform->matrix[1][1] == -pixman_fixed_1)
+   flags |= FAST_PATH_SCALE_TRANSFORM;
+   if (abs(m[0][0]) == pixman_fixed_1 &&
+   abs(m[1][1]) == pixman_fixed_1)
{
-   flags |= FAST_PATH_ROTATE_180_TRANSFORM;
+   /* no scaling */
+   nearest_ok = TRUE;
+   if (m[0][0] < 0 && m[1][1] < 0)
+   flags |= FAST_PATH_ROTATE_180_TRANSFORM;
}
-   flags |= FAST_PATH_SCALE_TRANSFORM;
}
-   else if (image->common.transform->matrix[0][0] == 0 &&
-image->common.transform->matrix[1][1] == 0)
+   else if (m[0][0] == 0 &&
+m[1][1] == 0)
{
-   pixman_fixed_t m01 = image->common.transform->matrix[0][1];
-   pixman_fixed_t m10 = image->common.transform->matrix[1][0];
+   /* x/y axis are swapped, 90 degree rotation */
+   if (abs(m[0][1]) == pixman_fixed_1 &&
+   abs(m[1][0]) == pixman_fixed_1)
+   {
+   /* no scaling */
+   nearest_ok = TRUE;
+   if (m[0][1] < 0 && m[1][0] > 0)
+   flags |= FAST_PATH_ROTATE_90_TRANSFORM;
+   else if (m[0][1] > 0 && m[1][0] < 0)
+   flags |= FAST_PATH_ROTATE_270_TRANSFORM;
+   }
+   }
 
-   if (m01 == -pixman_fixed_1 && m10 == pixman_fixed_1)
-   flags |= FAST_PATH_ROTATE_90_TRANSFORM;
-   else if (m01 == pixman_fixed_1 && m10 == -pixman_fixed_1)
-   flags |= FAST_PATH_ROTATE_270_TRANSFORM;
+   if (nearest_ok)
+   {
+   /* reject non-integer translation: */
+   if (pixman_fixed_frac (m[0][2] | m[1][2]))
+   nearest_ok = FALSE;
+   /* FIXME: there are some affine-test failures, showing
+* that handling of BILINEAR and NEAREST filter is not
+* quite equivalent when getting close to 32K for the
+* translation components of the matrix. That's likely
+* some bug, but for now just skip BILINEAR->NEAREST
+* optimization in this case.
+*/
+   else if (abs(m[0][2]) > pixman_int_to_fixed (3) ||
+abs(m[1][2]) > pixman_int_to_fixed (3))
+   nearest_ok = FALSE;
}
}
 
-   if (image->common.transform->matrix[0][0] > 0)
+   if (m[0][0] > 0)
flags |= FAST_PATH_X_UNIT_POSITIVE;
 
-   if (image->common.transform->matrix[1][0] == 0)
+   if (m[1][0] == 0)
flags |= FAST_PAT

[Pixman] [PATCH v14 13/22] pixman-filter: integral splitting is only needed for triangle filter

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Only the triangle is discontinuous at 0. The other filters resemble a cubic 
closely
enough that Simpsons integration works without splitting.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-filter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index f9ad45f..6d589c2 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -176,13 +176,13 @@ integral (pixman_kernel_t kernel1,
  * as LINEAR that are not differentiable at 0 will still
  * integrate properly.
  */
-else if (low < 0 && high > 0)
+else if (kernel1 == PIXMAN_KERNEL_LINEAR && low < 0 && high > 0)
 {
return
integral (kernel1, kernel2, scale, pos, low, 0) +
integral (kernel1, kernel2, scale, pos, 0, high);
 }
-else if (low < pos && high > pos)
+else if (kernel2 == PIXMAN_KERNEL_LINEAR && low < pos && high > pos)
 {
return
integral (kernel1, kernel2, scale, pos, low, pos) +
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 08/22] pixman-filter: rename "scale" to "size" when it is 1/scale

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This is to remove some confusion when reading the code. "scale" gets larger
as the picture gets larger, while "size" (ie the size of the filter) gets
smaller.

v14: Removed changes to integral function

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 18 +-
 pixman/pixman.h|  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index a29116a..c03a7f6 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -221,7 +221,7 @@ static void
 create_1d_filter (int  width,
  pixman_kernel_t  reconstruct,
  pixman_kernel_t  sample,
- double   scale,
+ double   size,
  int  n_phases,
  pixman_fixed_t *p)
 {
@@ -251,8 +251,8 @@ create_1d_filter (int  width,
double pos = x + 0.5 - frac;
double rlow = - filters[reconstruct].width / 2.0;
double rhigh = rlow + filters[reconstruct].width;
-   double slow = pos - scale * filters[sample].width / 2.0;
-   double shigh = slow + scale * filters[sample].width;
+   double slow = pos - size * filters[sample].width / 2.0;
+   double shigh = slow + size * filters[sample].width;
double c = 0.0;
double ilow, ihigh;
 
@@ -262,7 +262,7 @@ create_1d_filter (int  width,
ihigh = MIN (shigh, rhigh);
 
c = integral (reconstruct, ilow,
- sample, 1.0 / scale, ilow - pos,
+ sample, 1.0 / size, ilow - pos,
  ihigh - ilow);
}
 
@@ -335,12 +335,12 @@ filter_width (pixman_kernel_t reconstruct, 
pixman_kernel_t sample, double size)
 }
 
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
- * with the given kernels and scale parameters
+ * with the given kernels and size parameters
  */
 PIXMAN_EXPORT pixman_fixed_t *
 pixman_filter_create_separable_convolution (int *n_values,
-   pixman_fixed_t   scale_x,
-   pixman_fixed_t   scale_y,
+   pixman_fixed_t   size_x,
+   pixman_fixed_t   size_y,
pixman_kernel_t  reconstruct_x,
pixman_kernel_t  reconstruct_y,
pixman_kernel_t  sample_x,
@@ -348,8 +348,8 @@ pixman_filter_create_separable_convolution (int 
*n_values,
int  subsample_bits_x,
int  subsample_bits_y)
 {
-double sx = fabs (pixman_fixed_to_double (scale_x));
-double sy = fabs (pixman_fixed_to_double (scale_y));
+double sx = fabs (pixman_fixed_to_double (size_x));
+double sy = fabs (pixman_fixed_to_double (size_y));
 pixman_fixed_t *params;
 int subsample_x, subsample_y;
 int width, height;
diff --git a/pixman/pixman.h b/pixman/pixman.h
index 509ba5e..b012a33 100644
--- a/pixman/pixman.h
+++ b/pixman/pixman.h
@@ -845,12 +845,12 @@ typedef enum
 } pixman_kernel_t;
 
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
- * with the given kernels and scale parameters.
+ * with the given kernels and size parameters.
  */
 pixman_fixed_t *
 pixman_filter_create_separable_convolution (int *n_values,
-   pixman_fixed_t   scale_x,
-   pixman_fixed_t   scale_y,
+   pixman_fixed_t   size_x,
+   pixman_fixed_t   size_y,
pixman_kernel_t  reconstruct_x,
pixman_kernel_t  reconstruct_y,
pixman_kernel_t  sample_x,
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 16/22] pixman-filter: distribute normalization error over filter

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This removes a high-frequency spike in the middle of some filters that is
caused by math errors all being in the same direction.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-filter.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 36dd811..ab62e0a 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -282,8 +282,18 @@ create_1d_filter (int  width,
p[x] = t;
}
 
+   /* Distribute any remaining error over all samples */
if (new_total != pixman_fixed_1)
-   p[width / 2] += (pixman_fixed_1 - new_total);
+   {
+   pixman_fixed_t delta = new_total - pixman_fixed_1;
+   pixman_fixed_t t = 0;
+   for (x = 0; x < width; ++x)
+   {
+   pixman_fixed_t new_t = delta * (x + 1) / width;
+   p[x] += new_t - t;
+   t = new_t;
+   }
+   }
 
p += width;
 }
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14] Implement PIXMAN_FILTER_GOOD/BEST

[spitzak]

Changes from previous version:

* The actual change to filtering separated from other changes that I think
  have more chance at being accepted.
* Changes to integral function split into mulitple commits
* Whitespace fixes
* Good/best patch has fix for bug noticed by Søren

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

The position of only one subsample was wrong as ceil() was done on an integer.
Use a different function for all odd numbers of subsamples that gets this right.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-filter.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index ac89dda..f9ad45f 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -252,7 +252,10 @@ create_1d_filter (int  width,
 * and sample positions.
 */
 
-   pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
+   if (n_phases & 1)
+   pos = frac - width / 2.0;
+   else
+   pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
 
total = 0;
for (x = 0; x < width; ++x, ++pos)
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 13/14] pixman-image: Implement PIXMAN_FILTER_GOOD/BEST as separable convolutions

[Bill Spitzak]

I believe on the good/best patch you are worried about the freeing of 
the filter array. However in commit 756b54f6 you made pixman_set_filter 
always copy the one provided by the caller. This means my code works 
because the filter cannot be changed away from/to good/best without also 
freeing the previous data.


The commit just says "Add boolean returns to various setters" so I am 
wondering if this is a mistake. There is also a comparison of the 
old/new pointers which won't work if the filter array is copied.


___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 14/14] demos/scale: default to GOOD and locked-together axis

[Bill Spitzak]

On Fri, Mar 4, 2016 at 4:19 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
> From: Bill Spitzak <spit...@gmail.com>
>>
>> This makes the demo match normal behavior of pixman/cairo at startup.
>>
>
> Defaulting to 'locked' makes sense, but in the light of the comments to
> the earlier patches, defaulting to GOOD doesn't.
>

Since GOOD is the default for Cairo/Pixman, I think it does make sense.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 13/14] pixman-image: Implement PIXMAN_FILTER_GOOD/BEST as separable convolutions

[Bill Spitzak]

On Fri, Mar 4, 2016 at 4:17 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> Detects and uses PIXMAN_FILTER_NEAREST for all 8 90-degree rotations and
>> reflections when the scale is 1.0 and integer translation.
>>
>> GOOD uses:
>>
>>  scale < 1/16 : BOX.BOX at size 16
>>  scale < 3/4 : BOX.BOX at size 1/scale
>>  larger : BOX.BOX at size 1
>>
>>  If both directions have a scale >= 3/4 or a scale of 1/2 and an integer
>>  translation, the faster PIXMAN_FILTER_BILINEAR code is used. This is
>>  compatable at these scales with older versions of pixman where bilinear
>>  was always used for GOOD.
>>
>> BEST uses:
>>
>>  scale < 1/24 : BOX.BOX at size 24
>>  scale < 1/16 : BOX.BOX at size 1/scale
>>  scale < 1 : IMPULSE.LANCZOS2 at size 1/scale
>>  scale < 2.333 : IMPULSE.LANCZOS2 at size 1
>>  scale < 128 : BOX.LANCZOS2 at size 1/(scale-1) (antialiased square
>> pixels)
>>  larger : BOX.LANCZOS2 at size 1/127 (antialias blur gets thicker)
>>
>> v8: Cutoff in BEST between IMPULSE.LANCZOS2 and BOX.LANCZOS2 adjusted for
>> a better match between the filters.
>>
>> v9: Use the new negative subsample controls to scale the subsamples. These
>> were chosen by finding the lowest number that did not add visible
>> artifacts to the zone plate image.
>>
>> Scale demo altered to default to GOOD and locked-together x+y scale
>>
>> Fixed divide-by-zero from all-zero matrix found by stress-test
>>
>> v11: Whitespace and formatting fixes
>>  Moved demo changes to a later patch
>>
>> v12: Whitespace and formatting fixes
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>>
>
> The short answer to this patch is NACK - it doesn't make sense to change
> the meaning of GOOD and BEST.
>

Okay that is going to need a big WTF?

There are at least three reasons:
>
> 1. Pixman is a low-level API that is in the camp of "do what I say" not
> "do what I mean". When users such as cairo specify GOOD and BEST, they are
> essentially asking Pixman to make a tradeoff that it doesn't have the
> knowledge to do. As such, I think it was a mistake to have these values in
> the first place (and indeed also a mistake to have them in the Render
> extension since X11 is also supposed to be a do-what-I-say API). As such, I
> think they should remain aliases to BILINEAR as they are now.
>

I have no idea why having three ways to say BILINEAR is necessary to "do
what I say". One seems sufficient.

2. The motivation for doing this in Pixman (and not in cairo) is that
> supposedly the X server will then pick it up automatically and use the
> high-quality filters when PictFilterBest and PictFilterGood is set. But
> that doesn't actually work because the X server doesn't map those Render
> filters to the corresponding Pixman filters. See:
>
>  https://cgit.freedesktop.org/xorg/xserver/tree/fb/fbpict.c#n420
>
> So it looks to me like the whole scheme just won't work. But even if it
> did, you would still need to make all the hardware drivers do the same
> thing, so it's not just a matter of adding some constants in that function.
>

If X is not currently sending GOOD/BEST through, then this is even a better
reason why it is ok to change them, as it cannot possibly change the
behavior of any X programs. All other pixman-using programs are directly
linked with the library. I did figure X would remain broken for long after
this is fixed, and Cairo would use the image backend for such transforms
until X is updated, or it is replaced with Wayland (where the image backend
is used all the time).

The way to go is to add a new SeparateConvolution filter to Render and make
> fb call into Pixman to implement it. Once that is in place, the hardware
> drivers can then implement it.
>

You seem to think the hardware drivers will implement this version of
SeparateConvolution? Really???

The purpose of this was so that the hardware drivers could implement
something they call "good" and something they call "best" using whatever
hardware resources are available. This is not going to happen if they are
expected to read the filter array.

3. The patch is completely broken as written. I could go into details, but
> there is no point since I don't think the patch should be pushed even if
> fixed. I'll just point out that if the environment variable
> PIXMAN_DISABLE=fast is set, the patch does absolutely nothing (you can
> verify this with the scale program). And setting this envi

Re: [Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

[Bill Spitzak]

On Fri, Mar 4, 2016 at 3:49 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Tue, Feb 16, 2016 at 4:45 PM, Bill Spitzak <spit...@gmail.com> wrote:
>
>> I have a new version of this patch, which fixes a math error that
>> caused it to produce too many samples at small sizes (large scales). I
>> am not clear if I can submit a v14 of just this patch or I should
>> instead submit a v14 of the entire patch series.
>>
>
> I don't think this functionality belongs in pixman at all. Computing the
> number of subsamples is essentially a performance/quality tradeoff, which
> is something that cairo and other clients need to make for themselves. In
> general, pixman is not the place for heuristics; its API should be
> predictable and explicit.
>
> So, NACK from me on this patch.
>

The scale demo is almost useless without this as you have to adjust this
every time you change the scale.

Most software I am familiar with use a fixed number of samples for the
width of the filter, not per integer. This was an attempt to simulate that.

I guess the scale demo could do this math, though that makes it rather
difficult to use the scale demo to figure out the best settings.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 10/14] pixman-filter: Gaussian fixes

[Bill Spitzak]

On Fri, Mar 4, 2016 at 3:39 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

>
>
> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> The SIGMA term drops out on simplification.
>>
>
> But SIGMA has a meaning here: it's the standard deviation for the normal
> distribution corresponding to the Gaussian in question, so unless there is
> some good argument for reducing the formula, I think it should stay the way
> it is.
>

The simplification does not happen due to inexact math.



>
> Expanding the size is fine with me
>

This is based on checking other software for the widths it uses for such
filters.

There is a bit of hf at the cutoffs, enlarging the size reduces this. It is
very common for "gaussian" to actually be this function multiplied by a
sinc window to avoid this, but I did not think that change was a good idea.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 08/14] pixman-filter: Corrections to the integral() function

[Bill Spitzak]

On Fri, Mar 4, 2016 at 3:12 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> The IMPULSE special-cases did not sample the center of the of the region.
>> This caused it to sample the filters outside their range, and produce
>> assymetric filters and other errors. Fixing this required changing the
>> arguments to integral() so the correct point could be determined.
>>
>> I fixed the nice filter and the integration to directly produce normalized
>> values. Re-normalization is still needed for impulse.box or
>> impulse.triangle
>> so I did not remove it.
>>
>> Distribute fixed error over all filter samples, to remove a high-frequency
>> bit of noise in the center of some filters (lancoz at large scale value).
>>
>> box.box, which I expect will be very common as it is the proposed "good"
>> filter,
>> was made a lot faster and more accurate. This is easy as the caller
>> already
>> intersected the two boxes, so the width is the integral.
>>
>> v7: This is a merge of 4 patches and lots of new code cleanup and fixes
>>  determined by examining the gnuplot output
>>
>
> Please split this back out into separate patches that each fix one problem:
>
> - Fixing IMPULSE special cases
> - Distributing errors evenly
> - BOX.BOX speedups
>
> There are several of these changes I'm not convinced about, and they are
> hard to review as one big patch.
>

I will try to split them up. This is the unfortunate result of trying to
rebase them together, I merged because of conflicts. As you don't like the
argument renaming I might as well fix all of these at the same time.

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>
>> index 8b8fb82..e82871f 100644
>> --- a/pixman/pixman-filter.c
>> +++ b/pixman/pixman-filter.c
>> @@ -79,7 +79,7 @@ sinc (double x)
>>  }
>>
>>  static double
>> -lanczos (double x, int n)
>> +lanczos (double x, double n)
>>  {
>>  return sinc (x) * sinc (x * (1.0 / n));
>>  }
>>
>
> What is the point of this change? I don't think Lanczos makes sense with a
> non-integral number of lobes.
>

It removes the int->float cast, but I think this was mostly accidental.
Inline would probably remove the cast as well, and the division too.

@@ -99,7 +99,7 @@ lanczos3_kernel (double x)
>>  static double
>>  nice_kernel (double x)
>>  {
>> -return lanczos3_kernel (x * 0.75);
>> +return lanczos3_kernel (x * 0.75) * 0.75;
>>  }
>>
>
> I don't think this makes sense given that normalization is happening later
> anyway.
>

It is the only non-normalized one, and fixing this was very useful for
finding out whether bugs were in the normalization or in the filter
generators. Also it is plausable that future ones may be able to remove the
normalization, though I could not because of the box and triangle being so
non-continuous that they needed it.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 09/14] pixman-filter: Nested polynomial for cubic

[Bill Spitzak]

On Fri, Mar 4, 2016 at 3:05 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> v11: Restored range checks
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>> Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
>>
>
> What's the point of this? All this does is putting ax^2 outside of the
> parenthesis?
>

Nested form reduces the number of multiplications (from 23 to 18 in this
case), without changing how many additions are done. And compilers can take
advantage of ax+b instructions with this (I did not check if that actually
happened, however).
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 07/14] pixman-filter: Correct Simpsons integration

[Bill Spitzak]

On Fri, Mar 4, 2016 at 3:00 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> Simpsons uses cubic curve fitting, with 3 samples defining each cubic.
>> This
>> makes the weights of the samples be in a pattern of 1,4,2,4,2...4,1, and
>> then
>> dividing the result by 3.
>>
>> The previous code was using weights of 1,2,6,6...6,2,1. Since it divided
>> by
>> 3 this produced about 2x the desired value (the normalization fixed this).
>> Also this is effectively a linear interpolation, not Simpsons integration.
>>
>
> It is not true that the previous code used these weights because it only
> ran for half the segments (it had += 2). The intention with the code was to
> do both the 4 and the 2 weight in one loop, but for that to work a1 and a2
> would have to be updated, which the code didn't do. So it was indeed buggy,
> and I think the new code is correct.
>

You are right, the old code did weights of 1,2,0,6,0,6,...,2,1. This would
explain why it worked even when I removed normalization.

There should be spaces before the parentheses in the SAMPLE() macros,
> though.
>

Ok will fix.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 06/14] pixman-filter: reduce amount of malloc/free/memcpy to generate filter

[Bill Spitzak]

On Fri, Mar 4, 2016 at 2:52 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

>
>
> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> Rearranged so that the entire block of memory for the filter pair
>> is allocated first, and then filled in. Previous version allocated
>> and freed two temporary buffers for each filter and did an extra
>> memcpy.
>>
>> v8: small refactor to remove the filter_width function
>>
>> v10: Restored filter_width function but with arguments changed to
>>  match later patches
>>
>> v11: Removed unused arg and pointer from filter_width function
>>  Whitespace fixes.
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>> Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
>>
>
> I'm not opposed to this patch if it is correct (and I'll believe the
> existing Reviewed-by for correctness), but I'm also don't think there is
> much point to this -- these malloc() and free() calls are unlikely to be
> performance bottlenecks.
>
> Acked-by: Soren Sandmann <soren.sandm...@gmail.com>
>

This is certainly correct, it would have been obvious in my tests if it did
not work.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 05/14] pixman-filter: Consistency in arg names to integral ()

[Bill Spitzak]

On Fri, Mar 4, 2016 at 2:51 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> Rename kernel1/2 to reconstruct/sample to match the other functions.
>> Rename "scale" to "size" to avoid confusion with the scale being applied
>> to the image, which is the reciprocol of this value.
>>
>> v10: Renamed "scale" to "size"
>>
>
> I don't really agree with this patch. The intention behind the code was
> that the integral() function shouldn't know anything about scaling or
> images; all it would do was compute integrals over functions, but it
> wouldn't know or care what those functions were used for.
>

Okay will keep the arguments as before. In fact "scale" is used as expected
in the old version of the code (larger numbers mean the picture is being
enlarged). I still think renaming kernel1/2 to be the same name used in
other functions makes things a lot easier to follow.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 03/14] demos/scale: Only generate filters when used for separable

[Bill Spitzak]

On Fri, Mar 4, 2016 at 2:44 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

>
>
> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> This makes the speed of the demo more accurate, as the filter generation
>> is a visible fraction of the time it takes to do a transform. This also
>> prevents the output of unused filters in the gnuplot option in the next
>> patch.
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>> Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
>>
>
> This depends on patch 2, which I don't think should be accepted. However,
> if patch 2 is pushed, this one is an improvement that should also be
> pushed. It would make sense to squash these two patches and also include
> the gray-out functionality in that squashed patch.
>

Even without patch 2 it could be set to bilinear, so this actually is
useful. You are probably right that it does not apply cleanly without 2.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 02/14] demos/scale: Added pulldown to choose PIXMAN_FILTER_* value

[Bill Spitzak]

On Fri, Mar 4, 2016 at 2:41 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:

> On Wed, Feb 10, 2016 at 1:25 AM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> This allows testing of GOOD/BEST and to do comparisons between
>> the basic filters and PIXMAN_FILTER_SEPARABLE_CONVOLUTION settings.
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>> Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
>>
>
>
> Given that I don't think the GOOD/BEST/FAST filters should be changed (see
> upcoming comments to patch 13), there is not much point to this patch.
> However, if it ends up going in, the non-filter settings should certainly
> be grayed out when the filter is not SEPARABLE since they have no effect in
> that case. (Also, of course, if the GOOD/BEST/FAST filters stay as simple
> aliases, as they should, only NEAREST and LINEAR make sense in the dropdown
> here).
>

The ability to adjust the settings for separable while it was set to other
filters was very useful, actually. Is there a way in GTK to gray out a
widget while not making it not work? If not I guess disabling them will
work.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

[Bill Spitzak]

On 02/16/2016 01:58 PM, Oded Gabbay wrote:

On Tue, Feb 16, 2016 at 11:45 PM, Bill Spitzak <spit...@gmail.com> wrote:

I have a new version of this patch, which fixes a math error that
caused it to produce too many samples at small sizes (large scales). I
am not clear if I can submit a v14 of just this patch or I should
instead submit a v14 of the entire patch series.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman


My suggestion is to wait for Soren's review (I believe we will see
something around the start of next week) and when you fix his
comments, add this fix as well and then mark it as v14.

Oded


Have you been able to contact Søren?


___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v13 08/14] pixman-filter: Corrections to the integral() function

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

The IMPULSE special-cases did not sample the center of the of the region.
This caused it to sample the filters outside their range, and produce
assymetric filters and other errors. Fixing this required changing the
arguments to integral() so the correct point could be determined.

I fixed the nice filter and the integration to directly produce normalized
values. Re-normalization is still needed for impulse.box or impulse.triangle
so I did not remove it.

Distribute fixed error over all filter samples, to remove a high-frequency
bit of noise in the center of some filters (lancoz at large scale value).

box.box, which I expect will be very common as it is the proposed "good" filter,
was made a lot faster and more accurate. This is easy as the caller already
intersected the two boxes, so the width is the integral.

v7: This is a merge of 4 patches and lots of new code cleanup and fixes
 determined by examining the gnuplot output

v9: Restored the recursion splitting at zero for linear filter

v10: Small change from here moved to previous Simpsons patch so it compiles
 Merged patch to get correct subsample positions when subsample_bits==0

v11: Whitespace fixes

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Acked-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 154 +
 1 file changed, 79 insertions(+), 75 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 8b8fb82..e82871f 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -79,7 +79,7 @@ sinc (double x)
 }
 
 static double
-lanczos (double x, int n)
+lanczos (double x, double n)
 {
 return sinc (x) * sinc (x * (1.0 / n));
 }
@@ -99,7 +99,7 @@ lanczos3_kernel (double x)
 static double
 nice_kernel (double x)
 {
-return lanczos3_kernel (x * 0.75);
+return lanczos3_kernel (x * 0.75) * 0.75;
 }
 
 static double
@@ -147,45 +147,51 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
 };
 
-/* This function scales the @sample filter by @size, then
- * aligns @x1 in @reconstruct with @x2 in @sample and
- * and integrates the product of the kernels across @width.
+/* This function scales the @sample filter by @size, shifts it by @pos,
+ * and then integrates the product of the kernels across low..high
  *
  * This function assumes that the intervals are within
  * the kernels in question. E.g., the caller must not
  * try to integrate a linear kernel ouside of [-1:1]
  */
 static double
-integral (pixman_kernel_t reconstruct, double x1,
- pixman_kernel_t sample, double size, double x2,
- double width)
+integral (pixman_kernel_t reconstruct,
+ pixman_kernel_t sample, double size, double pos,
+ double low, double high)
 {
+if (high < low)
+{
+   return 0.0;
+}
+else if (sample == PIXMAN_KERNEL_IMPULSE)
+{
+   return filters[reconstruct].func (-pos);
+}
+else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
+{
+   return filters[sample].func (-pos / size) / size;
+}
+else if (reconstruct == PIXMAN_KERNEL_BOX && sample == PIXMAN_KERNEL_BOX)
+{
+   assert (high <= low + 1.0);
+   return (high - low) / size;
+}
 /* If the integration interval crosses zero, break it into
  * two separate integrals. This ensures that filters such
  * as LINEAR that are not differentiable at 0 will still
  * integrate properly.
  */
-if (x1 < 0 && x1 + width > 0)
+else if (reconstruct == PIXMAN_KERNEL_LINEAR && low < 0 && high > 0)
 {
return
-   integral (reconstruct, x1, sample, size, x2, - x1) +
-   integral (reconstruct, 0, sample, size, x2 - x1, width + x1);
+   integral (reconstruct, sample, size, pos, low, 0) +
+   integral (reconstruct, sample, size, pos, 0, high);
 }
-else if (x2 < 0 && x2 + width > 0)
+else if (sample == PIXMAN_KERNEL_LINEAR && low < pos && high > pos)
 {
return
-   integral (reconstruct, x1, sample, size, x2, - x2) +
-   integral (reconstruct, x1 - x2, sample, size, 0, width + x2);
-}
-else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
-{
-   assert (width == 0.0);
-   return filters[sample].func (x2 / size);
-}
-else if (sample == PIXMAN_KERNEL_IMPULSE)
-{
-   assert (width == 0.0);
-   return filters[reconstruct].func (x1);
+   integral (reconstruct, sample, size, pos, low, pos) +
+   integral (reconstruct, sample, size, pos, pos, high);
 }
 else
 {
@@ -197,32 +203,30 @@ integral (pixman_kernel_t reconstruct, double x1,
 * filter is 6 wide.
 */
 #define N_SEGMENTS 12
-#define SAMPLE(a1, a2) 

[Pixman] [PATCH v13 03/14] demos/scale: Only generate filters when used for separable

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This makes the speed of the demo more accurate, as the filter generation
is a visible fraction of the time it takes to do a transform. This also
prevents the output of unused filters in the gnuplot option in the next
patch.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 demos/scale.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index 06821e3..6d7ad2a 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -258,16 +258,25 @@ rescale (GtkWidget *may_be_null, app_t *app)
 pixman_transform_from_pixman_f_transform (, );
 pixman_image_set_transform (app->original, );
 
-params = pixman_filter_create_separable_convolution (
-_params,
-sx * 65536.0 + 0.5,
-   sy * 65536.0 + 0.5,
-   get_value (app, filters, "reconstruct_x_combo_box"),
-   get_value (app, filters, "reconstruct_y_combo_box"),
-   get_value (app, filters, "sample_x_combo_box"),
-   get_value (app, filters, "sample_y_combo_box"),
-   gtk_adjustment_get_value (app->subsample_adjustment),
-   gtk_adjustment_get_value (app->subsample_adjustment));
+if (get_value (app, filter_types, "filter_combo_box") ==
+   PIXMAN_FILTER_SEPARABLE_CONVOLUTION)
+{
+   params = pixman_filter_create_separable_convolution (
+   _params,
+   sx * 65536.0 + 0.5,
+   sy * 65536.0 + 0.5,
+   get_value (app, filters, "reconstruct_x_combo_box"),
+   get_value (app, filters, "reconstruct_y_combo_box"),
+   get_value (app, filters, "sample_x_combo_box"),
+   get_value (app, filters, "sample_y_combo_box"),
+   gtk_adjustment_get_value (app->subsample_adjustment),
+   gtk_adjustment_get_value (app->subsample_adjustment));
+}
+else
+{
+   params = 0;
+   n_params = 0;
+}
 
 pixman_image_set_filter (app->original,
get_value (app, filter_types, "filter_combo_box"),
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v13 13/14] pixman-image: Implement PIXMAN_FILTER_GOOD/BEST as separable convolutions

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Detects and uses PIXMAN_FILTER_NEAREST for all 8 90-degree rotations and
reflections when the scale is 1.0 and integer translation.

GOOD uses:

 scale < 1/16 : BOX.BOX at size 16
 scale < 3/4 : BOX.BOX at size 1/scale
 larger : BOX.BOX at size 1

 If both directions have a scale >= 3/4 or a scale of 1/2 and an integer
 translation, the faster PIXMAN_FILTER_BILINEAR code is used. This is
 compatable at these scales with older versions of pixman where bilinear
 was always used for GOOD.

BEST uses:

 scale < 1/24 : BOX.BOX at size 24
 scale < 1/16 : BOX.BOX at size 1/scale
 scale < 1 : IMPULSE.LANCZOS2 at size 1/scale
 scale < 2.333 : IMPULSE.LANCZOS2 at size 1
 scale < 128 : BOX.LANCZOS2 at size 1/(scale-1) (antialiased square pixels)
 larger : BOX.LANCZOS2 at size 1/127 (antialias blur gets thicker)

v8: Cutoff in BEST between IMPULSE.LANCZOS2 and BOX.LANCZOS2 adjusted for
a better match between the filters.

v9: Use the new negative subsample controls to scale the subsamples. These
were chosen by finding the lowest number that did not add visible
artifacts to the zone plate image.

Scale demo altered to default to GOOD and locked-together x+y scale

Fixed divide-by-zero from all-zero matrix found by stress-test

v11: Whitespace and formatting fixes
 Moved demo changes to a later patch

v12: Whitespace and formatting fixes

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-image.c | 327 ++
 1 file changed, 249 insertions(+), 78 deletions(-)

diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 1ff1a49..5f52dd7 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "pixman-private.h"
 
@@ -274,112 +275,282 @@ compute_image_info (pixman_image_t *image)
  FAST_PATH_X_UNIT_POSITIVE |
  FAST_PATH_Y_UNIT_ZERO |
  FAST_PATH_AFFINE_TRANSFORM);
+   switch (image->common.filter)
+   {
+   case PIXMAN_FILTER_CONVOLUTION:
+   break;
+   case PIXMAN_FILTER_SEPARABLE_CONVOLUTION:
+   flags |= FAST_PATH_SEPARABLE_CONVOLUTION_FILTER;
+   break;
+   default:
+   flags |= (FAST_PATH_NEAREST_FILTER | 
FAST_PATH_NO_CONVOLUTION_FILTER);
+   break;
+   }
 }
 else
 {
+   pixman_fixed_t (*m)[3] = image->common.transform->matrix;
+   double dx, dy;
+   int nearest_ok, bilinear_ok;
+
flags |= FAST_PATH_HAS_TRANSFORM;
 
-   if (image->common.transform->matrix[2][0] == 0  &&
-   image->common.transform->matrix[2][1] == 0  &&
-   image->common.transform->matrix[2][2] == pixman_fixed_1)
+   nearest_ok = FALSE;
+   bilinear_ok = FALSE;
+
+   if (m[2][0] == 0&&
+   m[2][1] == 0&&
+   m[2][2] == pixman_fixed_1)
{
+   /* no perspective */
flags |= FAST_PATH_AFFINE_TRANSFORM;
 
-   if (image->common.transform->matrix[0][1] == 0 &&
-   image->common.transform->matrix[1][0] == 0)
+   if (m[0][1] == 0 &&
+   m[1][0] == 0)
{
-   if (image->common.transform->matrix[0][0] == -pixman_fixed_1 &&
-   image->common.transform->matrix[1][1] == -pixman_fixed_1)
+   /* no tilt of either axis */
+   flags |= FAST_PATH_SCALE_TRANSFORM;
+   if (abs(m[0][0]) == pixman_fixed_1 &&
+   abs(m[1][1]) == pixman_fixed_1)
{
-   flags |= FAST_PATH_ROTATE_180_TRANSFORM;
+   /* no scaling */
+   nearest_ok = TRUE;
+   if (m[0][0] < 0 && m[1][1] < 0)
+   flags |= FAST_PATH_ROTATE_180_TRANSFORM;
}
-   flags |= FAST_PATH_SCALE_TRANSFORM;
}
-   else if (image->common.transform->matrix[0][0] == 0 &&
-image->common.transform->matrix[1][1] == 0)
+   else if (m[0][0] == 0 &&
+m[1][1] == 0)
{
-   pixman_fixed_t m01 = image->common.transform->matrix[0][1];
-   pixman_fixed_t m10 = image->common.transform->matrix[1][0];
-
-   if (m01 == -pixman_fixed_1 && m10 == pixman_fixed_1)
-   flags |= FAST_PATH_ROTATE_90_TRANSFORM;
-   else if (m01 == pixman_fixed_1 && m10 == -pixman_fixed_1)
-   flags |= FAST_PATH_ROTATE_270_TRANSFORM;
+   /* x/y axis are swapped, 90 degree rotation */
+   if 

[Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

The intention here is to produce approximately the same number of samples for
each filter size (ie width*samples is the same). This means the caller can
pass a constant rather than a different value for each size. To avoid conflict
with existing code, negative numbers are used to indicate that -n samples are
needed at size==1.

The original idea was to scale n by 1/size, but experiments led to it being
scaled by 2/((size+1)*min(size,1)). This approaches twice as many samples as
size goes to either zero or infinity. For larger sizes this is scaling by the
width of a BOX.BOX filter, but for smaller size I don't have a good
explanation.

This function was arrived at experimentally by testing the scaling of the zone
plate image rotated slightly. The smallest number of samples that did not
produce more artifacts was plotted for various scales which led to the above
formula and a value of 12 for -n.

The computed value is then rounded up to the next power of 2 to get the
subsample_bits.

The scale demo is modified to allow these negative numbers, and initially
uses -12.

v11: Put subsample calculation in it's own function
 Minor changes to comments
v12: More info in the commit message
v13: Corrected mistakes in the commit message and comments about the scale 
factor

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Acked-by: Oded Gabbay <oded.gab...@gmail.com>
---
 demos/scale.ui |  5 +++--
 pixman/pixman-filter.c | 37 +
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/demos/scale.ui b/demos/scale.ui
index b62cbfb..1e77f56 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -24,12 +24,12 @@
 10
   
   
-0
+-256
 12
 1
 1
 0
-4
+-12
   
   
 
@@ -321,6 +321,7 @@
   
 True
subsample_adjustment
+   -12
   
   
 1
diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 56e25b1..cb9f72b 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -346,6 +346,41 @@ filter_width (pixman_kernel_t reconstruct, pixman_kernel_t 
sample, double size)
 return ceil (filters[reconstruct].width + size * filters[sample].width);
 }
 
+/* Turn negative number into ceil(ln2(-n * 2/(size+1)*min(size,1)))
+ *
+ * The original idea was to scale n by 1/size, but experiments led to
+ * it being scaled by 2/((size+1)*min(size,1)). This approaches twice
+ * as many samples as size goes to either zero or infinity. For larger
+ * sizes this is scaling by the width of a BOX.BOX filter, but for
+ * smaller size I don't have a good explanation.
+ *
+ * This function was arrived at experimentally by testing the scaling
+ * of the zone plate image rotated slightly. The smallest number of
+ * samples that did not produce more artifacts was plotted for various
+ * scales which led to the above formula and a value of 12 for -n.
+ */
+static int
+subsample_bits (int subsample_bits, pixman_kernel_t sample, double size)
+{
+if (subsample_bits < 0)
+{
+   double desired_samples = -subsample_bits;
+   if (sample == PIXMAN_KERNEL_IMPULSE)
+   ; /* For x.IMPULSE no scaling is done */
+   else if (size >= 1.0)
+   desired_samples *= 2.0 / (size + 1.0);
+   else
+   desired_samples *= 2.0 / ((size + 1.0) * size);
+   if (desired_samples <= 1.0)
+   subsample_bits = 0;
+   else if (desired_samples >= 256.0)
+   subsample_bits = 8;
+   else
+   subsample_bits = (int) ceil (log2(desired_samples) - .01);
+}
+return subsample_bits;
+}
+
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and size parameters
  */
@@ -367,9 +402,11 @@ pixman_filter_create_separable_convolution (int
 *n_values,
 int width, height;
 
 width = filter_width (reconstruct_x, sample_x, sx);
+subsample_bits_x = subsample_bits (subsample_bits_x, sample_x, sx);
 subsample_x = (1 << subsample_bits_x);
 
 height = filter_width (reconstruct_y, sample_y, sy);
+subsample_bits_y = subsample_bits (subsample_bits_y, sample_y, sy);
 subsample_y = (1 << subsample_bits_y);
 
 *n_values = 4 + width * subsample_x + height * subsample_y;
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v13 09/14] pixman-filter: Nested polynomial for cubic

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

v11: Restored range checks

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index e82871f..e5ef8e6 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -109,14 +109,16 @@ general_cubic (double x, double B, double C)
 
 if (ax < 1)
 {
-   return ((12 - 9 * B - 6 * C) * ax * ax * ax +
-   (-18 + 12 * B + 6 * C) * ax * ax + (6 - 2 * B)) / 6;
+   return (((12 - 9 * B - 6 * C) * ax +
+(-18 + 12 * B + 6 * C)) * ax * ax +
+   (6 - 2 * B)) / 6;
 }
-else if (ax >= 1 && ax < 2)
+else if (ax < 2)
 {
-   return ((-B - 6 * C) * ax * ax * ax +
-   (6 * B + 30 * C) * ax * ax + (-12 * B - 48 * C) *
-   ax + (8 * B + 24 * C)) / 6;
+   return -B - 6 * C) * ax +
+ (6 * B + 30 * C)) * ax +
+(-12 * B - 48 * C)) * ax +
+   (8 * B + 24 * C)) / 6;
 }
 else
 {
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v13 05/14] pixman-filter: Consistency in arg names to integral ()

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Rename kernel1/2 to reconstruct/sample to match the other functions.
Rename "scale" to "size" to avoid confusion with the scale being applied
to the image, which is the reciprocol of this value.

v10: Renamed "scale" to "size"

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 48 +++-
 pixman/pixman.h|  6 +++---
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 3981c8b..b70da1f 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -147,8 +147,8 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
 };
 
-/* This function scales @kernel2 by @scale, then
- * aligns @x1 in @kernel1 with @x2 in @kernel2 and
+/* This function scales the @sample filter by @size, then
+ * aligns @x1 in @reconstruct with @x2 in @sample and
  * and integrates the product of the kernels across @width.
  *
  * This function assumes that the intervals are within
@@ -156,8 +156,8 @@ static const filter_info_t filters[] =
  * try to integrate a linear kernel ouside of [-1:1]
  */
 static double
-integral (pixman_kernel_t kernel1, double x1,
- pixman_kernel_t kernel2, double scale, double x2,
+integral (pixman_kernel_t reconstruct, double x1,
+ pixman_kernel_t sample, double size, double x2,
  double width)
 {
 /* If the integration interval crosses zero, break it into
@@ -168,31 +168,31 @@ integral (pixman_kernel_t kernel1, double x1,
 if (x1 < 0 && x1 + width > 0)
 {
return
-   integral (kernel1, x1, kernel2, scale, x2, - x1) +
-   integral (kernel1, 0, kernel2, scale, x2 - x1, width + x1);
+   integral (reconstruct, x1, sample, size, x2, - x1) +
+   integral (reconstruct, 0, sample, size, x2 - x1, width + x1);
 }
 else if (x2 < 0 && x2 + width > 0)
 {
return
-   integral (kernel1, x1, kernel2, scale, x2, - x2) +
-   integral (kernel1, x1 - x2, kernel2, scale, 0, width + x2);
+   integral (reconstruct, x1, sample, size, x2, - x2) +
+   integral (reconstruct, x1 - x2, sample, size, 0, width + x2);
 }
-else if (kernel1 == PIXMAN_KERNEL_IMPULSE)
+else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
 {
assert (width == 0.0);
-   return filters[kernel2].func (x2 * scale);
+   return filters[sample].func (x2 / size);
 }
-else if (kernel2 == PIXMAN_KERNEL_IMPULSE)
+else if (sample == PIXMAN_KERNEL_IMPULSE)
 {
assert (width == 0.0);
-   return filters[kernel1].func (x1);
+   return filters[reconstruct].func (x1);
 }
 else
 {
/* Integration via Simpson's rule */
 #define N_SEGMENTS 128
 #define SAMPLE(a1, a2) \
-   (filters[kernel1].func ((a1)) * filters[kernel2].func ((a2) * scale))
+   (filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / size))

double s = 0.0;
double h = width / (double)N_SEGMENTS;
@@ -221,16 +221,14 @@ static pixman_fixed_t *
 create_1d_filter (int *width,
  pixman_kernel_t  reconstruct,
  pixman_kernel_t  sample,
- double   scale,
+ double   size,
  int  n_phases)
 {
 pixman_fixed_t *params, *p;
 double step;
-double size;
 int i;
 
-size = scale * filters[sample].width + filters[reconstruct].width;
-*width = ceil (size);
+*width = ceil (size * filters[sample].width + filters[reconstruct].width);
 
 p = params = malloc (*width * n_phases * sizeof (pixman_fixed_t));
 if (!params)
@@ -259,8 +257,8 @@ create_1d_filter (int *width,
double pos = x + 0.5 - frac;
double rlow = - filters[reconstruct].width / 2.0;
double rhigh = rlow + filters[reconstruct].width;
-   double slow = pos - scale * filters[sample].width / 2.0;
-   double shigh = slow + scale * filters[sample].width;
+   double slow = pos - size * filters[sample].width / 2.0;
+   double shigh = slow + size * filters[sample].width;
double c = 0.0;
double ilow, ihigh;
 
@@ -270,7 +268,7 @@ create_1d_filter (int *width,
ihigh = MIN (shigh, rhigh);
 
c = integral (reconstruct, ilow,
- sample, 1.0 / scale, ilow - pos,
+ sample, size, ilow - pos,
  ihigh - ilow);
}
 
@@ -339,12 +337,12 @@ gnuplot_filter(int width, int samples, const 
pixman_fixed_t* p)
 #endif
 
 /* Create the parameter l

[Pixman] [PATCH v13 06/14] pixman-filter: reduce amount of malloc/free/memcpy to generate filter

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Rearranged so that the entire block of memory for the filter pair
is allocated first, and then filled in. Previous version allocated
and freed two temporary buffers for each filter and did an extra
memcpy.

v8: small refactor to remove the filter_width function

v10: Restored filter_width function but with arguments changed to
 match later patches

v11: Removed unused arg and pointer from filter_width function
 Whitespace fixes.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 55 +-
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index b70da1f..69ac3ab 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -217,23 +217,17 @@ integral (pixman_kernel_t reconstruct, double x1,
 }
 }
 
-static pixman_fixed_t *
-create_1d_filter (int *width,
+static void
+create_1d_filter (int  width,
  pixman_kernel_t  reconstruct,
  pixman_kernel_t  sample,
  double   size,
- int  n_phases)
+ int  n_phases,
+ pixman_fixed_t *p)
 {
-pixman_fixed_t *params, *p;
 double step;
 int i;
 
-*width = ceil (size * filters[sample].width + filters[reconstruct].width);
-
-p = params = malloc (*width * n_phases * sizeof (pixman_fixed_t));
-if (!params)
-return NULL;
-
 step = 1.0 / n_phases;
 
 for (i = 0; i < n_phases; ++i)
@@ -248,8 +242,8 @@ create_1d_filter (int *width,
 * and sample positions.
 */
 
-   x1 = ceil (frac - *width / 2.0 - 0.5);
-x2 = x1 + *width;
+   x1 = ceil (frac - width / 2.0 - 0.5);
+   x2 = x1 + width;
 
total = 0;
 for (x = x1; x < x2; ++x)
@@ -277,7 +271,7 @@ create_1d_filter (int *width,
 }
 
/* Normalize */
-   p -= *width;
+   p -= width;
 total = 1 / total;
 new_total = 0;
for (x = x1; x < x2; ++x)
@@ -289,10 +283,8 @@ create_1d_filter (int *width,
}
 
if (new_total != pixman_fixed_1)
-   *(p - *width / 2) += (pixman_fixed_1 - new_total);
+   *(p - width / 2) += (pixman_fixed_1 - new_total);
 }
-
-return params;
 }
 
 #ifdef PIXMAN_GNUPLOT
@@ -336,6 +328,12 @@ gnuplot_filter(int width, int samples, const 
pixman_fixed_t* p)
 }
 #endif
 
+static int
+filter_width (pixman_kernel_t reconstruct, pixman_kernel_t sample, double size)
+{
+return ceil (filters[reconstruct].width + size * filters[sample].width);
+}
+
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and size parameters
  */
@@ -352,42 +350,35 @@ pixman_filter_create_separable_convolution (int   
  *n_values,
 {
 double sx = fabs (pixman_fixed_to_double (size_x));
 double sy = fabs (pixman_fixed_to_double (size_y));
-pixman_fixed_t *horz = NULL, *vert = NULL, *params = NULL;
+pixman_fixed_t *params;
 int subsample_x, subsample_y;
 int width, height;
 
+width = filter_width (reconstruct_x, sample_x, sx);
 subsample_x = (1 << subsample_bits_x);
-subsample_y = (1 << subsample_bits_y);
 
-horz = create_1d_filter (, reconstruct_x, sample_x, sx, subsample_x);
-vert = create_1d_filter (, reconstruct_y, sample_y, sy, 
subsample_y);
+height = filter_width (reconstruct_y, sample_y, sy);
+subsample_y = (1 << subsample_bits_y);
 
-if (!horz || !vert)
-goto out;
-
 *n_values = 4 + width * subsample_x + height * subsample_y;
 
 params = malloc (*n_values * sizeof (pixman_fixed_t));
 if (!params)
-goto out;
+   return NULL;
 
 params[0] = pixman_int_to_fixed (width);
 params[1] = pixman_int_to_fixed (height);
 params[2] = pixman_int_to_fixed (subsample_bits_x);
 params[3] = pixman_int_to_fixed (subsample_bits_y);
 
-memcpy (params + 4, horz,
-   width * subsample_x * sizeof (pixman_fixed_t));
-memcpy (params + 4 + width * subsample_x, vert,
-   height * subsample_y * sizeof (pixman_fixed_t));
+create_1d_filter (width, reconstruct_x, sample_x, sx, subsample_x,
+ params + 4);
+create_1d_filter (height, reconstruct_y, sample_y, sy, subsample_y,
+ params + 4 + width * subsample_x);
 
 #ifdef PIXMAN_GNUPLOT
 gnuplot_filter(width, subsample_x, params+4);
 #endif
 
-out:
-free (horz);
-free (vert);
-
 return params;
 }
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v13] Implement PIXMAN_FILTER_GOOD/BEST

[spitzak]

Changes since previous version:
- Corrected mistaken reviewd-by to acked-by
- Added another acked-by
- Corrected errors in comments about the negative subsample patch

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 11/14] pixman-filter: Negative subsample values produces scaled result

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

The intention here is to produce approximately the same number of samples for
each filter size (ie width*samples is the same). This means the caller can
pass a constant rather than a different value for each size. To avoid conflict
with existing code, negative numbers are used to indicate that -n samples are
needed at size==1.

For larger size the width of a BOX.BOX filter is used (the number of samples
is scaled by 2/(size+1)). This may be more than are needed for other filters
which increase in width faster.

For smaller filters it seems 1/size would be needed to keep the same number
of samples on the high-frequency portions. But it appears to be acceptable to
reduce them, I used 2/((size+1)*size) which makes about 1/2 as many samples
as size approaches zero.

These functions were arrived at experimentally by testing for visible
artifacts in the scaling of the zone plate image.

The computed value is then rounded up to the next power of 2 to get the
subsample_bits.

The scale demo is modified to allow these negative numbers, and initially
uses -12.

v11: Put subsample calculation in it's own function
 Minor changes to comments
v12: More info in the commit message

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 demos/scale.ui |  5 +++--
 pixman/pixman-filter.c | 33 +
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/demos/scale.ui b/demos/scale.ui
index b62cbfb..1e77f56 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -24,12 +24,12 @@
 10
   
   
-0
+-256
 12
 1
 1
 0
-4
+-12
   
   
 
@@ -321,6 +321,7 @@
   
 True
subsample_adjustment
+   -12
   
   
 1
diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 56e25b1..e57b154 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -346,6 +346,37 @@ filter_width (pixman_kernel_t reconstruct, pixman_kernel_t 
sample, double size)
 return ceil (filters[reconstruct].width + size * filters[sample].width);
 }
 
+/* Turn negative number into approximately ceil(ln2(-n / size))
+ * Actual function is somewhat non-linear. For size > 1 it uses
+ * the width/2 of BOX.BOX instead of size. For size < 1 it reduces
+ * the samples by 1/2 as size approaches zero.
+ */
+static int
+subsample_bits (int subsample_bits, pixman_kernel_t sample, double size)
+{
+if (subsample_bits < 0)
+{
+   /* The intention was to do -n / size rounded up to the next power of 2,
+  but this non-linear function seems to work better. For large size
+  it is the width of the BOX.BOX filter. For small size it reduces
+  samples by 2 at maximum. */
+   double desired_samples = -subsample_bits;
+   if (sample == PIXMAN_KERNEL_IMPULSE)
+   ; /* For x.IMPULSE no scaling is done */
+   else if (size >= 1.0)
+   desired_samples *= 2.0 / (size + 1.0);
+   else
+   desired_samples *= 2.0 / ((size + 1.0) * size);
+   if (desired_samples <= 1.0)
+   subsample_bits = 0;
+   else if (desired_samples >= 256.0)
+   subsample_bits = 8;
+   else
+   subsample_bits = (int) ceil (log2(desired_samples) - .01);
+}
+return subsample_bits;
+}
+
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and size parameters
  */
@@ -367,9 +398,11 @@ pixman_filter_create_separable_convolution (int
 *n_values,
 int width, height;
 
 width = filter_width (reconstruct_x, sample_x, sx);
+subsample_bits_x = subsample_bits (subsample_bits_x, sample_x, sx);
 subsample_x = (1 << subsample_bits_x);
 
 height = filter_width (reconstruct_y, sample_y, sy);
+subsample_bits_y = subsample_bits (subsample_bits_y, sample_y, sy);
 subsample_y = (1 << subsample_bits_y);
 
 *n_values = 4 + width * subsample_x + height * subsample_y;
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 12/14] pixman-filter: Add description to pixman_filter_create_separable_convolution()

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

v9: Described arguments and more filter combinations, fixed some errors.

v11: Further correction, in particular replaced "scale" with "size"

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Acked-by: Oded Gabbay <oded.gab...@redhat.com>
---
 pixman/pixman-filter.c | 51 ++
 1 file changed, 51 insertions(+)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index e57b154..d858cc6 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -379,6 +379,57 @@ subsample_bits (int subsample_bits, pixman_kernel_t 
sample, double size)
 
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and size parameters
+ *
+ * Returns pointer to the block of numbers
+ *
+ * n_values: pointer to integer to store size of the block
+ *
+ * size_x/y: amount to stretch the sample filter. The best value to
+ * use is the derivative of the sample location. This value is
+ * approximately 1/scale when scaling an image. If size < 1.0 then you
+ * will get square pixels as an image is enlarged, using 1.0 instead
+ * will get the more customary "blurry" enlargement.
+ *
+ * reconstruct_x/y: filter that is not resized.
+ *
+ * sample_x/y: filter that is resized. Result is convolved with reconstruct 
filter.
+ *
+ * subsample_bits_x/y: If positive there are 2^n subpixel positions computed 
for
+ * the filter. If negative then -n (rounded up to the next power of 2) 
positions
+ * are calculated when size==1, but fewer are computed for larger sizes and
+ * more for smaller sizes.
+ *
+ * Some interesting reconstruct.sample combinations:
+ *
+ *  IMPULSE.x - Uses the sample function only. This is what many other pieces 
of
+ *  software do. Does not work for size < 1.0, so you must pass
+ *  max(size, 1.0). Or use x.IMPULSE for size < 1.0.
+ *
+ *  x.IMPULSE - Same as IMPULSE.x at size==1, size is ignored.
+ *
+ *  LINEAR.IMPULSE - Same as the bilinear filter. At size==1 this is the exact
+ *   same shape as BOX.BOX and IMPULSE.LINEAR, making it 
possible
+ *   to do seamless transitions between these filters at 
size==1.
+ *
+ *  BOX.BOX - Produces a trapazoid-shape. Narrowest possible filter with 
antialiasing.
+ *Matches a *lot* of other software, some call it "box", others 
call it
+ *"linear".
+ *
+ *  BOX.x - For size < 1.0 this produces square pixels. For size > 1.0 this
+ *  approaches IMPULSE.x as size gets larger.
+ *
+ *  BOX.LINEAR - At size==1 this is what some software calls a Quadratic filter
+ *
+ *  IMPULSE.LINEAR - Some software calls this "linear" or "triangle". Not a 
good filter.
+ *
+ *  LINEAR.LINEAR - At size==1 this is what some software calls "cubic 
interpolation".
+ *
+ *  IMPULSE.LANCZOS2 - Close to what a lot of other software calls "cubic 
interpolation"
+ *
+ *  IMPULSE.CUBIC - A third thing often called "cubic interpolation", 
sometimes called
+ *  "Mitchell" in other software.
+ *
+ *  IMPULSE.GAUSSIAN - Best removal of aliasing but usually considered too 
blurry
  */
 PIXMAN_EXPORT pixman_fixed_t *
 pixman_filter_create_separable_convolution (int *n_values,
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 06/14] pixman-filter: reduce amount of malloc/free/memcpy to generate filter

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Rearranged so that the entire block of memory for the filter pair
is allocated first, and then filled in. Previous version allocated
and freed two temporary buffers for each filter and did an extra
memcpy.

v8: small refactor to remove the filter_width function

v10: Restored filter_width function but with arguments changed to
 match later patches

v11: Removed unused arg and pointer from filter_width function
 Whitespace fixes.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 55 +-
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index b70da1f..69ac3ab 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -217,23 +217,17 @@ integral (pixman_kernel_t reconstruct, double x1,
 }
 }
 
-static pixman_fixed_t *
-create_1d_filter (int *width,
+static void
+create_1d_filter (int  width,
  pixman_kernel_t  reconstruct,
  pixman_kernel_t  sample,
  double   size,
- int  n_phases)
+ int  n_phases,
+ pixman_fixed_t *p)
 {
-pixman_fixed_t *params, *p;
 double step;
 int i;
 
-*width = ceil (size * filters[sample].width + filters[reconstruct].width);
-
-p = params = malloc (*width * n_phases * sizeof (pixman_fixed_t));
-if (!params)
-return NULL;
-
 step = 1.0 / n_phases;
 
 for (i = 0; i < n_phases; ++i)
@@ -248,8 +242,8 @@ create_1d_filter (int *width,
 * and sample positions.
 */
 
-   x1 = ceil (frac - *width / 2.0 - 0.5);
-x2 = x1 + *width;
+   x1 = ceil (frac - width / 2.0 - 0.5);
+   x2 = x1 + width;
 
total = 0;
 for (x = x1; x < x2; ++x)
@@ -277,7 +271,7 @@ create_1d_filter (int *width,
 }
 
/* Normalize */
-   p -= *width;
+   p -= width;
 total = 1 / total;
 new_total = 0;
for (x = x1; x < x2; ++x)
@@ -289,10 +283,8 @@ create_1d_filter (int *width,
}
 
if (new_total != pixman_fixed_1)
-   *(p - *width / 2) += (pixman_fixed_1 - new_total);
+   *(p - width / 2) += (pixman_fixed_1 - new_total);
 }
-
-return params;
 }
 
 #ifdef PIXMAN_GNUPLOT
@@ -336,6 +328,12 @@ gnuplot_filter(int width, int samples, const 
pixman_fixed_t* p)
 }
 #endif
 
+static int
+filter_width (pixman_kernel_t reconstruct, pixman_kernel_t sample, double size)
+{
+return ceil (filters[reconstruct].width + size * filters[sample].width);
+}
+
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and size parameters
  */
@@ -352,42 +350,35 @@ pixman_filter_create_separable_convolution (int   
  *n_values,
 {
 double sx = fabs (pixman_fixed_to_double (size_x));
 double sy = fabs (pixman_fixed_to_double (size_y));
-pixman_fixed_t *horz = NULL, *vert = NULL, *params = NULL;
+pixman_fixed_t *params;
 int subsample_x, subsample_y;
 int width, height;
 
+width = filter_width (reconstruct_x, sample_x, sx);
 subsample_x = (1 << subsample_bits_x);
-subsample_y = (1 << subsample_bits_y);
 
-horz = create_1d_filter (, reconstruct_x, sample_x, sx, subsample_x);
-vert = create_1d_filter (, reconstruct_y, sample_y, sy, 
subsample_y);
+height = filter_width (reconstruct_y, sample_y, sy);
+subsample_y = (1 << subsample_bits_y);
 
-if (!horz || !vert)
-goto out;
-
 *n_values = 4 + width * subsample_x + height * subsample_y;
 
 params = malloc (*n_values * sizeof (pixman_fixed_t));
 if (!params)
-goto out;
+   return NULL;
 
 params[0] = pixman_int_to_fixed (width);
 params[1] = pixman_int_to_fixed (height);
 params[2] = pixman_int_to_fixed (subsample_bits_x);
 params[3] = pixman_int_to_fixed (subsample_bits_y);
 
-memcpy (params + 4, horz,
-   width * subsample_x * sizeof (pixman_fixed_t));
-memcpy (params + 4 + width * subsample_x, vert,
-   height * subsample_y * sizeof (pixman_fixed_t));
+create_1d_filter (width, reconstruct_x, sample_x, sx, subsample_x,
+ params + 4);
+create_1d_filter (height, reconstruct_y, sample_y, sy, subsample_y,
+ params + 4 + width * subsample_x);
 
 #ifdef PIXMAN_GNUPLOT
 gnuplot_filter(width, subsample_x, params+4);
 #endif
 
-out:
-free (horz);
-free (vert);
-
 return params;
 }
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 14/14] demos/scale: default to GOOD and locked-together axis

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This makes the demo match normal behavior of pixman/cairo at startup.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 demos/scale.c  | 10 +-
 demos/scale.ui |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index 6d7ad2a..d1fce5d 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -340,7 +340,7 @@ on_expose (GtkWidget *da, GdkEvent *event, gpointer data)
 
 static void
 set_up_combo_box (app_t *app, const char *box_name,
-  int n_entries, const named_int_t table[])
+  int n_entries, const named_int_t table[], int active)
 {
 GtkWidget *widget = get_widget (app, box_name);
 GtkListStore *model;
@@ -366,7 +366,7 @@ set_up_combo_box (app_t *app, const char *box_name,
gtk_list_store_set (model, , 0, info->name, -1);
 }
 
-gtk_combo_box_set_active (GTK_COMBO_BOX (widget), 0);
+gtk_combo_box_set_active (GTK_COMBO_BOX (widget), active);
 
 g_signal_connect (widget, "changed", G_CALLBACK (rescale), app);
 }
@@ -374,7 +374,7 @@ set_up_combo_box (app_t *app, const char *box_name,
 static void
 set_up_filter_box (app_t *app, const char *box_name)
 {
-set_up_combo_box (app, box_name, G_N_ELEMENTS (filters), filters);
+set_up_combo_box (app, box_name, G_N_ELEMENTS (filters), filters, 0);
 }
 
 static char *
@@ -422,14 +422,14 @@ app_new (pixman_image_t *original)
 widget = get_widget (app, "drawing_area");
 g_signal_connect (widget, "expose_event", G_CALLBACK (on_expose), app);
 
-set_up_combo_box (app, "filter_combo_box", G_N_ELEMENTS (filter_types), 
filter_types);
+set_up_combo_box (app, "filter_combo_box", G_N_ELEMENTS (filter_types), 
filter_types, 3);
 set_up_filter_box (app, "reconstruct_x_combo_box");
 set_up_filter_box (app, "reconstruct_y_combo_box");
 set_up_filter_box (app, "sample_x_combo_box");
 set_up_filter_box (app, "sample_y_combo_box");
 
 set_up_combo_box (
-app, "repeat_combo_box", G_N_ELEMENTS (repeats), repeats);
+   app, "repeat_combo_box", G_N_ELEMENTS (repeats), repeats, 0);
 
 g_signal_connect (
gtk_builder_get_object (app->builder, "lock_checkbutton"),
diff --git a/demos/scale.ui b/demos/scale.ui
index 1e77f56..13e0e0d 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -177,6 +177,7 @@
  id="lock_checkbutton">
Lock X and Y 
Dimensions
0.0
+   True
  
   
 False
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 04/14] pixman-image: Added enable-gnuplot config to view filters in gnuplot

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

If enable-gnuplot is configured, then you can pipe the output of a pixman-using 
program
to gnuplot and get a continuously-updated plot of the horizontal filter. This
works well with demos/scale to test the filter generation.

The plot is all the different subposition filters shuffled together. This is
misleading in a few cases:

  IMPULSE.BOX - goes up and down as the subfilters have different numbers of 
non-zero samples
  IMPULSE.TRIANGLE - somewhat crooked for the same reason
  1-wide filters - looks triangular, but a 1-wide box would be more accurate

v7: First time this ability was included

v8: Use config option
Moved code to the filter generator
Modified scale demo to not call filter generator a second time.

v10: Only print if successful generation of plots
 Use #ifdef, not #if

v11: small whitespace fixes

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 configure.ac   | 13 +
 pixman/pixman-filter.c | 45 +
 2 files changed, 58 insertions(+)

diff --git a/configure.ac b/configure.ac
index 6b2134e..e833e45 100644
--- a/configure.ac
+++ b/configure.ac
@@ -834,6 +834,19 @@ fi
 AC_SUBST(PIXMAN_TIMERS)
 
 dnl ===
+dnl gnuplot
+
+AC_ARG_ENABLE(gnuplot,
+   [AC_HELP_STRING([--enable-gnuplot],
+   [enable output of filters that can be piped to gnuplot 
[default=no]])],
+   [enable_gnuplot=$enableval], [enable_gnuplot=no])
+
+if test $enable_gnuplot = yes ; then
+   AC_DEFINE(PIXMAN_GNUPLOT, 1, [enable output that can be piped to gnuplot])
+fi
+AC_SUBST(PIXMAN_GNUPLOT)
+
+dnl ===
 dnl GTK+
 
 AC_ARG_ENABLE(gtk,
diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index b2bf53f..3981c8b 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -297,6 +297,47 @@ create_1d_filter (int *width,
 return params;
 }
 
+#ifdef PIXMAN_GNUPLOT
+/* If enable-gnuplot is configured, then you can pipe the output of a
+ * pixman-using program to gnuplot and get a continuously-updated plot
+ * of the horizontal filter. This works well with demos/scale to test
+ * the filter generation.
+ *
+ * The plot is all the different subposition filters shuffled
+ * together. This is misleading in a few cases:
+ *
+ *  IMPULSE.BOX - goes up and down as the subfilters have different
+ *   numbers of non-zero samples
+ *  IMPULSE.TRIANGLE - somewhat crooked for the same reason
+ *  1-wide filters - looks triangular, but a 1-wide box would be more
+ *  accurate
+ */
+static void
+gnuplot_filter(int width, int samples, const pixman_fixed_t* p)
+{
+int x,y;
+printf("plot '-' with linespoints\n");
+printf("%g 0\n", - width * .5);
+for (x = 0; x < width; ++x) {
+   for (y = 0; y < samples; ++y) {
+   int yy;
+   if (width & 1)
+   yy = y;
+   else if (y >= samples / 2)
+   yy = y - samples / 2;
+   else
+   yy = samples / 2 + y;
+   printf("%g %g\n",
+  x - width * 0.5 + (y + 0.5) * (1.0 / samples),
+  pixman_fixed_to_double(p[(yy + 1) * width - x - 1]));
+   }
+}
+printf("%g 0\n", width * .5);
+printf("e\n");
+fflush(stdout);
+}
+#endif
+
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and scale parameters
  */
@@ -342,6 +383,10 @@ pixman_filter_create_separable_convolution (int
 *n_values,
 memcpy (params + 4 + width * subsample_x, vert,
height * subsample_y * sizeof (pixman_fixed_t));
 
+#ifdef PIXMAN_GNUPLOT
+gnuplot_filter(width, subsample_x, params+4);
+#endif
+
 out:
 free (horz);
 free (vert);
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 01/14] demos/scale: Compute filter size using boundary of xformed ellipse, not rectangle

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This is much more accurate and less blurry. In particular the filtering does
not change as the image is rotated.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 demos/scale.c | 102 +++---
 1 file changed, 61 insertions(+), 41 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index d00307e..0995ad0 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -55,50 +55,70 @@ get_widget (app_t *app, const char *name)
 return widget;
 }
 
-static double
-min4 (double a, double b, double c, double d)
-{
-double m1, m2;
-
-m1 = MIN (a, b);
-m2 = MIN (c, d);
-return MIN (m1, m2);
-}
-
-static double
-max4 (double a, double b, double c, double d)
-{
-double m1, m2;
-
-m1 = MAX (a, b);
-m2 = MAX (c, d);
-return MAX (m1, m2);
-}
-
+/* Figure out the boundary of a diameter=1 circle transformed into an ellipse
+ * by trans. Proof that this is the correct calculation:
+ *
+ * Transform x,y to u,v by this matrix calculation:
+ *
+ *  |u|   |a c| |x|
+ *  |v| = |b d|*|y|
+ *
+ * Horizontal component:
+ *
+ *  u = ax+cy (1)
+ *
+ * For each x,y on a radius-1 circle (p is angle to the point):
+ *
+ *  x^2+y^2 = 1
+ *  x = cos(p)
+ *  y = sin(p)
+ *  dx/dp = -sin(p) = -y
+ *  dy/dp = cos(p) = x
+ *
+ * Figure out derivative of (1) relative to p:
+ *
+ *  du/dp = a(dx/dp) + c(dy/dp)
+ *= -ay + cx
+ *
+ * The min and max u are when du/dp is zero:
+ *
+ *  -ay + cx = 0
+ *  cx = ay
+ *  c = ay/x  (2)
+ *  y = cx/a  (3)
+ *
+ * Substitute (2) into (1) and simplify:
+ *
+ *  u = ax + ay^2/x
+ *= a(x^2+y^2)/x
+ *= a/x (because x^2+y^2 = 1)
+ *  x = a/u (4)
+ *
+ * Substitute (4) into (3) and simplify:
+ *
+ *  y = c(a/u)/a
+ *  y = c/u (5)
+ *
+ * Square (4) and (5) and add:
+ *
+ *  x^2+y^2 = (a^2+c^2)/u^2
+ *
+ * But x^2+y^2 is 1:
+ *
+ *  1 = (a^2+c^2)/u^2
+ *  u^2 = a^2+c^2
+ *  u = hypot(a,c)
+ *
+ * Similarily the max/min of v is at:
+ *
+ *  v = hypot(b,d)
+ *
+ */
 static void
 compute_extents (pixman_f_transform_t *trans, double *sx, double *sy)
 {
-double min_x, max_x, min_y, max_y;
-pixman_f_vector_t v[4] =
-{
-   { { 1, 1, 1 } },
-   { { -1, 1, 1 } },
-   { { -1, -1, 1 } },
-   { { 1, -1, 1 } },
-};
-
-pixman_f_transform_point (trans, [0]);
-pixman_f_transform_point (trans, [1]);
-pixman_f_transform_point (trans, [2]);
-pixman_f_transform_point (trans, [3]);
-
-min_x = min4 (v[0].v[0], v[1].v[0], v[2].v[0], v[3].v[0]);
-max_x = max4 (v[0].v[0], v[1].v[0], v[2].v[0], v[3].v[0]);
-min_y = min4 (v[0].v[1], v[1].v[1], v[2].v[1], v[3].v[1]);
-max_y = max4 (v[0].v[1], v[1].v[1], v[2].v[1], v[3].v[1]);
-
-*sx = (max_x - min_x) / 2.0;
-*sy = (max_y - min_y) / 2.0;
+*sx = hypot (trans->m[0][0], trans->m[0][1]) / trans->m[2][2];
+*sy = hypot (trans->m[1][0], trans->m[1][1]) / trans->m[2][2];
 }
 
 typedef struct
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 07/14] pixman-filter: Correct Simpsons integration

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Simpsons uses cubic curve fitting, with 3 samples defining each cubic. This
makes the weights of the samples be in a pattern of 1,4,2,4,2...4,1, and then
dividing the result by 3.

The previous code was using weights of 1,2,6,6...6,2,1. Since it divided by
3 this produced about 2x the desired value (the normalization fixed this).
Also this is effectively a linear interpolation, not Simpsons integration.

With this fix the integration is accurate enough that the number of samples
could be reduced a lot. Multiples of 12 seem to work best.

v7: Merged with patch to reduce from 128 samples to 16
v9: Changed samples from 16 to 12
v10: Fixed rebase error that made it not compile
v11: minor whitespace change

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 69ac3ab..8b8fb82 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -189,13 +189,19 @@ integral (pixman_kernel_t reconstruct, double x1,
 }
 else
 {
-   /* Integration via Simpson's rule */
-#define N_SEGMENTS 128
+   /* Integration via Simpson's rule
+* See http://www.intmath.com/integration/6-simpsons-rule.php
+* 12 segments (6 cubic approximations) seems to produce best
+* result for lanczos3.linear, which was the combination that
+* showed the most errors.  This makes sense as the lanczos3
+* filter is 6 wide.
+*/
+#define N_SEGMENTS 12
 #define SAMPLE(a1, a2) \
(filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / size))

double s = 0.0;
-   double h = width / (double)N_SEGMENTS;
+   double h = width / N_SEGMENTS;
int i;
 
s = SAMPLE (x1, x2);
@@ -204,11 +210,14 @@ integral (pixman_kernel_t reconstruct, double x1,
{
double a1 = x1 + h * i;
double a2 = x2 + h * i;
+   s += 4 * SAMPLE(a1, a2);
+   }
 
-   s += 2 * SAMPLE (a1, a2);
-
-   if (i >= 2 && i < N_SEGMENTS - 1)
-   s += 4 * SAMPLE (a1, a2);
+   for (i = 2; i < N_SEGMENTS; i += 2)
+   {
+   double a1 = x1 + h * i;
+   double a2 = x2 + h * i;
+   s += 2 * SAMPLE(a1, a2);
}
 
s += SAMPLE (x1 + width, x2 + width);
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12] Implement PIXMAN_FILTER_GOOD/BEST

[spitzak]

Changes since last version:
- Removed blank line between signed-off and reviewed-by in commit message
- More explanation of the -n subsamples idea in commit message
- Code style fixes in #13
- More fixes to the big descriptive comment in #12
- Added more reviewed/acked comments
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 02/14] demos/scale: Added pulldown to choose PIXMAN_FILTER_* value

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This allows testing of GOOD/BEST and to do comparisons between
the basic filters and PIXMAN_FILTER_SEPARABLE_CONVOLUTION settings.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 demos/scale.c  | 14 +-
 demos/scale.ui | 40 ++--
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index 0995ad0..06821e3 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -127,6 +127,15 @@ typedef struct
 intvalue;
 } named_int_t;
 
+static const named_int_t filter_types[] =
+{
+{ "Separable", PIXMAN_FILTER_SEPARABLE_CONVOLUTION },
+{ "Nearest",   PIXMAN_FILTER_NEAREST },
+{ "Bilinear",  PIXMAN_FILTER_BILINEAR },
+{ "Good",  PIXMAN_FILTER_GOOD },
+{ "Best",  PIXMAN_FILTER_BEST },
+};
+
 static const named_int_t filters[] =
 {
 { "Box",   PIXMAN_KERNEL_BOX },
@@ -260,7 +269,9 @@ rescale (GtkWidget *may_be_null, app_t *app)
gtk_adjustment_get_value (app->subsample_adjustment),
gtk_adjustment_get_value (app->subsample_adjustment));
 
-pixman_image_set_filter (app->original, 
PIXMAN_FILTER_SEPARABLE_CONVOLUTION, params, n_params);
+pixman_image_set_filter (app->original,
+   get_value (app, filter_types, "filter_combo_box"),
+   params, n_params);
 
 pixman_image_set_repeat (
 app->original, get_value (app, repeats, "repeat_combo_box"));
@@ -402,6 +413,7 @@ app_new (pixman_image_t *original)
 widget = get_widget (app, "drawing_area");
 g_signal_connect (widget, "expose_event", G_CALLBACK (on_expose), app);
 
+set_up_combo_box (app, "filter_combo_box", G_N_ELEMENTS (filter_types), 
filter_types);
 set_up_filter_box (app, "reconstruct_x_combo_box");
 set_up_filter_box (app, "reconstruct_y_combo_box");
 set_up_filter_box (app, "sample_x_combo_box");
diff --git a/demos/scale.ui b/demos/scale.ui
index ee985dd..b62cbfb 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -191,12 +191,23 @@
 8
 6
 
+  
+True
+1
+bFilter:/b
+True
+  
+
+
   
 True
 1
 bReconstruct X:/b
 True
   
+  
+1
+  
 
 
   
@@ -206,7 +217,7 @@
 True
   
   
-1
+2
   
 
 
@@ -217,7 +228,7 @@
 True
   
   
-2
+3
   
 
 
@@ -228,7 +239,7 @@
 True
   
   
-3
+4
   
 
 
@@ -239,7 +250,7 @@
 True
   
   
-4
+5
   
 
 
@@ -250,7 +261,15 @@
 True
   
   
-5
+6
+  
+
+
+  
+True
+  
+  
+1
   
 
 
@@ -259,6 +278,7 @@
   
   
 1
+1
   
 
 
@@ -267,7 +287,7 @@
   
   
 1
-1
+2
   
 
 
@@ -276,7 +296,7 @@
   
   
 1
-2
+3
   
 
 
@@ -285,7 +305,7 @@
   

[Pixman] [PATCH v12 08/14] pixman-filter: Corrections to the integral() function

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

The IMPULSE special-cases did not sample the center of the of the region.
This caused it to sample the filters outside their range, and produce
assymetric filters and other errors. Fixing this required changing the
arguments to integral() so the correct point could be determined.

I fixed the nice filter and the integration to directly produce normalized
values. Re-normalization is still needed for impulse.box or impulse.triangle
so I did not remove it.

Distribute fixed error over all filter samples, to remove a high-frequency
bit of noise in the center of some filters (lancoz at large scale value).

box.box, which I expect will be very common as it is the proposed "good" filter,
was made a lot faster and more accurate. This is easy as the caller already
intersected the two boxes, so the width is the integral.

v7: This is a merge of 4 patches and lots of new code cleanup and fixes
 determined by examining the gnuplot output

v9: Restored the recursion splitting at zero for linear filter

v10: Small change from here moved to previous Simpsons patch so it compiles
 Merged patch to get correct subsample positions when subsample_bits==0

v11: Whitespace fixes

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 154 +
 1 file changed, 79 insertions(+), 75 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 8b8fb82..e82871f 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -79,7 +79,7 @@ sinc (double x)
 }
 
 static double
-lanczos (double x, int n)
+lanczos (double x, double n)
 {
 return sinc (x) * sinc (x * (1.0 / n));
 }
@@ -99,7 +99,7 @@ lanczos3_kernel (double x)
 static double
 nice_kernel (double x)
 {
-return lanczos3_kernel (x * 0.75);
+return lanczos3_kernel (x * 0.75) * 0.75;
 }
 
 static double
@@ -147,45 +147,51 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
 };
 
-/* This function scales the @sample filter by @size, then
- * aligns @x1 in @reconstruct with @x2 in @sample and
- * and integrates the product of the kernels across @width.
+/* This function scales the @sample filter by @size, shifts it by @pos,
+ * and then integrates the product of the kernels across low..high
  *
  * This function assumes that the intervals are within
  * the kernels in question. E.g., the caller must not
  * try to integrate a linear kernel ouside of [-1:1]
  */
 static double
-integral (pixman_kernel_t reconstruct, double x1,
- pixman_kernel_t sample, double size, double x2,
- double width)
+integral (pixman_kernel_t reconstruct,
+ pixman_kernel_t sample, double size, double pos,
+ double low, double high)
 {
+if (high < low)
+{
+   return 0.0;
+}
+else if (sample == PIXMAN_KERNEL_IMPULSE)
+{
+   return filters[reconstruct].func (-pos);
+}
+else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
+{
+   return filters[sample].func (-pos / size) / size;
+}
+else if (reconstruct == PIXMAN_KERNEL_BOX && sample == PIXMAN_KERNEL_BOX)
+{
+   assert (high <= low + 1.0);
+   return (high - low) / size;
+}
 /* If the integration interval crosses zero, break it into
  * two separate integrals. This ensures that filters such
  * as LINEAR that are not differentiable at 0 will still
  * integrate properly.
  */
-if (x1 < 0 && x1 + width > 0)
+else if (reconstruct == PIXMAN_KERNEL_LINEAR && low < 0 && high > 0)
 {
return
-   integral (reconstruct, x1, sample, size, x2, - x1) +
-   integral (reconstruct, 0, sample, size, x2 - x1, width + x1);
+   integral (reconstruct, sample, size, pos, low, 0) +
+   integral (reconstruct, sample, size, pos, 0, high);
 }
-else if (x2 < 0 && x2 + width > 0)
+else if (sample == PIXMAN_KERNEL_LINEAR && low < pos && high > pos)
 {
return
-   integral (reconstruct, x1, sample, size, x2, - x2) +
-   integral (reconstruct, x1 - x2, sample, size, 0, width + x2);
-}
-else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
-{
-   assert (width == 0.0);
-   return filters[sample].func (x2 / size);
-}
-else if (sample == PIXMAN_KERNEL_IMPULSE)
-{
-   assert (width == 0.0);
-   return filters[reconstruct].func (x1);
+   integral (reconstruct, sample, size, pos, low, pos) +
+   integral (reconstruct, sample, size, pos, pos, high);
 }
 else
 {
@@ -197,32 +203,30 @@ integral (pixman_kernel_t reconstruct, double x1,
 * filter is 6 wide.
 */
 #define N_SEGMENTS 12
-#define SAMPLE(a1, a2) 

[Pixman] [PATCH v12 13/14] pixman-image: Implement PIXMAN_FILTER_GOOD/BEST as separable convolutions

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Detects and uses PIXMAN_FILTER_NEAREST for all 8 90-degree rotations and
reflections when the scale is 1.0 and integer translation.

GOOD uses:

 scale < 1/16 : BOX.BOX at size 16
 scale < 3/4 : BOX.BOX at size 1/scale
 larger : BOX.BOX at size 1

 If both directions have a scale >= 3/4 or a scale of 1/2 and an integer
 translation, the faster PIXMAN_FILTER_BILINEAR code is used. This is
 compatable at these scales with older versions of pixman where bilinear
 was always used for GOOD.

BEST uses:

 scale < 1/24 : BOX.BOX at size 24
 scale < 1/16 : BOX.BOX at size 1/scale
 scale < 1 : IMPULSE.LANCZOS2 at size 1/scale
 scale < 2.333 : IMPULSE.LANCZOS2 at size 1
 scale < 128 : BOX.LANCZOS2 at size 1/(scale-1) (antialiased square pixels)
 larger : BOX.LANCZOS2 at size 1/127 (antialias blur gets thicker)

v8: Cutoff in BEST between IMPULSE.LANCZOS2 and BOX.LANCZOS2 adjusted for
a better match between the filters.

v9: Use the new negative subsample controls to scale the subsamples. These
were chosen by finding the lowest number that did not add visible
artifacts to the zone plate image.

Scale demo altered to default to GOOD and locked-together x+y scale

Fixed divide-by-zero from all-zero matrix found by stress-test

v11: Whitespace and formatting fixes
 Moved demo changes to a later patch

v12: Whitespace and formatting fixes

Signed-off-by: Bill Spitzak <spit...@gmail.com>
---
 pixman/pixman-image.c | 327 ++
 1 file changed, 249 insertions(+), 78 deletions(-)

diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 1ff1a49..5f52dd7 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "pixman-private.h"
 
@@ -274,112 +275,282 @@ compute_image_info (pixman_image_t *image)
  FAST_PATH_X_UNIT_POSITIVE |
  FAST_PATH_Y_UNIT_ZERO |
  FAST_PATH_AFFINE_TRANSFORM);
+   switch (image->common.filter)
+   {
+   case PIXMAN_FILTER_CONVOLUTION:
+   break;
+   case PIXMAN_FILTER_SEPARABLE_CONVOLUTION:
+   flags |= FAST_PATH_SEPARABLE_CONVOLUTION_FILTER;
+   break;
+   default:
+   flags |= (FAST_PATH_NEAREST_FILTER | 
FAST_PATH_NO_CONVOLUTION_FILTER);
+   break;
+   }
 }
 else
 {
+   pixman_fixed_t (*m)[3] = image->common.transform->matrix;
+   double dx, dy;
+   int nearest_ok, bilinear_ok;
+
flags |= FAST_PATH_HAS_TRANSFORM;
 
-   if (image->common.transform->matrix[2][0] == 0  &&
-   image->common.transform->matrix[2][1] == 0  &&
-   image->common.transform->matrix[2][2] == pixman_fixed_1)
+   nearest_ok = FALSE;
+   bilinear_ok = FALSE;
+
+   if (m[2][0] == 0&&
+   m[2][1] == 0&&
+   m[2][2] == pixman_fixed_1)
{
+   /* no perspective */
flags |= FAST_PATH_AFFINE_TRANSFORM;
 
-   if (image->common.transform->matrix[0][1] == 0 &&
-   image->common.transform->matrix[1][0] == 0)
+   if (m[0][1] == 0 &&
+   m[1][0] == 0)
{
-   if (image->common.transform->matrix[0][0] == -pixman_fixed_1 &&
-   image->common.transform->matrix[1][1] == -pixman_fixed_1)
+   /* no tilt of either axis */
+   flags |= FAST_PATH_SCALE_TRANSFORM;
+   if (abs(m[0][0]) == pixman_fixed_1 &&
+   abs(m[1][1]) == pixman_fixed_1)
{
-   flags |= FAST_PATH_ROTATE_180_TRANSFORM;
+   /* no scaling */
+   nearest_ok = TRUE;
+   if (m[0][0] < 0 && m[1][1] < 0)
+   flags |= FAST_PATH_ROTATE_180_TRANSFORM;
}
-   flags |= FAST_PATH_SCALE_TRANSFORM;
}
-   else if (image->common.transform->matrix[0][0] == 0 &&
-image->common.transform->matrix[1][1] == 0)
+   else if (m[0][0] == 0 &&
+m[1][1] == 0)
{
-   pixman_fixed_t m01 = image->common.transform->matrix[0][1];
-   pixman_fixed_t m10 = image->common.transform->matrix[1][0];
-
-   if (m01 == -pixman_fixed_1 && m10 == pixman_fixed_1)
-   flags |= FAST_PATH_ROTATE_90_TRANSFORM;
-   else if (m01 == pixman_fixed_1 && m10 == -pixman_fixed_1)
-   flags |= FAST_PATH_ROTATE_270_TRANSFORM;
+   /* x/y axis are swapped, 90 degree rotation */
+   if 

[Pixman] [PATCH v12 03/14] demos/scale: Only generate filters when used for separable

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

This makes the speed of the demo more accurate, as the filter generation
is a visible fraction of the time it takes to do a transform. This also
prevents the output of unused filters in the gnuplot option in the next
patch.

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 demos/scale.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index 06821e3..6d7ad2a 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -258,16 +258,25 @@ rescale (GtkWidget *may_be_null, app_t *app)
 pixman_transform_from_pixman_f_transform (, );
 pixman_image_set_transform (app->original, );
 
-params = pixman_filter_create_separable_convolution (
-_params,
-sx * 65536.0 + 0.5,
-   sy * 65536.0 + 0.5,
-   get_value (app, filters, "reconstruct_x_combo_box"),
-   get_value (app, filters, "reconstruct_y_combo_box"),
-   get_value (app, filters, "sample_x_combo_box"),
-   get_value (app, filters, "sample_y_combo_box"),
-   gtk_adjustment_get_value (app->subsample_adjustment),
-   gtk_adjustment_get_value (app->subsample_adjustment));
+if (get_value (app, filter_types, "filter_combo_box") ==
+   PIXMAN_FILTER_SEPARABLE_CONVOLUTION)
+{
+   params = pixman_filter_create_separable_convolution (
+   _params,
+   sx * 65536.0 + 0.5,
+   sy * 65536.0 + 0.5,
+   get_value (app, filters, "reconstruct_x_combo_box"),
+   get_value (app, filters, "reconstruct_y_combo_box"),
+   get_value (app, filters, "sample_x_combo_box"),
+   get_value (app, filters, "sample_y_combo_box"),
+   gtk_adjustment_get_value (app->subsample_adjustment),
+   gtk_adjustment_get_value (app->subsample_adjustment));
+}
+else
+{
+   params = 0;
+   n_params = 0;
+}
 
 pixman_image_set_filter (app->original,
get_value (app, filter_types, "filter_combo_box"),
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 09/14] pixman-filter: Nested polynomial for cubic

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

v11: Restored range checks

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index e82871f..e5ef8e6 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -109,14 +109,16 @@ general_cubic (double x, double B, double C)
 
 if (ax < 1)
 {
-   return ((12 - 9 * B - 6 * C) * ax * ax * ax +
-   (-18 + 12 * B + 6 * C) * ax * ax + (6 - 2 * B)) / 6;
+   return (((12 - 9 * B - 6 * C) * ax +
+(-18 + 12 * B + 6 * C)) * ax * ax +
+   (6 - 2 * B)) / 6;
 }
-else if (ax >= 1 && ax < 2)
+else if (ax < 2)
 {
-   return ((-B - 6 * C) * ax * ax * ax +
-   (6 * B + 30 * C) * ax * ax + (-12 * B - 48 * C) *
-   ax + (8 * B + 24 * C)) / 6;
+   return -B - 6 * C) * ax +
+ (6 * B + 30 * C)) * ax +
+(-12 * B - 48 * C)) * ax +
+   (8 * B + 24 * C)) / 6;
 }
 else
 {
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v12 05/14] pixman-filter: Consistency in arg names to integral ()

[spitzak]

From: Bill Spitzak <spit...@gmail.com>

Rename kernel1/2 to reconstruct/sample to match the other functions.
Rename "scale" to "size" to avoid confusion with the scale being applied
to the image, which is the reciprocol of this value.

v10: Renamed "scale" to "size"

Signed-off-by: Bill Spitzak <spit...@gmail.com>
Reviewed-by: Oded Gabbay <oded.gab...@gmail.com>
---
 pixman/pixman-filter.c | 48 +++-
 pixman/pixman.h|  6 +++---
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 3981c8b..b70da1f 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -147,8 +147,8 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
 };
 
-/* This function scales @kernel2 by @scale, then
- * aligns @x1 in @kernel1 with @x2 in @kernel2 and
+/* This function scales the @sample filter by @size, then
+ * aligns @x1 in @reconstruct with @x2 in @sample and
  * and integrates the product of the kernels across @width.
  *
  * This function assumes that the intervals are within
@@ -156,8 +156,8 @@ static const filter_info_t filters[] =
  * try to integrate a linear kernel ouside of [-1:1]
  */
 static double
-integral (pixman_kernel_t kernel1, double x1,
- pixman_kernel_t kernel2, double scale, double x2,
+integral (pixman_kernel_t reconstruct, double x1,
+ pixman_kernel_t sample, double size, double x2,
  double width)
 {
 /* If the integration interval crosses zero, break it into
@@ -168,31 +168,31 @@ integral (pixman_kernel_t kernel1, double x1,
 if (x1 < 0 && x1 + width > 0)
 {
return
-   integral (kernel1, x1, kernel2, scale, x2, - x1) +
-   integral (kernel1, 0, kernel2, scale, x2 - x1, width + x1);
+   integral (reconstruct, x1, sample, size, x2, - x1) +
+   integral (reconstruct, 0, sample, size, x2 - x1, width + x1);
 }
 else if (x2 < 0 && x2 + width > 0)
 {
return
-   integral (kernel1, x1, kernel2, scale, x2, - x2) +
-   integral (kernel1, x1 - x2, kernel2, scale, 0, width + x2);
+   integral (reconstruct, x1, sample, size, x2, - x2) +
+   integral (reconstruct, x1 - x2, sample, size, 0, width + x2);
 }
-else if (kernel1 == PIXMAN_KERNEL_IMPULSE)
+else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
 {
assert (width == 0.0);
-   return filters[kernel2].func (x2 * scale);
+   return filters[sample].func (x2 / size);
 }
-else if (kernel2 == PIXMAN_KERNEL_IMPULSE)
+else if (sample == PIXMAN_KERNEL_IMPULSE)
 {
assert (width == 0.0);
-   return filters[kernel1].func (x1);
+   return filters[reconstruct].func (x1);
 }
 else
 {
/* Integration via Simpson's rule */
 #define N_SEGMENTS 128
 #define SAMPLE(a1, a2) \
-   (filters[kernel1].func ((a1)) * filters[kernel2].func ((a2) * scale))
+   (filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / size))

double s = 0.0;
double h = width / (double)N_SEGMENTS;
@@ -221,16 +221,14 @@ static pixman_fixed_t *
 create_1d_filter (int *width,
  pixman_kernel_t  reconstruct,
  pixman_kernel_t  sample,
- double   scale,
+ double   size,
  int  n_phases)
 {
 pixman_fixed_t *params, *p;
 double step;
-double size;
 int i;
 
-size = scale * filters[sample].width + filters[reconstruct].width;
-*width = ceil (size);
+*width = ceil (size * filters[sample].width + filters[reconstruct].width);
 
 p = params = malloc (*width * n_phases * sizeof (pixman_fixed_t));
 if (!params)
@@ -259,8 +257,8 @@ create_1d_filter (int *width,
double pos = x + 0.5 - frac;
double rlow = - filters[reconstruct].width / 2.0;
double rhigh = rlow + filters[reconstruct].width;
-   double slow = pos - scale * filters[sample].width / 2.0;
-   double shigh = slow + scale * filters[sample].width;
+   double slow = pos - size * filters[sample].width / 2.0;
+   double shigh = slow + size * filters[sample].width;
double c = 0.0;
double ilow, ihigh;
 
@@ -270,7 +268,7 @@ create_1d_filter (int *width,
ihigh = MIN (shigh, rhigh);
 
c = integral (reconstruct, ilow,
- sample, 1.0 / scale, ilow - pos,
+ sample, size, ilow - pos,
  ihigh - ilow);
}
 
@@ -339,12 +337,12 @@ gnuplot_filter(int width, int samples, const 
pixman_fixed_t* p)
 #endif
 
 /* Create the parameter l