|
_al_stretch_blit questionable behaviour |
Edgar Reynaldo
Major Reynaldo
May 2007
|
Due to an error with how I called stretch_blit, I wound up debugging and I discovered that on most platforms in all cases stretch_blit calls _al_stretch_blit, so naturally I was looking it over to see if anything was wrong with it. I came to the lines where it actually stretches the bitmap and I discovered this line (allegro/src/c/cstretch.c line 404) : (*stretch_line)(bmp_write_line(dst, y) + dxofs, src->line[sy] + sxofs);
/* vtable hook; not called if dest is a memory surface */ if (src->vtable->do_stretch_blit && !is_memory_bitmap(dst)) { src->vtable->do_stretch_blit(src, dst, sx, sy, sw, sh, dx, dy, dw, dh, masked); return; } The problem is that the type of the source bitmap is not checked for at all, and it is accessed using the line pointer, which the manual says it is not safe to do for non memory bitmaps : The manual said: The simplest approach will only work with memory bitmaps (obtained from create_bitmap(), grabber datafiles, and image files) and sub-bitmaps of memory bitmaps. This uses a table of char pointers, called `line', which is a part of the bitmap structure and contains pointers to the start of each line of the image. The manual goes on to elaborate a little bit : The manual said: If you want to write to the screen as well as to memory bitmaps, you need to use some helper macros, because the video memory may not be part of your normal address space. This simple routine will work for any linear screen, eg. a VESA linear framebuffers: void linear_screen_putpixel(BITMAP *bmp, int x, int y, int color) { bmp_select(bmp); bmp_write8((unsigned long)bmp->line[y]+x, color); }
So in this case, they use the line pointer to write to the screen, but the manual goes on and says when this will not work : The manual said: This still won't work in banked SVGA modes, however, or on platforms like Windows that do special processing inside the bank switching functions. For more flexible access to bitmap memory, you need to call the following routines (They're referring to bmp_write_line, bmp_read_line, bmp_unwrite_line). They are implemented as inline assembler routines, so they are not as inefficient as they might seem. If the bitmap doesn't require bank switching (ie. it is a memory bitmap, mode 13h screen, etc), these functions just return bmp->line[line]. However, I tested stretch_blit on Vista using a video bitmap as the source and a memory bitmap as the destination, and nothing bad happened at all, so I'm confused. The manual says it's wrong to use line pointers on non memory bitmaps, but _al_stretch_blit does exactly that. So is it safe to use line pointers on any bitmap, or isn't it? Append The manual said: Moreover, the source must be a memory bitmap. However, if that is true, then why are there entries in gfx_capabilities for the following flags : The manual said:
GFX_HW_VRAM_STRETCH_BLIT: GFX_HW_SYS_STRETCH_BLIT: GFX_HW_VRAM_STRETCH_BLIT_MASKED: GFX_HW_SYS_STRETCH_BLIT_MASKED: This indicates that the source may be a system or video bitmap when drawing onto another video bitmap. I wish the manual would stop contradicting itself. Is it correct to assume that given all the information so far, that when the destination bitmap is a memory bitmap, then the source bitmap can (should) only be a memory bitmap? That would fit _al_stretch_blit's assumption about using a line pointer on the source bitmap, however, the source code of _al_stretch_blit should reflect that if it is true. Append 2 Append 3 Apparent options : 2) Modify _al_stretch_blit to work for any combination of bitmap types. This would require using bmp_read_line on the source to read a line into a temporary array and then stretching the line normally. 3) Modify _al_stretch_blit to ASSERT that the source is a memory bitmap. So, any opinions on this? Append 4 My Website! | EAGLE GUI Library Demos | My Deviant Art Gallery | Spiraloid Preview | A4 FontMaker | Skyline! (Missile Defense) Eagle and Allegro 5 binaries | Older Allegro 4 and 5 binaries | Allegro 5 compile guide |
Trent Gamblin
Member #261
April 2000
|
I vaguely recall fixing stretch_blit a few years back when there was an unrelated bug in it. From what I remember, on some platforms, my game would crash when using a non memory source bitmap. I can't remember if it was DOS or Linux console or anything, just a vague recollection of bad things happening. My suggestion would be to document those display flags and say that they mean the hardware is capable of it but Allegro is not, for a quick fix. Now if you could come up with a real fix, that allowed any type of bitmap to be used, that would be a better fix, providing that there is no significant speed decrease. This function is highly optimized right now and I'm going to guess that on a low end machine (which I think is now the main target for Allegro 4) it would make a significant difference to all variations if you made it more generic. But as I say, if you can do it without hurting performance, that would be great.
|
Edgar Reynaldo
Major Reynaldo
May 2007
|
Trent Gamblin said: My suggestion would be to document those display flags and say that they mean the hardware is capable of it but Allegro is not, for a quick fix. Isn't that a contradiction though? The flags indicate that Allegro is capable of it, as far as I know. They do say that the destination is the screen though, and not just any video bitmap, but I don't personally know if there is a difference. Trent Gamblin said: Now if you could come up with a real fix, that allowed any type of bitmap to be used, that would be a better fix, providing that there is no significant speed decrease. Technically, it should be as simple as using an if else statement to preserve the code that is currently there for memory source bitmaps and allow for the source to be a system or video bitmap. The only other thing necessary would be to make a copy of the line from the source system/video bitmap and pass that to the 'stretch_line' function pointer instead of 'src->line[sy] + sxofs'. The code I'm referring to is at the end of src/c/cstretch.c in the al_stretch_blit function on lines 399-413 : 399 /* Stretch it */
400
401 bmp_select(dst);
402
403 for (; y < dyend; y++, sy += syinc) {
404 (*stretch_line)(bmp_write_line(dst, y) + dxofs, src->line[sy] + sxofs);
405 if (yc <= 0) {
406 sy++;
407 yc += ycinc;
408 }
409 else
410 yc -= ycdec;
411 }
412
413 bmp_unwrite_line(dst);
It would become something like : 1 /* Stretch it */
2 if (is_memory_bitmap(src)) {
3 bmp_select(dst);
4
5 for (; y < dyend; y++, sy += syinc) {
6 (*stretch_line)(bmp_write_line(dst, y) + dxofs, src->line[sy] + sxofs);
7 if (yc <= 0) {
8 sy++;
9 yc += ycinc;
10 }
11 else
12 yc -= ycdec;
13 }
14
15 bmp_unwrite_line(dst);
16 }
17 else {
18 // get bits per pixel, multiply times width, add padding so it aligns with
19 // a multiple of four
20 char* srccopy = (char*)malloc(size + pad);
21 if (!srccopy) {return;}
22 for (; y < dyend; y++, sy += syinc) {
23 bmp_select(src);
24 memcpy(srccopy , (char*)bmp_read_line(src , sy) , size);
25 bmp_select(dst);
26
27 (*stretch_line)(bmp_write_line(dst, y) + dxofs, srccopy + sxofs);
28 if (yc <= 0) {
29 sy++;
30 yc += ycinc;
31 }
32 else
33 yc -= ycdec;
34 }
35 bmp_unwrite_line(src);
36 bmp_unwrite_line(dst);
37 free(srccopy);
38 }
Trent Gamblin said: This function is highly optimized right now and I'm going to guess that on a low end machine (which I think is now the main target for Allegro 4) it would make a significant difference to all variations if you made it more generic. But as I say, if you can do it without hurting performance, that would be great. It may be slightly slower due to the allocation and memcpy, but that's for stretching from system and video bitmaps where it was previously unsupported, and the original routine is only altered by an if check, which is negligible. My Website! | EAGLE GUI Library Demos | My Deviant Art Gallery | Spiraloid Preview | A4 FontMaker | Skyline! (Missile Defense) Eagle and Allegro 5 binaries | Older Allegro 4 and 5 binaries | Allegro 5 compile guide |
Trent Gamblin
Member #261
April 2000
|
Ok.. well create patches for this and the ddraw alpha issue and I'll test them and apply them. I plan to be doing a little more 4.4 development when I get the motherboard Matthew sent me.
|
Edgar Reynaldo
Major Reynaldo
May 2007
|
I haven't forgotten about this if you were wondering, I just haven't gotten to it yet. What with Allegro 4.4.2 coming out recently, there's no real rush anyway. My Website! | EAGLE GUI Library Demos | My Deviant Art Gallery | Spiraloid Preview | A4 FontMaker | Skyline! (Missile Defense) Eagle and Allegro 5 binaries | Older Allegro 4 and 5 binaries | Allegro 5 compile guide |
|