|
Stretch blit bug? |
Torbjörn Olsson
Member #6,792
January 2006
|
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? Best regards 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) /* ASSERT(src); if ((sw <= 0) || (sh <= 0) || (dw <= 0) || (dh <= 0)) if (dst->clip) { dxbeg = ((dx > dst->cl) ? dx : dst->cl); /* Bresenham algorithm uses difference between points, not number of points. */ /* How much to add to src-coordinates, when moving to the next dst-coordinate. */ if (dh == 0) /* Walk in x direction until dxbeg and save Bresenham state there. */ /* Save Bresenham algorithm state with offset fixups. */ /* Find out which stretcher should be used. */ ASSERT(stretch_line); /* Walk in y direction until we reach first non-clipped line. */ if(1)//HACK-WARNING for (x = dy, y = sy; x < dybeg; x++, y += yinc) { /* Stretch all non-clipped lines. */ Now it seems to work like I want it to, but can you see any problems with my changes? |
A J
Member #3,025
December 2002
|
which looks better ? ___________________________ |
Michael Faerber
Member #4,800
July 2004
|
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.
-- |
Elias
Member #358
May 2000
|
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. -- |
Thomas Harte
Member #33
April 2000
|
Quote: Anyone up to fixing it? 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... [My site] [Tetrominoes] |
Elias
Member #358
May 2000
|
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: Quote: r3020 | tjaden | 2003-02-15 02:29:39 +0100 (Sat, 15 Feb 2003) | 4 lines Compensate for a problem in stretch_bitmap where the first column or row can 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.. -- |
Torbjörn Olsson
Member #6,792
January 2006
|
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 |
Evert
Member #794
November 2000
|
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. |
Elias
Member #358
May 2000
|
Quote: Elias (first post): Did you look at the scaling around the borders of the bitmap? 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?) -- |
Torbjörn Olsson
Member #6,792
January 2006
|
Evert: I agree 100% Elias: There are two if(1)-blocks (one for vertical and one for horisontal), did you use both of them? |
Elias
Member #358
May 2000
|
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. -- |
Torbjörn Olsson
Member #6,792
January 2006
|
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. |
|