<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2013/9/13 Søren Sandmann <span dir="ltr"><<a href="mailto:sandmann@cs.au.dk" target="_blank">sandmann@cs.au.dk</a>></span><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="im">LE GARREC Vincent <<a href="mailto:legarrec.vincent@gmail.com">legarrec.vincent@gmail.com</a>> writes:<br>
<br>
> So, what should I do ? Is it okay to make a cast for every multiplication<br>
> of stride by height ?<br>
<br>
</div>There are a couple of things that have to be kept in mind:<br>
<br>
- The stride can be negative (this corresponds to the lines of the image<br>
being stored upside down), so storing or casting the stride to size_t<br>
probably won't work.<br></blockquote><div><br>You're right, I should use ssize_t.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- Even if you fix the multiplication issues, has limitations in other<br>
places that prevents access to large images working well. See<br>
<br>
<a href="https://bugs.freedesktop.org/show_bug.cgi?id=46277" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=46277</a><br>
<br>
for example. Fixing the multiplications may still be worthwhile<br>
though, since we may some day get large images working.<br><div class="im"></div></blockquote><div><br>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).<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">
> I'm asking because I don't want to work on a patch that will be denied<br>
> because my solution is wrong.<br>
<br>
</div>I don't know what the best way to do it is, but requiring all rowstride<br>
multiplications to have a cast probably isn't going to be maintainable,<br>
because people will forget about it when writing new code.<span class=""><font color="#888888"><br></font></span></blockquote><div><br><div>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 <br>
</div><div>#define STRIDE_MULT(stride, y) ((ssize_t)(stride)*(y))<br></div><div>to show that's important to not use just "*"<br></div><div><br></div>And about forgetting to do the cast in new code, that's what a bug is ^_^ And that's why review is important. <br>
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class=""><font color="#888888">
Søren<br>
</font></span></blockquote></div><br></div></div>