[PATCH xserver] miext/damage: take care of the coordinate mode in damagePolyPoint

Cedric Roux sed at free.fr
Wed Sep 12 17:14:18 UTC 2018


The mode (CoordModeOrigin or CoordModePrevious) was not taken into
account when computing the box. The result was a bad drawing of
points in some situations (on my hardware/software configuration,
calling XDrawString followed by XDrawPoints in the mode
CoordModePrevious).

Signed-off-by: Cedric Roux <sed at free.fr>
---
 miext/damage/damage.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/miext/damage/damage.c b/miext/damage/damage.c
index de14d5cc8..f3ae4ebbc 100644
--- a/miext/damage/damage.c
+++ b/miext/damage/damage.c
@@ -829,16 +829,36 @@ damagePolyPoint(DrawablePtr pDrawable,
 
         /* this could be slow if the points were spread out */
 
-        while (--nptTmp) {
-            pptTmp++;
-            if (box.x1 > pptTmp->x)
-                box.x1 = pptTmp->x;
-            else if (box.x2 < pptTmp->x)
-                box.x2 = pptTmp->x;
-            if (box.y1 > pptTmp->y)
-                box.y1 = pptTmp->y;
-            else if (box.y2 < pptTmp->y)
-                box.y2 = pptTmp->y;
+        if (mode == CoordModePrevious) {
+            int x = box.x1;
+            int y = box.y1;
+
+            while (--nptTmp) {
+                pptTmp++;
+                x += pptTmp->x;
+                y += pptTmp->y;
+                if (box.x1 > x)
+                    box.x1 = x;
+                else if (box.x2 < x)
+                    box.x2 = x;
+                if (box.y1 > y)
+                    box.y1 = y;
+                else if (box.y2 < y)
+                    box.y2 = y;
+            }
+        }
+        else {
+            while (--nptTmp) {
+                pptTmp++;
+                if (box.x1 > pptTmp->x)
+                    box.x1 = pptTmp->x;
+                else if (box.x2 < pptTmp->x)
+                    box.x2 = pptTmp->x;
+                if (box.y1 > pptTmp->y)
+                    box.y1 = pptTmp->y;
+                else if (box.y2 < pptTmp->y)
+                    box.y2 = pptTmp->y;
+            }
         }
 
         box.x2++;
-- 
2.17.1

To (maybe) see the bug you can run the following program.
If things are working correctly we should see two G clefs.
But we see only one full G clef and just one point for the
second clef because of the bug. I tested it on a computer
using the nouveau driver. The computer is not very recent,
I don't know if that matters or not. The X server was
version 1.19.6.

When not using acceleration the two G clefs are drawn properly
(well, the X server probably doesn't call damagePolyPoint in
this case).

On another computer the two clefs are drawn properly (version 1.20.0).
But to me, there is still a bug, damagePolylines and damageFillPolygon
discriminate between CoordModeOrigin and CoordModePrevious.

Plus, applying the patch on the computer where things fail fixes
the problem.

#include <stdio.h>
#include <X11/Xlib.h>

void plot(Display *d, Window w, Pixmap xp, int window_width, int window_height)
{
  GC gc;
  XGCValues v;
  XPoint p[] = {
    {x:18,y:20},{x:1,y:0},{x:1,y:0},{x:-3,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:-3,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:-5,y:1},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:-5,y:1},{x:1,y:0},{x:1,y:0},
    {x:3,y:0},{x:1,y:0},{x:-6,y:1},{x:1,y:0},{x:4,y:0},{x:1,y:0},{x:-7,y:1},
    {x:1,y:0},{x:1,y:0},{x:4,y:0},{x:1,y:0},{x:-7,y:1},{x:1,y:0},{x:1,y:0},
    {x:4,y:0},{x:1,y:0},{x:-7,y:1},{x:1,y:0},{x:1,y:0},{x:4,y:0},{x:1,y:0},
    {x:-7,y:1},{x:1,y:0},{x:1,y:0},{x:3,y:0},{x:1,y:0},{x:1,y:0},{x:-6,y:1},
    {x:1,y:0},{x:3,y:0},{x:1,y:0},{x:1,y:0},{x:-6,y:1},{x:1,y:0},{x:2,y:0},
    {x:1,y:0},{x:1,y:0},{x:-5,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:-4,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:-5,y:1},
    {x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:-5,y:1},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:1,y:0},{x:-6,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:-6,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:-5,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:-6,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:2,y:0},{x:-6,y:1},
    {x:1,y:0},{x:1,y:0},{x:1,y:0},{x:3,y:0},{x:-7,y:1},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:4,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:-10,y:1},{x:1,y:0},
    {x:1,y:0},{x:4,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:-11,y:1},{x:1,y:0},{x:5,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:-13,y:1},{x:1,y:0},{x:1,y:0},{x:4,y:0},{x:1,y:0},
    {x:2,y:0},{x:3,y:0},{x:1,y:0},{x:1,y:0},{x:-14,y:1},{x:1,y:0},{x:5,y:0},
    {x:1,y:0},{x:2,y:0},{x:4,y:0},{x:1,y:0},{x:-14,y:1},{x:1,y:0},{x:5,y:0},
    {x:3,y:0},{x:1,y:0},{x:4,y:0},{x:1,y:0},{x:-15,y:1},{x:1,y:0},{x:5,y:0},
    {x:4,y:0},{x:4,y:0},{x:1,y:0},{x:-15,y:1},{x:1,y:0},{x:1,y:0},{x:4,y:0},
    {x:1,y:0},{x:3,y:0},{x:4,y:0},{x:1,y:0},{x:-14,y:1},{x:1,y:0},{x:5,y:0},
    {x:3,y:0},{x:4,y:0},{x:1,y:0},{x:-14,y:1},{x:1,y:0},{x:1,y:0},{x:7,y:0},
    {x:3,y:0},{x:1,y:0},{x:-12,y:1},{x:1,y:0},{x:1,y:0},{x:6,y:0},{x:2,y:0},
    {x:1,y:0},{x:1,y:0},{x:-11,y:1},{x:1,y:0},{x:1,y:0},{x:5,y:0},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:-9,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:1,y:0},{x:-6,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:1,y:0},{x:0,y:1},{x:0,y:1},{x:1,y:0},{x:-1,y:1},
    {x:1,y:0},{x:-7,y:1},{x:1,y:0},{x:1,y:0},{x:5,y:0},{x:-8,y:1},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:1,y:0},{x:4,y:0},{x:-8,y:1},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:1,y:0},{x:4,y:0},{x:-8,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:1,y:0},{x:3,y:0},{x:1,y:0},{x:-7,y:1},{x:1,y:0},{x:1,y:0},{x:4,y:0},
    {x:-6,y:1},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},{x:1,y:0},
    {x:-4,y:1},{x:1,y:0},{x:1,y:0}
  };

  /* clear */
  gc = DefaultGC(d, DefaultScreen(d));
  v.foreground = WhitePixel(d, DefaultScreen(d));
  XChangeGC(d, gc, GCForeground, &v);
  XFillRectangle(d, xp, gc, 0, 0, window_width, window_height);
  v.foreground = BlackPixel(d, DefaultScreen(d));
  XChangeGC(d, gc, GCForeground, &v);

#if 1
  /* draw points */
  p[0].x = 10;
  p[0].y = 10;
  XDrawPoints(d, xp, DefaultGC(d, DefaultScreen(d)),
              p, sizeof(p)/sizeof(XPoint), CoordModePrevious);
#endif

  /* draw string */
  XDrawString(d, xp, DefaultGC(d, DefaultScreen(d)),
              100, 100, "test", 4);

  /* draw points */
  p[0].x = 40;
  p[0].y = 10;
  XDrawPoints(d, xp, DefaultGC(d, DefaultScreen(d)),
              p, sizeof(p)/sizeof(XPoint), CoordModePrevious);
}

int main(void)
{
  int window_width = 200;
  int window_height = 200;
  Display *d = XOpenDisplay(0);
  Window w = XCreateSimpleWindow(d, DefaultRootWindow(d), 0, 0,
          window_width, window_height,
          0, 0, WhitePixel(d, DefaultScreen(d)));
  Pixmap xp = XCreatePixmap(d, w, window_width, window_height,
           DefaultDepth(d, DefaultScreen(d)));

  plot(d, w, xp, window_width, window_height);

  XSelectInput(d, w, ExposureMask);
  XMapWindow(d, w);

  while (1) {
    XEvent ev;
    XNextEvent(d, &ev);
    switch (ev.type) {
      case Expose:
        XCopyArea(d, xp, w, DefaultGC(d, DefaultScreen(d)),
                  0, 0, window_width, window_height, 0, 0);
        break;
      default: printf("got event %d\n", ev.type); break;
    }
  }
}


More information about the xorg-devel mailing list