Mac OS X Image Loader Issue (5.0.2.1)
Todd Cope

I've noticed that on the OS X port the function al_load_bitmap_f() is not completely implemented. It currently reads in the entire file, making it not work with my custom formats. Often I will have a PNG or set of PNGs embedded in a larger file and with the current native image loader all data past the first embedded image is skipped, causing my loaders to break.

Are there any plans to fix this? If not it would be good to mention somewhere that this functionality is broken in the native image loader so someone what wants to use it on OS X will know to turn WANT_NATIVE_IMAGE_LOADER off.

Evert

I'm not sure I understand what the problem is and so I'm not sure what to suggest (but if you're putting multiple resources in a single file, the proper way to deal with that is to write an I/O interface for it, but the easiest thing to do is go through PhysFS).
However, if there's a problem and you have a way to fix it, we gladly accept patches.

Thomas Fjellstrom

I think we were supposed to add subfile or "chunk" support at some point to deal with that sort of use case. I thought Matthew or Peter had worked on it at some point.

Evert

Ah, ok. Probably not for the OS X native loader then. Not sure how to implement this there, but yes, this should be done then.

Todd Cope

Here's what one of my custom format loaders looks like:

#SelectExpand
1T3F_ANIMATION * t3f_load_animation_f(ALLEGRO_FILE * fp) 2{ 3 T3F_ANIMATION * ap; 4 int i; 5 char header[12] = {0}; 6 7 al_fread(fp, header, 12); 8 if(!check_header(header)) 9 { 10 return NULL; 11 } 12 13 ap = t3f_create_animation(); 14 if(ap) 15 { 16 ap->bitmaps = al_fread16le(fp); 17 for(i = 0; i < ap->bitmaps; i++) 18 { 19 ap->bitmap[i] = al_load_bitmap_f(fp, ".png"); 20 if(!ap->bitmap[i]) 21 { 22 printf("failed to load bitmap %d\n", i); 23 } 24 } 25 ap->frames = al_fread16le(fp); 26 for(i = 0; i < ap->frames; i++) 27 { 28 ap->frame[i] = malloc(sizeof(T3F_ANIMATION_FRAME)); 29 if(!ap->frame[i]) 30 { 31 return NULL; 32 } 33 ap->frame[i]->bitmap = al_fread16le(fp); 34 ap->frame[i]->x = t3f_fread_float(fp); 35 ap->frame[i]->y = t3f_fread_float(fp); 36 ap->frame[i]->z = t3f_fread_float(fp); 37 ap->frame[i]->width = t3f_fread_float(fp); 38 ap->frame[i]->height = t3f_fread_float(fp); 39 ap->frame[i]->angle = t3f_fread_float(fp); 40 ap->frame[i]->ticks = al_fread32le(fp); 41 ap->frame[i]->flags = al_fread32le(fp); 42 } 43 ap->flags = al_fread32le(fp); 44 } 45 t3f_animation_build_frame_list(ap); 46 return ap; 47}

This works perfectly with the Allegro image loaders but fails on the OS X native loader (haven't tried the Windows native loader). I already looked at the code for the native loader and I can't see any obvious way to fix it. It would require knowing the size of the embedded image file ahead of time and there's no way of knowing that without adding some extra code to read to the end of the image. I didn't see anything in the CGImage API that would be useful for this.

Evert
Todd Cope said:

I already looked at the code for the native loader and I can't see any obvious way to fix it. It would require knowing the size of the embedded image file ahead of time and there's no way of knowing that without adding some extra code to read to the end of the image. I didn't see anything in the CGImage API that would be useful for this.

That's what I was afraid of.
Is it possible to figure out after the fact and seek to that location in the file?

Peter Wang

You will need to record the size of the PNG file within the larger file. Then you may read the number of bytes into a buffer, wrap it with [al_open_memfile] and load the image from the memfile.

The planned chunk functionality would be slightly more efficient because you wouldn't need to read the data into a temporary buffer. No one's working on it right now.

Matthew Leverton
Todd Cope said:

This works perfectly with the Allegro image loaders

It is not guaranteed to work, so there's nothing to fix on OS X.

I wrote slice code that should be sufficient for what you are doing. If I can find it, I'll post it later. It looks something like:

ALLEGRO_FILE *slice = al_open_slice(fp, size, flags);
ap->bitmap[i] = al_load_bitmap_f(slice, ".png");
al_fclose(slice);

The size is still sometimes necessary because some loaders might do a SEEK_END, and we cannot control what the OS loaders do.

Todd Cope

That's what I was looking into but I can't see any way to know how much data the CGImage function actually used to load the image file.

There might be a way to get the size after the fact and seek to where we need to be. I don't know if it would work but maybe this would help:

NSImage *image = NSImageFromAllegroBitmap(bmp);
NSArray *reps = [image representations];
NSData *nsdata = [NSBitmapImageRep representationOfImageRepsInArray: reps usingType: type properties: nil];
size_t size = (size_t)[nsdata length];

This is how image saving works so maybe we could do something like this:

#SelectExpand
1static ALLEGRO_BITMAP *really_load_image(char *buffer, const char * ident, int *size) 2{ 3 ALLEGRO_BITMAP *bmp = NULL; 4 void *pixels = NULL; 5 /* Note: buffer is now owned (and later freed) by the data object. */ 6 NSData *nsdata = [NSData dataWithBytesNoCopy:buffer length:size]; 7 NSImage *image = [[NSImage alloc] initWithData:nsdata]; 8 bool premul = !(al_get_new_bitmap_flags() & ALLEGRO_NO_PREMULTIPLIED_ALPHA); 9 /* get the size to pass back to the caller so we can seek to the right place */ 10 NSBitmapImageFileType type; 11 if (!strcmp(ident, ".bmp")) { 12 type = NSBMPFileType; 13 } 14 else if (!strcmp(ident, ".jpg") || !strcmp(ident, ".jpeg")) { 15 type = NSJPEGFileType; 16 } 17 else if (!strcmp(ident, ".gif")) { 18 type = NSGIFFileType; 19 } 20 else if (!strcmp(ident, ".tif") || !strcmp(ident, ".tiff")) { 21 type = NSTIFFFileType; 22 } 23 else if (!strcmp(ident, ".png")) { 24 type = NSPNGFileType; 25 } 26 else { 27 return false; 28 } 29 NSData *size_nsdata = [NSBitmapImageRep representationOfImageRepsInArray: reps usingType: type properties: nil]; 30 *size = (int)[size_nsdata length]; 31 32 if (!image) 33 return NULL; 34 35 /* Get the image representations */ 36 NSArray *reps = [image representations]; 37 NSImageRep *image_rep = [reps objectAtIndex: 0]; 38 39 // Note: Do we want to support this on OSX 10.5? It doesn't have 40 // CGImageForProposedRect... 41 //CGImageRef cgimage = [image_rep CGImageForProposedRect: nil context: nil hints: nil]; 42 43 if (!image_rep) { 44 [image release]; 45 return NULL; 46 } 47 48 /* Get the actual size in pixels from the representation */ 49 int w = [image_rep pixelsWide]; 50 int h = [image_rep pixelsHigh]; 51 52 ALLEGRO_DEBUG("Read image of size %dx%d\n", w, h); 53 54 /* Now we need to draw the image into a memory buffer. */ 55 pixels = al_malloc(w * h * 4); 56 57 CGFloat whitePoint[3] = { 58 1, 1, 1 59 }; 60 CGFloat blackPoint[3] = { 61 0, 0, 0 62 }; 63 CGFloat gamma[3] = { 64 2.2, 2.2, 2.2 65 }; 66 CGFloat matrix[9] = { 67 1, 1, 1, 68 1, 1, 1, 69 1, 1, 1 70 }; 71 /* This sets up a color space that results in identical values 72 * as the image data itself, which is the same as the standalone 73 * libpng loader 74 */ 75 CGColorSpaceRef colour_space = 76 CGColorSpaceCreateCalibratedRGB( 77 whitePoint, blackPoint, gamma, matrix 78 ); 79 CGContextRef context = CGBitmapContextCreate(pixels, w, h, 8, w * 4, 80 colour_space, 81 kCGImageAlphaPremultipliedLast); 82 83 [NSGraphicsContext saveGraphicsState]; 84 [NSGraphicsContext setCurrentContext:[NSGraphicsContext 85 graphicsContextWithGraphicsPort:context flipped:NO]]; 86 [image drawInRect:NSMakeRect(0, 0, w, h) 87 fromRect:NSZeroRect 88 operation : NSCompositeCopy 89 fraction : 1.0]; 90 [NSGraphicsContext restoreGraphicsState]; 91 92 //CGContextDrawImage(context, CGRectMake(0.0, 0.0, (CGFloat)w, (CGFloat)h), 93 // cgimage); 94 CGContextRelease(context); 95 CGColorSpaceRelease(colour_space); 96 97 [image release]; 98 99 /* Then create a bitmap out of the memory buffer. */ 100 bmp = al_create_bitmap(w, h); 101 if (bmp) { 102 ALLEGRO_LOCKED_REGION *lock = al_lock_bitmap(bmp, 103 ALLEGRO_PIXEL_FORMAT_ABGR_8888_LE, ALLEGRO_LOCK_WRITEONLY); 104 int i; 105 if (!premul) { 106 int x; 107 for (i = 0; i < h; i++) { 108 unsigned char *src_ptr = (unsigned char *)pixels + w * 4 * i; 109 unsigned char *dest_ptr = (unsigned char *)lock->data + 110 lock->pitch * i; 111 for (x = 0; x < w; x++) { 112 unsigned char r, g, b, a; 113 r = *src_ptr++; 114 g = *src_ptr++; 115 b = *src_ptr++; 116 a = *src_ptr++; 117 // NOTE: avoid divide by zero by adding a fraction 118 float alpha_mul = 255.0f / (a+0.001f); 119 r *= alpha_mul; 120 g *= alpha_mul; 121 b *= alpha_mul; 122 *dest_ptr++ = r; 123 *dest_ptr++ = g; 124 *dest_ptr++ = b; 125 *dest_ptr++ = a; 126 } 127 } 128 } 129 else { 130 for (i = 0; i < h; i++) { 131 memcpy(lock->data + lock->pitch * i, pixels + w * 4 * i, w * 4); 132 } 133 } 134 al_unlock_bitmap(bmp); 135 } 136 al_free(pixels); 137 return bmp; 138}

This might work depending on how NSImage handles representations. If it spits back the exact data it read when you retrieve the representation it should work.

You will need to record the size of the PNG file within the larger file.

Edit: I can't know the size of the PNG before recording it. I would have to write a temporary file, get the size, then rewrite the data into my own file.

It is not guaranteed to work, so there's nothing to fix on OS X.

Since the function exists and doesn't work then it needs to be fixed. There is no note of "not gauranteed to work" in the manual. If the al_load_*_f() functions are meant to be used with memfiles then it should be noted somewhere.

Peter Wang

al_load_*_f are allowed to read as much data as they like (this is a documentation bug). Some file formats simply don't have a well defined end. TGA is one. I don't remember if PNG is one.

Todd Cope said:

I can't know the size of the PNG before recording it. I would have to write a temporary file, get the size, then rewrite the data into my own file.

That's what the packfiles in A4 does. Other possibilities: write to a memory buffer first. Or reserve a few bytes, write the PNG, then seek back to the old location and fill in the length then.

Matthew Leverton
Todd Cope said:

Since the function exists and doesn't work then it needs to be fixed. There is no note of "not gauranteed to work" in the manual. If the al_load_*_f() functions are meant to be used with memfiles then it should be noted somewhere.

They aren't meant to work only with memfiles, but they do require a full random access I/O interface that represents a single file.

The native loaders (that we didn't write and have no control over) might do something like seek(SEEK_SET, 0) or seek(SEEK_END, 16). If you supply a pointer to a something that represents more than a single file, there's no way for Allegro to guarantee that it works.

I don't think the proper solution is to try to hack Allegro to work around those limitations in every single loader and saver (images, sounds, fonts, etc). Instead, file slices could be used over top any underlying interface.

I proposed (and partially implemented) something like:

ALLEGRO_FILE *slice = al_fopen_slice(fp, 0, "rw+");
al_save_bitmap_f(slice, ".png", bmp);
al_fclose(slice);

In this case, the slice is opened with an initial length of 0 for read (r) and write (w) and is expandable (+).

A slice is always opened up at the current position of the file. (Opening two slices on the same file at the same time is undefined.) When closing the slice, the original file is moved to the end of the slice.

I was also considering a "memory buffer" (m) option that would automatically store everything in memory so that it could be used for write only streams. For example:

ALLEGRO_FILE *ftp_file = open_ftp_file("ftp://allegro.cc/foo.png");
ALLEGRO_FILE *slice = al_fopen_slice(ftp_file, 0, "mrw+");
al_save_bitmap_f(slice, ".png", bmp);
al_fclose(slice); // memory buffer is flushed  

Todd Cope

The native loaders (that we didn't write and have no control over) might do something like seek(SEEK_SET, 0) or seek(SEEK_END, 16). If you supply a pointer to a something that represents more than a single file, there's no way for Allegro to guarantee that it works.

The docs should mention that the stream passed to the al_load_*_f() functions should represent a single file or else the behavior is undefined. Maybe even suggest using a memfile (or slices if those are implemented) so people can understand how to use them. I know I am not the only one who assumes you can just pass an open ALLEGRO_FILE stream. I can see the writer of the OS X loader was assuming this, too, based on the comments in the code.

Thread #607213. Printed from Allegro.cc