From owner-freebsd-ports-bugs@FreeBSD.ORG Mon May 8 08:30:10 2006 Return-Path: X-Original-To: freebsd-ports-bugs@hub.freebsd.org Delivered-To: freebsd-ports-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7E82816A403 for ; Mon, 8 May 2006 08:30:10 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id D3A3F43D46 for ; Mon, 8 May 2006 08:30:09 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k488U9YM037120 for ; Mon, 8 May 2006 08:30:09 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k488U9OZ037119; Mon, 8 May 2006 08:30:09 GMT (envelope-from gnats) Resent-Date: Mon, 8 May 2006 08:30:09 GMT Resent-Message-Id: <200605080830.k488U9OZ037119@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-ports-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Peter Jeremy Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A725A16A405 for ; Mon, 8 May 2006 08:24:57 +0000 (UTC) (envelope-from peterjeremy@optushome.com.au) Received: from mail04.syd.optusnet.com.au (mail04.syd.optusnet.com.au [211.29.132.185]) by mx1.FreeBSD.org (Postfix) with ESMTP id E9A3443D45 for ; Mon, 8 May 2006 08:24:55 +0000 (GMT) (envelope-from peterjeremy@optushome.com.au) Received: from turion.vk2pj.dyndns.org (c220-239-19-236.belrs4.nsw.optusnet.com.au [220.239.19.236]) by mail04.syd.optusnet.com.au (8.12.11/8.12.11) with ESMTP id k488Or28025517 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO) for ; Mon, 8 May 2006 18:24:54 +1000 Received: from turion.vk2pj.dyndns.org (localhost.vk2pj.dyndns.org [127.0.0.1]) by turion.vk2pj.dyndns.org (8.13.6/8.13.6) with ESMTP id k488OrYQ001058; Mon, 8 May 2006 18:24:53 +1000 (EST) (envelope-from peter@turion.vk2pj.dyndns.org) Received: (from peter@localhost) by turion.vk2pj.dyndns.org (8.13.6/8.13.6/Submit) id k488Orpv001057; Mon, 8 May 2006 18:24:53 +1000 (EST) (envelope-from peter) Message-Id: <200605080824.k488Orpv001057@turion.vk2pj.dyndns.org> Date: Mon, 8 May 2006 18:24:53 +1000 (EST) From: Peter Jeremy To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: peter@vk2pj.dyndns.org Subject: ports/96971: [patch] graphics/xv incorrectly handles xwd files X-BeenThere: freebsd-ports-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Peter Jeremy List-Id: Ports bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 May 2006 08:30:10 -0000 >Number: 96971 >Category: ports >Synopsis: [patch] graphics/xv incorrectly handles xwd files >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-ports-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Mon May 08 08:30:08 GMT 2006 >Closed-Date: >Last-Modified: >Originator: Peter Jeremy >Release: FreeBSD 6.1-STABLE amd64 >Organization: n/a >Environment: System: FreeBSD turion.vk2pj.dyndns.org 6.1-STABLE FreeBSD 6.1-STABLE #9: Mon May 8 12:06:14 EST 2006 root@turion.vk2pj.dyndns.org:/usr/obj/usr/src/sys/turion amd64 xv-3.10a_5 >Description: With 24/32-bit xwd files, xv swaps the red and blue channels. With 16-bit xwd files, the image is very dark green (almost black). Both problems are caused by hard-coding the channel order and offsets, rather than using the colour masks in the xwd header. xv reads the input into a 24-bit internal image, which is then displayed. The lack of brightness in the 16-bit display is because the colour values are copied into the low-order bits of the internal pixmap rather than the high order bits. The green hue is because the green channel has 6 bits, whereas red and blue only have 5 bits, making the green twice as (relatively) bright. Thanks to Callum Gibson for initial pointers to the bug as well as 16-bit dumps. >How-To-Repeat: Do a window or screen dump using xwd from a 24/32-bit TrueColor display. Display the screen dump using xv. Do a window or screen dump using xwd from a 16-bit TrueColor display. Display the screen dump using xv. >Fix: The following patch replaces the byte-swap routines as well - I initially suspected that the problem was at least partially caused by the non-portability of "union cheat". Whilst that was not a problem, the resultant code is cleaner and I did not want to expend further effort removing the change. Rather than hard-coding the shift values (and the mask for 24-bit colour), the shift value is calculated from the mask during initialisation. Since the low colour bits must be shifted left whilst all other shifts are right, each pixel colour is shifted so the MSB of the colour is in bit 31 and then shifted right 24 bits to correctly locate the colour in an 8-bit field. --- xvxwd.c~ Sat May 6 07:29:07 2006 +++ xvxwd.c Sat May 6 17:24:12 2006 @@ -19,7 +19,7 @@ */ #include "xv.h" - +#include /***************************** x11wd.h *****************************/ #define X11WD_FILE_VERSION 7 @@ -66,23 +66,25 @@ typedef byte pixel; +#define bs_short(x) bswap16(x) +#define bs_long(x) bswap32(x) + /* local functions */ static int getinit PARM((FILE *, int*, int*, int*, CARD32 *, CARD32, PICINFO *)); static CARD32 getpixnum PARM((FILE *)); static int xwdError PARM((char *)); static void xwdWarning PARM((char *)); -static int bs_short PARM((int)); -static CARD32 bs_long PARM((CARD32)); static int readbigshort PARM((FILE *, CARD16 *)); static int readbiglong PARM((FILE *, CARD32 *)); static int readlittleshort PARM((FILE *, CARD16 *)); static int readlittlelong PARM((FILE *, CARD32 *)); static int writebigshort PARM((FILE *, int)); static int writebiglong PARM((FILE *, CARD32)); - +static int get_shift PARM((CARD32)); static byte *pic8, *pic24; static CARD32 red_mask, green_mask, blue_mask; +static int red_shift, green_shift, blue_shift; static int bits_per_item, bits_used, bit_shift, bits_per_pixel; static char buf[4]; static char *byteP; @@ -181,24 +183,9 @@ CARD32 ul; ul = getpixnum(ifp); - switch (bits_per_pixel) { - case 16: - *xP++ = ((ul & red_mask) >> 0); - *xP++ = ((ul & green_mask) >> 5); - *xP++ = ((ul & blue_mask) >> 10); - break; - - case 24: - case 32: - *xP++ = (ul ) & 0xff; - *xP++ = (ul>> 8) & 0xff; - *xP++ = (ul>>16) & 0xff; - break; - - default: - xwdError("True/Direct only supports 16, 24, and 32 bits"); - return 0; - } + *xP++ = ((ul & red_mask) << red_shift) >> 24; + *xP++ = ((ul & green_mask) << green_shift) >> 24; + *xP++ = ((ul & blue_mask) << blue_shift) >> 24; } for (col=0; colheader_size = bs_long(h11P->header_size); h11P->file_version = bs_long(h11P->file_version); + if (h11P->file_version != X11WD_FILE_VERSION) + return(xwdError("unsupported X11 XWD file version")); h11P->pixmap_format = bs_long(h11P->pixmap_format); h11P->pixmap_depth = bs_long(h11P->pixmap_depth); h11P->pixmap_width = bs_long(h11P->pixmap_width); @@ -427,6 +416,10 @@ green_mask = h11P->green_mask; blue_mask = h11P->blue_mask; + red_shift = get_shift(red_mask); + green_shift = get_shift(green_mask); + blue_shift = get_shift(blue_mask); + byteP = (char *) buf; shortP = (CARD16 *) buf; longP = (CARD32 *) buf; @@ -531,44 +524,6 @@ /* - * Byte-swapping junk. - */ - -union cheat { - CARD32 l; - CARD16 s; - CARD8 c[sizeof(CARD32)]; -}; - -static int bs_short(s) - int s; -{ - union cheat u; - unsigned char t; - - u.s = (CARD16) s; - t = u.c[0]; u.c[0] = u.c[1]; u.c[1] = t; - return (int) u.s; -} - -static CARD32 bs_long(l) - CARD32 l; -{ - union cheat u; - unsigned char t; - - u.l = l; - t = u.c[0]; u.c[0] = u.c[3]; u.c[3] = t; - t = u.c[1]; u.c[1] = u.c[2]; u.c[2] = t; - return u.l; -} - - - - - - -/* * Endian I/O. */ @@ -642,4 +597,17 @@ putc((s>>8)&0xff, out); putc(s&0xff, out); return 0; +} + + +static int get_shift(val) + CARD32 val; +{ + int shift; + + if (!val) + return (0); + for (shift = 0; !(val & 0x80000000); shift++) + val <<= 1; + return (shift); } >Release-Note: >Audit-Trail: >Unformatted: