<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 10, 2015 at 9:35 AM, Ben Avison <span dir="ltr"><<a href="mailto:bavison@riscosopen.org" target="_blank">bavison@riscosopen.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, 10 Sep 2015 12:46:50 +0100, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
you're right, documenting this is important. However, I think this<br>
particular patch is not the best place, and here is why.<br>
<br>
When we recently discussed this, both I and Siarhei had the opinion<br>
that this needs to be done in two separate steps:<br>
<br>
1. Remove the useless 8e fuzz margins.<br>
<br>
2. Change the meaning of the COVER_CLIP_BILINEAR flag so that it is no<br>
   longer safe for fetchers to always fetch a 2x2 pixel block.<br></blockquote></span></blockquote><div><br></div><div>This sounds exactly right to me.<br><br></div><div>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.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
</span>
I sense we're taking a slightly different perspective on the problem<br>
here. I don't really see these two steps as different in spirit. In the<br>
first one, the flag calculation was allowing extra space to permit the<br>
corresponding fast path to be a bit sloppy with its coordinate<br>
transformations. In the second one, the flag calculation was allowing<br>
extra space to permit the corresponding fast path to be a bit sloppy with<br>
loading data that it doesn't need. Apart from meaning that less efficient<br>
fast paths sometimes get used, this also means a lot of unnecessary cache<br>
lines get loaded in many cases, which has got to hurt performance.<br></blockquote><div><br></div><div>I believe you are confusing the implementation of the bilinear with this flag.<br><br></div><div>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.<br></div><div><br></div><div>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.<br><br></div><div>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.<br><br></div><div>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.<br><br></div><div>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.<br><br></div></div></div></div>