On Fri, 29 Apr 2016 10:15:37 +0100 Emil Velikov <emil.l.veli...@gmail.com> wrote:
> On 27 April 2016 at 18:46, Bill Spitzak <spit...@gmail.com> wrote: > > On Wed, Apr 27, 2016 at 2:03 AM, Pekka Paalanen <ppaala...@gmail.com> > > wrote: > >> > >> On Wed, 27 Apr 2016 09:56:44 +0100 > >> Emil Velikov <emil.l.veli...@gmail.com> wrote: > >> > >> > On 26 April 2016 at 19:12, Bill Spitzak <spit...@gmail.com> wrote: > >> > > The old code is comparing pixman_fixed_48_16_t values to > >> > > pixman_fixed_16_16_t values, thus it is checking for truncation of > >> > > overflow > >> > > values. > >> > > > >> > Indeed it does. I'll grep more before asking silly questions ;-) > >> > > >> > > It would probably be better to clamp these overflowed values, like > >> > > pixman_transform_point_31_16 is doing to clamp to the > >> > > pixman_fixed_48_16 > >> > > result. Right now the result is an odd mix of clamping and modulus. A > >> > > rewrite to go directly to clamped pixman_fixed_16_16 values would be > >> > > even > >> > > better. > >> > > > >> > Sounds like a plan. Sadly I doubt I'll get to it any time soon. > >> > >> Wasn't the point of the boolean return from these functions to tell the > >> caller to drop what it is doing because it cannot be done properly? > > > > > > Dropping a fill is a lot worse than trying to simulate it using the clamped > > path. It will produce a wrong result if one of the edges connected to a > > clamped point passes through the clip, but this often does not happen, and > > the transition to the wrong result is gradual as the point moves outside the > > clamped region. > > > > More importantly the caller cannot do anything with the return values right > > now, as they are modulus MAX_16_16+1. Even the direction they are in is > > lost. > > > I think that keeping the user provided memory as-is when the function > does not succeed is a good idea. > Afaics currently the contents get overwritten regardless of the result. > > This is what you guys were on about, right ? Or perhaps you're > thinking about spinning v2 of the function with different > signature/behaviour ? Hi Emil, I think the conclusion was that the comparisons are not useless, and this patch should be dropped. You noted it yourself that this patch causes a regression in the test suite. Thanks, pq
pgp6Le5fmbKgX.pgp
Description: OpenPGP digital signature
_______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pixman