[cairo] Pixman sampling coordinates (Re: White seams (lines) appearing between objects

[Carl Worth]

On Sun, 20 Jan 2008 04:24:37 +0100, Bertram Felgenhauer wrote:
> However, the good news is that the patch is still correct. In
> fact we have some degree of freedom in rounding the left and
> right edges for N_BITS > 1, as long as it only affects pixels
> with 0% coverage.

Bertram,

I really appreciate you looking into all of this, and with such
attention to detail. Thank you!

> The bad news is that it makes hitting the  rxs == 0  case in
> fbRasterizeEdges{4,8} more likely.
>
> The good news is that we can safely eliminate that check if we
> assign the right edge to the rightmost pixel of the scanline
> instead, i.e.

Very nice. I always considered the "if (rxs)" case I added to be an
ugly band-aid, but I hadn't found the proper fix. So this will be a
nice cleanup to have.

> Actually it's even true after my previous patch for  a1-traps-sample,
> as long as pixman_fixed_t has at least 2*n fraction bits, which is
> currently the case. But with the plans to change to 20.12 precision,
> that may change soon.

Am I reading this correctly that your previous patch introduced this
fragility, but with the new approach the fragility is gone? That
sounds very encouraging, so thanks.

> Patch 2a:
>     Alternative fix for the  a1-traps-sample  testcase which
>     keeps the rounding version of RenderSamplesX().

I'm a little confused by what patch sequencing you intend here. Also,
the patches you've attached, (which look like they came from "git
show" or so), are not accepted by "git am". I think that's a bug in
git, (not accepting the same syntax it emits), but there it is.

Do you have a complete git branch you can put somewhere that I can
just pull?

>  	/* clip X */
> -	lx = l->x;
> +#if N_BITS == 1
> +	lx = l->x + X_FRAC_FIRST(1);
> +	if (lx < 0)
> +	    lx = 0;
> +	rx = r->x + X_FRAC_FIRST(1);
> +	if (pixman_fixed_to_int (rx) >= width)
> +	    rx = pixman_int_to_fixed (width);
> +#else
>  	if (lx < 0)
>  	    lx = 0;
>  	rx = r->x;
>  	if (pixman_fixed_to_int (rx) >= width)
>  	    rx = pixman_int_to_fixed (width);
> +#endif

Let's please fix the above to keep the common code outside of the
#ifdef, (perhaps a #else won't even be necessary). From this context
it looks like an assignment to lx is missing from th #else case
anyway, right?

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20080121/83fb419c/attachment.pgp