[Pixman] Make pixman compatible with big image

[Siarhei Siamashka]

On Mon, 16 Sep 2013 22:24:38 +0200
LE GARREC Vincent <legarrec.vincent at gmail.com> wrote:

> 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.

Review is not going to help much in this case. With so many possible
places where this can be broken, sooner or later bugs are going to slip
into the code unnoticed. Or some of the less obvious similar problems
may remain latent.

Extending the pixman test programs to also use huge strides for the
images on 64-bit systems can possibly help to improve testing coverage.
If this gets implemented right, the real memory usage should not
increase too much, because the huge gaps between scanlines are not
going to be allocated in RAM unless they are really accessed.

It would help a lot to have an update for the test suite, which could
verify that we are on the right track and the patch is improving
something.

-- 
Best regards,
Siarhei Siamashka