[Pixman] [PATCH 0/7] Cover scaling patches & request for assistance

[Oded Gabbay]

On Mon, Aug 24, 2015 at 11:41 PM, Ben Avison <bavison at riscosopen.org> wrote:
> Some back story...
>
> First there was this patch:
>
> http://patchwork.freedesktop.org/patch/49937/
>
> Back last October, Søren had this to say about it:
>
>> A concern I have here is that code might access pixels outside the
>> image that have weight 0. Ie., in the bilinear case, some code might
>> attempt to read the pixel at bits[-1] and then multiply it with 0. But
>> we can't assume that bits[-1] is readable.
>>
>> If I remember correctly, the +pixman_fixed_e * 8 stuff was intended to
>> handle this case.
>>
>> I think it would be worthwhile to have a test that uses fence_malloc
>> for the source buffer and the matrix mentioned in the commit. In fact,
>> the fence_malloc() testing could benefit from being extended in
>> various ways:
>>
>>       - having fence pages both before and after the image
>>       - having fence pages in the 'stride' part of the image
>
> Towards this goal, the following patches were posted to the list - and they
> seem to have escaped Patchwork's notice:
>
> http://lists.freedesktop.org/archives/pixman/2015-May/003644.html
> http://lists.freedesktop.org/archives/pixman/2015-May/003645.html
> http://lists.freedesktop.org/archives/pixman/2015-May/003646.html
> http://lists.freedesktop.org/archives/pixman/2015-May/003678.html
>
> (note that there were a few minor outstanding points on the first three).
> This series relies upon the test program implemented by those patches to
> prove its correctness, so it would be helpful if they could be finished off
> and committed.
>

So these patches didn't escape patchwork. As part of the cleanup, I
moved them to "Changes requested" status:

http://patchwork.freedesktop.org/patch/48887/
http://patchwork.freedesktop.org/patch/48888/
http://patchwork.freedesktop.org/patch/48889/
http://patchwork.freedesktop.org/patch/50516/

You can read the email thread of each patch there and I understood
that Pekka will fix / have already fixed the comments, but I didn't
see he sent the fixed patches to the mailing list.

Pekka,
I think you can just commit the fixed versions, or if you want, you
can post them and I can also help review them.

           Oded

> If you look in detail at the patches in this series, you'll see that there's
> an outstanding change for a single routine - the one and only scanline fetch
> iter in pixman-ssse3.c, which handles bilinear scaled a8r8g8b8 source
> images. This could probably be fixed using either of the two methods I used
> in the other patches, but pixman-fast-path.c is the most elegant. My problem
> is that I don't know SSSE3 (or any other x86 for that matter) so it would
> represent a big learning curve for me for the sake of just this one
> function.
>
> Is anyone able to help out? I've got a load of other scaling-related goodies
> lined up, but it doesn't make much sense to post them while all these
> fundamentals are still outstanding.
>
> Ben Avison (7):
>   Refactor calculation of cover flags
>   More accurate FAST_PATH_SAMPLES_COVER_CLIP_NEAREST
>   Split FAST_PATH_SAMPLES_COVER_CLIP_BILINEAR flag
>   More accurate FAST_PATH_SAMPLES_COVER_CLIP_BILINEAR
>   armv7/mips/sse2: Fix bounds violations in bilinear cover scaled fast
>     paths
>   pixman-fast-path: Fix bounds violations in bilinear cover fetcher
>   test: Make image size calculation match COVER_CLIP definition again
>
>  pixman/pixman-fast-path.c |   35 +++++++++++++++++--------
>  pixman/pixman-inlines.h   |   63 ++++++++++++++++++++++++++++++++++++++++++++-
>  pixman/pixman-private.h   |    1 +
>  pixman/pixman-ssse3.c     |    2 +-
>  pixman/pixman.c           |   37 +++++++++++++-------------
>  test/affine-bench.c       |   11 +++-----
>  6 files changed, 110 insertions(+), 39 deletions(-)
>
> --
> 1.7.5.4
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman