Hello,
two faults:
1. draw_sprite failure to find correct place
2. system_bitmap with subbitmaps crashes when you destroy the subbitmaps

From testing I've found that when you draw_sprite a subbitmap of a video or system bitmap onto a different system or video bitmap, it doubles the x/y and you end up with the wrong portion showing (this is following from my thread trying to explain things).Also, I've found through testing that if you have a subbitmap of a system bitmap when you try to destroy the subbitmaps you get a crash. Video and memory bitmaps are ok.

Attached is a sample piece of code and some windows exe's to show how subbitmaps onto video or system bitmaps don't work, and how system bitmaps that have subbitmaps crash the system.

As you can see from the images, 'original.jpg' is how it works fine with memory bitmaps, but when you use video/system bitmaps you get it wrong, as in 'doublingerror.jpg'. sheet.bmp is the source bitmap.

If you look closely you can see what is happening is it is doubling the x and y position when you blit a video/memory subbitmap onto a system/video bitmap.

1. if you compile as is, it will work.
2. if you uncomment the line: //#define nonmemorybitmap you will get the error
3. if you change the line 41 to system bitmap you get the crash.

masked blit is fine. The problem, I believe, is the bit of code in the windows acceleration source file for draw_sprite that tries to determine the correct place.

$ g++ main.cpp `allegro-config --libs --static`
main.cpp:9: error: `main' must return `int'
main.cpp: In function `int main(...)':
main.cpp:84: error: return-statement with no value, in function declared with a 
   non-void return type

Fixed those.

I'm on Linux, and everything worked fine.

I've found the genuine and demonstrable Allegro bug! As Neil Walker observes (elsewhere), ddraw_draw_sprite looks like this:

The problem relates to [x/y]_ofs, which seem to be used to hold the top left corner of the area of a bitmap that is actually in use. With ordinary bitmaps they are both set to 0. With sub bitmaps they are set somewhere else in order to only display a window within the source bitmap. The Allegro 2.x days of just fiddling with ->line pointers are obviously long behind.

The problem is that both ddraw_draw_sprite and ddraw_masked_blit apply the sub bitmap constraints. I believe (but cannot test, as still in OS X) that the following should fix the problem:

Note the only change, near the top:

      sx = 0;
      sy = 0;

Which relate to lines 155 and 156 of src/win/wddaccel.c of the Allegro source.

Please could someone test this theory?

Found it: http://www.allegro.cc/dev/make-diff.php. The diff, as shown below, is attached. Should this patch make the grade, please acknowledge both Neil Walker & myself in the authors file as I only got to ddraw_draw_sprite following a message by him on the Retrospec mailing list.

It's only two lines :-) As soon as someone confirms it works I'll commit it. I'm sure it's correct anyway. Thanks to all involved.

Sorry, yes, I've found the cause of that but am otherwise lost as I haven't traced create_sub_bitmap yet.

destroy_bitmap is at line 1418 of src/graphics.c. It has a special branch for anything that passes is_video_bitmap, which includes sub bitmaps. Specifically, starting on line 1423 of that same file:

On DirectX, gfx_driver->destroy_video_bitmap directly deletes the bitmap no questions asked. So the memory for the parent bitmap is destroyed and both it and all sub bitmaps of it become invalid. Obviously an is_sub_bitmap needs to be checked somewhere, but I have no idea whether that is meant to be the responsibility of the platform neutral code or the driver specific.

I have to admit that I couldn't see a purpose for it. Perhaps the change to making that function sit on top of masked_blit was a late addition to the code?

Hmmm. Then I've misunderstood the code a little. But line 1471 of src/graphics.c jumps to line 656 of src/win/wddbmp.c, then on to line 299 which is an IDirectDrawSurface2_Release, freeing the memory of the parent bitmap.

The code change works, three cheers for Thomas

Attached is an executable that used to fail, and now it works. I've included the source code, the bitmap, the .lib and the new dll if anyone wants to try or use a new .lib/dll file.

To see it failing simply rename the .DLL so that the exe will use the original dll and watch it show the wrong graphics!

btw, before the code is checked in, does someone want to see if all the clipping code is necessary? if you call masked_blit directly it doesn't do any of that.

This example code can also be used to see that when you use a SYSTEM bitmap (follow the comments) it crashes when you have subbitmaps and try to delete a sub-bitmap.

ps, to celebrate I've upped my AXL_FRAMS project from 70% to 90%

Hmm, yes, but a bit ugly. What do you think of this patch? (untested)
All this is a bit theoretical (not the bug Neil pointed out), since no gfx or system drivers have non-null create_sub_bitmap methods anyway.

Patch committed, cheers.

For a moment I was thinking you were talking about draw_sprite() and masked_blit() in general, but it's the Windows-specific hardware accelerated implementation, right?
In general, you cannot always use masked_blit() where you can use draw_sprite().

Regarding the tests, they do look a bit redundant (or missing in the case of ddraw_masked_blit). What happens if you remove them altogether? Is clipping still performed as it should? What about maksed_blit? Does that do proper clipping?

Although DirectDraw supersets the Allegro clipping functionality and the DirectDraw driver creates a clipper it appears that there is no DirectDraw version of set_clip[_rect]. So it seems to me that ddraw_masked_blit should clip "incorrectly" based on a reading of the current manual but if it were "correct" then there would be no need for the clipping in ddraw_draw_sprite.

Can anyone verify that?

What do you want testing exactly, just use masked_blit to draw a bitmap onto another but make the co-ordinates outside the target bitmap?

Don't worry, I'll test it later as I can easily check all permutations of video/system/memory bitmaps and sub-bitmaps in all resolutions, colour depths and windowed modes with the power of AXL