From e7ce6468fcaf977aa52a0c76ba654faa8fcc3ed3 Mon Sep 17 00:00:00 2001 From: dan Date: Mon, 6 Nov 2000 04:21:25 +0000 Subject: [PATCH] - fixed code in wrlib dependant on the order of evaluation. code like *ptr++ = *ptr++ = *ptr++ = color; is wrong, because there is no guarantee that ptr will be incremented _between_ the assignment operations. it can be incremented after all assignment operations as well. Because of this both of these are valid implementations for a compiler: a. assign, increment, assign, increment, assign, increment b. assign, assign, assign, increment by 3 In case b. only the first memory location of the 3 will be modified, being assigned 3 times the same value, while the other 2 remain unchanged. For example egcs-2.91.66 (and possibly gcc-2.95.x too) implement this in the second way (like in case b.) Also the order in which the assignement is made is undefined (left to right or right to left). this fixed the problem we had with greyscale jpegs showing up in red, and possibly other problems related to pseudocolor and greyscale displays. --- ChangeLog | 3 +++ wrlib/ChangeLog | 15 +++++++++++++++ wrlib/convert.c | 21 ++++++++++----------- wrlib/draw.c | 6 +++--- wrlib/jpeg.c | 4 +++- wrlib/view.c | 6 ++++-- 6 files changed, 38 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 661efc40..f9528ce1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,9 @@ Changes since version 0.62.1: startup. - replaced free() with wfree() wherever appropriate - fixed some memory leaks generated by wstrappend() +- fixed code that was dependant on the order of evaluation in wrlib. This + fixed a number of problems, like greyscale jpegs that showed up in red + and possibly the problems with pseudocolor displays. Changes since version 0.62.0: diff --git a/wrlib/ChangeLog b/wrlib/ChangeLog index c914d9ff..044cadcd 100644 --- a/wrlib/ChangeLog +++ b/wrlib/ChangeLog @@ -1,3 +1,18 @@ +- fixed code dependant on the order of evaluation. code like + *ptr++ = *ptr++ = *ptr++ = color; + is wrong, because there is no guarantee that ptr will be incremented + _between_ the assignment operations. it can be incremented after all + assignment operations as well. Because of this both of these are valid + implementations for a compiler: + a. assign, increment, assign, increment, assign, increment + b. assign, assign, assign, increment by 3 + In case b. only the first memory location of the 3 will be modified, being + assigned 3 times the same value, while the other 2 remain unchanged. + For example egcs-2.91.66 (and possibly gcc-2.95.x too) implement this in + the second way (like in case b.) + Also the order in which the assignement is made is undefined (left to right + or right to left). + - added RMakeCenteredImage() - Added code to draw pixels and lines. Both writing absolute values, or diff --git a/wrlib/convert.c b/wrlib/convert.c index 31f18948..547e2757 100644 --- a/wrlib/convert.c +++ b/wrlib/convert.c @@ -593,10 +593,10 @@ image2StandardPseudoColor(RContext *ctx, RImage *image) if (ctx->attribs->render_mode == RBestMatchRendering) { for (y=0; yheight; y++) { - for (x=0; xwidth; x++, ptr+=channels-3) { + for (x=0; xwidth; x++, ptr+=channels) { /* reduce pixel */ - pixel = (rtable[*ptr++] + gtable[*ptr++] - + btable[*ptr++] + base_pixel) & 0xffffffff; + pixel = (rtable[*ptr] + gtable[*(ptr+1)] + + btable[*(ptr+2)] + base_pixel) & 0xffffffff; XPutPixel(ximg->image, x, y, pixel); } @@ -625,7 +625,8 @@ image2StandardPseudoColor(RContext *ctx, RImage *image) err[x++] = ptr[x1++]; err[x++] = ptr[x1++]; } - err[x++] = err[x++] = err[x++] = 0; + err[x] = err[x+1] = err[x+2] = 0; + x += 3; /* convert and dither the image to XImage */ for (y=0, ofs=0; yheight; y++) { if (yheight-1) { @@ -742,8 +743,8 @@ image2GrayScale(RContext *ctx, RImage *image) for (y=0; yheight; y++) { for (x=0; xwidth; x++) { /* reduce pixel */ - g = table[(*ptr++ * 30 + *ptr++ * 59 + *ptr++ * 11)/100]; - + g = table[(*ptr * 30 + *(ptr+1) * 59 + *(ptr+2) * 11)/100]; + ptr += 3; /*data[ofs] = ctx->colors[g].pixel;*/ XPutPixel(ximg->image, x, y, ctx->colors[g].pixel); } @@ -776,14 +777,12 @@ image2GrayScale(RContext *ctx, RImage *image) for (y=0; yheight; y++) { if (yheight-1) { int x1; - for (x=0, x1=(y+1)*image->width*3; - xwidth; - x1+=channels-3) { - ngerr[x] = (ptr[x1++]*30 + ptr[x1++]*59 + ptr[x1++]*11)/100; + for (x=0, x1=(y+1)*image->width*3; xwidth; x1+=channels) { + ngerr[x] = (ptr[x1]*30 + ptr[x1+1]*59 + ptr[x1+2]*11)/100; } /* last column */ x1-=channels; - ngerr[x] = (ptr[x1++]*30 + ptr[x1++]*59 + ptr[x1++]*11)/100; + ngerr[x] = (ptr[x1]*30 + ptr[x1+1]*59 + ptr[x1+2]*11)/100; } for (x=0; xwidth; x++) { /* reduce pixel */ diff --git a/wrlib/draw.c b/wrlib/draw.c index fc2df662..ebe2d4f8 100644 --- a/wrlib/draw.c +++ b/wrlib/draw.c @@ -98,9 +98,9 @@ RPutPixel(RImage *image, int x, int y, RColor *color) alpha = color->alpha; nalpha = 255 - alpha; - *ptr++ = (((int)*ptr * nalpha) + (r * alpha))/256; - *ptr++ = (((int)*ptr * nalpha) + (g * alpha))/256; - *ptr++ = (((int)*ptr * nalpha) + (b * alpha))/256; + *ptr = (((int)*ptr * nalpha) + (r * alpha))/256; ptr++; + *ptr = (((int)*ptr * nalpha) + (g * alpha))/256; ptr++; + *ptr = (((int)*ptr * nalpha) + (b * alpha))/256; ptr++; if (image->format == RRGBAFormat) { *ptr = alpha + ((int)*ptr * nalpha)/256; } diff --git a/wrlib/jpeg.c b/wrlib/jpeg.c index 33e73f5c..9d94ed2a 100644 --- a/wrlib/jpeg.c +++ b/wrlib/jpeg.c @@ -185,7 +185,9 @@ RLoadJPEG(RContext *context, char *file_name, int index) jpeg_read_scanlines(&cinfo, buffer,(JDIMENSION) 1); bptr = buffer[0]; for (i=0; i