[comp.windows.x] Definite problem with Xlib image code, not-so-definite solution

toddb@tekcrl.crl (Todd Brunhoff) (09/27/88)

I believe there's a bug in the Xlib PutImage code for
    - scanline unit >= 16
    - format is XYPixmap (or depth one ZPixmap)
    - source (client) and destination (server) bit ordering or byte ordering
      do not match.

Given the situation above, let me show you how the code in
lib/X/XPutImage.c is correct for half the cases and incorrect for the
other half.  Lest I lose your attention, first the incorrect case.
Assume that we have an image with the following properties:
    - scanline unit is 16
    - format is XYPixmap
    - byte order is MSBFirst
    - bit order is LSBFirst
    - width is 8, height is 1

Now, the legend in the code found in lib/X/XPutImage.c says that my bit
and byte ordering for this image would look like

	byte0 byte1
	----- -----
	15-08 07-00

(Find this on the line labeled '2Ml' about line 340 in lib/X/XPutImage.c)
Since the width is 8 pixels (or bits, with depth=1), then my
interesting data in in the second byte.  Remember this. Now assume that
the server has the following properties:
    - scanline unit is 16 (screen bitmap pad)
    - byte order is MSBFirst
    - bit order is MSBFirst

Note that the server's properties and the image in the client differ only
in the bit order.  The lib/X/XPutImage.c code should then change the data
to look like

	byte0 byte1
	----- -----
	00-07 08-15

In this case, SendXYImage() would eventally get called and would
accomplish the massaging by a call to the (computed) function
SwapBitsAndTwoBytes with the arguments:
  srclen = 1
	Computed by line 464
		 bytes_per_src = ((long)req->width + req->leftPad + 7) >> 3;
	where req->width is 8 and req->leftPad is computed to be 0.

  srcinc = 2
  destinc = 2
  height = 1

	static int
	SwapBitsAndTwoBytes (src, dest, srclen, srcinc, destinc, height)
	    register unsigned char *src, *dest;
	    long srclen, srcinc, destinc;
	    unsigned int height;
	{
	    long length = ROUNDUP(srclen, 2);
	    register long h, n;
	    register unsigned char *rev = _reverse_byte;

	    srcinc -= length;
	    destinc -= length;
	    for (h = height; --h >= 0; src += srcinc, dest += destinc) {
    --->	if ((h == 0) && (srclen != length)) {
    --->	    length -= 2;
    --->	    *(dest + length + 1) = rev[*(src + length)];
		}
		for (n = length; n > 0; n -= 2, src += 2) {
		    *dest++ = rev[*(src + 1)];
		    *dest++ = rev[*src];
		}
	    }
	}

Note that when it does the last row, it tries to avoid reading off the
end of the 'src' pointer by doing something special (all the
SwapSomethingOrOther functions in this file do that).  You should
see that for this example, only the lines marked with the arrows
will be executed.  But this means that instead of getting the bits
07-00 in byte one moved to byte 0, we get:

	byte0 byte1
	----- -----
	??-?? 08-15

This only affects the last 1-3 bytes of the last line of the image,
depending on how many bytes have to be swapped.  There is no problem if the
image has bit-order MSBFirst and the server has bit-order LSBFirst;
this represents the half of the cases that are correct.

The solution, I believe, is to change the 3rd arg in final call to the
swapfunc function at about line 511 from

	(*swapfunc)((unsigned char *)src, (unsigned char *)dest,
                    bytes_per_src, bytes_per_line,
                    bytes_per_dest, req->height);

to

	(*swapfunc)((unsigned char *)src, (unsigned char *)dest,
                    bytes_per_line, bytes_per_line,
                    bytes_per_dest, req->height);

But there is some tom-foolery having to do with the x-offset that
is not documented here, and that I don't understand.  I can't imagine
a case where the 'srclen' argument to the swapfunc function should
not be image->bytes_per_line.  I have always believed that the purpose
of the special code in the SwapSomethingOrOther functions was to prevent
reading off the end of a client's image when the image->bytes_per_line
is less than the computed bytes_per_dest (for example, when the server's
scanline unit is greater than the image's).

Help.

- ---------------
Usenet:       {ucbvax,decvax,allegra,uw-beaver,hplabs}!tektronix!crl!toddb
{CS,ARPA}net: toddb%tekcrl.crl.tek.com@relay.cs.net                  c--Q Q
US:           Todd Brunhoff; Visual Systems Lab; Tektronix, Inc.         `
              Box 500  MS 50-662, Beaverton OR 97077                     -
Phone:        (503) 627-1121

------- End of Forwarded Message