Bug#533361: [grahn: Henning's patch tested, and kind of working]
Scripsit Jörgen Grahn
> Just in case you too do not get mail notifications whenever a Debian
> bug gets updated. I wrote this:
Oops, thanks for the heads-up. I meant to subscribe to the PTS for
xcftools, but never got it done after I resigned as a Debian dev a
couple of months ago.
> Henning, sorry for not replying earlier -- I somehow believed the BTS
> would Cc: me on everything, and didn't check the web interface until
> today.
I thought that too -- otherwise I'd have Cc'ed you directly.
> However, the resulting image still looks strange.
...
> What L2.png actually contains is the 10x10 canvas-visible part and
> everything to the south and east of that. The rest is clipped, i.e.
> filled -b white.
Yes, this is a bug. There were a few more places where I had stored
canvas-based coordinates in unsigned variables. Embarassing.
The attached patch ought to fix fixed all known cases. It is based on
the clean 1.0.4 source.
I don't think any of the newly fixed places are security
relevant. They just cause parts of the source image to be ignored, or
in certain cases wrong pixels to be read. So if someone does a
security upload to lenny, the previous patch is probably the least
invasive one to use.
I plan to release 1.0.5 with the fixes Thursday or Friday -- other
stuff gets in the way until then.
> Also, for me personally, this doesn't matter because now that xcf2pnm
> no longer crashes, I can see that I do not want the -C effect. I want
> my layers clipped to the canvas, just like xcf2pnm does by default.
I will try to think of improvements to the wording of the manpage such
that others will not be similarly confused. Suggestions welcome.
--
Henning Makholm "Al lykken er i ét ord: Overvægtig!"
? 533361b.patch
? autotools-stamp
? configure-stamp
? foo.png
? test-stamp
Index: flatten.c
===================================================================
RCS file: /home/makcvs/repository/source/xcftools/flatten.c,v
retrieving revision 1.27
diff -u -r1.27 flatten.c
--- flatten.c 22 Feb 2006 00:01:04 -0000 1.27
+++ flatten.c 29 Jun 2009 22:10:22 -0000
@@ -619,14 +619,14 @@
fillTile(&toptile,0);
for( where.t = spec->dim.c.t; where.t < spec->dim.c.b; where.t=where.b ) {
- where.b = (where.t+TILE_HEIGHT) - where.t % TILE_HEIGHT ;
+ where.b = TILE_TOP(where.t)+TILE_HEIGHT ;
if( where.b > spec->dim.c.b ) where.b = spec->dim.c.b ;
nrows = where.b - where.t ;
for( y = 0; y < nrows ; y++ )
rows[y] = xcfmalloc(4*(spec->dim.c.r-spec->dim.c.l));
for( where.l = spec->dim.c.l; where.l < spec->dim.c.r; where.l=where.r ) {
- where.r = (where.l+TILE_WIDTH) - where.l % TILE_WIDTH ;
+ where.r = TILE_LEFT(where.l)+TILE_WIDTH ;
if( where.r > spec->dim.c.r ) where.r = spec->dim.c.r ;
ncols = where.r - where.l ;
Index: pixels.c
===================================================================
RCS file: /home/makcvs/repository/source/xcftools/pixels.c,v
retrieving revision 1.14
diff -u -r1.14 pixels.c
--- pixels.c 13 Feb 2006 03:04:03 -0000 1.14
+++ pixels.c 29 Jun 2009 22:10:22 -0000
@@ -361,8 +361,8 @@
if( isSubrect(want,dim->c) &&
(want.l - dim->c.l) % TILE_WIDTH == 0 &&
(want.t - dim->c.t) % TILE_HEIGHT == 0 ) {
- unsigned tx = (want.l - dim->c.l) / TILE_WIDTH ;
- unsigned ty = (want.t - dim->c.t) / TILE_WIDTH ;
+ int tx = TILE_NUM(want.l - dim->c.l);
+ int ty = TILE_NUM(want.t - dim->c.t);
if( want.r == TILEXn(*dim,tx+1) && want.b == TILEYn(*dim,ty+1) ) {
/* The common case? An entire single tile from the layer */
copyTilePixels(tile,tiles->tileptrs[tx + ty*dim->tilesx],tiles->params);
@@ -375,8 +375,10 @@
unsigned width = want.r-want.l ;
rgba *pixvert = tile->pixels ;
rgba *pixhoriz ;
- unsigned y, ty, l0, l1, lstart, lnum ;
- unsigned x, tx, c0, c1, cstart, cnum ;
+ int y, ty, l0, l1 ;
+ int x, tx, c0, c1 ;
+ unsigned lstart, lnum ;
+ unsigned cstart, cnum ;
if( !isSubrect(want,dim->c) ) {
if( want.l < dim->c.l ) pixvert += (dim->c.l - want.l),
@@ -394,7 +396,7 @@
fprintf(stderr,"jig0 (%d-%d),(%d-%d)\n",left,right,top,bottom);
#endif
- for( y=want.t, ty=(want.t-dim->c.t)/TILE_HEIGHT, l0=TILEYn(*dim,ty);
+ for( y=want.t, ty=TILE_NUM(want.t-dim->c.t), l0=TILEYn(*dim,ty);
y<want.b;
pixvert += lnum*width, ty++, y=l0=l1 ) {
l1 = TILEYn(*dim,ty+1) ;
@@ -402,7 +404,7 @@
lnum = (l1 > want.b ? want.b : l1) - y ;
pixhoriz = pixvert ;
- for( x=want.l, tx=(want.l-dim->c.l)/TILE_WIDTH, c0=TILEXn(*dim,tx);
+ for( x=want.l, tx=TILE_NUM(want.l-dim->c.l), c0=TILEXn(*dim,tx);
x<want.r;
pixhoriz += cnum, tx++, x=c0=c1 ) {
c1 = TILEXn(*dim,tx+1);
Index: xcftools.h
===================================================================
RCS file: /home/makcvs/repository/source/xcftools/xcftools.h,v
retrieving revision 1.17
diff -u -r1.17 xcftools.h
--- xcftools.h 13 Feb 2006 03:04:03 -0000 1.17
+++ xcftools.h 29 Jun 2009 22:10:22 -0000
@@ -144,8 +144,17 @@
const char* xcfString(uint32_t ptr,uint32_t *after);
/* These are hardcoded in the Gimp sources: */
-#define TILE_WIDTH 64
-#define TILE_HEIGHT 64
+#define TILE_SHIFT 6
+#define TILE_WIDTH (1<<TILE_SHIFT)
+#define TILE_HEIGHT (1<<TILE_SHIFT)
+/* These definitions of TILE_LEFT and TILE_TOP work correctly for negative
+ * numbers, but on the other hand depend on TILE_WIDTH and TILE_HEIGHT
+ * being powers of 2. That's okay, because the tile size cannot change
+ * anyway.
+ */
+#define TILE_LEFT(x) ((x) & -TILE_WIDTH)
+#define TILE_TOP(y) ((y) & -TILE_HEIGHT)
+#define TILE_NUM(x) ((x) >> TILE_SHIFT)
struct tileDimensions {
struct rect c ;
Reply to: