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

Mon Mar 7 19:24:45 UTC 2016

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

> On Fri, 4 Mar 2016 19:12:36 -0800
> Bill Spitzak <spitzak at gmail.com> wrote:
>
> > On Fri, Mar 4, 2016 at 4:17 PM, Søren Sandmann <soren.sandmann at gmail.com
> >
> > wrote:
> >
> > > On Wed, Feb 10, 2016 at 1:25 AM, <spitzak at gmail.com> wrote:
> > >
> > >> From: Bill Spitzak <spitzak at 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 <spitzak at 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 whatever
> > hardware resources are available. This is not going to happen if they are
> > expected to read the filter array.
>
> That will violate the do-what-I-say policy.
>
> > 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 environment
> variable
> > > is not supposed to change Pixman's behavior, only potentially its
> > > performance.
> > >
> >
> > Will look into that. I would like the rest of the detail please.
>
> That won't help this patch get through.
>
> > Currently Linux 2D rendering is a joke. Nobody was using the
> > SeparateConvolution until I patched up Cairo to do it, but I sure figured
> > that was a very temporary patch until it could be done in a proper
> > location. Users want "good", not "low level api". So now Linux is 1.5
> years
> > further behind. Really sad.
>
> Users do not use Pixman. Pixman is an implementation detail of Render
> and Cairo. Pixman *is* a low level API, lower than Render or Cairo.

Please look at the new patch, where I split this from other changes so at
least you can push the other changes that fix the filter generator.

I agree that users should use Render/Cairo but because the api to them just
has "good" and "best" it makes no difference to the user what the
communication between it and Pixman looks like. And I believe the current
design is making it impossible to do optimizations expected on modern
systems as Pixman is lacking significant information that it needs.

Currently Pixman cannot optimize the separable convolution because it does
not know enough information about the filters. The code I put in Cairo does
about the best that I can manage, but this duplicates matrix analysis that
Pixman is doing anyway. And it lacks optimizations that Pixman could do if
it knew the filter was normalized and symmetric (I now plan to add a patch
so Pixman looks at the filter to figure these out).

X11 might have bugs sending GOOD/BEST through, but it completely lacks any
attempt to send the separable convolution filters through. I know which of
these problems would be easier to fix!

Hardware/shader implementations do not use a data structure resembling the
convolution array at all. The most common approach is to bilinear interpret
a 1-d image that represents the filter (yes I know there are trickier
ones). Take a look at the gnuplot output and you will see that constructing
such an image by reinterlacing the subsamples fails badly on the box and
triangle filters. Knowledge of the filter would allow a perfect lookup
image to be created trivially, or for shaders/hardware to directly
calculate them (bilinear is by far the most common hardware supported
filter, it is a triangle, but I doubt any implementation uses a lookup
table for it!).

The current api is not usable for non-affine transforms, as the filter has
to change size depending on the location. All the optimization attempts in
Cairo are useless as they have to be performed per-pixel, not per-transform.

I suppose an api that looks more like the api to the filter *generator*,
with an enumeration or other description of the functions, would work and
maybe be more in keeping with the Pixman design. I'm just not very certain
that any users want this. If Cairo just translates Good->set1 and
Best->set2, then all effort in supporting other sets is wasted code that
will never be used.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pixman/attachments/20160307/b1417678/attachment.html>