[Pixman] [RFC 1/2] Remove seemingly unneeded comparison(s)

[Emil Velikov]

On 27 April 2016 at 18:46, Bill Spitzak <spitzak at gmail.com> wrote:
> On Wed, Apr 27, 2016 at 2:03 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>
>> On Wed, 27 Apr 2016 09:56:44 +0100
>> Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>
>> > On 26 April 2016 at 19:12, Bill Spitzak <spitzak at 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 ?

-Emil