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

[Siarhei Siamashka]

On Tue, 22 Sep 2015 13:54:54 +0100
"Ben Avison" <bavison at riscosopen.org> wrote:

> On Mon, 21 Sep 2015 06:32:48 +0100, Siarhei Siamashka <siarhei.siamashka at gmail.com> wrote:
> > Since we are trying to justify the 8e extra safety margin removal in
> > the context of this patch, this is what I wanted to see explained in
> > the commit message. But maybe I'm just bad at math and it was perfectly
> > obvious to everyone else without any special proof.
> 
> I think it's just that if you've come on board since the old rounding
> code was removed (as I have) it's hard to see why this would ever have
> been a problem. If you express your P vector as a linear combination of
> vectors
> 
> | frac(x_dst) |   | int(x_dst) |
> | frac(y_dst) | + | int(y_dst) |
> | 0x10000     |   | 0          |

We can only do this if we are sure that the distributive property is
maintained and "A * (B + C)" is really equivalent to "A * B + A * C".
Yes, this is true for matrix multiplication in its canonical form.
However we are not doing perfectly accurate matrix multiplication and
instead have a reasonably close approximation with rounding. And for
example, floating-point calculations are not distributive.

> then it's clear that the first component is an invariant (0x8000, 0x8000,
> 0x10000) irrespective of whether you reach P in a stepwise manner or not,
> and that the other one uses only integers. Any integer multiplied by a
> fixed-point number is a fixed-point number (of the same number of
> fractional digits) without any rounding errors, so in the absence of any
> intermediate rounding steps, the rounding error of the expression as a
> whole is the same as the rounding error of the first component, and that
> hasn't changed.

You already mention the rounding implementation details and making
assumptions about them. And I just expanded the whole matrix
multiplication in the way it is implemented in pixman, so that we
only have to look at ordinary integer calculations.

> >> Proof:
> >>
> >> All implementations must give the same numerical results as
> >> bits_image_fetch_pixel_nearest() / bits_image_fetch_pixel_bilinear().
> >>
> >> The former does
> >>     int x0 = pixman_fixed_to_int (x - pixman_fixed_e);
> >> which maps directly to the new test for the nearest flag, when you consider
> >> that x0 must fall in the interval [0,width).
> >>
> >> The latter does
> >>     x1 = x - pixman_fixed_1 / 2;
> >>     x1 = pixman_fixed_to_int (x1);
> >>     x2 = x1 + 1;
> >> but then x2 is brought back in range by the repeat() function, so it can't
> >> stray beyond the end of the source line.
> >
> > The wording does not look very good here. It seems to imply that the
> > repeat() function has some special use in the latter (BILINEAR) case.
> > But the repeat handling is exactly the same for NEAREST and BILINEAR.
> > Also for NONE repeat and fetching pixels from the outside of the source
> > image bounds, we are not bringing the coordinate back into range but
> > interpreting the pixel value as zero.
> 
> I can't follow your argument there - I don't think I implied that
> repeat() acted differently for the bilinear case?

It's just that you had a special emphasis on the x2 variable here. And
x2 is only used for the bilinear case.

> On NONE repeat, yes I neglected that detail, but I was generalising. How
> about:
> 
> The latter does
>      x1 = x - pixman_fixed_1 / 2;
>      x1 = pixman_fixed_to_int (x1);
>      x2 = x1 + 1;
> but any values of x2 that correspond to a pixel offset beyond the end of
> the source line are never used to dereference the pixel array. In the
> case of NONE repeat, a pixel value of zero is substituted, and otherwise
> the action of the repeat() function, when applied to x2, is to select a
> different pixel offset which *does* lie within the source line (the exact
> choice depends upon the repeat type selected).

Replace "x2" -> "x1" and "beyond the end" -> "beyond the beginning" in
this paragraph and it will be still true.


-- 
Best regards,
Siarhei Siamashka