From 67a8d833ea1448777d9570479d9040f2f335fdc9 Mon Sep 17 00:00:00 2001 From: frutiemax Date: Thu, 2 Jul 2020 13:04:49 -0400 Subject: [PATCH] Part of #11159: ImageImporter::ImportResult uses std::vector (#12076) --- src/openrct2/CmdlineSprite.cpp | 58 +++++++++-------------- src/openrct2/drawing/ImageImporter.cpp | 40 ++++++---------- src/openrct2/drawing/ImageImporter.h | 7 ++- src/openrct2/object/ObjectJsonHelpers.cpp | 2 - test/tests/ImageImporterTests.cpp | 7 ++- 5 files changed, 44 insertions(+), 70 deletions(-) diff --git a/src/openrct2/CmdlineSprite.cpp b/src/openrct2/CmdlineSprite.cpp index 08c4dff176..99d42fa2e9 100644 --- a/src/openrct2/CmdlineSprite.cpp +++ b/src/openrct2/CmdlineSprite.cpp @@ -244,9 +244,8 @@ static bool sprite_file_export(rct_g1_element* spriteHeader, const char* outPath } } -static bool sprite_file_import( - const char* path, int16_t x_offset, int16_t y_offset, bool keep_palette, bool forceBmp, rct_g1_element* outElement, - uint8_t** outBuffer, int* outBufferLength, int32_t mode) +static std::optional sprite_file_import( + const char* path, int16_t x_offset, int16_t y_offset, bool keep_palette, bool forceBmp, int32_t mode) { try { @@ -266,17 +265,13 @@ static bool sprite_file_import( ImageImporter importer; auto image = Imaging::ReadFromFile(path, format); - auto result = importer.Import(image, x_offset, y_offset, flags, static_cast(mode)); - *outElement = result.Element; - *outBuffer = static_cast(result.Buffer); - *outBufferLength = static_cast(result.BufferLength); - return true; + return importer.Import(image, x_offset, y_offset, flags, static_cast(mode)); } catch (const std::exception& e) { fprintf(stderr, "%s\n", e.what()); - return false; + return std::nullopt; } } @@ -643,12 +638,8 @@ int32_t cmdline_for_sprite(const char** argv, int32_t argc) } } - rct_g1_element spriteElement; - uint8_t* buffer; - - int32_t bufferLength; - if (!sprite_file_import( - imagePath, x_offset, y_offset, false, false, &spriteElement, &buffer, &bufferLength, gSpriteMode)) + auto importResult = sprite_file_import(imagePath, x_offset, y_offset, false, false, gSpriteMode); + if (importResult == std::nullopt) return -1; if (!sprite_file_open(spriteFilePath)) @@ -658,7 +649,7 @@ int32_t cmdline_for_sprite(const char** argv, int32_t argc) } spriteFileHeader.num_entries++; - spriteFileHeader.total_size += bufferLength; + spriteFileHeader.total_size += static_cast(importResult->Buffer.size()); spriteFileEntries = static_cast( realloc(spriteFileEntries, spriteFileHeader.num_entries * sizeof(rct_g1_element))); @@ -666,12 +657,13 @@ int32_t cmdline_for_sprite(const char** argv, int32_t argc) spriteFileData = static_cast(realloc(spriteFileData, spriteFileHeader.total_size)); sprite_entries_make_absolute(); - spriteFileEntries[spriteFileHeader.num_entries - 1] = spriteElement; - std::memcpy(spriteFileData + (spriteFileHeader.total_size - bufferLength), buffer, bufferLength); - spriteFileEntries[spriteFileHeader.num_entries - 1].offset = spriteFileData - + (spriteFileHeader.total_size - bufferLength); + spriteFileEntries[spriteFileHeader.num_entries - 1] = importResult->Element; + + const auto& buffer = importResult->Buffer; + std::memcpy(spriteFileData + (spriteFileHeader.total_size - buffer.size()), buffer.data(), buffer.size()); + spriteFileEntries[spriteFileHeader.num_entries - 1].offset = spriteFileData + + (spriteFileHeader.total_size - buffer.size()); - free(buffer); if (!sprite_file_save(spriteFilePath)) return -1; @@ -768,14 +760,10 @@ int32_t cmdline_for_sprite(const char** argv, int32_t argc) // Resolve absolute sprite path auto imagePath = platform_get_absolute_path(json_string_value(path), directoryPath); - rct_g1_element spriteElement; - uint8_t* buffer; - int bufferLength; - - if (!sprite_file_import( - imagePath.c_str(), x_offset == nullptr ? 0 : json_integer_value(x_offset), - y_offset == nullptr ? 0 : json_integer_value(y_offset), keep_palette, forceBmp, &spriteElement, &buffer, - &bufferLength, gSpriteMode)) + auto importResult = sprite_file_import( + imagePath.c_str(), x_offset == nullptr ? 0 : json_integer_value(x_offset), + y_offset == nullptr ? 0 : json_integer_value(y_offset), keep_palette, forceBmp, gSpriteMode); + if (importResult == std::nullopt) { fprintf(stderr, "Could not import image file: %s\nCanceling\n", imagePath.c_str()); json_decref(sprite_list); @@ -790,7 +778,7 @@ int32_t cmdline_for_sprite(const char** argv, int32_t argc) } spriteFileHeader.num_entries++; - spriteFileHeader.total_size += bufferLength; + spriteFileHeader.total_size += static_cast(importResult->Buffer.size()); spriteFileEntries = static_cast( realloc(spriteFileEntries, spriteFileHeader.num_entries * sizeof(rct_g1_element))); @@ -798,12 +786,12 @@ int32_t cmdline_for_sprite(const char** argv, int32_t argc) spriteFileData = static_cast(realloc(spriteFileData, spriteFileHeader.total_size)); sprite_entries_make_absolute(); - spriteFileEntries[spriteFileHeader.num_entries - 1] = spriteElement; - std::memcpy(spriteFileData + (spriteFileHeader.total_size - bufferLength), buffer, bufferLength); - spriteFileEntries[spriteFileHeader.num_entries - 1].offset = spriteFileData - + (spriteFileHeader.total_size - bufferLength); + spriteFileEntries[spriteFileHeader.num_entries - 1] = importResult->Element; - free(buffer); + const auto& buffer = importResult->Buffer; + std::memcpy(spriteFileData + (spriteFileHeader.total_size - buffer.size()), buffer.data(), buffer.size()); + spriteFileEntries[spriteFileHeader.num_entries - 1].offset = spriteFileData + + (spriteFileHeader.total_size - buffer.size()); if (!sprite_file_save(spriteFilePath)) { diff --git a/src/openrct2/drawing/ImageImporter.cpp b/src/openrct2/drawing/ImageImporter.cpp index 2e771c005b..5ecc827d9a 100644 --- a/src/openrct2/drawing/ImageImporter.cpp +++ b/src/openrct2/drawing/ImageImporter.cpp @@ -37,11 +37,9 @@ ImportResult ImageImporter::Import( const auto height = image.Height; auto pixels = GetPixels(image.Pixels.data(), width, height, flags, mode); - auto [buffer, bufferLength] = flags & IMPORT_FLAGS::RLE ? EncodeRLE(pixels.data(), width, height) - : EncodeRaw(pixels.data(), width, height); + auto buffer = flags & IMPORT_FLAGS::RLE ? EncodeRLE(pixels.data(), width, height) : EncodeRaw(pixels.data(), width, height); rct_g1_element outElement; - outElement.offset = static_cast(buffer); outElement.width = width; outElement.height = height; outElement.flags = (flags & IMPORT_FLAGS::RLE ? G1_FLAG_RLE_COMPRESSION : G1_FLAG_BMP); @@ -51,8 +49,8 @@ ImportResult ImageImporter::Import( ImportResult result; result.Element = outElement; - result.Buffer = buffer; - result.BufferLength = bufferLength; + result.Buffer = std::move(buffer); + result.Element.offset = result.Buffer.data(); return result; } @@ -108,19 +106,19 @@ std::vector ImageImporter::GetPixels( return buffer; } -std::tuple ImageImporter::EncodeRaw(const int32_t* pixels, uint32_t width, uint32_t height) +std::vector ImageImporter::EncodeRaw(const int32_t* pixels, uint32_t width, uint32_t height) { auto bufferLength = width * height; - auto buffer = static_cast(std::malloc(bufferLength)); + std::vector buffer(bufferLength); for (size_t i = 0; i < bufferLength; i++) { auto p = pixels[i]; buffer[i] = (p == PALETTE_TRANSPARENT ? 0 : static_cast(p)); } - return std::make_tuple(buffer, bufferLength); + return buffer; } -std::tuple ImageImporter::EncodeRLE(const int32_t* pixels, uint32_t width, uint32_t height) +std::vector ImageImporter::EncodeRLE(const int32_t* pixels, uint32_t width, uint32_t height) { struct RLECode { @@ -129,18 +127,14 @@ std::tuple ImageImporter::EncodeRLE(const int32_t* pixels, uint32 }; auto src = pixels; - auto buffer = static_cast(std::malloc((height * 2) + (width * height * 16))); - if (buffer == nullptr) - { - throw std::bad_alloc(); - } + std::vector buffer((height * 2) + (width * height * 16)); - std::fill_n(buffer, (height * 2) + (width * height * 16), 0x00); - auto yOffsets = reinterpret_cast(buffer); - auto dst = buffer + (height * 2); + std::fill_n(buffer.data(), (height * 2) + (width * height * 16), 0x00); + auto yOffsets = reinterpret_cast(buffer.data()); + auto dst = buffer.data() + (height * 2); for (uint32_t y = 0; y < height; y++) { - yOffsets[y] = static_cast(dst - buffer); + yOffsets[y] = static_cast(dst - buffer.data()); auto previousCode = static_cast(nullptr); auto currentCode = reinterpret_cast(dst); @@ -213,13 +207,9 @@ std::tuple ImageImporter::EncodeRLE(const int32_t* pixels, uint32 } } - auto bufferLength = static_cast(dst - buffer); - buffer = static_cast(realloc(buffer, bufferLength)); - if (buffer == nullptr) - { - throw std::bad_alloc(); - } - return std::make_tuple(buffer, bufferLength); + auto bufferLength = static_cast(dst - buffer.data()); + buffer.resize(bufferLength); + return buffer; } int32_t ImageImporter::CalculatePaletteIndex( diff --git a/src/openrct2/drawing/ImageImporter.h b/src/openrct2/drawing/ImageImporter.h index 6c1d108fe6..233e3851aa 100644 --- a/src/openrct2/drawing/ImageImporter.h +++ b/src/openrct2/drawing/ImageImporter.h @@ -28,8 +28,7 @@ namespace OpenRCT2::Drawing struct ImportResult { rct_g1_element Element{}; - void* Buffer{}; - size_t BufferLength{}; + std::vector Buffer; }; enum class IMPORT_MODE @@ -53,8 +52,8 @@ namespace OpenRCT2::Drawing private: static std::vector GetPixels( const uint8_t* pixels, uint32_t width, uint32_t height, IMPORT_FLAGS flags, IMPORT_MODE mode); - static std::tuple EncodeRaw(const int32_t* pixels, uint32_t width, uint32_t height); - static std::tuple EncodeRLE(const int32_t* pixels, uint32_t width, uint32_t height); + static std::vector EncodeRaw(const int32_t* pixels, uint32_t width, uint32_t height); + static std::vector EncodeRLE(const int32_t* pixels, uint32_t width, uint32_t height); static int32_t CalculatePaletteIndex( IMPORT_MODE mode, int16_t* rgbaSrc, int32_t x, int32_t y, int32_t width, int32_t height); diff --git a/src/openrct2/object/ObjectJsonHelpers.cpp b/src/openrct2/object/ObjectJsonHelpers.cpp index 6b12cf4a05..e9c96b6de2 100644 --- a/src/openrct2/object/ObjectJsonHelpers.cpp +++ b/src/openrct2/object/ObjectJsonHelpers.cpp @@ -414,7 +414,6 @@ namespace ObjectJsonHelpers auto importResult = importer.Import(image, 0, 0, ImageImporter::IMPORT_FLAGS::RLE); result.push_back(std::make_unique(importResult.Element)); - std::free(importResult.Buffer); } catch (const std::exception& e) { @@ -450,7 +449,6 @@ namespace ObjectJsonHelpers g1Element.x_offset = x; g1Element.y_offset = y; result.push_back(std::make_unique(g1Element)); - std::free(importResult.Buffer); } catch (const std::exception& e) { diff --git a/test/tests/ImageImporterTests.cpp b/test/tests/ImageImporterTests.cpp index 397098f019..ce7b9f5485 100644 --- a/test/tests/ImageImporterTests.cpp +++ b/test/tests/ImageImporterTests.cpp @@ -43,7 +43,7 @@ TEST_F(ImageImporterTests, Import_Logo) auto image = Imaging::ReadFromFile(logoPath, IMAGE_FORMAT::PNG_32); auto result = importer.Import(image, 3, 5, ImageImporter::IMPORT_FLAGS::RLE); - ASSERT_EQ(result.Buffer, result.Element.offset); + ASSERT_EQ(result.Buffer.data(), result.Element.offset); ASSERT_EQ(128, result.Element.width); ASSERT_EQ(128, result.Element.height); ASSERT_EQ(3, result.Element.x_offset); @@ -52,8 +52,7 @@ TEST_F(ImageImporterTests, Import_Logo) // Check to ensure RLE data doesn't change unexpectedly. // Update expected hash if change is expected. - ASSERT_NE(nullptr, result.Buffer); - auto hash = GetHash(result.Buffer, result.BufferLength); + ASSERT_NE(nullptr, result.Buffer.data()); + auto hash = GetHash(result.Buffer.data(), result.Buffer.size()); ASSERT_EQ(0xCEF27C7D, hash); - free(result.Buffer); }