From 1985d28ef905753423b3ee407c8bf4ee9165302b Mon Sep 17 00:00:00 2001 From: Andreas Sandberg Date: Sat, 23 May 2015 13:37:01 +0100 Subject: [PATCH] base: Clean up bitmap generation code The bitmap generation code is hard to follow and incorrectly uses the size of an enum member to calculate the size of a pixel. This changeset cleans up the code and adds some documentation. --- src/base/bitmap.cc | 94 ++++++++++++++++++++++++++-------------------- src/base/bitmap.hh | 68 +++++++++++++++++++-------------- 2 files changed, 93 insertions(+), 69 deletions(-) diff --git a/src/base/bitmap.cc b/src/base/bitmap.cc index 80d836b2f..d83a30be3 100644 --- a/src/base/bitmap.cc +++ b/src/base/bitmap.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010 ARM Limited + * Copyright (c) 2010, 2015 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -37,26 +37,59 @@ * Authors: William Wang * Ali Saidi * Chris Emmons + * Andreas Sandberg */ +#include "base/bitmap.hh" + #include -#include "base/bitmap.hh" #include "base/misc.hh" -const size_t Bitmap::sizeofHeaderBuffer = sizeof(Magic) + sizeof(Header) + - sizeof(Info); - // bitmap class ctor -Bitmap::Bitmap(VideoConvert::Mode _mode, uint16_t w, uint16_t h, uint8_t *d) - : mode(_mode), height(h), width(w), data(d), - vc(mode, VideoConvert::rgb8888, width, height), headerBuffer(0) +Bitmap::Bitmap(VideoConvert::Mode mode, uint16_t w, uint16_t h, uint8_t *d) + : height(h), width(w), + header(getCompleteHeader()), + data(d), + vc(mode, VideoConvert::rgb8888, width, height) +{ +} + +Bitmap::~Bitmap() { } -Bitmap::~Bitmap() { - if (headerBuffer) - delete [] headerBuffer; +const Bitmap::CompleteV1Header +Bitmap::getCompleteHeader() const +{ + const uint32_t pixel_array_size(sizeof(PixelType) * width * height); + const uint32_t file_size(sizeof(CompleteV1Header) + pixel_array_size); + + const CompleteV1Header header = { + // File header + { + {'B','M'}, /* Magic */ + file_size, + 0, 0, /* Reserved */ + sizeof(CompleteV1Header) /* Offset to pixel array */ + }, + // Info/DIB header + { + sizeof(InfoHeaderV1), + width, + height, + 1, /* Color planes */ + 32, /* Bits per pixel */ + 0, /* No compression */ + pixel_array_size, /* Image size in bytes */ + 2835, /* x pixels per meter (assume 72 DPI) */ + 2835, /* y pixels per meter (assume 72 DPI) */ + 0, /* Colors in color table */ + 0 /* Important color count (0 == all are important) */ + } + }; + + return header; } void @@ -64,43 +97,24 @@ Bitmap::write(std::ostream *bmp) const { assert(data); - // header is always the same for a bitmap object; compute the info once per - // bitmap object - if (!headerBuffer) { - // For further information see: - // http://en.wikipedia.org/wiki/BMP_file_format - Magic magic = {{'B','M'}}; - Header header = { - static_cast(sizeof(VideoConvert::Rgb8888)) * - width * height, 0, 0, 54}; - Info info = {static_cast(sizeof(Info)), width, height, 1, - static_cast(sizeof(VideoConvert::Rgb8888)) * 8, - 0, static_cast(sizeof(VideoConvert::Rgb8888)) * - width * height, 1, 1, 0, 0}; - - char *p = headerBuffer = new char[sizeofHeaderBuffer]; - memcpy(p, &magic, sizeof(Magic)); - p += sizeof(Magic); - memcpy(p, &header, sizeof(Header)); - p += sizeof(Header); - memcpy(p, &info, sizeof(Info)); - } - // 1. write the header - bmp->write(headerBuffer, sizeofHeaderBuffer); + bmp->write(reinterpret_cast(&header), sizeof(header)); // 2. write the bitmap data - uint8_t *tmp = vc.convert(data); - uint32_t *tmp32 = (uint32_t*)tmp; + const uint8_t *pixels(vc.convert(data)); // BMP start store data left to right starting with the bottom row // so we need to do some creative flipping - for (int i = height - 1; i >= 0; i--) - for (int j = 0; j < width; j++) - bmp->write((char*)&tmp32[i * width + j], sizeof(uint32_t)); + for (int y = height - 1; y >= 0; y--) { + for (int x = 0; x < width; x++) { + bmp->write( + (const char *)&pixels[sizeof(PixelType) * (y * width + x)], + sizeof(PixelType)); + } + } bmp->flush(); - delete [] tmp; + delete[] pixels; } diff --git a/src/base/bitmap.hh b/src/base/bitmap.hh index e9ad15473..b06aff10f 100644 --- a/src/base/bitmap.hh +++ b/src/base/bitmap.hh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010 ARM Limited + * Copyright (c) 2010, 2015 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -37,12 +37,14 @@ * Authors: William Wang * Ali Saidi * Chris Emmons + * Andreas Sandberg */ #ifndef __BASE_BITMAP_HH__ #define __BASE_BITMAP_HH__ -#include +#include +#include "base/compiler.hh" #include "base/vnc/convert.hh" /** @@ -54,8 +56,10 @@ class Bitmap { public: - /** Create a Bitmap creator that takes data in the given mode & size - * and outputs to an fstream + /** + * Create a bitmap that takes data in a given mode & size and + * outputs to an ostream. + * * @param mode the type of data that is being provided * @param h the hight of the image * @param w the width of the image @@ -63,52 +67,42 @@ class Bitmap */ Bitmap(VideoConvert::Mode mode, uint16_t w, uint16_t h, uint8_t *d); - /** Destructor */ ~Bitmap(); - /** Provide the converter with the data that should be output. It will be - * converted into rgb8888 and write out when write() is called. + /** + * Provide the converter with the data that should be output. It + * will be converted into rgb8888 and written when write() is + * called. + * * @param d the data */ void rawData(uint8_t* d) { data = d; } - /** Write the provided data into the fstream provided + /** + * Write the frame buffer data into the provided ostream + * * @param bmp stream to write to */ void write(std::ostream *bmp) const; - /** Gets a hash over the bitmap for quick comparisons to other bitmaps. + /** + * Gets a hash over the bitmap for quick comparisons to other bitmaps. + * * @return hash of the bitmap */ uint64_t getHash() const { return vc.getHash(data); } private: - VideoConvert::Mode mode; - uint16_t height; - uint16_t width; - uint8_t *data; - - VideoConvert vc; - - mutable char *headerBuffer; - static const size_t sizeofHeaderBuffer; - - struct Magic - { + struct FileHeader { unsigned char magic_number[2]; - }; - - struct Header - { uint32_t size; uint16_t reserved1; uint16_t reserved2; uint32_t offset; - }; + } M5_ATTR_PACKED; - struct Info - { + struct InfoHeaderV1 { /* Aka DIB header */ uint32_t Size; uint32_t Width; uint32_t Height; @@ -120,7 +114,23 @@ class Bitmap uint32_t YPelsPerMeter; uint32_t ClrUsed; uint32_t ClrImportant; - }; + } M5_ATTR_PACKED; + + struct CompleteV1Header { + FileHeader file; + InfoHeaderV1 info; + } M5_ATTR_PACKED; + + typedef uint32_t PixelType; + + const CompleteV1Header getCompleteHeader() const; + + const uint16_t height; + const uint16_t width; + const CompleteV1Header header; + uint8_t *data; + + VideoConvert vc; }; #endif // __BASE_BITMAP_HH__ -- 2.30.2