From d8c4ed5118d849cbb6a2526aea71ecd9fdd6f42d Mon Sep 17 00:00:00 2001 From: rubidium Date: Fri, 6 Nov 2009 23:01:52 +0000 Subject: [PATCH] (svn r17989) [0.7] -Backport from trunk: - Fix: Use 24bpp BMP format instead of 32bpp for screenshots. Saves space and is supported by more image viewers (r17943) - Fix: Close BMP file when making screenshot fails (r17941) - Fix: Deadlock when trying to create screenshot with too long name (including path) (r17936) - Fix: 32bpp BMP screenshots were in wrong colours on big endian machines and broken when screen width was not a multiple of 4 (r17910, r17909) --- src/screenshot.cpp | 128 ++++++++++++++++++++++++++++++--------------- 1 file changed, 87 insertions(+), 41 deletions(-) diff --git a/src/screenshot.cpp b/src/screenshot.cpp index 17b18993ea..cc3260e5b7 100644 --- a/src/screenshot.cpp +++ b/src/screenshot.cpp @@ -41,6 +41,7 @@ struct ScreenshotFormat { #pragma pack(push, 1) #endif +/** BMP File Header (stored in little endian) */ struct BitmapFileHeader { uint16 type; uint32 size; @@ -53,6 +54,7 @@ assert_compile(sizeof(BitmapFileHeader) == 14); #pragma pack(pop) #endif +/** BMP Info Header (stored in little endian) */ struct BitmapInfoHeader { uint32 size; int32 width, height; @@ -61,46 +63,63 @@ struct BitmapInfoHeader { }; assert_compile(sizeof(BitmapInfoHeader) == 40); +/** Format of palette data in BMP header */ struct RgbQuad { byte blue, green, red, reserved; }; assert_compile(sizeof(RgbQuad) == 4); -/* generic .BMP writer */ +/** Pixel data in 24bpp BMP */ +struct RgbTriplet { + byte b, g, r; +}; +assert_compile(sizeof(RgbTriplet) == 3); + +/** + * Generic .BMP writer + * @param name file name including extension + * @param callb callback used for gathering rendered image + * @param userdata parameters forwarded to #callb + * @param w width in pixels + * @param h height in pixels + * @param pixelformat bits per pixel + * @param paletter colour paletter (for 8bpp mode) + * @return was everything ok? + */ static bool MakeBmpImage(const char *name, ScreenshotCallback *callb, void *userdata, uint w, uint h, int pixelformat, const Colour *palette) { - BitmapFileHeader bfh; - BitmapInfoHeader bih; - RgbQuad rq[256]; - FILE *f; - uint i, padw; - uint n, maxlines; - uint pal_size = 0; - uint bpp = pixelformat / 8; + uint bpp; // bytes per pixel + switch (pixelformat) { + case 8: bpp = 1; break; + /* 32bpp mode is saved as 24bpp BMP */ + case 32: bpp = 3; break; + /* Only implemented for 8bit and 32bit images so far */ + default: return false; + } - /* only implemented for 8bit and 32bit images so far. */ - if (pixelformat != 8 && pixelformat != 32) return false; - - f = fopen(name, "wb"); + FILE *f = fopen(name, "wb"); if (f == NULL) return false; - /* each scanline must be aligned on a 32bit boundary */ - padw = Align(w, 4); + /* Each scanline must be aligned on a 32bit boundary */ + uint bytewidth = Align(w * bpp, 4); // bytes per line in file - if (pixelformat == 8) pal_size = sizeof(RgbQuad) * 256; + /* Size of palette. Only present for 8bpp mode */ + uint pal_size = pixelformat == 8 ? sizeof(RgbQuad) * 256 : 0; - /* setup the file header */ + /* Setup the file header */ + BitmapFileHeader bfh; bfh.type = TO_LE16('MB'); - bfh.size = TO_LE32(sizeof(bfh) + sizeof(bih) + pal_size + padw * h * bpp); + bfh.size = TO_LE32(sizeof(BitmapFileHeader) + sizeof(BitmapInfoHeader) + pal_size + bytewidth * h); bfh.reserved = 0; - bfh.off_bits = TO_LE32(sizeof(bfh) + sizeof(bih) + pal_size); + bfh.off_bits = TO_LE32(sizeof(BitmapFileHeader) + sizeof(BitmapInfoHeader) + pal_size); - /* setup the info header */ + /* Setup the info header */ + BitmapInfoHeader bih; bih.size = TO_LE32(sizeof(BitmapInfoHeader)); bih.width = TO_LE32(w); bih.height = TO_LE32(h); bih.planes = TO_LE16(1); - bih.bitcount = TO_LE16(pixelformat); + bih.bitcount = TO_LE16(bpp * 8); bih.compression = 0; bih.sizeimage = 0; bih.xpels = 0; @@ -108,43 +127,66 @@ static bool MakeBmpImage(const char *name, ScreenshotCallback *callb, void *user bih.clrused = 0; bih.clrimp = 0; + /* Write file header and info header */ + if (fwrite(&bfh, sizeof(bfh), 1, f) != 1 || fwrite(&bih, sizeof(bih), 1, f) != 1) { + fclose(f); + return false; + } + if (pixelformat == 8) { - /* convert the palette to the windows format */ - for (i = 0; i != 256; i++) { + /* Convert the palette to the windows format */ + RgbQuad rq[256]; + for (uint i = 0; i < 256; i++) { rq[i].red = palette[i].r; rq[i].green = palette[i].g; rq[i].blue = palette[i].b; rq[i].reserved = 0; } + /* Write the palette */ + if (fwrite(rq, sizeof(rq), 1, f) != 1) { + fclose(f); + return false; + } } - /* write file header and info header and palette */ - if (fwrite(&bfh, sizeof(bfh), 1, f) != 1) return false; - if (fwrite(&bih, sizeof(bih), 1, f) != 1) return false; - if (pixelformat == 8) if (fwrite(rq, sizeof(rq), 1, f) != 1) return false; + /* Try to use 64k of memory, store between 16 and 128 lines */ + uint maxlines = Clamp(65536 / (w * pixelformat / 8), 16, 128); // number of lines per iteration - /* use by default 64k temp memory */ - maxlines = Clamp(65536 / padw, 16, 128); + uint8 *buff = MallocT(maxlines * w * pixelformat / 8); // buffer which is rendered to + uint8 *line = AllocaM(uint8, bytewidth); // one line, stored to file + memset(line, 0, bytewidth); - /* now generate the bitmap bits */ - uint8 *buff = CallocT(padw * maxlines * bpp); // by default generate 128 lines at a time. - - /* start at the bottom, since bitmaps are stored bottom up. */ + /* Start at the bottom, since bitmaps are stored bottom up */ do { - /* determine # lines */ - n = min(h, maxlines); + uint n = min(h, maxlines); h -= n; - /* render the pixels */ - callb(userdata, buff, h, padw, n); + /* Render the pixels */ + callb(userdata, buff, h, w, n); - /* write each line */ - while (n) - if (fwrite(buff + (--n) * padw * bpp, padw * bpp, 1, f) != 1) { + /* Write each line */ + while (n-- != 0) { + if (pixelformat == 8) { + /* Move to 'line', leave last few pixels in line zeroed */ + memcpy(line, buff + n * w, w); + } else { + /* Convert from 'native' 32bpp to BMP-like 24bpp. + * Works for both big and little endian machines */ + Colour *src = ((Colour *)buff) + n * w; + RgbTriplet *dst = (RgbTriplet *)line; + for (uint i = 0; i < w; i++) { + dst[i].r = src[i].r; + dst[i].g = src[i].g; + dst[i].b = src[i].b; + } + } + /* Write to file */ + if (fwrite(line, bytewidth, 1, f) != 1) { free(buff); fclose(f); return false; } + } } while (h != 0); free(buff); @@ -539,7 +581,11 @@ static char *MakeScreenshotName(const char *ext) snprintf(&_screenshot_name[len], lengthof(_screenshot_name) - len, ".%s", ext); for (serial = 1;; serial++) { - snprintf(filename, lengthof(filename), "%s%s", _personal_dir, _screenshot_name); + if (snprintf(filename, lengthof(filename), "%s%s", _personal_dir, _screenshot_name) >= (int)lengthof(filename)) { + /* We need more characters than MAX_PATH -> end with error */ + filename[0] = '\0'; + break; + } if (!FileExists(filename)) break; /* If file exists try another one with same name, but just with a higher index */ snprintf(&_screenshot_name[len], lengthof(_screenshot_name) - len, "#%d.%s", serial, ext);