Hello,
I was in the process of testing 4.2.1 release to see if it fixes the system bitmap problem with sub-bitmaps (http://allegro.cc/forums/thread/589858/646845#target) - and it does, however I've now come across quite a major bug.

Before I get out the debugger and fail to find the fault, just wondered if anyone else has come across this. It is a windows build, btw.

Basically, when you use grab_font_from_bitmap(f) when the bitmap is not a memory bitmap, (i.e. video or system bitmap) it takes about 15 to 20 seconds to make the call. I have 5 fonts to be grabbed so that's about two minutes just waiting for a program to start. I guess it's a locking problem? In 4.2.0 it is immediate (milliseconds) for any bitmap type.

[edit]tell a lie, with memory bitmaps grab_from_bitmap akes about 2 seconds per bitmap font, so something is definitely up[/edit]

The bitmap is fine and I can run 4.2.0 on it and all is well.

I guess the simplest question is, if it's related specifically to the grab_font code, can anyone do a diff to see what has changed?

Here are the changes in fontbmp.c from 4.2.0 to 4.2.1. The _AL_MALLOC/_AL_FREE changes are nothing, so the only thing that may make a difference is the first hunk.

The first section was changed due to this:

r5721 (orig r5674):  elias | 2006-01-19 07:36:55 +1100

Patch to add support for anti-aliased bitmap fonts. It does the following:
- Allow to read in alpha fonts. If alpha values are detected in a 32bit font bitmap, the top left pixel is used as mask color instead of yel- Add a make_trans_font function. This simply changes the vtable of the passed color font to one using draw_trans_sprite for render_char.
- Since there were now 3 locations using bitmap_has_alpha, made it an internal function.

But bitmap_has_alpha is implemented very inefficiently:

So I guess that's the problem.

and the fix is?

I'm not at my dev machine at the minute, but to prove it, I'll redo the bitmaps in 8bpp and see if it does anything. But how can scanning through a bitmap take so long? and why is it using getpixel and not _getpixel? and shouldn't there be a flag in the functions to turn this new alpha/alias checking off? and why is the get alpha called for 8bpp explicitly then in the get alpha code returned immediately if 8bpp? so many questions

btw, pardon my French but those if statements at the top look awefully messy.

Cheers.

Reading from video bitmaps is VERY VERY slow.

these are system bitmaps as well that take 20 or so seconds. memory bitmaps have also increased from milliseconds to seconds.

I would assume system bitmaps are just as slow.. I'm not 100% sure though.

if you take a stab at a memory bitmap taking 100ms to scan in 4.2 (it was immediate for me for 5 bitmaps) and in the new one it takes 1200ms (it took about 7 seconds in 4.2.1), then that's a 12x decrease in speed with the new code, even with memory bitmaps.

But I think Peter is right and the fault is the way the new code patch works for checking for aliased/alpha. For starters there's one check looking for 8bpp but the function it calls returns immediately if it is 8bpp, then there's the use of getpixel.

This is what I'm saying, its reading every single pixel from the bitmap, and checking if theres a non solid alpha value. That reading is comparatively slow with memorybitmaps, but much slower on non memory bitmaps (which you first started talking about).

Or you can use any bitmap that isn't 32 bpp.

Because it's checking the entire bitmap for the presence of an alpha channel for each character you grab from it, that's why. It breaks out of this as soon as it finds an alpha value, but the worst case is obviously going to be the case where no alpha value is set at all.
It shouldn't be very difficult to provide a fix for this that only does the check once per call of grab_font_from_bitmap(), give me half an hour or so.

Because _getpixel() only works on 8 bit memory bitmaps whereas getpixel() works on any type of bitmap in any colour depth?
Beside which, you're not supposed to call grab_font_from_bitmap() in speed-critical code. In other words, the speed difference between getpixel() and _getpixel*() should not be that important.
That said, feel free to optimise it and submit a patch (EDIT: see attachment).

I don't think so. That would really make things messy.

It isn't called for 8bpp. The code checks first if the bitmap has an alpha channel (false for all but 32 bpp, which can have one), then if it's 8bpp:

static void font_find_character(BITMAP *bmp, int *x, int *y, int *w, int *h)
{
   int c;

   if (_bitmap_has_alpha(bmp)) {
      c = getpixel(bmp, 0, 0);
   }
   else if (bitmap_color_depth(bmp) == 8) {
      c = 255;
   }
   else {
      c = makecol_depth(bitmap_color_depth(bmp), 255, 255, 0);
   }

How would you do this then?

Now, personal opinion: it makes no sense whatsoever to call grab_font_from_bitmap on a video or system bitmap, precisely because you need to read data back from teh bitmap. This should be obvious, but if you want we can add a sentence to the documentation explaining this.

EDIT: patch attached.

It's still a lot slower for memory bitmaps, but liveable I guess.

I've always treated system bitmaps as if they were memory bitmaps, but faster if available.

My mistake, I didn't notice the diff -

Excellent, thanks Evert. The obvious fixes are always the best. Hopefully it'll make it to the next version.

Just so there's no confusion, this means you tested it and it corrects the problem for you, right? I know the code compiles and links fine, but I didn't have an opportunity to check that it actually works. It should, but it wouldn't be the first time I write code that "should" work but doesn't.

I'll commit it as soon as someone tests it and confirms it works as intended.

Erm, no. For 4.2.1 I was lazy and just got the pre-built version. If no-one takes you up by the end of today to recompile (I can test it) a new dll, I'll do it tomorrow, if someone can tell me the diff tool to use (windows).

WinMerge.

Reglar patch should be fine as well.

I use winmerge and oddly just tried to find out how to read in diff files a few minutes ago, but all I could find was an option to create a diff from two files, not open one up.

http://gnuwin32.sourceforge.net/packages/patch.htm

To apply the patch, when in the allegro dir:

patch -p0 -i fontbmp.diff

You can always add --dry-run to see if everything is ok before you actually apply the patch.

Winmerge, TortoiseSVN and many others can create patches, or you can use the original:

http://gnuwin32.sourceforge.net/packages/diffutils.htm

bump

I haven't forgotten, I've been ill most of the week.