[Pixman] Make pixman compatible with big image

[LE GARREC Vincent]

2013/9/13 Søren Sandmann <sandmann at cs.au.dk>

> LE GARREC Vincent <legarrec.vincent at gmail.com> writes:
>
> > So, what should I do ? Is it okay to make a cast for every multiplication
> > of stride by height ?
>
> There are a couple of things that have to be kept in mind:
>
> - The stride can be negative (this corresponds to the lines of the image
>   being stored upside down), so storing or casting the stride to size_t
>   probably won't work.
>

You're right, I should use ssize_t.


> - Even if you fix the multiplication issues, has limitations in other
>   places that prevents access to large images working well. See
>
>         https://bugs.freedesktop.org/show_bug.cgi?id=46277
>
>   for example. Fixing the multiplications may still be worthwhile
>   though, since we may some day get large images working.
>

I took a look at this bug. To make it works with pixman, struct
pixman_vector should uses pixman_fixed_48_16_t instead of pixman_fixed_t
(and remove IS_16_16) but it will create a breakage of the API. I think
that solution will make that bug disappears but will not make the whole
library okay (a bit like my first patch with the stride's problem where I
thought that just one line needed to be changed).


> > I'm asking because I don't want to work on a patch that will be denied
> > because my solution is wrong.
>
> I don't know what the best way to do it is, but requiring all rowstride
> multiplications to have a cast probably isn't going to be maintainable,
> because people will forget about it when writing new code.
>

I spend some time to think about it and I believe that's the simplest
solution. If you think that using (ssize_t) will be forget in new code, why
not using a macro like
#define STRIDE_MULT(stride, y) ((ssize_t)(stride)*(y))
to show that's important to not use just "*"

And about forgetting to do the cast in new code, that's what a bug is ^_^
And that's why review is important.

Søren
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20130916/c151e91e/attachment.html>