Allegro.cc - Online Community

Allegro.cc Forums » Allegro Development » _al_stretch_blit questionable behaviour

This thread is locked; no one can reply to it. rss feed Print
_al_stretch_blit questionable behaviour
Edgar Reynaldo
Major Reynaldo
May 2007
avatar

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);

Now the destination bitmap is guaranteed to be a memory bitmap by these lines (src/c/cstretch.c lines 259-263) - actually if do_stretch_blit is NULL in the source bitmap's vtable then the destination could be any kind of bitmap.

   /* 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
Well, I looked at stretch_blit in the manual, and it says :

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:
Indicates that stretched blitting of video bitmaps onto the screen is implemented using hardware acceleration.

GFX_HW_SYS_STRETCH_BLIT:
Indicates that stretched blitting of system bitmaps onto the screen is implemented using hardware acceleration.

GFX_HW_VRAM_STRETCH_BLIT_MASKED:
Indicates that masked stretched blitting (including stretch_sprite) of video bitmaps onto the screen is implemented using hardware acceleration. NOTE: some display drivers may show artifacts when this function is used. If the image does not look correct try updating your video drivers.

GFX_HW_SYS_STRETCH_BLIT_MASKED:
Indicates that masked stretched blitting (including stretch_sprite) of system bitmaps onto the screen is implemented using hardware acceleration. NOTE: some display drivers may show artefact's when this function is used. If the image does not look correct try updating your video drivers.

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
Bueller?

Append 3
So, to review :
1) The manual says it's wrong to use line pointers on non memory bitmaps, but _al_stretch_blit does exactly that.
2) The manual says that for stretch_blit that the source must be a memory bitmap, but there are flags in gfx_capabilities that indicate the possibility of hardware accelerated stretching from system and video bitmaps to video bitmaps.

Apparent options :
1) Change the manual entry for stretch_blit to say that the source bitmap must be a memory bitmap except when gfx_capabilities has the following flags set (as appropriate) :
GFX_HW_VRAM_STRETCH_BLIT
GFX_HW_SYS_STRETCH_BLIT
GFX_HW_VRAM_STRETCH_BLIT_MASKED
GFX_HW_SYS_STRETCH_BLIT_MASKED

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
Since I haven't gotten any replies to this, I guess I will take it to the [AD] mailing list and see if I get any replies there.

Trent Gamblin
Member #261
April 2000
avatar

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
avatar

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 :

cstretch.c#SelectExpand
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 :

#SelectExpand
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.

Trent Gamblin
Member #261
April 2000
avatar

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
avatar

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.

Go to: