[PATCH xts] Remove pad adjustments from Xproto/GetImage/PutImage

[Aaron Plattner]

On 12/17/2012 12:38 PM, Peter Harris wrote:
> GetImage adjusts the length field incorrectly, and PutImage does not
> adjust the length field at all.
>
> Since the only stored images are those from GetImage, it makes more
> sense to remove the pad code entirely than to fix it.
>
> Signed-off-by: Peter Harris <pharris at opentext.com>
> ---

Pushed, 6fc68f..f98e350.

> Xtest defaults to using half the width of your monitor for the pPutImage
> test. Nobody noticed the bug before now because the noop pad adjustment
> case is the common case. 640, 800, 1024, and 1280 are all an even
> multiple of 32.
>
> I noticed because my monitor in portrait mode has a width of 1050.
>
>   xts5/src/libproto/RcvRep.c  |   34 +++++++---------------------------
>   xts5/src/libproto/SendReq.c |   38 ++++----------------------------------
>   2 files changed, 11 insertions(+), 61 deletions(-)
>
> diff --git a/xts5/src/libproto/RcvRep.c b/xts5/src/libproto/RcvRep.c
> index fca2265..c375d5a 100644
> --- a/xts5/src/libproto/RcvRep.c
> +++ b/xts5/src/libproto/RcvRep.c
> @@ -507,26 +507,16 @@ int client;   /* */
>   	case X_GetImage:
>   		{
>   /*
> - *	Images are stored in the test programs in client byte order and
> - *	unpadded.  This allows images to be independent of the server.
> - *	However the server will send images in server byte order and
> - *	padded.  This routine unpacks from server form into client-normal
> - *	form.  Note that we're assuming client-normal images are padded to
> - *	byte boundary; otherwise the translation is more complicated.
> - *	Similarly, left-pad must be zero.
> + *	Images are stored in the test programs in server byte order and
> + *	padding.
>    */
>
> -		int row, col = 1;
> -		unsigned char my_sex = *((unsigned char *) &col) ^ 1;
> -		unsigned char server_sex =
> -			(Xst_clients[client].cl_dpy) -> byte_order;
> -		long flip = my_sex ^ server_sex;  /* assume MSBFirst == 1 */
>   		int server_pad = (Xst_clients[client].cl_dpy) -> bitmap_pad;
> -		int dst_width /*in bytes*/ =
> +		int byte_width /*in bytes*/ =
>   			(Xst_clients[client].cl_imagewidth + 7) >> 3;
> -		int src_width /*in bytes*/ = dst_width +
> -			((dst_width % (server_pad>>3)) == 0 ? 0 :
> -			 (server_pad>>3) - dst_width % (server_pad>>3));
> +		int src_width /*in bytes*/ = byte_width +
> +			((byte_width % (server_pad>>3)) == 0 ? 0 :
> +			 (server_pad>>3) - byte_width % (server_pad>>3));
>   			
>   		char *dst = (char *)rp + sizeof(xReply);
>
> @@ -540,17 +530,7 @@ int client;   /* */
>   	                    break;
>                   }
>
> -		rp->generic.length =
> -			(dst_width * Xst_clients[client].cl_imageheight) >> 2;
> -
> -		for (row = 0; row < Xst_clients[client].cl_imageheight; row++)
> -			for(col = 0; col < src_width; col++)
> -
> -				if (col < dst_width)  {
> -				    *(dst++) = *((char *)((long)rbp++ ^ flip));
> -				}  else  {
> -					rbp++;
> -				}
> +		memcpy(dst, rbp, calculated_length * 4);
>   	        }
>   		break;
>   	case X_ListInstalledColormaps:
> diff --git a/xts5/src/libproto/SendReq.c b/xts5/src/libproto/SendReq.c
> index 0320594..4677a31 100644
> --- a/xts5/src/libproto/SendReq.c
> +++ b/xts5/src/libproto/SendReq.c
> @@ -876,34 +876,12 @@ int pollreq;
>   	case X_PutImage:
>   		{
>   /*
> - *	Images are stored in the test programs in client byte order and
> - *	unpadded.  This allows images to be independent of the server.
> - *	However the server will expect images in server byte order and
> - *	padded.  This routine sends an altered xPutImageReq which the server
> - *	will like.  Note that we're assuming client-normal form means that
> - *	rows are padded to a byte boundary; otherwise the translation is
> - *	more complex.  Similarly, left-pad must be zero.
> + *	Images are stored in the test programs in server byte order and
> + *	padding.
>    */
>
> -		int row, col = 1;
> -		unsigned char my_sex = *((unsigned char *) &col) ^ 1;
> -		unsigned char server_sex =
> -			(Xst_clients[client].cl_dpy) -> byte_order;
> -		long flip = my_sex ^ server_sex;  /* assume MSBFirst == 1 */
> -		int server_pad = (Xst_clients[client].cl_dpy) -> bitmap_pad;
> -		int src_width /*in bytes*/ =
> -			(int)(((xPutImageReq *)rp)->width + 7) >> 3;
> -		int dst_width /*in bytes*/ = src_width +
> -			((src_width % (server_pad>>3)) == 0 ? 0 :
> -			 (server_pad>>3) - src_width % (server_pad>>3));
>   		char *src = (char *)rp + sizeof(xPutImageReq);
>   		char **dst = (&(Get_Display(client)->bufptr));
> -		char *drop;
> -
> -		if (((xPutImageReq *)rp)->leftPad != 0)  {
> -			Log_Err("leftPad != 0; not supported in Send_Req()\n");
> -			Abort();
> -		}
>
>   		send1(client,(long) ((xPutImageReq *)rp)->reqType);
>   		send1(client,(long) ((xPutImageReq *)rp)->format);
> @@ -929,16 +907,8 @@ int pollreq;
>   			break;
>   		}
>   		squeeze_me_in(client, n);
> -		for (row = 0; n > 0 && row < (int)((xPutImageReq *)rp)->height; row++)
> -			for(col = 0; col < dst_width; col++) {
> -				if (col < src_width)  {
> -					drop = (char *)((long)(*dst)++ ^ flip);
> -					*drop = *(src++);
> -				}  else  {
> -					(*dst)++;
> -				}
> -				n--;
> -			}
> +		memcpy(*dst, src, n);
> +		(*dst) += n;
>   	        }
>   		break;
>   	case X_GetImage:
>


-- 
Aaron