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

[Pekka Paalanen]

On Fri, 29 Apr 2016 10:15:37 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

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

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/pixman/attachments/20160429/bf82b8d5/attachment.sig>