From e9857132c8aa301849527ef08d722d425cc1ec85 Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Tue, 29 Sep 2026 15:58:57 -0700 Subject: [PATCH 1/5] feat: add bitmap lossiness state contract --- bitmap.cpp | 42 +++++++++----- bitmap.h | 9 +++ test_runner.sh | 6 +- tests/bitmap_tests.cpp | 128 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 169 insertions(+), 16 deletions(-) create mode 100644 tests/bitmap_tests.cpp diff --git a/bitmap.cpp b/bitmap.cpp index 3aa4030..bf08ddf 100644 --- a/bitmap.cpp +++ b/bitmap.cpp @@ -55,6 +55,7 @@ struct bmpfile_dib_info void Bitmap::open(std::string filename) { std::ifstream file(filename.c_str(), std::ios::in | std::ios::binary); + lossy = false; //clear data if already holds information for(int i=0; i MAX_RGB || current.red < MIN_RGB || - current.green > MAX_RGB || current.green < MIN_RGB || - current.blue > MAX_RGB || current.blue < MIN_RGB ) - { + if( static_cast(pixels[row].size()) != width ) + { valid = false; - break; - } + } + for(int column=0; valid && column < width; column++) + { + Pixel current = pixels[row][column]; + if( current.red > MAX_RGB || current.red < MIN_RGB || + current.green > MAX_RGB || current.green < MIN_RGB || + current.blue > MAX_RGB || current.blue < MIN_RGB ) + { + valid = false; + } + } } } return valid; } +// ---------------------------------------------------------------------------- +bool Bitmap::isLossy() +{ + return lossy; +} + // ---------------------------------------------------------------------------- /** * Provides a vector of vector of pixels representing the bitmap @@ -292,4 +303,5 @@ PixelMatrix Bitmap::toPixelMatrix() void Bitmap::fromPixelMatrix(const PixelMatrix & values) { pixels = values; + lossy = false; } diff --git a/bitmap.h b/bitmap.h index 173c6d2..3d12fc4 100644 --- a/bitmap.h +++ b/bitmap.h @@ -37,6 +37,7 @@ class Bitmap { private: PixelMatrix pixels; + bool lossy = false; public: /** @@ -68,6 +69,14 @@ class Bitmap **/ bool isImage(); + /** + * Reports whether opening the current image discarded color information + * or precision while converting it to RGB pixels. + * + * @return true only when the most recent successful open was lossy + **/ + bool isLossy(); + /** * Provides a vector of vector of pixels representing the bitmap * diff --git a/test_runner.sh b/test_runner.sh index 973a36f..dd38c55 100755 --- a/test_runner.sh +++ b/test_runner.sh @@ -1,3 +1,7 @@ #!/bin/bash -g++ -c bitmap.cpp -o bitmap.o \ No newline at end of file +set -eu + +test_binary="${TMPDIR:-/tmp}/bitmap-tests" +g++ -std=c++11 -Wall -Wextra -pedantic bitmap.cpp tests/bitmap_tests.cpp -o "$test_binary" +"$test_binary" diff --git a/tests/bitmap_tests.cpp b/tests/bitmap_tests.cpp new file mode 100644 index 0000000..7080fc3 --- /dev/null +++ b/tests/bitmap_tests.cpp @@ -0,0 +1,128 @@ +#include "../bitmap.h" + +#include +#include +#include +#include +#include +#include + +namespace +{ +int failures = 0; +int fixture_number = 0; + +void expect(bool condition, const std::string & message) +{ + if (!condition) + { + std::cerr << "FAIL: " << message << '\n'; + ++failures; + } +} + +void append_u16(std::vector & bytes, unsigned int value) +{ + bytes.push_back(static_cast(value & 0xff)); + bytes.push_back(static_cast((value >> 8) & 0xff)); +} + +void append_u32(std::vector & bytes, unsigned long value) +{ + bytes.push_back(static_cast(value & 0xff)); + bytes.push_back(static_cast((value >> 8) & 0xff)); + bytes.push_back(static_cast((value >> 16) & 0xff)); + bytes.push_back(static_cast((value >> 24) & 0xff)); +} + +std::string write_fixture(const std::vector & bytes) +{ + const std::string path = "/tmp/bitmap-test-" + + std::to_string(fixture_number++) + ".bmp"; + std::ofstream output(path.c_str(), std::ios::binary); + output.write(reinterpret_cast(&bytes[0]), bytes.size()); + return path; +} + +std::vector make_24_bpp_fixture() +{ + std::vector bytes; + bytes.push_back('B'); + bytes.push_back('M'); + append_u32(bytes, 62); + append_u16(bytes, 0); + append_u16(bytes, 0); + append_u32(bytes, 54); + append_u32(bytes, 40); + append_u32(bytes, 2); + append_u32(bytes, 1); + append_u16(bytes, 1); + append_u16(bytes, 24); + append_u32(bytes, 0); + append_u32(bytes, 8); + append_u32(bytes, 0); + append_u32(bytes, 0); + append_u32(bytes, 0); + append_u32(bytes, 0); + bytes.push_back(30); + bytes.push_back(20); + bytes.push_back(10); + bytes.push_back(60); + bytes.push_back(50); + bytes.push_back(40); + bytes.push_back(0); + bytes.push_back(0); + return bytes; +} + +void test_public_state_contract() +{ + Bitmap bitmap; + expect(!bitmap.isImage(), "a default bitmap is not an image"); + expect(!bitmap.isLossy(), "a default bitmap is not lossy"); + + bitmap.open("/tmp/bitmap-test-does-not-exist.bmp"); + expect(!bitmap.isImage(), "a failed open leaves an empty image"); + expect(!bitmap.isLossy(), "a failed open resets lossiness"); + + PixelMatrix pixels(1, std::vector(1, Pixel(1, 2, 3))); + bitmap.fromPixelMatrix(pixels); + expect(bitmap.isImage(), "a valid pixel matrix is an image"); + expect(!bitmap.isLossy(), "a caller-provided matrix has no lossy source"); +} + +void test_existing_24_bpp_behavior() +{ + const std::string path = write_fixture(make_24_bpp_fixture()); + Bitmap bitmap; + bitmap.open(path); + const PixelMatrix pixels = bitmap.toPixelMatrix(); + + expect(bitmap.isImage(), "24-bpp BI_RGB opens successfully"); + expect(!bitmap.isLossy(), "24-bpp BI_RGB is lossless"); + expect(pixels.size() == 1 && pixels[0].size() == 2, + "24-bpp dimensions are preserved"); + if (pixels.size() == 1 && pixels[0].size() == 2) + { + expect(pixels[0][0].red == 10 && pixels[0][0].green == 20 && + pixels[0][0].blue == 30, "first BGR pixel becomes RGB"); + expect(pixels[0][1].red == 40 && pixels[0][1].green == 50 && + pixels[0][1].blue == 60, "second BGR pixel becomes RGB"); + } + std::remove(path.c_str()); +} +} + +int main() +{ + test_public_state_contract(); + test_existing_24_bpp_behavior(); + + if (failures != 0) + { + std::cerr << failures << " test assertion(s) failed\n"; + return EXIT_FAILURE; + } + std::cout << "All bitmap tests passed\n"; + return EXIT_SUCCESS; +} From ff9fc0b76742c8f4d3cb352618489012ffc7c69a Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Tue, 29 Sep 2026 16:08:23 -0700 Subject: [PATCH 2/5] feat: decode common BMP formats safely --- bitmap.cpp | 1011 +++++++++++++++++++++++++++++----------- tests/bitmap_tests.cpp | 436 ++++++++++++++++- 2 files changed, 1177 insertions(+), 270 deletions(-) diff --git a/bitmap.cpp b/bitmap.cpp index bf08ddf..b9c3807 100644 --- a/bitmap.cpp +++ b/bitmap.cpp @@ -1,307 +1,780 @@ -#include -#include #include "bitmap.h" -#include -typedef unsigned char uchar_t; -typedef unsigned int uint32_t; -typedef unsigned short int uint16_t; -typedef signed int int32_t; -typedef signed short int int16_t; - -const int MIN_RGB=0; -const int MAX_RGB=255; -const int BMP_MAGIC_ID=2; +#include +#include +#include +#include +#include +#include -// -------------------------------------------------------------- -// Windows BMP-specific format data -struct bmpfile_magic +namespace +{ +const int MIN_RGB = 0; +const int MAX_RGB = 255; +const std::uint32_t BI_RGB = 0; +const std::uint32_t BI_RLE8 = 1; +const std::uint32_t BI_RLE4 = 2; +const std::uint32_t BI_BITFIELDS = 3; +const std::uint32_t LCS_SRGB = 0x73524742; +const std::uint64_t MAX_DECODED_PIXELS = 100000000; + +struct Header { - uchar_t magic[BMP_MAGIC_ID]; + std::uint32_t dib_size; + std::int32_t width; + std::int32_t signed_height; + std::uint32_t height; + std::uint16_t bits_per_pixel; + std::uint32_t compression; + std::uint32_t image_size; + std::uint32_t colors_used; + std::uint32_t pixel_offset; + std::uint32_t red_mask; + std::uint32_t green_mask; + std::uint32_t blue_mask; + std::uint32_t alpha_mask; + std::size_t palette_offset; + bool core; + bool top_down; + bool color_metadata_lost; + + Header() + : dib_size(0), width(0), signed_height(0), height(0), + bits_per_pixel(0), compression(BI_RGB), image_size(0), + colors_used(0), pixel_offset(0), red_mask(0), green_mask(0), + blue_mask(0), alpha_mask(0), palette_offset(0), core(false), + top_down(false), color_metadata_lost(false) + { + } }; -struct bmpfile_header +struct MaskInfo { - uint32_t file_size; - uint16_t creator1; - uint16_t creator2; - uint32_t bmp_offset; + std::uint32_t mask; + unsigned int shift; + unsigned int bits; + std::uint64_t maximum; }; -struct bmpfile_dib_info +bool range_fits(std::size_t offset, std::size_t length, std::size_t size) { - uint32_t header_size; - int32_t width; - int32_t height; - uint16_t num_planes; - uint16_t bits_per_pixel; - uint32_t compression; - uint32_t bmp_byte_size; - int32_t hres; - int32_t vres; - uint32_t num_colors; - uint32_t num_important_colors; -}; + return offset <= size && length <= size - offset; +} +bool read_u16(const std::vector & bytes, std::size_t offset, + std::uint16_t & value) +{ + if (!range_fits(offset, 2, bytes.size())) + { + return false; + } + value = static_cast(bytes[offset]) | + static_cast(bytes[offset + 1] << 8); + return true; +} -// -------------------------------------------------------------- -/** - * Opens a file as its name is provided and reads pixel-by-pixel the colors - * into a matrix of RGB pixels. Any errors will cout but will result in an - * empty matrix (with no rows and no columns). - * - * @param name of the filename to be opened and read as a matrix of pixels -**/ -void Bitmap::open(std::string filename) +bool read_u32(const std::vector & bytes, std::size_t offset, + std::uint32_t & value) +{ + if (!range_fits(offset, 4, bytes.size())) + { + return false; + } + value = static_cast(bytes[offset]) | + (static_cast(bytes[offset + 1]) << 8) | + (static_cast(bytes[offset + 2]) << 16) | + (static_cast(bytes[offset + 3]) << 24); + return true; +} + +bool read_s32(const std::vector & bytes, std::size_t offset, + std::int32_t & value) +{ + std::uint32_t unsigned_value = 0; + if (!read_u32(bytes, offset, unsigned_value)) + { + return false; + } + value = static_cast(unsigned_value); + return true; +} + +bool load_file(const std::string & filename, std::vector & bytes) +{ + std::ifstream file(filename.c_str(), std::ios::in | std::ios::binary); + if (!file) + { + return false; + } + file.seekg(0, std::ios::end); + const std::streamoff length = file.tellg(); + if (length < 0 || static_cast(length) > + std::numeric_limits::max()) + { + return false; + } + file.seekg(0, std::ios::beg); + bytes.resize(static_cast(length)); + if (!bytes.empty()) + { + file.read(reinterpret_cast(&bytes[0]), bytes.size()); + } + return file.good() || file.eof(); +} + +bool is_known_dib_size(std::uint32_t size) +{ + return size == 12 || size == 40 || size == 52 || size == 56 || + size == 108 || size == 124; +} + +bool valid_encoding(const Header & header) +{ + if (header.core) + { + return header.compression == BI_RGB && + (header.bits_per_pixel == 1 || header.bits_per_pixel == 4 || + header.bits_per_pixel == 8 || header.bits_per_pixel == 24); + } + if (header.compression == BI_RGB) + { + return header.bits_per_pixel == 1 || header.bits_per_pixel == 4 || + header.bits_per_pixel == 8 || header.bits_per_pixel == 16 || + header.bits_per_pixel == 24 || header.bits_per_pixel == 32; + } + if (header.compression == BI_RLE8) + { + return header.bits_per_pixel == 8 && !header.top_down; + } + if (header.compression == BI_RLE4) + { + return header.bits_per_pixel == 4 && !header.top_down; + } + return header.compression == BI_BITFIELDS && + (header.bits_per_pixel == 16 || header.bits_per_pixel == 32); +} + +bool has_nonzero_bytes(const std::vector & bytes, + std::size_t offset, std::size_t length) { - std::ifstream file(filename.c_str(), std::ios::in | std::ios::binary); - lossy = false; - //clear data if already holds information - for(int i=0; i & bytes, Header & header) +{ + if (!range_fits(0, 18, bytes.size()) || bytes[0] != 'B' || bytes[1] != 'M' || + !read_u32(bytes, 10, header.pixel_offset) || + !read_u32(bytes, 14, header.dib_size) || + !is_known_dib_size(header.dib_size)) + { + return false; + } + + std::uint16_t planes = 0; + if (header.dib_size == 12) + { + std::uint16_t width = 0; + std::uint16_t height = 0; + if (!range_fits(14, 12, bytes.size()) || + !read_u16(bytes, 18, width) || !read_u16(bytes, 20, height) || + !read_u16(bytes, 22, planes) || + !read_u16(bytes, 24, header.bits_per_pixel) || + width == 0 || height == 0) + { + return false; + } + header.core = true; + header.width = width; + header.signed_height = height; + header.height = height; + header.palette_offset = 26; + } + else + { + if (!range_fits(14, header.dib_size, bytes.size()) || + !read_s32(bytes, 18, header.width) || + !read_s32(bytes, 22, header.signed_height) || + !read_u16(bytes, 26, planes) || + !read_u16(bytes, 28, header.bits_per_pixel) || + !read_u32(bytes, 30, header.compression) || + !read_u32(bytes, 34, header.image_size) || + !read_u32(bytes, 46, header.colors_used) || + header.width <= 0 || header.signed_height == 0 || + header.signed_height == std::numeric_limits::min()) + { + return false; + } + header.top_down = header.signed_height < 0; + header.height = static_cast(header.top_down + ? -header.signed_height : header.signed_height); + header.palette_offset = 14 + header.dib_size; + + if (header.compression == BI_BITFIELDS) + { + if (header.dib_size == 40) + { + if (!read_u32(bytes, header.palette_offset, header.red_mask) || + !read_u32(bytes, header.palette_offset + 4, header.green_mask) || + !read_u32(bytes, header.palette_offset + 8, header.blue_mask)) + { + return false; + } + header.palette_offset += 12; + } + else if (!read_u32(bytes, 54, header.red_mask) || + !read_u32(bytes, 58, header.green_mask) || + !read_u32(bytes, 62, header.blue_mask)) + { + return false; + } + if (header.dib_size >= 56 && !read_u32(bytes, 66, header.alpha_mask)) + { + return false; + } + } + + if (header.dib_size >= 108) + { + std::uint32_t color_space = 0; + if (!read_u32(bytes, 70, color_space)) + { + return false; + } + const bool calibrated_values = has_nonzero_bytes(bytes, 74, 48); + header.color_metadata_lost = calibrated_values || + (color_space != 0 && color_space != LCS_SRGB); + if (header.dib_size == 124) + { + std::uint32_t profile_size = 0; + if (!read_u32(bytes, 130, profile_size)) + { + return false; + } + header.color_metadata_lost = header.color_metadata_lost || + profile_size != 0; + } + } + } + + const std::uint64_t pixel_count = + static_cast(header.width) * header.height; + return planes == 1 && pixel_count <= MAX_DECODED_PIXELS && + valid_encoding(header) && header.palette_offset <= header.pixel_offset && + header.pixel_offset <= bytes.size(); +} + +bool read_palette(const std::vector & bytes, + const Header & header, std::vector & palette) +{ + if (header.bits_per_pixel > 8) + { + return true; + } + const std::uint32_t maximum = 1U << header.bits_per_pixel; + const std::uint32_t count = header.core || header.colors_used == 0 + ? maximum : header.colors_used; + if (count == 0 || count > maximum) + { + return false; + } + const std::size_t entry_size = header.core ? 3 : 4; + const std::uint64_t palette_bytes = + static_cast(count) * entry_size; + if (palette_bytes > header.pixel_offset - header.palette_offset || + !range_fits(header.palette_offset, static_cast(palette_bytes), + bytes.size())) + { + return false; + } + palette.reserve(count); + for (std::uint32_t index = 0; index < count; ++index) + { + const std::size_t offset = header.palette_offset + index * entry_size; + palette.push_back(Pixel(bytes[offset + 2], bytes[offset + 1], + bytes[offset])); + } + return true; +} + +bool checked_stride(const Header & header, std::size_t & stride) +{ + const std::uint64_t row_bits = + static_cast(header.width) * header.bits_per_pixel; + const std::uint64_t row_bytes = ((row_bits + 31) / 32) * 4; + if (row_bytes > std::numeric_limits::max()) + { + return false; + } + stride = static_cast(row_bytes); + return true; +} + +bool describe_mask(std::uint32_t mask, unsigned int stored_bits, + MaskInfo & info) +{ + if (mask == 0 || (stored_bits < 32 && (mask >> stored_bits) != 0)) + { + return false; + } + unsigned int shift = 0; + while (((mask >> shift) & 1U) == 0U) + { + ++shift; + } + unsigned int bits = 0; + std::uint32_t shifted = mask >> shift; + while ((shifted & 1U) != 0U) + { + ++bits; + shifted >>= 1; + } + if (shifted != 0 || bits == 0) + { + return false; + } + info.mask = mask; + info.shift = shift; + info.bits = bits; + info.maximum = bits == 32 ? 0xffffffffULL : ((1ULL << bits) - 1); + return true; +} + +int decode_component(std::uint32_t value, const MaskInfo & info) +{ + const std::uint64_t component = (value & info.mask) >> info.shift; + return static_cast((component * 255 + info.maximum / 2) / + info.maximum); +} + +bool decode_uncompressed(const std::vector & bytes, + const Header & header, + const std::vector & palette, + PixelMatrix & pixels, bool & lossy) +{ + std::size_t stride = 0; + if (!checked_stride(header, stride)) + { + return false; + } + const std::uint64_t data_size = + static_cast(stride) * header.height; + if (data_size > bytes.size() - header.pixel_offset) + { + return false; + } + + MaskInfo red = {0, 0, 0, 0}; + MaskInfo green = {0, 0, 0, 0}; + MaskInfo blue = {0, 0, 0, 0}; + MaskInfo alpha = {0, 0, 0, 0}; + const bool bitfields = header.compression == BI_BITFIELDS; + const bool has_alpha = bitfields && header.alpha_mask != 0; + if (bitfields) + { + if ((header.red_mask & header.green_mask) != 0 || + (header.red_mask & header.blue_mask) != 0 || + (header.green_mask & header.blue_mask) != 0 || + !describe_mask(header.red_mask, header.bits_per_pixel, red) || + !describe_mask(header.green_mask, header.bits_per_pixel, green) || + !describe_mask(header.blue_mask, header.bits_per_pixel, blue) || + (has_alpha && ((header.alpha_mask & (header.red_mask | + header.green_mask | header.blue_mask)) != 0 || + !describe_mask(header.alpha_mask, header.bits_per_pixel, alpha)))) + { + return false; + } + lossy = lossy || red.bits > 8 || green.bits > 8 || blue.bits > 8; + } + + pixels.assign(header.height, + std::vector(static_cast(header.width))); + for (std::uint32_t stored_row = 0; stored_row < header.height; ++stored_row) + { + const std::size_t row_offset = header.pixel_offset + + static_cast(stored_row) * stride; + const std::uint32_t target_row = header.top_down + ? stored_row : header.height - 1 - stored_row; + for (std::int32_t column = 0; column < header.width; ++column) + { + Pixel pixel; + if (header.bits_per_pixel == 1 || header.bits_per_pixel == 4 || + header.bits_per_pixel == 8) + { + std::uint32_t palette_index = 0; + if (header.bits_per_pixel == 1) + { + palette_index = (bytes[row_offset + column / 8] >> + (7 - (column % 8))) & 1U; + } + else if (header.bits_per_pixel == 4) + { + const unsigned char packed = bytes[row_offset + column / 2]; + palette_index = column % 2 == 0 ? packed >> 4 : packed & 0xf; + } + else + { + palette_index = bytes[row_offset + column]; + } + if (palette_index >= palette.size()) + { + return false; + } + pixel = palette[palette_index]; + } + else if (header.bits_per_pixel == 16) + { + const std::size_t offset = row_offset + column * 2; + const std::uint32_t value = bytes[offset] | + (static_cast(bytes[offset + 1]) << 8); + if (bitfields) + { + pixel = Pixel(decode_component(value, red), + decode_component(value, green), + decode_component(value, blue)); + } + else + { + pixel = Pixel(static_cast(((value >> 10) & 0x1f) * 255 / 31), + static_cast(((value >> 5) & 0x1f) * 255 / 31), + static_cast((value & 0x1f) * 255 / 31)); + } + } + else if (header.bits_per_pixel == 24) + { + const std::size_t offset = row_offset + column * 3; + pixel = Pixel(bytes[offset + 2], bytes[offset + 1], bytes[offset]); + } + else + { + const std::size_t offset = row_offset + column * 4; + const std::uint32_t value = bytes[offset] | + (static_cast(bytes[offset + 1]) << 8) | + (static_cast(bytes[offset + 2]) << 16) | + (static_cast(bytes[offset + 3]) << 24); + if (bitfields) + { + pixel = Pixel(decode_component(value, red), + decode_component(value, green), + decode_component(value, blue)); + if (has_alpha && ((value & alpha.mask) >> alpha.shift) != + alpha.maximum) + { + lossy = true; + } + } + else + { + pixel = Pixel(bytes[offset + 2], bytes[offset + 1], + bytes[offset]); + } + } + pixels[target_row][column] = pixel; + } + } + return true; +} + +bool decode_rle(const std::vector & bytes, + const Header & header, const std::vector & palette, + PixelMatrix & pixels) +{ + std::size_t end = bytes.size(); + if (header.image_size != 0) + { + if (header.image_size > bytes.size() - header.pixel_offset) + { + return false; + } + end = header.pixel_offset + header.image_size; + } + std::vector > indexes( + header.height, std::vector(header.width, 0)); + std::size_t position = header.pixel_offset; + std::uint32_t x = 0; + std::uint32_t y = 0; + bool complete = false; + + while (!complete && range_fits(position, 2, end)) + { + const unsigned int count = bytes[position++]; + const unsigned int value = bytes[position++]; + if (count != 0) + { + if (y >= header.height || count > static_cast(header.width) - x) + { + return false; + } + for (unsigned int index = 0; index < count; ++index) + { + const unsigned int palette_index = header.compression == BI_RLE8 + ? value : (index % 2 == 0 ? value >> 4 : value & 0xf); + if (palette_index >= palette.size()) + { + return false; + } + indexes[y][x++] = static_cast(palette_index); + } + } + else if (value == 0) + { + x = 0; + ++y; + if (y > header.height) + { + return false; + } + } + else if (value == 1) + { + complete = true; + } + else if (value == 2) + { + if (!range_fits(position, 2, end)) + { + return false; + } + const std::uint32_t dx = bytes[position++]; + const std::uint32_t dy = bytes[position++]; + if (y >= header.height || dx > static_cast(header.width) - x || + dy >= header.height - y) + { + return false; + } + x += dx; + y += dy; + } + else + { + const unsigned int literal_count = value; + const unsigned int data_bytes = header.compression == BI_RLE8 + ? literal_count : (literal_count + 1) / 2; + const unsigned int padded_bytes = data_bytes + (data_bytes % 2); + if (y >= header.height || + literal_count > static_cast(header.width) - x || + !range_fits(position, padded_bytes, end)) + { + return false; + } + for (unsigned int index = 0; index < literal_count; ++index) + { + const unsigned int palette_index = header.compression == BI_RLE8 + ? bytes[position + index] + : (index % 2 == 0 ? bytes[position + index / 2] >> 4 + : bytes[position + index / 2] & 0xf); + if (palette_index >= palette.size()) + { + return false; + } + indexes[y][x++] = static_cast(palette_index); + } + position += padded_bytes; + } + } + if (!complete) + { + return false; + } + + pixels.assign(header.height, std::vector(header.width)); + for (std::uint32_t stored_row = 0; stored_row < header.height; ++stored_row) + { + const std::uint32_t target_row = header.height - 1 - stored_row; + for (std::int32_t column = 0; column < header.width; ++column) + { + const std::uint16_t palette_index = indexes[stored_row][column]; + if (palette_index >= palette.size()) + { + return false; + } + pixels[target_row][column] = palette[palette_index]; + } + } + return true; +} + +bool decode_bitmap(const std::vector & bytes, + PixelMatrix & pixels, bool & lossy) +{ + Header header; + std::vector palette; + if (!parse_header(bytes, header) || !read_palette(bytes, header, palette)) + { + return false; + } + lossy = header.color_metadata_lost; + try + { + if (header.compression == BI_RLE4 || header.compression == BI_RLE8) + { + return decode_rle(bytes, header, palette, pixels); + } + return decode_uncompressed(bytes, header, palette, pixels, lossy); + } + catch (const std::bad_alloc &) + { pixels.clear(); + lossy = false; + return false; + } +} + +void write_u16(std::ostream & output, std::uint16_t value) +{ + output.put(static_cast(value & 0xff)); + output.put(static_cast((value >> 8) & 0xff)); +} + +void write_u32(std::ostream & output, std::uint32_t value) +{ + output.put(static_cast(value & 0xff)); + output.put(static_cast((value >> 8) & 0xff)); + output.put(static_cast((value >> 16) & 0xff)); + output.put(static_cast((value >> 24) & 0xff)); +} +} - if (file.fail()) - { - std::cerr< row_data; - - for (int col = 0; col < dib_info.width; col++) - { - int blue = file.get(); - int green = file.get(); - int red = file.get(); - - row_data.push_back( Pixel(red, green, blue) ); - } - - // Rows are padded so that they're always a multiple of 4 - // bytes. This line skips the padding at the end of each row. - file.seekg(dib_info.width % 4, std::ios::cur); - - if (flip) - { - pixels.insert(pixels.begin(), row_data); - } - else - { - pixels.push_back(row_data); - } - } - - file.close(); - }//end else (is an image) - }//end else (can open file) +void Bitmap::open(std::string filename) +{ + pixels.clear(); + lossy = false; + + std::vector bytes; + PixelMatrix decoded_pixels; + bool decoded_lossy = false; + if (!load_file(filename, bytes)) + { + std::cerr << filename << " could not be opened.\n"; + } + else if (!decode_bitmap(bytes, decoded_pixels, decoded_lossy)) + { + std::cerr << filename << " is not a supported, valid BMP file.\n"; + } + else + { + pixels.swap(decoded_pixels); + lossy = decoded_lossy; + } } -// ---------------------------------------------------------------------------- -/** - * Saves the current image, represented by the matrix of pixels, as a - * Windows BMP file with the name provided by the parameter. File extension - * is not forced but should be .bmp. Any errors will print to cerr and will NOT - * attempt to save the file. - * - * @param name of the filename to be written as a bmp image -**/ void Bitmap::save(std::string filename) { - std::ofstream file(filename.c_str(), std::ios::out | std::ios::binary); - - if (file.fail()) - { - std::cerr<= 0; row--) - { - const std::vector & row_data = pixels[row]; - - for (int col = 0; col < row_data.size(); col++) - { - const Pixel& pix = row_data[col]; - - file.put((uchar_t)(pix.blue)); - file.put((uchar_t)(pix.green)); - file.put((uchar_t)(pix.red)); - } - - // Rows are padded so that they're always a multiple of 4 - // bytes. This line skips the padding at the end of each row. - for (int i = 0; i < row_data.size() % 4; i++) - { - file.put(0); - } - } - - file.close(); - } + if (!isImage()) + { + std::cerr << "Bitmap cannot be saved. It is not a valid image.\n"; + return; + } + + const std::uint64_t width = pixels[0].size(); + const std::uint64_t height = pixels.size(); + const std::uint64_t stride = ((width * 24 + 31) / 32) * 4; + const std::uint64_t image_size = stride * height; + const std::uint64_t file_size = 54 + image_size; + if (width > static_cast(std::numeric_limits::max()) || + height > static_cast(std::numeric_limits::max()) || + image_size > std::numeric_limits::max() || + file_size > std::numeric_limits::max()) + { + std::cerr << "Bitmap cannot be saved because it is too large.\n"; + return; + } + + std::ofstream file(filename.c_str(), std::ios::out | std::ios::binary); + if (!file) + { + std::cerr << filename << " could not be opened for editing.\n"; + return; + } + + file.put('B'); + file.put('M'); + write_u32(file, static_cast(file_size)); + write_u16(file, 0); + write_u16(file, 0); + write_u32(file, 54); + write_u32(file, 40); + write_u32(file, static_cast(width)); + write_u32(file, static_cast(height)); + write_u16(file, 1); + write_u16(file, 24); + write_u32(file, BI_RGB); + write_u32(file, static_cast(image_size)); + write_u32(file, 2835); + write_u32(file, 2835); + write_u32(file, 0); + write_u32(file, 0); + + const std::size_t padding = static_cast(stride - width * 3); + for (std::size_t stored_row = 0; stored_row < height; ++stored_row) + { + const std::vector & row = pixels[height - 1 - stored_row]; + for (std::size_t column = 0; column < row.size(); ++column) + { + file.put(static_cast(row[column].blue)); + file.put(static_cast(row[column].green)); + file.put(static_cast(row[column].red)); + } + for (std::size_t index = 0; index < padding; ++index) + { + file.put(0); + } + } + if (!file) + { + std::cerr << filename << " could not be written completely.\n"; + } } -// ---------------------------------------------------------------------------- -/** - * Validates whether or not the current matrix of pixels represents a - * proper image with non-zero-size rows and consistent non-zero-size - * columns for each row. In addition, each pixel in the matrix is validated - * to have red, green, and blue components with values between 0 and 255 - * - * @return boolean value of whether or not the matrix is a valid image - **/ bool Bitmap::isImage() { - const int height = pixels.size(); - bool valid = true; - - if( height == 0 ) - { - valid = false; - } - else - { - const int width = pixels[0].size(); - if( width == 0 ) - { - valid = false; - } - - for(int row=0; valid && row < height; row++) - { - if( static_cast(pixels[row].size()) != width ) - { - valid = false; - } - for(int column=0; valid && column < width; column++) - { - Pixel current = pixels[row][column]; - if( current.red > MAX_RGB || current.red < MIN_RGB || - current.green > MAX_RGB || current.green < MIN_RGB || - current.blue > MAX_RGB || current.blue < MIN_RGB ) - { - valid = false; - } - } - } - } - return valid; + if (pixels.empty() || pixels[0].empty()) + { + return false; + } + const std::size_t width = pixels[0].size(); + for (std::size_t row = 0; row < pixels.size(); ++row) + { + if (pixels[row].size() != width) + { + return false; + } + for (std::size_t column = 0; column < width; ++column) + { + const Pixel & current = pixels[row][column]; + if (current.red > MAX_RGB || current.red < MIN_RGB || + current.green > MAX_RGB || current.green < MIN_RGB || + current.blue > MAX_RGB || current.blue < MIN_RGB) + { + return false; + } + } + } + return true; } -// ---------------------------------------------------------------------------- bool Bitmap::isLossy() { - return lossy; + return lossy; } -// ---------------------------------------------------------------------------- -/** - * Provides a vector of vector of pixels representing the bitmap - * - * @return the bitmap image, represented by a matrix of RGB pixels -**/ PixelMatrix Bitmap::toPixelMatrix() { - PixelMatrix converted; - if( isImage() ) - { - converted = pixels; - } - - return converted; + return isImage() ? pixels : PixelMatrix(); } -// ---------------------------------------------------------------------------- -/** - * Overwrites the current bitmap with that represented by a matrix of - * pixels. Does not validate that the new matrix of pixels is a proper - * image. - * - * @param a matrix of pixels to represent a bitmap -**/ void Bitmap::fromPixelMatrix(const PixelMatrix & values) { - pixels = values; - lossy = false; + pixels = values; + lossy = false; } diff --git a/tests/bitmap_tests.cpp b/tests/bitmap_tests.cpp index 7080fc3..4954cd9 100644 --- a/tests/bitmap_tests.cpp +++ b/tests/bitmap_tests.cpp @@ -35,12 +35,24 @@ void append_u32(std::vector & bytes, unsigned long value) bytes.push_back(static_cast((value >> 24) & 0xff)); } +void set_u32(std::vector & bytes, std::size_t offset, + unsigned long value) +{ + bytes[offset] = static_cast(value & 0xff); + bytes[offset + 1] = static_cast((value >> 8) & 0xff); + bytes[offset + 2] = static_cast((value >> 16) & 0xff); + bytes[offset + 3] = static_cast((value >> 24) & 0xff); +} + std::string write_fixture(const std::vector & bytes) { const std::string path = "/tmp/bitmap-test-" + std::to_string(fixture_number++) + ".bmp"; std::ofstream output(path.c_str(), std::ios::binary); - output.write(reinterpret_cast(&bytes[0]), bytes.size()); + if (!bytes.empty()) + { + output.write(reinterpret_cast(&bytes[0]), bytes.size()); + } return path; } @@ -75,6 +87,121 @@ std::vector make_24_bpp_fixture() return bytes; } +std::vector make_info_fixture( + int width, int height, unsigned int bits_per_pixel, + unsigned long compression, const std::vector & palette, + const std::vector & masks, + const std::vector & pixel_bytes, + unsigned long dib_size = 40, bool meaningful_color_metadata = false) +{ + const bool appended_masks = compression == 3 && dib_size == 40; + const unsigned long mask_bytes = appended_masks ? 12 : 0; + const unsigned long pixel_offset = 14 + dib_size + mask_bytes + + static_cast(palette.size() * 4); + std::vector bytes; + bytes.push_back('B'); + bytes.push_back('M'); + append_u32(bytes, pixel_offset + pixel_bytes.size()); + append_u16(bytes, 0); + append_u16(bytes, 0); + append_u32(bytes, pixel_offset); + append_u32(bytes, dib_size); + append_u32(bytes, static_cast(width)); + append_u32(bytes, static_cast(height)); + append_u16(bytes, 1); + append_u16(bytes, bits_per_pixel); + append_u32(bytes, compression); + append_u32(bytes, pixel_bytes.size()); + append_u32(bytes, 0); + append_u32(bytes, 0); + append_u32(bytes, palette.size()); + append_u32(bytes, 0); + bytes.resize(14 + dib_size, 0); + + if (dib_size >= 52 && masks.size() >= 3) + { + set_u32(bytes, 14 + 40, masks[0]); + set_u32(bytes, 14 + 44, masks[1]); + set_u32(bytes, 14 + 48, masks[2]); + } + if (dib_size >= 56 && masks.size() >= 4) + { + set_u32(bytes, 14 + 52, masks[3]); + } + if (meaningful_color_metadata && dib_size >= 108) + { + set_u32(bytes, 14 + 56, 0x73524742UL); + set_u32(bytes, 14 + 96, 1000); + } + if (appended_masks) + { + append_u32(bytes, masks[0]); + append_u32(bytes, masks[1]); + append_u32(bytes, masks[2]); + } + for (std::size_t i = 0; i < palette.size(); ++i) + { + bytes.push_back(static_cast(palette[i].blue)); + bytes.push_back(static_cast(palette[i].green)); + bytes.push_back(static_cast(palette[i].red)); + bytes.push_back(0); + } + bytes.insert(bytes.end(), pixel_bytes.begin(), pixel_bytes.end()); + return bytes; +} + +std::vector make_core_fixture( + unsigned int width, unsigned int height, unsigned int bits_per_pixel, + const std::vector & palette, + const std::vector & pixel_bytes) +{ + const unsigned long pixel_offset = 26 + + static_cast(palette.size() * 3); + std::vector bytes; + bytes.push_back('B'); + bytes.push_back('M'); + append_u32(bytes, pixel_offset + pixel_bytes.size()); + append_u16(bytes, 0); + append_u16(bytes, 0); + append_u32(bytes, pixel_offset); + append_u32(bytes, 12); + append_u16(bytes, width); + append_u16(bytes, height); + append_u16(bytes, 1); + append_u16(bytes, bits_per_pixel); + for (std::size_t i = 0; i < palette.size(); ++i) + { + bytes.push_back(static_cast(palette[i].blue)); + bytes.push_back(static_cast(palette[i].green)); + bytes.push_back(static_cast(palette[i].red)); + } + bytes.insert(bytes.end(), pixel_bytes.begin(), pixel_bytes.end()); + return bytes; +} + +Pixel open_single_pixel(const std::vector & fixture, + bool expected_lossy, const std::string & context) +{ + const std::string path = write_fixture(fixture); + Bitmap bitmap; + bitmap.open(path); + const PixelMatrix pixels = bitmap.toPixelMatrix(); + expect(bitmap.isImage(), context + " opens"); + expect(bitmap.isLossy() == expected_lossy, context + " lossiness"); + expect(pixels.size() == 1 && pixels[0].size() == 1, + context + " dimensions"); + std::remove(path.c_str()); + return pixels.size() == 1 && pixels[0].size() == 1 + ? pixels[0][0] : Pixel(); +} + +void expect_color(const Pixel & pixel, int red, int green, int blue, + const std::string & context) +{ + expect(pixel.red == red && pixel.green == green && pixel.blue == blue, + context); +} + void test_public_state_contract() { Bitmap bitmap; @@ -111,12 +238,319 @@ void test_existing_24_bpp_behavior() } std::remove(path.c_str()); } + +void test_indexed_info_depths() +{ + std::vector two_colors; + two_colors.push_back(Pixel(0, 0, 0)); + two_colors.push_back(Pixel(12, 34, 56)); + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 1, 0, two_colors, std::vector(), + std::vector{0x80, 0, 0, 0}), false, "1-bpp INFO"), + 12, 34, 56, "1-bpp palette lookup"); + + std::vector sixteen_colors(16, Pixel()); + sixteen_colors[10] = Pixel(21, 43, 65); + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 4, 0, sixteen_colors, std::vector(), + std::vector{0xa0, 0, 0, 0}), false, "4-bpp INFO"), + 21, 43, 65, "4-bpp palette lookup"); + + std::vector colors(256, Pixel()); + colors[200] = Pixel(31, 63, 95); + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 8, 0, colors, std::vector(), + std::vector{200, 0, 0, 0}), false, "8-bpp INFO"), + 31, 63, 95, "8-bpp palette lookup"); +} + +void test_core_depths() +{ + std::vector palette2; + palette2.push_back(Pixel()); + palette2.push_back(Pixel(90, 80, 70)); + expect_color(open_single_pixel(make_core_fixture( + 1, 1, 1, palette2, std::vector{0x80, 0, 0, 0}), + false, "1-bpp CORE"), 90, 80, 70, "CORE RGBTRIPLE palette"); + + std::vector palette16(16, Pixel()); + palette16[3] = Pixel(3, 6, 9); + expect_color(open_single_pixel(make_core_fixture( + 1, 1, 4, palette16, std::vector{0x30, 0, 0, 0}), + false, "4-bpp CORE"), 3, 6, 9, "CORE 4-bpp decode"); + + std::vector palette256(256, Pixel()); + palette256[17] = Pixel(17, 34, 51); + expect_color(open_single_pixel(make_core_fixture( + 1, 1, 8, palette256, std::vector{17, 0, 0, 0}), + false, "8-bpp CORE"), 17, 34, 51, "CORE 8-bpp decode"); + + expect_color(open_single_pixel(make_core_fixture( + 1, 1, 24, std::vector(), + std::vector{7, 8, 9, 0}), false, "24-bpp CORE"), + 9, 8, 7, "CORE 24-bpp decode"); +} + +void test_direct_color_and_orientation() +{ + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 16, 0, std::vector(), std::vector(), + std::vector{0x00, 0x7c, 0, 0}), false, + "16-bpp RGB555"), 255, 0, 0, "RGB555 expansion"); + + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 32, 0, std::vector(), std::vector(), + std::vector{3, 2, 1, 255}), false, + "32-bpp BGRX"), 1, 2, 3, "reserved BGRX byte is ignored"); + + const std::vector rows = { + 30, 20, 10, 0, + 60, 50, 40, 0 + }; + const std::string path = write_fixture(make_info_fixture( + 1, -2, 24, 0, std::vector(), std::vector(), rows)); + Bitmap bitmap; + bitmap.open(path); + const PixelMatrix pixels = bitmap.toPixelMatrix(); + expect(pixels.size() == 2 && pixels[0][0].red == 10 && + pixels[1][0].red == 40, "negative height preserves top-down order"); + std::remove(path.c_str()); +} + +void test_bitfields_and_alpha_loss() +{ + const std::vector rgb565 = {0xf800, 0x07e0, 0x001f}; + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 16, 3, std::vector(), rgb565, + std::vector{0xe0, 0x07, 0, 0}), false, + "16-bpp RGB565"), 0, 255, 0, "RGB565 expansion"); + + const std::vector argb = { + 0x00ff0000, 0x0000ff00, 0x000000ff, 0xff000000 + }; + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 32, 3, std::vector(), argb, + std::vector{30, 20, 10, 255}, 56), false, + "opaque ARGB"), 10, 20, 30, "opaque alpha is lossless"); + + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 32, 3, std::vector(), argb, + std::vector{30, 20, 10, 64}, 56), true, + "transparent ARGB"), 10, 20, 30, + "transparent alpha preserves stored RGB"); + + const std::vector rgb101010 = { + 0x3ff00000, 0x000ffc00, 0x000003ff + }; + open_single_pixel(make_info_fixture( + 1, 1, 32, 3, std::vector(), rgb101010, + std::vector{0xff, 0x03, 0, 0}), true, + "10-bit bitfields"); +} + +void test_color_metadata_loss() +{ + open_single_pixel(make_info_fixture( + 1, 1, 24, 0, std::vector(), std::vector(), + std::vector{3, 2, 1, 0}, 108, false), false, + "default V4 metadata"); + open_single_pixel(make_info_fixture( + 1, 1, 24, 0, std::vector(), std::vector(), + std::vector{3, 2, 1, 0}, 108, true), true, + "meaningful V4 metadata"); +} + +void test_rle_imports() +{ + std::vector palette256(256, Pixel()); + palette256[7] = Pixel(70, 71, 72); + const std::vector rle8 = {1, 7, 0, 0, 0, 1}; + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 8, 1, palette256, std::vector(), rle8), + false, "RLE8"), 70, 71, 72, "RLE8 encoded run"); + + std::vector palette16(16, Pixel()); + palette16[10] = Pixel(100, 101, 102); + const std::vector rle4 = {1, 0xa0, 0, 0, 0, 1}; + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 4, 2, palette16, std::vector(), rle4), + false, "RLE4"), 100, 101, 102, "RLE4 encoded run"); + + std::vector palette(256, Pixel()); + palette[1] = Pixel(1, 2, 3); + palette[2] = Pixel(4, 5, 6); + palette[3] = Pixel(7, 8, 9); + const std::vector absolute = { + 0, 3, 1, 2, 3, 0, 0, 0, 0, 1 + }; + const std::string path = write_fixture(make_info_fixture( + 3, 1, 8, 1, palette, std::vector(), absolute)); + Bitmap bitmap; + bitmap.open(path); + const PixelMatrix pixels = bitmap.toPixelMatrix(); + expect(bitmap.isImage() && pixels[0].size() == 3, + "RLE8 absolute run opens"); + if (bitmap.isImage() && pixels[0].size() == 3) + { + expect(pixels[0][0].red == 1 && pixels[0][1].red == 4 && + pixels[0][2].red == 7, "RLE8 absolute indexes decode"); + } + std::remove(path.c_str()); +} + +void test_rle_commands_and_orientation() +{ + std::vector palette(256, Pixel()); + palette[1] = Pixel(10, 0, 0); + palette[2] = Pixel(20, 0, 0); + const std::vector commands = { + 0, 2, 1, 0, + 1, 2, + 0, 0, + 3, 1, + 0, 1 + }; + std::string path = write_fixture(make_info_fixture( + 3, 2, 8, 1, palette, std::vector(), commands)); + Bitmap bitmap; + bitmap.open(path); + PixelMatrix pixels = bitmap.toPixelMatrix(); + expect(bitmap.isImage() && pixels.size() == 2 && pixels[0].size() == 3, + "RLE8 delta and EOL commands open"); + if (bitmap.isImage() && pixels.size() == 2 && pixels[0].size() == 3) + { + expect(pixels[0][0].red == 10 && pixels[0][1].red == 10 && + pixels[0][2].red == 10, "RLE storage rows are bottom-up"); + expect(pixels[1][0].red == 0 && pixels[1][1].red == 20 && + pixels[1][2].red == 0, "RLE delta leaves palette index zero"); + } + std::remove(path.c_str()); + + std::vector palette16(16, Pixel()); + palette16[1] = Pixel(1, 0, 0); + palette16[2] = Pixel(2, 0, 0); + palette16[3] = Pixel(3, 0, 0); + const std::vector rle4_absolute = { + 0, 3, 0x12, 0x30, 0, 1 + }; + path = write_fixture(make_info_fixture( + 3, 1, 4, 2, palette16, std::vector(), rle4_absolute)); + bitmap.open(path); + pixels = bitmap.toPixelMatrix(); + expect(bitmap.isImage() && pixels[0][0].red == 1 && + pixels[0][1].red == 2 && pixels[0][2].red == 3, + "RLE4 absolute nibbles decode in high-low order"); + std::remove(path.c_str()); +} + +void test_save_round_trip_padding() +{ + for (int width = 1; width <= 4; ++width) + { + PixelMatrix source(2, std::vector(width)); + for (int row = 0; row < 2; ++row) + { + for (int column = 0; column < width; ++column) + { + source[row][column] = Pixel(10 + row, 20 + column, + 30 + row + column); + } + } + Bitmap saved; + saved.fromPixelMatrix(source); + const std::string path = "/tmp/bitmap-save-" + + std::to_string(width) + ".bmp"; + saved.save(path); + + Bitmap reopened; + reopened.open(path); + const PixelMatrix result = reopened.toPixelMatrix(); + expect(reopened.isImage() && !reopened.isLossy(), + "saved 24-bpp image reopens losslessly"); + bool matches = result.size() == source.size(); + for (std::size_t row = 0; matches && row < source.size(); ++row) + { + matches = result[row].size() == source[row].size(); + for (std::size_t column = 0; + matches && column < source[row].size(); ++column) + { + matches = result[row][column].red == source[row][column].red && + result[row][column].green == source[row][column].green && + result[row][column].blue == source[row][column].blue; + } + } + expect(matches, "save/reopen preserves every padding width"); + std::remove(path.c_str()); + } +} + +void test_malformed_inputs_fail_atomically() +{ + std::vector truncated = make_24_bpp_fixture(); + truncated.pop_back(); + truncated.pop_back(); + Bitmap bitmap; + std::string path = write_fixture(truncated); + bitmap.open(path); + expect(!bitmap.isImage() && !bitmap.isLossy(), + "truncated rows fail atomically"); + std::remove(path.c_str()); + + std::vector palette(256, Pixel()); + const std::vector crossing_run = {2, 1, 0, 1}; + path = write_fixture(make_info_fixture( + 1, 1, 8, 1, palette, std::vector(), crossing_run)); + bitmap.open(path); + expect(!bitmap.isImage(), "RLE runs cannot cross row bounds"); + std::remove(path.c_str()); + + std::vector bad_offset = make_24_bpp_fixture(); + set_u32(bad_offset, 10, 20); + path = write_fixture(bad_offset); + bitmap.open(path); + expect(!bitmap.isImage(), "pixel data cannot overlap its header"); + std::remove(path.c_str()); + + std::vector unknown_header = make_24_bpp_fixture(); + set_u32(unknown_header, 14, 41); + path = write_fixture(unknown_header); + bitmap.open(path); + expect(!bitmap.isImage(), "unknown DIB headers are rejected"); + std::remove(path.c_str()); + + const std::vector overlapping_masks = { + 0x00ff, 0x00f0, 0xff00 + }; + path = write_fixture(make_info_fixture( + 1, 1, 16, 3, std::vector(), overlapping_masks, + std::vector{0, 0, 0, 0})); + bitmap.open(path); + expect(!bitmap.isImage(), "overlapping bitfield masks are rejected"); + std::remove(path.c_str()); + + std::vector rle_palette(256, Pixel()); + const std::vector top_down_rle = {1, 0, 0, 1}; + path = write_fixture(make_info_fixture( + 1, -1, 8, 1, rle_palette, std::vector(), top_down_rle)); + bitmap.open(path); + expect(!bitmap.isImage(), "top-down RLE is rejected"); + std::remove(path.c_str()); +} } int main() { test_public_state_contract(); test_existing_24_bpp_behavior(); + test_indexed_info_depths(); + test_core_depths(); + test_direct_color_and_orientation(); + test_bitfields_and_alpha_loss(); + test_color_metadata_loss(); + test_rle_imports(); + test_rle_commands_and_orientation(); + test_save_round_trip_padding(); + test_malformed_inputs_fail_atomically(); if (failures != 0) { From a6d56b0c88f2d0792f7148bb551ccea1e0979519 Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Tue, 29 Sep 2026 16:12:32 -0700 Subject: [PATCH 3/5] docs: define BMP compatibility and safety limits --- README.md | 88 +++++++++++-- bitmap.cpp | 87 +++++++++---- bitmap.h | 19 +-- specs/issue-1-bmp-import.md | 249 ++++++++++++++++++++++++++++++++++++ tests/bitmap_tests.cpp | 40 ++++++ 5 files changed, 440 insertions(+), 43 deletions(-) create mode 100644 specs/issue-1-bmp-import.md diff --git a/README.md b/README.md index 5c8e533..19a2629 100644 --- a/README.md +++ b/README.md @@ -1,13 +1,13 @@ # C++ Bitmap header files -This repo is compatible with the [cpp-container](https://github.com/ChicoState/cpp-container) Docker container. +This repo is compatible with the [cpp-container](https://github.com/ChicoState/cpp-container) Docker container and requires C++11 or newer. ## Getting Started 1. Clone this repository onto your development environment 2. Copy `bitmap.h` and `bitmap.cpp` to your project directory 3. In your C++ project, include the header file with -`#include "bitmap.h"` and include bitmap in your compilation +`#include "bitmap.h"` and include `bitmap.cpp` in your compilation 4. Declare your variables of type *Bitmap* or *Pixel*. See the guides for the Bitmap and Pixel data types below. @@ -38,9 +38,65 @@ purpleDot.blue = 255; ## Bitmap -Represents a bitmap where a grid of pixels (in row-major order) -describes the color of each pixel within the image. Limited to Windows BMP -formatted images with no compression and 24 bit color depth. +Represents a bitmap where a grid of pixels in row-major order describes the +color of each pixel. Supported BMP inputs are normalized to 8-bit red, green, +and blue components. Saving always produces an uncompressed 24-bpp BMP. + +### Input compatibility + +| DIB header | Encoding | Supported depths | +|---|---|---| +| 12-byte `BITMAPCOREHEADER` | Uncompressed indexed/direct RGB | 1, 4, 8, 24 bpp | +| 40-byte `BITMAPINFOHEADER` and 52/56/108/124-byte extensions | `BI_RGB` | 1, 4, 8, 16, 24, 32 bpp | +| 40/52/56/108/124-byte headers | `BI_BITFIELDS` | 16, 32 bpp | +| 40/52/56/108/124-byte headers | `BI_RLE4` | 4 bpp | +| 40/52/56/108/124-byte headers | `BI_RLE8` | 8 bpp | + +The decoder supports bottom-up images and top-down uncompressed/bitfield +images. Microsoft defines RLE-compressed BMPs as bottom-up, so top-down RLE is +rejected. Palettes, bitfields, scanline padding, and row orientation are +converted internally; the returned matrix is always top-to-bottom RGB. + +### Lossiness rules + +`isLossy()` reports whether the most recent successful `open()` discarded +color information or precision while creating the RGB-only matrix. It does +not refer to image width, height, or pixel density; dimensions are not +resampled. + +| Import behavior | `isLossy()` | +|---|---:| +| Palette expansion, RLE expansion, row reordering, or padding removal | `false` | +| Expanding 5-bit or 6-bit RGB components to 8-bit components | `false` | +| Ignoring the undefined byte in 32-bpp `BI_RGB` BGRX data | `false` | +| Discarding a declared alpha channel whose pixels are all opaque | `false` | +| Discarding a declared alpha channel with any non-opaque pixel | `true` | +| Reducing an RGB bitfield component wider than 8 bits | `true` | +| Preserving numeric RGB values without applying meaningful V4/V5 color-profile or gamma metadata | `true` | + +When alpha is discarded, the stored red, green, and blue values are preserved +unchanged. The library does not composite pixels against an assumed +background. + +For a new `Bitmap`, after a failed `open()`, or after `fromPixelMatrix()`, +`isLossy()` returns `false`. Calling `save()` does not alter the value. + +### Limitations + +- Embedded `BI_JPEG` and `BI_PNG` payloads are not decoded. +- CMYK BMP encodings and later OS/2 2.x-specific headers/compression are not supported. +- Unknown DIB header sizes are rejected rather than guessed. +- ICC/profile and gamma transforms are not applied; numeric RGB values are retained and the import is marked lossy when meaningful metadata is present. +- Alpha is not retained in `PixelMatrix` or saved output. +- Input files larger than 512 MiB are rejected before allocation. +- Inputs whose decoded dimensions exceed 100 million pixels are rejected as a memory-safety limit. +- `save()` writes only bottom-up, uncompressed, 24-bpp `BI_RGB` files. + +The decoder behavior follows Microsoft's documentation for +[bitmap headers](https://learn.microsoft.com/en-us/windows/win32/gdi/bitmap-header-types), +[bitmap storage](https://learn.microsoft.com/en-us/windows/win32/gdi/bitmap-storage), +[`BITMAPINFOHEADER`](https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapinfoheader), +and [RLE compression](https://learn.microsoft.com/en-us/windows/win32/gdi/bitmap-compression). ### Functions @@ -48,9 +104,8 @@ formatted images with no compression and 24 bit color depth. `void open(std::string)` -*Opens a file as its name is provided and reads pixel-by-pixel the colors -into a matrix of RGB pixels. Any errors will cout but will result in an -empty matrix (with no rows and no columns).* +*Opens a supported BMP and converts it to a matrix of RGB pixels. Any error is +written to `std::cerr` and leaves an empty matrix.* *parameter: name of the filename to be opened and read as a matrix of pixels* @@ -58,10 +113,9 @@ empty matrix (with no rows and no columns).* `void save(std::string)` -*Saves the current image, represented by the matrix of pixels, as a -Windows BMP file with the name provided by the parameter. File extension -is not forced but should be .bmp. Any errors will cout and will NOT -attempt to save the file.* +*Saves the current matrix as an uncompressed 24-bpp Windows BMP. The file +extension is not forced but should be `.bmp`. Errors are written to +`std::cerr`.* #### isImage @@ -74,6 +128,14 @@ to have red, green, and blue components with values between 0 and 255* *return: boolean value of whether or not the matrix is a valid image* +#### isLossy + +`bool isLossy()` + +*Returns whether the most recent successful `open()` discarded color +information or precision while converting the input to RGB pixels. See the +lossiness table above for exact behavior.* + #### toPixelMatrix `std::vector > toPixelMatrix()` @@ -112,6 +174,7 @@ int main() //verify that the file opened was a valid image bool validBmp = image.isImage(); + bool lostColorInformation = image.isLossy(); if( validBmp == true ) { @@ -127,6 +190,7 @@ int main() image.fromPixelMatrix(bmp); image.save("example.bmp"); } + (void)lostColorInformation; return 0; } ``` diff --git a/bitmap.cpp b/bitmap.cpp index b9c3807..42fc009 100644 --- a/bitmap.cpp +++ b/bitmap.cpp @@ -16,6 +16,7 @@ const std::uint32_t BI_RLE8 = 1; const std::uint32_t BI_RLE4 = 2; const std::uint32_t BI_BITFIELDS = 3; const std::uint32_t LCS_SRGB = 0x73524742; +const std::uint64_t MAX_INPUT_BYTES = 512ULL * 1024 * 1024; const std::uint64_t MAX_DECODED_PIXELS = 100000000; struct Header @@ -33,6 +34,8 @@ struct Header std::uint32_t green_mask; std::uint32_t blue_mask; std::uint32_t alpha_mask; + std::uint32_t profile_offset; + std::uint32_t profile_size; std::size_t palette_offset; bool core; bool top_down; @@ -42,8 +45,9 @@ struct Header : dib_size(0), width(0), signed_height(0), height(0), bits_per_pixel(0), compression(BI_RGB), image_size(0), colors_used(0), pixel_offset(0), red_mask(0), green_mask(0), - blue_mask(0), alpha_mask(0), palette_offset(0), core(false), - top_down(false), color_metadata_lost(false) + blue_mask(0), alpha_mask(0), profile_offset(0), profile_size(0), + palette_offset(0), core(false), top_down(false), + color_metadata_lost(false) { } }; @@ -108,18 +112,27 @@ bool load_file(const std::string & filename, std::vector & bytes) } file.seekg(0, std::ios::end); const std::streamoff length = file.tellg(); - if (length < 0 || static_cast(length) > + if (length < 0 || static_cast(length) > MAX_INPUT_BYTES || + static_cast(length) > std::numeric_limits::max()) { return false; } file.seekg(0, std::ios::beg); - bytes.resize(static_cast(length)); + try + { + bytes.resize(static_cast(length)); + } + catch (const std::bad_alloc &) + { + return false; + } if (!bytes.empty()) { file.read(reinterpret_cast(&bytes[0]), bytes.size()); + return static_cast(file.gcount()) == bytes.size(); } - return file.good() || file.eof(); + return true; } bool is_known_dib_size(std::uint32_t size) @@ -256,13 +269,23 @@ bool parse_header(const std::vector & bytes, Header & header) (color_space != 0 && color_space != LCS_SRGB); if (header.dib_size == 124) { - std::uint32_t profile_size = 0; - if (!read_u32(bytes, 130, profile_size)) + std::uint32_t profile_data = 0; + if (!read_u32(bytes, 126, profile_data) || + !read_u32(bytes, 130, header.profile_size)) { return false; } + const std::uint64_t profile_offset = 14ULL + profile_data; + if (header.profile_size != 0 && + (profile_offset < header.pixel_offset || + profile_offset > bytes.size() || + header.profile_size > bytes.size() - profile_offset)) + { + return false; + } + header.profile_offset = static_cast(profile_offset); header.color_metadata_lost = header.color_metadata_lost || - profile_size != 0; + header.profile_size != 0; } } } @@ -373,6 +396,11 @@ bool decode_uncompressed(const std::vector & bytes, { return false; } + if (header.profile_size != 0 && header.profile_offset < + header.pixel_offset + data_size) + { + return false; + } MaskInfo red = {0, 0, 0, 0}; MaskInfo green = {0, 0, 0, 0}; @@ -445,9 +473,9 @@ bool decode_uncompressed(const std::vector & bytes, } else { - pixel = Pixel(static_cast(((value >> 10) & 0x1f) * 255 / 31), - static_cast(((value >> 5) & 0x1f) * 255 / 31), - static_cast((value & 0x1f) * 255 / 31)); + pixel = Pixel(static_cast((((value >> 10) & 0x1f) * 255 + 15) / 31), + static_cast((((value >> 5) & 0x1f) * 255 + 15) / 31), + static_cast(((value & 0x1f) * 255 + 15) / 31)); } } else if (header.bits_per_pixel == 24) @@ -490,13 +518,23 @@ bool decode_rle(const std::vector & bytes, PixelMatrix & pixels) { std::size_t end = bytes.size(); + if (header.profile_size != 0) + { + end = header.profile_offset; + } if (header.image_size != 0) { if (header.image_size > bytes.size() - header.pixel_offset) { return false; } - end = header.pixel_offset + header.image_size; + const std::size_t image_end = + static_cast(header.pixel_offset) + header.image_size; + if (image_end > end) + { + return false; + } + end = image_end; } std::vector > indexes( header.height, std::vector(header.width, 0)); @@ -607,15 +645,16 @@ bool decode_rle(const std::vector & bytes, bool decode_bitmap(const std::vector & bytes, PixelMatrix & pixels, bool & lossy) { - Header header; - std::vector palette; - if (!parse_header(bytes, header) || !read_palette(bytes, header, palette)) - { - return false; - } - lossy = header.color_metadata_lost; try { + Header header; + std::vector palette; + if (!parse_header(bytes, header) || + !read_palette(bytes, header, palette)) + { + return false; + } + lossy = header.color_metadata_lost; if (header.compression == BI_RLE4 || header.compression == BI_RLE8) { return decode_rle(bytes, header, palette, pixels); @@ -678,12 +717,16 @@ void Bitmap::save(std::string filename) const std::uint64_t width = pixels[0].size(); const std::uint64_t height = pixels.size(); + if (width > static_cast(std::numeric_limits::max()) || + height > static_cast(std::numeric_limits::max())) + { + std::cerr << "Bitmap cannot be saved because it is too large.\n"; + return; + } const std::uint64_t stride = ((width * 24 + 31) / 32) * 4; const std::uint64_t image_size = stride * height; const std::uint64_t file_size = 54 + image_size; - if (width > static_cast(std::numeric_limits::max()) || - height > static_cast(std::numeric_limits::max()) || - image_size > std::numeric_limits::max() || + if (image_size > std::numeric_limits::max() || file_size > std::numeric_limits::max()) { std::cerr << "Bitmap cannot be saved because it is too large.\n"; diff --git a/bitmap.h b/bitmap.h index 3d12fc4..1824598 100644 --- a/bitmap.h +++ b/bitmap.h @@ -29,9 +29,9 @@ typedef std::vector < std::vector > PixelMatrix; // ---------------------------------------------------------------------------- /** - * Represents a bitmap where a grid of pixels (in row-major order) - * describes the color of each pixel within the image. Limited to Windows BMP - * formatted images with no compression and 24 bit color depth. + * Represents a bitmap where a grid of pixels (in row-major order) describes + * the color of each pixel. Common Windows BMP depths and encodings are read + * into 8-bit red, green, and blue components. Images are saved as 24-bpp BMP. **/ class Bitmap { @@ -41,9 +41,8 @@ class Bitmap public: /** - * Opens a file as its name is provided and reads pixel-by-pixel the colors - * into a matrix of RGB pixels. Any errors will cout but will result in an - * empty matrix (with no rows and no columns). + * Opens a supported Windows BMP and converts it to a matrix of RGB pixels. + * Any errors are written to cerr and result in an empty matrix. * * @param name of the filename to be opened and read as a matrix of pixels **/ @@ -52,8 +51,9 @@ class Bitmap /** * Saves the current image, represented by the matrix of pixels, as a * Windows BMP file with the name provided by the parameter. File extension - * is not forced but should be .bmp. Any errors will cout and will NOT - * attempt to save the file. + * is not forced but should be .bmp. Any errors are written to cerr and do + * not change the current matrix or its lossiness state. The output format + * is always uncompressed 24-bpp BMP. * * @param name of the filename to be written as a bmp image **/ @@ -71,7 +71,8 @@ class Bitmap /** * Reports whether opening the current image discarded color information - * or precision while converting it to RGB pixels. + * or precision while converting it to RGB pixels. Spatial dimensions are + * never resampled. See README.md for the complete lossiness rules. * * @return true only when the most recent successful open was lossy **/ diff --git a/specs/issue-1-bmp-import.md b/specs/issue-1-bmp-import.md new file mode 100644 index 0000000..e0497fe --- /dev/null +++ b/specs/issue-1-bmp-import.md @@ -0,0 +1,249 @@ +# Implementation Plan: Common BMP Import to 24-bpp RGB Matrix + +## Feature Description +Expand `Bitmap::open()` so the library can read a defined set of commonly supported Windows BMP files that can be converted into the existing `PixelMatrix` representation, where every stored pixel is 24-bpp RGB through `Pixel.red`, `Pixel.green`, and `Pixel.blue`. For this issue, "commonly supported" means CORE/INFO/V2/V3/V4/V5 headers using indexed or direct `BI_RGB`, 16/32-bpp `BI_BITFIELDS`, or `BI_RLE4`/`BI_RLE8`. The public API remains source-compatible except for a new public `bool isLossy()` function. `isLossy()` reports whether the most recent successful `open()` discarded color information or precision that cannot be represented in the library's RGB-only matrix; it never refers to spatial dimensions. + +This feature makes the library useful with a much wider range of real BMP files while keeping the teaching-friendly `Pixel` and `Bitmap` API simple. + +## User Story +As a C++ user of the Bitmap library +I want to open common BMP variants and receive a normal RGB pixel matrix +So that I can process images without first converting them to uncompressed 24-bpp BMP files in another tool. + +## Problem Statement +The current implementation assumes a fixed 14-byte file header, a 40-byte Windows DIB header, uncompressed 24-bpp BGR pixel data, and native struct layout. It rejects every other bit depth and every compressed encoding, even when the BMP can be decoded into the existing RGB `PixelMatrix`. Issue #1 originally described multiple bit-depth support, but the desired scope is now broader: support any commonly supported BMP input that can be converted to a 24-bpp RGB matrix, and expose whether the conversion lost source image information. + +## Solution Statement +Replace the single 24-bpp read path with a private BMP decoder pipeline: + +1. Read little-endian BMP fields explicitly into a validated internal header model. +2. Support the common Windows DIB header families: the legacy 12-byte `BITMAPCOREHEADER` plus the 40-byte `BITMAPINFOHEADER` and its 52/56/108/124-byte extensions, always honoring `header_size` and `bmp_offset`. +3. Decode supported BMP pixel encodings into `PixelMatrix` while preserving row orientation and validating malformed/truncated inputs atomically. +4. Keep `save()` output as uncompressed 24-bpp `BI_RGB`. +5. Add `Bitmap::isLossy()` as an additive API that returns `true` after a successful `open()` only when source information was discarded during import. + +Recommended support matrix: + +| Encoding | Bits per pixel | Import plan | `isLossy()` | +|---|---:|---|---| +| `BI_RGB` indexed | 1, 4, 8 | Decode palette indexes through RGBQUAD color table | `false` | +| `BI_RGB` direct | 16 | Decode RGB 5-5-5 and expand to 8-bit channels | `false` because no source precision is discarded | +| `BI_RGB` direct | 24 | Decode existing BGR rows | `false` | +| `BI_RGB` direct | 32 | Decode BGRX RGB channels; ignore the undefined/reserved byte | `false`; an undeclared reserved byte is not image information | +| `BI_BITFIELDS` | 16, 32 | Decode masks, including RGB 5-6-5 and common 32-bit masks; preserve stored RGB values when dropping alpha | `true` only if at least one pixel in a declared alpha channel is non-opaque or an RGB channel has more than 8 bits of precision | +| `BI_RLE8` | 8 | Feasible; implement Microsoft RLE8 encoded, absolute, EOL, EOB, and delta modes | `false` | +| `BI_RLE4` | 4 | Feasible; implement Microsoft RLE4 encoded, absolute, EOL, EOB, and delta modes | `false` | +| `BI_JPEG` / `BI_PNG` | implied | Feasible only with a new image decoder dependency or platform API; explicitly out of scope for this issue | n/a | +| CMYK BMP variants | varies | Reject for now; RGB conversion policy is outside the current library model | n/a | +| 12-byte `BITMAPCOREHEADER` | 1, 4, 8, 24 | Decode its unsigned dimensions and RGBTRIPLE palette layout | `false` | +| Later OS/2 2.x headers/compression | varies | Reject; these are distinct, uncommon extensions rather than the legacy Windows-compatible CORE layout | n/a | + +Authoritative references for implementers: + +- Microsoft BITMAPINFOHEADER documents bit depth, top-down/bottom-up height, `BI_RGB`, `BI_BITFIELDS`, palette rules, and DWORD stride calculation: https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapinfoheader +- Microsoft Bitmap Storage documents the BMP file layout, color table, pixel-index array, and row order: https://learn.microsoft.com/en-us/windows/win32/gdi/bitmap-storage +- Microsoft Bitmap Compression documents `BI_RLE8` and `BI_RLE4` encoded/absolute modes and escape records: https://learn.microsoft.com/en-us/windows/win32/gdi/bitmap-compression +- Microsoft WMF compression enumeration documents common compression values and notes that bottom-up bitmaps can be compressed while top-down compressed bitmaps cannot: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-wmf/4e588f70-bd92-4a6f-b77f-35d0feaf7a57 + +## Relevant Files +Use these files to implement the feature: + +- `bitmap.h` + - Declares the public `Pixel`, `PixelMatrix`, and `Bitmap` API. + - Add `bool isLossy();` and a private flag such as `bool lossy;`. + - Update comments so read support and write support are not confused. +- `bitmap.cpp` + - Contains all current BMP parsing, saving, validation, and matrix conversion. + - Replace native packed-struct reads with explicit little-endian parsing helpers. + - Add private decoder helpers for palettes, bitfields, uncompressed rows, RLE rows, and lossiness tracking. +- `README.md` + - Update the public documentation with the new import support matrix and `isLossy()` behavior. + - Clarify that saving still writes 24-bpp uncompressed BMP files. + - Add explicit Supported, Lossy conversion, and Unsupported/limitations tables rather than describing compatibility only in prose. +- `test_runner.sh` + - Change from compile-only to compiling and running a dependency-free test executable. + +### New Files +- `tests/bitmap_tests.cpp` + - Add deterministic unit tests that generate tiny BMP fixtures in code and assert exact RGB matrices, `isImage()`, and `isLossy()`. +- `tests/fixtures/README.md` or inline fixture helpers + - Optional. Prefer inline fixture builders in `tests/bitmap_tests.cpp` unless binary fixture files are truly clearer. + +## Implementation Plan +### Phase 1: Foundation +Define the import contract, lossiness semantics, and test harness before touching decoder behavior. Build a small internal BMP parsing layer that reads fields safely and validates file/header invariants before allocating or exposing pixels. + +### Phase 2: Core Implementation +Implement import support in vertical slices: uncompressed indexed formats, uncompressed direct-color formats, bitfields, then RLE4/RLE8. Each slice should add tests before or alongside implementation and leave existing 24-bpp behavior intact. + +### Phase 3: Integration +Wire `isLossy()` through `open()`, `fromPixelMatrix()`, and failure paths; update documentation; keep `save()` as 24-bpp `BI_RGB`; and run the full validation command set. + +## Step by Step Tasks +IMPORTANT: Execute every step in order, top to bottom. + +### Task 1: Define API Contract and Test Harness +- Add `bool isLossy();` to `Bitmap` in `bitmap.h`. +- Add a private `bool lossy;` field initialized to `false`. +- Fix `isImage()` so an empty matrix returns `false` before accessing `pixels[0]`; the new failed-open contract must be safe to query. +- Define behavior: + - New/default `Bitmap`: `isLossy() == false`. + - Failed `open()`: image is empty and `isLossy() == false`. + - Successful `open()`: `isLossy()` reflects only loss during import into `PixelMatrix`. + - `fromPixelMatrix(...)`: `isLossy() == false` because there is no opened source image. + - `save(...)`: does not change `isLossy()`. +- Create `tests/bitmap_tests.cpp` with small assertion helpers and BMP byte-vector fixture builders. +- Update `test_runner.sh` to compile `bitmap.cpp` plus tests and run the executable. + +### Task 2: Replace Native Struct Reads with Safe Header Parsing +- Add private fixed-width little-endian readers for 16-bit and 32-bit signed/unsigned fields. +- Parse the BMP file header and DIB header without relying on compiler struct padding or host endianness. +- Validate magic bytes, DIB header size, planes, width, height, bit depth, compression, color table length, pixel offset, and file length before decoding. +- Preserve atomic failure semantics: never expose a partially decoded matrix. +- Support the known 12/40/52/56/108/124-byte DIB layouts; reject unknown layouts until their field and palette rules are defined. +- Detect V4/V5 non-default color-space, gamma, and profile metadata. Preserve numeric RGB values without applying color-profile conversion, and mark the import lossy when meaningful metadata is present but cannot be represented. + +### Task 3: Implement Shared Row, Palette, and Orientation Utilities +- Use the BMP stride formula for uncompressed RGB data: row bytes rounded up to the nearest 4-byte boundary. +- Preserve bottom-up and top-down behavior for supported uncompressed/bitfield BMPs. +- Read `RGBQUAD` palettes for 1/4/8-bpp `BI_RGB` images; when `biClrUsed` is zero, use `2^bits_per_pixel`. +- Validate palette availability and palette indexes. +- Read RGBTRIPLE palettes for 12-byte `BITMAPCOREHEADER` images and RGBQUAD palettes for the Windows INFO/V4/V5 families. + +### Task 4: Decode Uncompressed Indexed and Legacy CORE BMPs +- Decode 1-bpp pixels from high bit to low bit. +- Decode 4-bpp pixels from high nibble to low nibble. +- Decode 8-bpp pixels directly as palette indexes. +- Decode the 12-byte `BITMAPCOREHEADER` using unsigned dimensions, RGBTRIPLE palettes, bottom-up rows, and its standard 1/4/8/24-bpp depths. +- Ignore row padding, do not create extra pixels from partially used final bytes, and reject truncated data. +- Add tests for every supported CORE depth, odd widths, top-down and bottom-up orientation where legal, palette lookup, invalid palette references, offsets, and truncation. + +### Task 5: Decode Uncompressed Direct-Color BMPs +- Decode 16-bpp `BI_RGB` as RGB 5-5-5 and scale to 0-255 deterministically. +- Keep 24-bpp BGR decoding behavior unchanged. +- Decode 32-bpp `BI_RGB` as BGRX; the high byte is reserved and does not by itself make the conversion lossy. +- Add tests for 16-bpp endpoints/midpoints, 24-bpp regressions, 32-bpp reserved-byte behavior, stride padding, and orientation. + +### Task 6: Decode BI_BITFIELDS BMPs +- Support 16-bpp and 32-bpp `BI_BITFIELDS`. +- Read RGB masks from the documented mask location after the DIB header for BITMAPINFOHEADER-style files, or from V4/V5 header fields when applicable. +- Convert masked channels to 8-bit RGB based on mask width and shift. +- Recognize common RGB 5-6-5, RGB 5-5-5, XRGB8888, and ARGB8888 masks. +- Preserve stored RGB channel values when discarding alpha. Mark `lossy` true only when at least one decoded alpha value is non-opaque, or when an RGB mask carries more than 8 bits of precision and must be quantized. +- Add tests for 565, 555, XRGB, fully opaque ARGB, partially transparent ARGB, fully transparent ARGB, preserved RGB-under-alpha values, malformed masks, and unsupported mask layouts. + +### Checkpoint: Common uncompressed import +- All CORE/INFO/V2/V3/V4/V5 uncompressed and bitfield fixtures decode to exact RGB values. +- Existing 24-bpp behavior remains compatible. +- Failed imports leave an empty image, and both `isImage()` and `isLossy()` are safe to call. +- This is a shippable milestone before introducing the independent RLE state machine. + +### Task 7: Add Feasible Compressed BMP Import with RLE4 and RLE8 +- Implement `BI_RLE8` for 8-bpp indexed BMPs using encoded mode, absolute mode, EOL, EOB, and delta escape handling. +- Implement `BI_RLE4` for 4-bpp indexed BMPs using alternating high/low nibbles in encoded and absolute modes. +- Enforce bounds: runs and deltas cannot write outside the image matrix. +- Apply palette lookup after expanding indexes, or while writing decoded pixels. +- Reject top-down RLE images because Microsoft documents compressed BMPs as bottom-up only. +- Add tests for encoded runs, absolute runs, word padding in absolute mode, EOL, EOB, deltas, malformed streams, and out-of-bounds writes. + +### Checkpoint: Native BMP compression +- Both RLE variants pass complete command-mode and malformed-stream coverage. +- RLE expansion produces the same RGB matrix as an equivalent uncompressed indexed fixture and remains non-lossy. + +### Task 8: Keep Embedded JPEG/PNG BMPs Explicitly Out of Scope +- Document that `BI_JPEG` and `BI_PNG` are feasible only by adding a decoder dependency or platform-specific decoder. +- Do not add a decoder dependency in this issue. +- If support is requested later, create a separate issue covering dependency selection, licensing, security updates, and codec-specific tests. +- Add tests that current `BI_JPEG` and `BI_PNG` BMPs are rejected clearly without partial image state. + +### Task 9: Preserve and Tighten Save Behavior +- Keep `save()` output as 24-bpp uncompressed Windows BMP. +- Reuse checked stride/padding calculations so file size and row padding are correct for all image widths. +- Ensure saved images reopen to the same RGB matrix and `isLossy() == false` on the newly opened saved file. +- Add save/reopen tests for widths with 0, 1, 2, and 3 padding bytes. + +### Task 10: Update Public Documentation +- Update `bitmap.h` comments for `open()`, `save()`, and `isLossy()`. +- Add README compatibility tables listing supported header families, depth/encoding combinations, and compression modes. +- Add a README lossiness table covering non-opaque alpha, greater-than-8-bit RGB quantization, ignored non-default color metadata, and conversions that remain lossless. +- Add a README limitations table explicitly excluding embedded JPEG/PNG, CMYK, later OS/2-specific variants, ICC/profile conversion, alpha preservation, and alternate-depth/compressed output. +- State that `save()` remains 24-bpp `BI_RGB` only. +- Include a short example showing `image.open(...)`, `image.isImage()`, and `image.isLossy()`. + +### Task 11: Run Validation Commands +- Execute every command in the Validation Commands section. +- Fix any failing tests, warnings, or documentation/code mismatches before handing off. + +## Testing Strategy +### Unit Tests +Use generated in-memory/minimal file fixtures so each test controls headers, palettes, rows, padding, compression bytes, and truncation cases. Tests should write temporary BMP files under a test temp directory, call the public API, and assert public behavior only. + +Required coverage: + +- API state: default object, failed open, successful open, `fromPixelMatrix`, `save`. +- Uncompressed indexed: 1, 4, and 8 bpp. +- Uncompressed direct: 16, 24, and 32 bpp. +- Bitfields: 16-bpp 565/555 and common 32-bpp masks. +- RLE: RLE8 and RLE4 encoded mode, absolute mode, EOL, EOB, delta, and malformed streams. +- Orientation: positive-height bottom-up and negative-height top-down where supported. +- Padding: all row padding sizes. +- Atomic failure: unsupported/malformed/truncated files leave `isImage() == false` and `toPixelMatrix().empty()`. + +### Edge Cases +- Zero width, zero height, negative width, and minimum/maximum safe dimensions. +- Valid 12-byte CORE and 40/52/56/108/124-byte Windows headers; unknown header sizes; pixel offset before required metadata. +- `biClrUsed` smaller/larger than required palette indexes. +- Palette data overlapping pixel data. +- Arithmetic overflow in stride and image-size calculations. +- Truncated file in header, palette, mask, uncompressed pixel rows, and RLE streams. +- RLE runs crossing row boundaries. +- RLE delta moving outside bounds. +- Top-down compressed BMPs. +- 32-bpp `BI_RGB` data with an ignored reserved byte that is zero versus nonzero (both remain non-lossy). +- Alpha masks with all alpha values opaque versus varied values. +- Transparent pixels whose stored RGB values differ from their visible composited color, proving RGB is preserved rather than composited. +- V4/V5 default metadata versus meaningful non-default color-space/gamma/profile metadata. + +## Acceptance Criteria +- `Bitmap::open()` imports common BMP files that can be represented as RGB pixels: 12-byte CORE images at 1/4/8/24 bpp, `BI_RGB` 1/4/8/16/24/32 bpp, `BI_BITFIELDS` 16/32 bpp, `BI_RLE4`, and `BI_RLE8`. +- Imported images produce the expected `PixelMatrix` dimensions, orientation, and exact RGB values. +- `Bitmap::isLossy()` is available as a public function and follows the documented state rules. +- `isLossy()` returns `false` for palette expansion, RLE expansion, 16-bpp expansion, 24-bpp import, and RGB-only bitfield import. +- `isLossy()` returns `true` when import discards color information or precision: at least one declared alpha sample is non-opaque, an RGB component wider than 8 bits is quantized, or meaningful non-default color-management metadata is not applied. Reserved/undefined bytes and an entirely opaque alpha channel do not count. +- When alpha is discarded, stored RGB component values are preserved without compositing against any background. +- Unsupported compressed variants, embedded JPEG/PNG, CMYK BMPs, malformed BMPs, and unsafe dimensions fail cleanly. +- Existing code using `open`, `save`, `isImage`, `toPixelMatrix`, and `fromPixelMatrix` remains source-compatible. +- `save()` still writes uncompressed 24-bpp BMP files and round-trips valid `PixelMatrix` values. +- README and header comments accurately document the supported compatibility matrix, lossy cases, explicit limitations, write format, and `isLossy()` behavior. + +## Validation Commands +Execute every command to validate the feature works correctly with zero regressions. + +```bash +./test_runner.sh +``` + +```bash +g++ -std=c++11 -Wall -Wextra -pedantic -c bitmap.cpp -o /tmp/bitmap.o +``` + +```bash +g++ -std=c++11 -Wall -Wextra -pedantic example.cpp bitmap.cpp -o /tmp/bitmap-example +``` + +```bash +/tmp/bitmap-example +``` + +```bash +git diff --check +``` + +## Notes +- `isLossy()` means "did the library discard color information or precision while converting the opened BMP into the RGB-only `PixelMatrix`?" It does not report whether dimensions changed or whether the source had previously undergone lossy compression. The implementation never resamples; image dimensions remain unchanged. +- A declared alpha channel is lossy only when at least one alpha sample is non-opaque. RGB values are preserved unchanged when alpha is discarded; the importer never composites against an assumed background. +- V4/V5 profile and gamma conversion is deliberately out of scope. Non-default metadata is detected, numeric RGB values are preserved, and `isLossy()` reports the discarded color interpretation. +- RLE4 and RLE8 support is feasible without new dependencies because Microsoft documents the byte stream formats. They are lossless encodings of palette indexes. +- `BI_BITFIELDS` is not compression in the usual sense; it is direct RGB data with masks. It should be treated as a common import format. +- Embedded `BI_JPEG` and `BI_PNG` BMPs are feasible but not dependency-free. Supporting them would expand project scope into general image decoding, dependency selection, licensing, and security handling. +- Include the 12-byte `BITMAPCOREHEADER` because Microsoft lists it as a basic legacy BMP header and common decoders retain it for backward compatibility. Keep later OS/2 2.x-only headers and compression modes out of scope. diff --git a/tests/bitmap_tests.cpp b/tests/bitmap_tests.cpp index 4954cd9..36b9a6a 100644 --- a/tests/bitmap_tests.cpp +++ b/tests/bitmap_tests.cpp @@ -298,6 +298,12 @@ void test_direct_color_and_orientation() std::vector{0x00, 0x7c, 0, 0}), false, "16-bpp RGB555"), 255, 0, 0, "RGB555 expansion"); + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 16, 0, std::vector(), std::vector(), + std::vector{0x00, 0x40, 0, 0}), false, + "16-bpp RGB555 midpoint"), 132, 0, 0, + "RGB555 components are rounded to the nearest 8-bit value"); + expect_color(open_single_pixel(make_info_fixture( 1, 1, 32, 0, std::vector(), std::vector(), std::vector{3, 2, 1, 255}), false, @@ -360,6 +366,39 @@ void test_color_metadata_loss() "meaningful V4 metadata"); } +void test_v5_profile_bounds() +{ + std::vector valid = make_info_fixture( + 1, 1, 24, 0, std::vector(), std::vector(), + std::vector{3, 2, 1, 0}, 124, false); + const unsigned long profile_offset_from_dib = + static_cast(valid.size() - 14); + set_u32(valid, 14 + 112, profile_offset_from_dib); + set_u32(valid, 14 + 116, 4); + valid.push_back('I'); + valid.push_back('C'); + valid.push_back('C'); + valid.push_back(0); + set_u32(valid, 2, valid.size()); + open_single_pixel(valid, true, "bounded V5 profile metadata"); + + std::vector invalid = valid; + set_u32(invalid, 14 + 112, 0xfffffff0UL); + std::string path = write_fixture(invalid); + Bitmap bitmap; + bitmap.open(path); + expect(!bitmap.isImage() && !bitmap.isLossy(), + "out-of-bounds V5 profiles are rejected atomically"); + std::remove(path.c_str()); + + std::vector overlapping = valid; + set_u32(overlapping, 14 + 112, 124); + path = write_fixture(overlapping); + bitmap.open(path); + expect(!bitmap.isImage(), "V5 profiles cannot overlap pixel data"); + std::remove(path.c_str()); +} + void test_rle_imports() { std::vector palette256(256, Pixel()); @@ -547,6 +586,7 @@ int main() test_direct_color_and_orientation(); test_bitfields_and_alpha_loss(); test_color_metadata_loss(); + test_v5_profile_bounds(); test_rle_imports(); test_rle_commands_and_orientation(); test_save_round_trip_padding(); From 2ca7894a15c6354265095e90cd25b0491017ee83 Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Tue, 29 Sep 2026 16:13:15 -0700 Subject: [PATCH 4/5] test: cover BMP decoder edge cases --- tests/bitmap_tests.cpp | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/bitmap_tests.cpp b/tests/bitmap_tests.cpp index 36b9a6a..08515ce 100644 --- a/tests/bitmap_tests.cpp +++ b/tests/bitmap_tests.cpp @@ -352,6 +352,26 @@ void test_bitfields_and_alpha_loss() 1, 1, 32, 3, std::vector(), rgb101010, std::vector{0xff, 0x03, 0, 0}), true, "10-bit bitfields"); + + expect_color(open_single_pixel(make_info_fixture( + 1, 1, 16, 3, std::vector(), rgb565, + std::vector{0x1f, 0, 0, 0}, 52), false, + "52-byte bitfields header"), 0, 0, 255, + "V2 mask fields are read from the extended header"); + + const std::string transparent_path = write_fixture(make_info_fixture( + 1, 1, 32, 3, std::vector(), argb, + std::vector{30, 20, 10, 64}, 56)); + Bitmap transparent; + transparent.open(transparent_path); + const std::string saved_path = "/tmp/bitmap-lossy-save.bmp"; + transparent.save(saved_path); + expect(transparent.isLossy(), "save preserves source lossiness state"); + transparent.open("/tmp/bitmap-test-does-not-exist-after-loss.bmp"); + expect(!transparent.isLossy() && !transparent.isImage(), + "failed open resets prior lossiness and pixels"); + std::remove(transparent_path.c_str()); + std::remove(saved_path.c_str()); } void test_color_metadata_loss() @@ -480,6 +500,39 @@ void test_rle_commands_and_orientation() pixels[0][1].red == 2 && pixels[0][2].red == 3, "RLE4 absolute nibbles decode in high-low order"); std::remove(path.c_str()); + + palette16[4] = Pixel(4, 0, 0); + palette16[5] = Pixel(5, 0, 0); + const std::vector padded_rle4_absolute = { + 0, 5, 0x12, 0x34, 0x50, 0, + 0, 1 + }; + path = write_fixture(make_info_fixture( + 5, 1, 4, 2, palette16, std::vector(), + padded_rle4_absolute)); + bitmap.open(path); + pixels = bitmap.toPixelMatrix(); + expect(bitmap.isImage() && pixels[0][4].red == 5, + "RLE4 absolute data consumes word-alignment padding"); + std::remove(path.c_str()); +} + +void test_unsupported_encodings() +{ + const unsigned long unsupported_compressions[] = {4, 5, 11}; + for (std::size_t index = 0; + index < sizeof(unsupported_compressions) / + sizeof(unsupported_compressions[0]); ++index) + { + const std::string path = write_fixture(make_info_fixture( + 1, 1, 24, unsupported_compressions[index], std::vector(), + std::vector(), std::vector{0, 0, 0, 0})); + Bitmap bitmap; + bitmap.open(path); + expect(!bitmap.isImage() && !bitmap.isLossy(), + "JPEG, PNG, and CMYK BMP encodings are rejected"); + std::remove(path.c_str()); + } } void test_save_round_trip_padding() @@ -567,6 +620,16 @@ void test_malformed_inputs_fail_atomically() expect(!bitmap.isImage(), "overlapping bitfield masks are rejected"); std::remove(path.c_str()); + const std::vector noncontiguous_masks = { + 0x7001, 0x00e0, 0x001c + }; + path = write_fixture(make_info_fixture( + 1, 1, 16, 3, std::vector(), noncontiguous_masks, + std::vector{0, 0, 0, 0})); + bitmap.open(path); + expect(!bitmap.isImage(), "non-contiguous bitfield masks are rejected"); + std::remove(path.c_str()); + std::vector rle_palette(256, Pixel()); const std::vector top_down_rle = {1, 0, 0, 1}; path = write_fixture(make_info_fixture( @@ -589,6 +652,7 @@ int main() test_v5_profile_bounds(); test_rle_imports(); test_rle_commands_and_orientation(); + test_unsupported_encodings(); test_save_round_trip_padding(); test_malformed_inputs_fail_atomically(); From f4ee9082fa33fce41ed1a526e475a0b6361643a4 Mon Sep 17 00:00:00 2001 From: Kevin Buffardi Date: Tue, 29 Sep 2026 16:13:52 -0700 Subject: [PATCH 5/5] refactor: reduce RLE decoder memory use --- bitmap.cpp | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/bitmap.cpp b/bitmap.cpp index 42fc009..d5a5d59 100644 --- a/bitmap.cpp +++ b/bitmap.cpp @@ -536,8 +536,8 @@ bool decode_rle(const std::vector & bytes, } end = image_end; } - std::vector > indexes( - header.height, std::vector(header.width, 0)); + pixels.assign(header.height, + std::vector(header.width, palette[0])); std::size_t position = header.pixel_offset; std::uint32_t x = 0; std::uint32_t y = 0; @@ -561,7 +561,7 @@ bool decode_rle(const std::vector & bytes, { return false; } - indexes[y][x++] = static_cast(palette_index); + pixels[header.height - 1 - y][x++] = palette[palette_index]; } } else if (value == 0) @@ -615,7 +615,7 @@ bool decode_rle(const std::vector & bytes, { return false; } - indexes[y][x++] = static_cast(palette_index); + pixels[header.height - 1 - y][x++] = palette[palette_index]; } position += padded_bytes; } @@ -625,20 +625,6 @@ bool decode_rle(const std::vector & bytes, return false; } - pixels.assign(header.height, std::vector(header.width)); - for (std::uint32_t stored_row = 0; stored_row < header.height; ++stored_row) - { - const std::uint32_t target_row = header.height - 1 - stored_row; - for (std::int32_t column = 0; column < header.width; ++column) - { - const std::uint16_t palette_index = indexes[stored_row][column]; - if (palette_index >= palette.size()) - { - return false; - } - pixels[target_row][column] = palette[palette_index]; - } - } return true; }