libpixman 32bit regions patch

Carsten Haitzler (The Rasterman) raster at rasterman.com
Mon Jan 9 20:15:12 PST 2006


On Mon, 09 Jan 2006 16:57:58 -0800 Carl Worth <cworth at cworth.org> babbled:

> On Mon, 9 Jan 2006 22:43:22 +0900, Carsten Haitzler (The Rasterman) wrote:
> > On Mon, 9 Jan 2006 22:30:27 +0900 Carsten Haitzler (The Rasterman)
> > <raster at rasterman.com> babbled:
> > > On Mon, 9 Jan 2006 19:54:26 +0900 Carsten Haitzler (The Rasterman)
> > > <raster at rasterman.com> babbled:
> > > 
> > > > (for keith mostly) - here's a patch against 0.1.6 libpixman to make
> > > > regions 32bit (wll ok - int's - 32bit for all intents and purposes
> > > > these days) - i didnt make fb stuff 32bit though - i can if that makes
> > > > sense.
> > > 
> > > hmm - i found some other 16bit-isms in there. i'll update the patch.
> > 
> > ok- fixed them :) new patch attached.
> 
> This patch would be a lot easier to review if the obvious/automatic
> renaming were supplied as a separate patch from the subtle/manual
> storage and limit fixes.
> 
> From a very quick scan, the only thing I saw from the second class is:
> 
> 	-typedef struct pixman_box16 {
> 	-    short x1, y1, x2, y2;
> 	-} pixman_box16_t;
> 	+typedef struct pixman_box {
> 	+    int x1, y1, x2, y2;
> 	+} pixman_box_t;
> 
> Which should likely use int32_t (uint32_t ?) instead of a bare int.

well the internals use "int" so it matches that. i think i saw a user complain
once - about 6 or 7 years ago, on an anchient IRIX system that their compiler
used 16bit ints. there was a compiler option to use 32bitints. using that made
them happy. since then i have yet to hear from a single user about an int NOT
being 32bit. sure - in theory it can, but anyway - it matches what the
internals seemed to use which was what prompted me to change it as it was weird
that it was a hybrid 32/16bit system. i removed all the 16's as it was no
longer a valid indicator of type. also some internal fn calls used shorts for
passing co-ordinates as well - i changed them to int's on my 2nd patch. it
seesm the code is partly using int/short and partly using int62_t/int32_t and
likely needs to settle on one or the other - not both.

> Notably missing from the patch are many fixes including at least fixes
> to the following lines of code:

aah indeed - missed these. 

> -Carl
> 
> grep -n -e 'SHRT\|SHORT\|BOUND' *.[ch] /dev/null
> icimage.c:628:#define BOUND(v)  (int16_t) ((v) < MINSHORT ? MINSHORT : (v) >
> MAXSH\ORT ? MAXSHORT : (v)) icimage.c:644:      pRbox->x1 = BOUND(v);
> icimage.c:646:      pRbox->x2 = BOUND(v);
> icimage.c:648:      pRbox->y1 = BOUND(v);
> icimage.c:650:      pRbox->y2 = BOUND(v);
> icimage.c:918:    x2 = BOUND(v);
> icimage.c:921:    y2 = BOUND(v);
> icint.h:120:#define MAXSHORT SHRT_MAX
> icint.h:121:#define MINSHORT SHRT_MIN
> ictrap.c:77:    box->y1 = MAXSHORT;
> ictrap.c:78:    box->y2 = MINSHORT;
> ictrap.c:79:    box->x1 = MAXSHORT;
> ictrap.c:80:    box->x2 = MINSHORT;
> pixregion.c:1665:       if ((x2 = x1 + (int) prect->width) > SHRT_MAX)
> pixregion.c:1666:           x2 = SHRT_MAX;
> pixregion.c:1667:       if ((y2 = y1 + (int) prect->height) > SHRT_MAX)
> pixregion.c:1668:           y2 = SHRT_MAX;
> pixregion.c:1690:       if ((x2 = x1 + (int) prect->width) > SHRT_MAX)
> pixregion.c:1691:           x2 = SHRT_MAX;
> pixregion.c:1692:       if ((y2 = y1 + (int) prect->height) > SHRT_MAX)
> pixregion.c:1693:           y2 = SHRT_MAX;
> pixregion.c:2103:    if (((x1 - SHRT_MIN)|(y1 - SHRT_MIN)|(SHRT_MAX - x2)|
> (SHRT_MA\X - y2)) >= 0) pixregion.c:2117:    if (((x2 - SHRT_MIN)|(y2 -
> SHRT_MIN)|(SHRT_MAX - x1)|(SHRT_MA\X - y1)) <= 0) pixregion.c:2125:    if (x1
> < SHRT_MIN) pixregion.c:2126:       region->extents.x1 = SHRT_MIN;
> pixregion.c:2127:    else if (x2 > SHRT_MAX)
> pixregion.c:2128:       region->extents.x2 = SHRT_MAX;
> pixregion.c:2129:    if (y1 < SHRT_MIN)
> pixregion.c:2130:       region->extents.y1 = SHRT_MIN;
> pixregion.c:2131:    else if (y2 > SHRT_MAX)
> pixregion.c:2132:       region->extents.y2 = SHRT_MAX;
> pixregion.c:2143:           if (((x2 - SHRT_MIN)|(y2 - SHRT_MIN)|
> pixregion.c:2144:                (SHRT_MAX - x1)|(SHRT_MAX - y1)) <= 0)
> pixregion.c:2149:           if (x1 < SHRT_MIN)
> pixregion.c:2150:               pboxout->x1 = SHRT_MIN;
> pixregion.c:2151:           else if (x2 > SHRT_MAX)
> pixregion.c:2152:               pboxout->x2 = SHRT_MAX;
> pixregion.c:2153:           if (y1 < SHRT_MIN)
> pixregion.c:2154:               pboxout->y1 = SHRT_MIN;
> pixregion.c:2155:           else if (y2 > SHRT_MAX)
> pixregion.c:2156:               pboxout->y2 = SHRT_MAX;
> 
> 
> 
> 
> 
> 
> 


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler)    raster at rasterman.com
裸好多
Tokyo, Japan (東京 日本)



More information about the xorg mailing list