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

[Bill Spitzak]

On Thu, Sep 10, 2015 at 9:35 AM, Ben Avison <bavison at riscosopen.org> wrote:

> On Thu, 10 Sep 2015 12:46:50 +0100, Pekka Paalanen <ppaalanen at gmail.com>
> wrote:
>
>> you're right, documenting this is important. However, I think this
>> particular patch is not the best place, and here is why.
>>
>> When we recently discussed this, both I and Siarhei had the opinion
>> that this needs to be done in two separate steps:
>>
>> 1. Remove the useless 8e fuzz margins.
>>
>> 2. Change the meaning of the COVER_CLIP_BILINEAR flag so that it is no
>>    longer safe for fetchers to always fetch a 2x2 pixel block.
>>
>
This sounds exactly right to me.

Another way to say it is that it is safe to fetch all pixels that have a
non-zero weight. Certain bilinear positions will produce 2x2 blocks that
contain up to 3 pixels with a zero weight, those pixels may be outside the
source clip even when this flag is on.


> I sense we're taking a slightly different perspective on the problem
> here. I don't really see these two steps as different in spirit. In the
> first one, the flag calculation was allowing extra space to permit the
> corresponding fast path to be a bit sloppy with its coordinate
> transformations. In the second one, the flag calculation was allowing
> extra space to permit the corresponding fast path to be a bit sloppy with
> loading data that it doesn't need. Apart from meaning that less efficient
> fast paths sometimes get used, this also means a lot of unnecessary cache
> lines get loaded in many cases, which has got to hurt performance.
>

I believe you are confusing the implementation of the bilinear with this
flag.

For a coordinate of .5, pixel 0 will have a weight of 1.0, and all other
pixels will have a weight of 0.0. This includes both the pixel at -1 and
the pixel at 1. They both have a weight of zero.

It is useful for a bilinear algorithm to think about pairs of pixels, and
depending on the implementation the pair produced for the .5 coordinate may
be pixels 0 and 1, or pixels -1 and 0.

However this does not change the setting of COVER_CLIP_BILINEAR! No pixel
has changed it's weight, both pixel -1 and 1 remain with a weight of zero,
no matter which is chosen for the pair.

I think you are right that there are reasons for the bilinear
*implementation* to round down, so the weighted-zero pixel is at the lower
coordinate. This is because the special code to avoid fetching it can be at
the start of the loop, rather than at the end. But this has nothing to do
with COVER_CLIP_BILINEAR and should not be part of what sets it.

And it is true that there are a lot of broken bilinear fetches that do read
the weight-zero pixel. These need to be fixed. But this still does not
change the meaning of the flag.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20150910/60ab7081/attachment.html>