The stretch blit in cstretch.c behaves differently compared with the one in istretch.c.

You can see the difference in the files I have attached.

Basically what I want to to is to enlarge a bitmap 10 times. In the istretch.c version each pixel becomes a 10x10 area, but using cstretch.c the pixels around the borders are not expanded as much as those in the middle.

If I could I would use istretch.c, but I am not developing for x86, so I can not.

My questions:

Is this a known bug?
Will there be/is there a solution?

Best regards
Torbjörn Olsson

UPDATE:

Ok, it seems to me that cstretch.c is different by design. I changed this code to make it work like I want it to:

(Note the code with the HACK-WARNING comment)

/*
* Stretch blit work-horse.
*/
static void _al_stretch_blit(BITMAP *src, BITMAP *dst,
int sx, int sy, int sw, int sh,
int dx, int dy, int dw, int dh,
int masked)
{
int x, y, fixup;
int i1, i2, dd;
int xinc, yinc;
int dxbeg, dxend;
int dybeg, dyend;
int sxofs, dxofs;
void (*stretch_line)(unsigned long dptr, unsigned char *sptr);

ASSERT(src);
ASSERT(dst);

if ((sw <= 0) || (sh <= 0) || (dw <= 0) || (dh <= 0))
return;

if (dst->clip) {
dybeg = ((dy > dst->ct) ? dy : dst->ct);
dyend = (((dy + dh) < dst->cb) ? (dy + dh) : dst->cb);
if (dybeg >= dyend)
return;

dxbeg = ((dx > dst->cl) ? dx : dst->cl);
dxend = (((dx + dw) < dst->cr) ? (dx + dw) : dst->cr);
if (dxbeg >= dxend)
return;
}
else {
dxbeg = dx;
dxend = dx + dw;
dybeg = dy;
dyend = dy + dh;
}

/* Bresenham algorithm uses difference between points, not number of points. */
sw--;
sh--;
dw--;
dh--;

/* How much to add to src-coordinates, when moving to the next dst-coordinate. */
if (dw == 0)
xinc = 0;
else {
xinc = sw / dw;
sw %= dw;
}

if (dh == 0)
yinc = 0;
else {
yinc = sh / dh;
sh %= dh;
}

/* Walk in x direction until dxbeg and save Bresenham state there. */
i2 = (dd = (i1 = 2 * sw) - dw) - dw;

if(1)//HACK-WARNING
{
i1 = 2 * sw + 2;
dd = 2 * sw - 2 * dw;
i2 = dd;
}

for (x = dx, y = sx; x < dxbeg; x++, y += xinc) {
if (dd >= 0)
y++, dd += i2;
else
dd += i1;
}

/* Save Bresenham algorithm state with offset fixups. */
switch (bitmap_color_depth(dst)) {
case 15:
case 16:
fixup = sizeof(short);
break;
case 24:
fixup = 3;
break;
case 32:
fixup = sizeof(long);
break;
default:
fixup = 1;
break;
}
sxofs = y * fixup;
dxofs = x * fixup;
_al_stretch.i1 = i1;
_al_stretch.i2 = i2;
_al_stretch.dd = dd;
_al_stretch.dw = (dxend - dxbeg) * fixup;
_al_stretch.sinc = xinc * fixup;

/* Find out which stretcher should be used. */
if (masked) {
switch (bitmap_color_depth(dst)) {
#ifdef ALLEGRO_COLOR8
case 8:
if (is_linear_bitmap(dst))
stretch_line = stretch_masked_line8;
else {
#ifdef GFX_MODEX
stretch_line = stretch_masked_linex;
#else
return;
#endif
}
break;
#endif
#ifdef ALLEGRO_COLOR16
case 15:
stretch_line = stretch_masked_line15;
break;
case 16:
stretch_line = stretch_masked_line16;
break;
#endif
#ifdef ALLEGRO_COLOR24
case 24:
stretch_line = stretch_masked_line24;
break;
#endif
#ifdef ALLEGRO_COLOR32
case 32:
stretch_line = stretch_masked_line32;
break;
#endif
default:
return;
}
}
else {
switch (bitmap_color_depth(dst)) {
#ifdef ALLEGRO_COLOR8
case 8:
if (is_linear_bitmap(dst))
stretch_line = stretch_line8;
else {
#ifdef GFX_MODEX
stretch_line = stretch_linex;
#else
return;
#endif
}
break;
#endif
#ifdef ALLEGRO_COLOR16
case 15:
stretch_line = stretch_line15;
break;
case 16:
stretch_line = stretch_line16;
break;
#endif
#ifdef ALLEGRO_COLOR24
case 24:
stretch_line = stretch_line24;
break;
#endif
#ifdef ALLEGRO_COLOR32
case 32:
stretch_line = stretch_line32;
break;
#endif
default:
return;
}
}

ASSERT(stretch_line);

/* Walk in y direction until we reach first non-clipped line. */
i2 = (dd = (i1 = 2 * sh) - dh) - dh;

if(1)//HACK-WARNING
{
i1 = 2 * sh + 2;
dd = 2 * sh - 2 * dh;
i2 = dd;
}

for (x = dy, y = sy; x < dybeg; x++, y += yinc) {
if (dd >= 0)
y++, dd += i2;
else
dd += i1;
}

/* Stretch all non-clipped lines. */
bmp_select(dst);
for (; x < dyend; x++, y += yinc) {
(*stretch_line)(bmp_write_line(dst, x) + dxofs, src->line[y] + sxofs);
if (dd >= 0)
y++, dd += i2;
else
dd += i1;
}
bmp_unwrite_line(dst);
}

Now it seems to work like I want it to, but can you see any problems with my changes?

which looks better ?
does cstrech make a nicer looking image?

I'm not experienced with the Allegro internals code and therefore won't comment on the code itself, but if you post code, you should use the [ code] and [ /code] tags. See Mockup information.

You used code tags now, but still killed the indentation

Anyway, it seems to be a bug. Looking at the changelogs, the functional part of the code in cstretch.c hasn't been touched since 6 years (that's how far SVN logs go back..), so the problem must be the initial implementation of the bresenham scaler.

[Edit: http://websvn.bafserv.com/wsvn/allegro/allegro/branches/4.2/src/c/cstretch.c?op=blame&rev=0&sc=0]

Anyone up to fixing it?

I can't notice any difference when adding your if(1) {} part.. probably I missed something there.

Any chance of just completely replacing _al_stretch_blit? Besides being hard to follow - and Allegro's doesn't even seem to be the common form - Bresenham trades divides outside the inner loop for conditionals within it. So on modern CPUs it isn't a smart move, especially since average BITMAP sizes are increasing over time.

I'll have a quick hack at a more conventional _al_stretch_blit, if everyone else is busy...

Sure. As I understand it, the asm code will be dropped more sooner than later anyway, so no need to try to match the cstrech.c version with the istretch.c one.

But maybe you shold wait for Peter, he apparently fixed something in istretch.c a while back:

The change is marked as 3020 here: http://websvn.bafserv.com/wsvn/allegro/allegro/branches/4.2/src/i386/istretch.c?op=blame&rev=0&sc=0

And I assume, they use sort of the same algorithm, just implemented a bit differently, so someone understanding the code might be able to fix it easily..

Thanks for your replies.

AJ: The istretch version is preferable from my point of view.

Michael Faerber: Thanks for the tip.

Elias (first post): Did you look at the scaling around the borders of the bitmap?

Thomas Harte: Great, please let me know how it goes.

Elias (second post): Do you mean istretch will be dropped? Then I guess its time to fix cstretch soon.

Best regards
Torbjörn Olsson

From a code maintenance point of view, having both a C version of the code and an ASM version is a bad idea - especially if we want to rewrite things anyway.

As for bugs in the C version of the code, I've never really noticed any but then again, I hardly use scaling anyway.

Yes, it looked wrong both with and without that if(1) block for me. (Couldn't notice any difference at all.. but I didn't check for possible compilation problems, so maybe I looked twice at the same version?)

Evert: I agree 100%

Elias: There are two if(1)-blocks (one for vertical and one for horisontal), did you use both of them?

Ah, yes, I had only seen one, and put at the wrong place From the example it looks like it fixes the problem.

So - since you commented it as "HACK" - is this a proper fix or not? If yes, maybe you could rewrite it without the if(1) and make a patch.

Thomas: In that case, it wouldn't be necessary to rewrite it - but if you create an improved and better understandable bitmap scaler, we would of course use that.

Since I don't completely understand the code, I can not be sure that it works in all cases.

I have tested it a bit, but only for very simple test cases.

But even if my solution does work, it is probably not the best possible, so if someone with more experience with the code could take a look at it, and maybe even be able to optimize it a bit, that would be great.