[cairo] Pixman sampling coordinates (Re: White seams (lines) appearing between objects
Bertram Felgenhauer
bertram.felgenhauer at googlemail.com
Sat Jan 19 19:24:37 PST 2008
Bertram Felgenhauer wrote:
[snip]
> Patch 2: fix cairo's a1-traps-sample test
> This patch actually fixes a problem with pixman's RenderSamplesX()
> macro, which could assign a subpixel a coverage of over 100%.
Sorry, I was hallucinating when I wrote that. RenderSamplesX()
divides its result by STEP_X_SMALL(n), which will truncate all
the excess coverage.
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.
Consider a scanline's view of three adjacent pixels,
Pixel 0 || Pixel 1 || Pixel 2
with subpixels,
| sp n || sp 0 | sp 1 | ... | sp | sp n || sp 0 |
Now consider a span that covers less than half of subpixel n of
pixel 0:
| >||------|------|-...-|------|-< || |
| sp n || sp 0 | sp 1 | ... | sp | sp n || sp 0 |
It does not matter whether we assign the left end to pixel 0 (where
it will result in a coverage of 0%) or to pixel 1 (resulting in
a coverage of 100%).
Pixman assigns the end points to the pixels they're in, so the left
edge will be assigned to pixel 0.
With my patch from the previous mail, pixman will "round up", and
assign the left end point in the example to pixel 1.
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.
if (pixman_fixed_to_int (rx) >= width)
rx = pixman_int_to_fixed (width) - 1;
This requires RenderSamplesX(-1, n) == N_X_FRAC(n) to result
in a pixel of 100% coverage. This is true for the original rounding
version of RenderSamplesX.
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.
New patches:
Patch 2a:
Alternative fix for the a1-traps-sample testcase which
keeps the rounding version of RenderSamplesX().
Patch 4:
Eliminate the if (rxs) check in fbRasterizeEdges{4,8}()
as discussed above.
There are no changes in the testsuite results.
regards,
Bertram
-------------- next part --------------
commit 43a0e46ad92aecc94a68e6087985421e4d311b9e
Author: Bertram Felgenhauer <int-e at gmx.de>
Date: Sun Jan 20 04:03:34 2008 +0100
fix cairo's a1-traps-sample test
For 1 bit alpha targets, the left and right boundaries of edges were
not rounded correctly.
diff --git a/pixman/pixman-edge-imp.h b/pixman/pixman-edge-imp.h
index 2cf6d73..0827fd2 100644
--- a/pixman/pixman-edge-imp.h
+++ b/pixman/pixman-edge-imp.h
@@ -46,12 +46,20 @@ rasterizeEdges (pixman_image_t *image,
int rxi;
/* 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
/* Skip empty (or backwards) sections */
if (rx > lx)
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index 33a9e7c..075feb6 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -748,7 +748,7 @@ union pixman_image
#define MAX_ALPHA(n) ((1 << (n)) - 1)
#define N_Y_FRAC(n) ((n) == 1 ? 1 : (1 << ((n)/2)) - 1)
-#define N_X_FRAC(n) ((1 << ((n)/2)) + 1)
+#define N_X_FRAC(n) ((n) == 1 ? 1 : (1 << ((n)/2)) + 1)
#define STEP_Y_SMALL(n) (pixman_fixed_1 / N_Y_FRAC(n))
#define STEP_Y_BIG(n) (pixman_fixed_1 - (N_Y_FRAC(n) - 1) * STEP_Y_SMALL(n))
-------------- next part --------------
commit b014da634015b03418e165767eb1585767f8c5a7
Author: Bertram Felgenhauer <int-e at gmx.de>
Date: Sun Jan 20 04:07:32 2008 +0100
eliminate a special case in fbRasterizeEdges{4,8}.
diff --git a/pixman/pixman-edge-imp.h b/pixman/pixman-edge-imp.h
index 0827fd2..8a0b682 100644
--- a/pixman/pixman-edge-imp.h
+++ b/pixman/pixman-edge-imp.h
@@ -58,7 +58,11 @@ rasterizeEdges (pixman_image_t *image,
lx = 0;
rx = r->x;
if (pixman_fixed_to_int (rx) >= width)
- rx = pixman_int_to_fixed (width);
+ /* Use the last pixel of the scanline, covered 100%.
+ * We can't use the first pixel following the scanline,
+ * because accessing it could result in a buffer overrun.
+ */
+ rx = pixman_int_to_fixed (width) - 1;
#endif
/* Skip empty (or backwards) sections */
@@ -117,12 +121,7 @@ rasterizeEdges (pixman_image_t *image,
AddAlpha (N_X_FRAC(N_BITS));
StepAlpha;
}
- /* Do not add in a 0 alpha here. This check is necessary
- * to avoid a buffer overrun when rx is exactly on a pixel
- * boundary.
- */
- if (rxs != 0)
- AddAlpha (rxs);
+ AddAlpha (rxs);
}
}
#endif
diff --git a/pixman/pixman-edge.c b/pixman/pixman-edge.c
index 1ed6f10..4a9df9a 100644
--- a/pixman/pixman-edge.c
+++ b/pixman/pixman-edge.c
@@ -147,7 +147,11 @@ fbRasterizeEdges8 (pixman_image_t *image,
lx = 0;
rx = r->x;
if (pixman_fixed_to_int (rx) >= width)
- rx = pixman_int_to_fixed (width);
+ /* Use the last pixel of the scanline, covered 100%.
+ * We can't use the first pixel following the scanline,
+ * because accessing it could result in a buffer overrun.
+ */
+ rx = pixman_int_to_fixed (width) - 1;
/* Skip empty (or backwards) sections */
if (rx > lx)
@@ -235,11 +239,7 @@ fbRasterizeEdges8 (pixman_image_t *image,
add_saturate_8 (ap + lxi, N_X_FRAC(8), rxi - lxi);
}
- /* Do not add in a 0 alpha here. This check is
- * necessary to avoid a buffer overrun, (when rx
- * is exactly on a pixel boundary). */
- if (rxs)
- WRITE(image, ap + rxi, clip255 (READ(image, ap + rxi) + rxs));
+ WRITE(image, ap + rxi, clip255 (READ(image, ap + rxi) + rxs));
}
}
More information about the cairo
mailing list