[Pixman] [PATCH 1/4] Change conditions for setting FAST_PATH_SAMPLES_COVER_CLIP flags

[Søren Sandmann]

Ben Avison <bavison at riscosopen.org> writes:

> For what it's worth, my experience has been that treating the operation
> as a two-pass operation (separable into horizontal/vertical passes)
> generally outperforms the 2x2 convolution filter approach by a wide
> margin. But let's say we still try to describe it in those terms.

Yes, especially for upscaling, where the same scanline can be reused
many times; this approach is used in a couple of places in pixman
already: The SSSE3 and 'fast' bilinear iterators work this way.

But this is an optimization/implementation detail. Those iterators still
act as if they were doing convolutions with 2x2 matrices.

> You're making an assumption that the coefficients of that 2x2 matrix are
>
> / (1 - frac(x)) . (1 - frac(y))     frac(x) . (1 - frac(y)) \
> |                                                           |
> \ (1 - frac(x)) . frac(y)           frac(x) . frac(y)       /
>
> However, other definitions will work, such as
>
> / frac(-x) . frac(-y)               (1 - frac(-x)) . frac(-y)       \
> |                                                                   |
> \ frac(-x) . (1 - frac(-y))         (1 - frac(-x)) . (1 - frac(-y)) /
>
> These matrix coefficients are not part of the public Pixman API, but they
> do affect how you align the matrix. In the first case, the position of
> the NW coefficient is always found by rounding *down* the coordinates, as
> you say; in the second, the position of the SE coefficient is always
> found by rounding *up* the coordinates.
>
> You might think this makes little difference, but as I've described
> previously, there are benefits to the latter approach.

That is a good point, and it may actually be the explanation for why I
left out BILINEAR from the rounding.txt document: All reasonable choices
of rounding will lead to precisely the same results, and the aim of the
document was to describe rounding, not filtering.

> If you're only going to have one flag, it makes sense for it to be the
> most inclusive one. That way, any given fast path can always add
> additional checks to detect cases they can't handle and pass them on to a
> secondary routine. This is effectively the approach I took here:

Well, the fast path system is based on the idea that each fast path
describes what it wants from the operation, and if it matches, it should
do what it claimed to be capable of and not pass things off to secondary
routines. There are known issues with this system of course, but IMO ad
hoc workarounds inside the fast paths themselves will quickly turn into
a mess.

> http://lists.freedesktop.org/archives/pixman/2015-September/003946.html
>
> It can't be done the other way round - once we have filtered out an
> operation for consideration of bilinear fast paths, there's no way a fast
> path can say "actually, yes I could have handled that if you had asked
> me".
>
>> A separate possibility is a flag that says "all pixels whose weights are
>> non-zero are inside the borders of the source image". Is this useful
>> information? It might be, and if so, it could be conveyed through some
>> new flag, though I'd echo Siarhei's comment about whether this is
>> something that happens in practice.
>
> Here's an example I've just thought of which nicely illustrates how the
> existing scheme leads to a suboptimal solution. Say we are aiming to plot
> an image such that it is reduced in size vertically by a factor of 2, but
> left unchanged horizontally. The overall effect desired is to find the
> mean of each two vertically neighbouring pixels.
>
> The set of Y coordinates, once transformed into source space, would be
>   1.0, 3.0, 5.0, ... (height * 2 - 1.0)
> The set of X coordinates, once transformed into source space, would be
>   0.5, 1.5, 2.5, ... (width - 0.5)
>
> Under the existing scheme, this would not be passed to any COVER fast
> paths - because it involves the X coordinate at exactly (width - 0.5).
> And yet it's obvious to a human that no pixels outside the source image
> are involved. To slightly reduce the X increment to avoid this would
> be highly undesirable, as it could lead to a marked softening and/or
> sharpening as you move from left to right across the image. Trimming off
> the rightmost pixel might also reasonably be frowned upon, so we'd be
> left with having to use a slower fast path, for no good reason.

Okay, that's a good example; I'll buy that this could happen in
practice. If all existing fast paths and iterators can be
changed/verified to not read outside their bounds, I have no objection
to changing the meaning of the existing flag (or adding a new one if
that's more convenient).

I realized that there is another use for a new flag like this: It could
likely also be used here:

    http://cgit.freedesktop.org/pixman/tree/pixman/pixman.c#n662

so that images would be considered opaque in more cases, leading to more
cases of reducing the operator from OVER to SRC.


Søren