[Glamor] [PATCH] Fix a bug for trapezoid clip
Zhigang Gong
zhigang.gong at linux.intel.com
Fri Jun 15 04:45:10 PDT 2012
Junyan，
Thanks for the patch, I just pushed and by default trapezoid shader is
disabled.
Although this patch fixes some problem, It still seems that we still have
accurate problem even
with this patch. Most of the cases look like just slightly accurate error.
But one case is more clear
for me, you can see a1-clip-fill-rule.
The reference image get correct antialiasing result which is still
symmetrical, but with this patch
The output image is obviously dissymmetrical.
We don't pursue exactly match the reference image, as we are using float but
the reference method
is using fixed function. But we can't reduce the quality. Technically, we
should get identical or even
better quality than the reference image.
> -----Original Message-----
> From:
> glamor-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.org
> [mailto:glamor-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.o
> rg] On Behalf Of junyan.he at linux.intel.com
> Sent: Friday, June 15, 2012 9:00 AM
> To: zhigang.gong at linux.intel.com
> Cc: Junyan He; glamor at lists.freedesktop.org
> Subject: [Glamor] [PATCH] Fix a bug for trapezoid clip
>
> From: Junyan He <junyan.he at linux.intel.com>
>
> We find in some cases the trapezoid will be render as a triangle and the
> left edge and right edge will cross with each other just bellow the top
or
> over the bottom. The distance between the cross poind and the top or
> bottom is less than pixman_fixed_1_minus_e, so after the fixed
> converted to int, the cross point has the same value with the top or
> botton and the triangle should not be affected. But in our clip logic,
the
> cross point will be clipped out. So add a logic to fix this problem.
>
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
> src/glamor_priv.h | 2 +-
> src/glamor_trapezoid.c | 115
> ++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 108 insertions(+), 9 deletions(-)
>
> diff --git a/src/glamor_priv.h b/src/glamor_priv.h index 1b45a71..055077c
> 100644
> --- a/src/glamor_priv.h
> +++ b/src/glamor_priv.h
> @@ -965,7 +965,7 @@ glamor_poly_line(DrawablePtr pDrawable, GCPtr
> pGC, int mode, int npt, #define GLAMOR_PIXMAP_DYNAMIC_UPLOAD
> #ifndef GLAMOR_GLES2 #define GLAMOR_GRADIENT_SHADER -#define
> GLAMOR_TRAPEZOID_SHADER
> +//#define GLAMOR_TRAPEZOID_SHADER
> #endif
> #define GLAMOR_TEXTURED_LARGE_PIXMAP 1
> #if 0
> diff --git a/src/glamor_trapezoid.c b/src/glamor_trapezoid.c index
> cc6a706..6f85769 100644
> --- a/src/glamor_trapezoid.c
> +++ b/src/glamor_trapezoid.c
> @@ -38,6 +38,8 @@
>
> #ifdef GLAMOR_TRAPEZOID_SHADER
>
> +#define DEBUG_CLIP_VTX 0
> +
> #define POINT_INSIDE_CLIP_RECT(point, rect) \
> (point[0] >= IntToxFixed(rect->x1) \
> && point[0] <= IntToxFixed(rect->x2) \
> @@ -45,6 +47,28 @@
> && point[1] <= IntToxFixed(rect->y2))
>
> static xFixed
> +_glamor_lines_crossfixedY (xLineFixed *l, xLineFixed *r) {
> + xFixed dx1 = l->p2.x - l->p1.x;
> + xFixed dx2 = r->p2.x - r->p1.x;
> + xFixed dy1 = l->p2.y - l->p1.y;
> + xFixed dy2 = r->p2.y - r->p1.y;
> + xFixed_32_32 tmp = (xFixed_32_32) dy2 * dy1;
> + xFixed_32_32 dividend1 = (tmp >> 32) * (l->p1.x - r->p1.x);
> + tmp = (xFixed_32_32) dx1 * dy2;
> + xFixed_32_32 dividend2 = (tmp >> 32) * l->p1.y;
> + tmp = (xFixed_32_32) dy1 * dx2;
> + xFixed_32_32 dividend3 = (tmp >> 32) * r->p1.y;
> + xFixed_32_32 divisor = ((xFixed_32_32) dx1 * (xFixed_32_32) dy2
> + - (xFixed_32_32) dy1 * (xFixed_32_32)
> dx2)
> +>> 32;
> +
> + if (divisor)
> + return (xFixed)((dividend2 - dividend1 - dividend3) /
divisor);
> +
> + return 0xFFFFFFFF;
> +}
> +
> +static xFixed
> _glamor_linefixedX (xLineFixed *l, xFixed y, Bool ceil) {
> xFixed dx = l->p2.x - l->p1.x;
> @@ -67,22 +91,86 @@ _glamor_linefixedY (xLineFixed *l, xFixed x, Bool
> ceil) }
>
> static Bool
> -point_inside_trapezoid(int point[2], xTrapezoid * trap)
> +point_inside_trapezoid(int point[2], xTrapezoid * trap, xFixed cut_y)
> {
> int ret = TRUE;
> int tmp;
> - if (point[1] > trap->bottom
> - || point[1] < trap->top)
> + if (point[1] > trap->bottom) {
> + ret = FALSE;
> + if (DEBUG_CLIP_VTX) {
> + ErrorF("Out of Trap bottom, point[1] = %d(0x%x)), "
> + "bottom = %d(0x%x)\n",
> + (unsigned int)xFixedToInt(point[1]),
point[1],
> + (unsigned int)xFixedToInt(trap->bottom),
> + (unsigned int)trap->bottom);
> + }
> +
> + return ret;
> + }
> +
> + if (point[1] < trap->top) {
> ret = FALSE;
> + if (DEBUG_CLIP_VTX) {
> + ErrorF("Out of Trap top, point[1] = %d(0x%x)), "
> + "top = %d(0x%x)\n",
> + (unsigned int)xFixedToInt(point[1]), point[1],
> + (unsigned int)xFixedToInt(trap->top),
> + (unsigned int)trap->top);
> + }
> +
> + return ret;
> + }
>
> tmp = _glamor_linefixedX (&trap->left, point[1], FALSE);
> - if (point[0] < tmp)
> + if (point[0] < tmp) {
> ret = FALSE;
> +
> + if (abs(cut_y - trap->top) < pixman_fixed_1_minus_e &&
> + abs(point[1] - trap->top) <
pixman_fixed_1_minus_e
> &&
> + tmp - point[0] < pixman_fixed_1_minus_e) {
> + ret = TRUE;
> + } else if (abs(cut_y - trap->bottom) <
pixman_fixed_1_minus_e
> &&
> + point[1] - trap->bottom < pixman_fixed_1_minus_e
> &&
> + tmp - point[0] < pixman_fixed_1_minus_e) {
> + ret = TRUE;
> + }
> +
> + if (DEBUG_CLIP_VTX && !ret) {
> + ErrorF("Out of Trap left, point[0] = %d(0x%x)), "
> + "left = %d(0x%x)\n",
> + (unsigned int)xFixedToInt(point[0]),
point[0],
> + (unsigned int)xFixedToInt(tmp), (unsigned
int)tmp);
> + }
> +
> + if (!ret)
> + return ret;
> + }
>
> tmp = _glamor_linefixedX (&trap->right, point[1], TRUE);
> - if (point[0] > tmp)
> + if (point[0] > tmp) {
> ret = FALSE;
>
> + if (abs(cut_y - trap->top) < pixman_fixed_1_minus_e &&
> + abs(point[1] - trap->top) <
pixman_fixed_1_minus_e
> &&
> + point[0] - tmp < pixman_fixed_1_minus_e) {
> + ret = TRUE;
> + } else if (abs(cut_y - trap->bottom) <
pixman_fixed_1_minus_e
> &&
> + abs(point[1] - trap->bottom) <
> pixman_fixed_1_minus_e &&
> + point[0] - tmp < pixman_fixed_1_minus_e) {
> + ret = TRUE;
> + }
> +
> + if (DEBUG_CLIP_VTX && !ret) {
> + ErrorF("Out of Trap right, point[0] = %d(0x%x)), "
> + "right = %d(0x%x)\n",
> + (unsigned int)xFixedToInt(point[0]),
point[0],
> + (unsigned int)xFixedToInt(tmp), (unsigned
int)tmp);
> + }
> +
> + if (!ret)
> + return ret;
> + }
> +
> return ret;
> }
>
> @@ -125,12 +213,11 @@ glamor_flush_composite_triangles(ScreenPtr
> screen)
> glamor_put_dispatch(glamor_priv);
> }
>
> -#define DEBUG_CLIP_VTX 0
> -
> static Bool
> _glamor_clip_trapezoid_vertex(xTrapezoid * trap, BoxPtr pbox,
> int vertex[6], int *num)
> {
> + xFixed edge_cross_y = 0xFFFFFFFF;
> int tl[2];
> int bl[2];
> int tr[2];
> @@ -217,7 +304,7 @@ _glamor_clip_trapezoid_vertex(xTrapezoid * trap,
> BoxPtr pbox,
>
> #define ADD_VERTEX_IF_INSIDE(vtx) \
> if(POINT_INSIDE_CLIP_RECT(vtx, pbox) \
> - && point_inside_trapezoid(vtx, trap)){ \
> + && point_inside_trapezoid(vtx, trap, edge_cross_y)){ \
> tmp_vtx[vertex_num] = xFixedToInt(vtx[0]); \
> tmp_vtx[vertex_num + 1] = xFixedToInt(vtx[1]); \
> vertex_num += 2; \
> @@ -239,6 +326,18 @@ _glamor_clip_trapezoid_vertex(xTrapezoid *
> trap, BoxPtr pbox,
> "the Rect\n"); \
> }
>
> + /*Trap's right edge cut right edge. */
> + if((!IS_TRAP_EDGE_VERTICAL((&trap->left))) ||
> + (!IS_TRAP_EDGE_VERTICAL((&trap->right)))) {
> + edge_cross_y = _glamor_lines_crossfixedY((&trap->left),
> (&trap->right));
> + if (DEBUG_CLIP_VTX) {
> + ErrorF("Trap's left edge cut right edge at %d(0x%x),
"
> + "trap_top = %x, trap_bottom = %x\n",
> + xFixedToInt(edge_cross_y), edge_cross_y,
> + (unsigned int)trap->top, (unsigned
int)trap->bottom);
> + }
> + }
> +
> /*Trap's TopLeft, BottomLeft, TopRight and BottomRight. */
> CACULATE_CUT_VERTEX(tl, 1, FALSE, trap->top, (&trap->left));
> CACULATE_CUT_VERTEX(bl, 1, FALSE, trap->bottom, (&trap->left));
> --
> 1.7.7.6
>
> _______________________________________________
> Glamor mailing list
> Glamor at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/glamor
More information about the Glamor
mailing list