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

2018-09-21 Thread Eric Anholt
Cedric Roux  writes:

> 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 

Thanks for the fix!  I wrote up a little testsuite for damage that we
can use to test these bugs in the future and made a MR for the whole
thing:

https://gitlab.freedesktop.org/xorg/xserver/merge_requests/23


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

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

2018-09-13 Thread Cedric Roux
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 
---
 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 
#include 

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