[Pixman] [PATCH v3 1/2] Remove the 8e extra safety margin in COVER_CLIP analysis

[Siarhei Siamashka]

On Fri, 25 Sep 2015 14:38:46 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Fri, 25 Sep 2015 07:38:34 +0300
> Siarhei Siamashka <siarhei.siamashka at gmail.com> wrote:
> 
> > On Wed, 23 Sep 2015 11:40:32 +0300
> > Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > 
> > > From: Ben Avison <bavison at riscosopen.org>
> > > 
> > > As discussed in
> > > http://lists.freedesktop.org/archives/pixman/2015-August/003905.html
> > > 
> > > the 8 * pixman_fixed_e (8e) adjustment which was applied to the transformed
> > > coordinates is a legacy of rounding errors which used to occur in old
> > > versions of Pixman, but which no longer apply. For any affine transform,
> > > you are now guaranteed to get the same result by transforming the upper
> > > coordinate as though you transform the lower coordinate and add (size-1)
> > > steps of the increment in source coordinate space. No projective
> > > transform routines use the COVER_CLIP flags, so they cannot be affected.
> > > 
> > > Proof by Siarhei Siamashka:
> 
> > > Cc: Siarhei Siamashka <siarhei.siamashka at gmail.com>
> > > Signed-off-by: Ben Avison <bavison at riscosopen.org>
> > > [Pekka: adjusted commit message, left affine-bench changes for another patch]
> > > [Pekka: add commit message parts from Siarhei]
> > > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > 
> > > ---
> > > 
> > > v2 is part 1/2 of the patch "Change conditions for setting
> > > FAST_PATH_SAMPLES_COVER_CLIP flags".
> > > 
> > > This is v3 with a yet more bikeshedded commit message.
> > > 
> > > Ben, Siarhei,
> > > 
> > > for the record, gentlemen, please give your reviewed-by's
> > > so I can just push these two out without any confusion. :-)
> > 
> > Yes, sure. Even the older commit message already included a
> > link to the mailing list archive, so I did not have any
> > problems with it. This final revision is also fine.
> > 
> > Reviewed-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>
> 
> With all the comments, well-actuallys and by-the-ways, it's sometimes
> hard to tell. :-)
> 
> I wasn't sure if you meant to give R-b also for 2/2, so I put only
> Ben's R-b there.
> 
> Pushed:
>    23525b4..2876d8d  master -> master

Thanks!

It was one of these patches, where the commit message is much larger
than the patch itself. The difficult part was not about doing the
modifications to the code, but about deciding whether this is really
safe to do. An off-by-one error can be pretty much devastating.

About the 2/2 patch, it goes to the "test" directory and does not
introduce any real risk. The quality bar for such code is generally
much lower and it does not need a thorough review. Maybe I should
have given my R-b for it though, but it's not a big deal.

-- 
Best regards,
Siarhei Siamashka