From 95fbae2483c77193986885d6eba31e8912a91937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 8 Nov 2022 12:11:33 +0100 Subject: [PATCH] TgaImporter: recognize and skip TGA 2 file footer. Those may eventually get parsed and exposed via image "extras", once those are a thing, for now it's all just ignored. --- doc/changelog.dox | 2 + .../TgaImporter/Test/TgaImporterTest.cpp | 124 ++++++++++++++++-- src/MagnumPlugins/TgaImporter/TgaHeader.h | 8 ++ src/MagnumPlugins/TgaImporter/TgaImporter.cpp | 54 +++++++- src/MagnumPlugins/TgaImporter/TgaImporter.h | 3 + 5 files changed, 178 insertions(+), 13 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 32d7776e7..882c5b0b1 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -635,6 +635,8 @@ See also: @relativeref{Trade::TextureType,CubeMapArray} in order to be able to distinguish what's the intended texture use, e.g. whether it's a 3D texture with filtering along Z or if it's a 2D array with discrete slices. +- @relativeref{Trade,TgaImporter} now recognizes and skips TGA 2 file footers + instead of treating them as actual image data - @ref magnum-imageconverter "magnum-imageconverter" has a new `--in-place` option for converting images in-place - In order to reduce the amount of exported symbols, a single no-op diff --git a/src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp b/src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp index 6e8a3bad6..bff564ef1 100644 --- a/src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp +++ b/src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp @@ -57,6 +57,8 @@ struct TgaImporterTest: TestSuite::Tester { void grayscale8(); void grayscale8Rle(); + void tga2(); + void openMemory(); void openTwice(); void importTwice(); @@ -65,6 +67,18 @@ struct TgaImporterTest: TestSuite::Tester { PluginManager::Manager _manager{"nonexistent"}; }; +constexpr const char Grayscale8Rle[]{ + 0, 0, 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 8, 0, + /* 2 pixels as-is */ + '\x01', 1, 2, + /* 1 pixel 2x repeated */ + '\x81', 3, + /* 1 pixel as-is */ + '\x00', 5, + /* 1 pixel 1x repeated */ + '\x00', 6 +}; + constexpr const char Color24[] = { 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0, 1, 2, 3, 2, 3, 4, @@ -122,6 +136,37 @@ const struct { {"invalid image type", {InPlaceInit, { 0, 0, 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }}, "unsupported image type: 9"}, + {"TGA 2 file too short", {InPlaceInit, { + 0, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0, + 0, 0, 0, 0, 0, 0, 0, /* One byte for the sizes missing here */ + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}, "TGA 2 file too short, expected at least 44 bytes but got 43"}, + {"TGA 2 extension offset overlaps with file header", {InPlaceInit, { + 0, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0, + 17, 0, 0, 0, 0, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}, "TGA 2 extension offset 17 overlaps with file header"}, + {"TGA 2 extension offset overlaps with file footer", {InPlaceInit, { + 0, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0, + 19, 0, 0, 0, 0, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}, "TGA 2 extension offset 19 out of bounds for 44 bytes and a 26-byte file footer"}, + {"TGA 2 developer area offset overlaps with file header", {InPlaceInit, { + 0, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0, + 0, 0, 0, 0, 17, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}, "TGA 2 developer area offset 17 overlaps with file header"}, + {"TGA 2 developer area offset overlaps with file footer", {InPlaceInit, { + 0, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0, + 0, 0, 0, 0, 19, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}, "TGA 2 developer area offset 19 out of bounds for 44 bytes and a 26-byte file footer"}, + {"TGA 2 developer area offset overlaps with extension area", {InPlaceInit, { + 0, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0, + '\xdd', '\xee', '\xee', + 19, 0, 0, 0, 18, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}, "TGA 2 developer area offset 18 overlaps with extensions at 19 bytes"}, {"RLE too large", {InPlaceInit, { 0, 0, 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0, /* 3 pixels as-is */ @@ -145,6 +190,45 @@ const struct { "Trade::TgaImporter::image2D(): converting from BGRA to RGBA\n"} }; +/* Tga2Data footer offsets rely on this */ +static_assert(sizeof(Grayscale8Rle) == 27, "size of grayscale data not 27 bytes"); +const struct { + const char* name; + Containers::Array footer; +} Tga2Data[]{ + {"just the footer", {InPlaceInit, { + 0, 0, 0, 0, 0, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}}, + {"extension", {InPlaceInit, { + '\xee', '\xee', + 27, 0, 0, 0, 0, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}}, + {"developer area", {InPlaceInit, { + '\xdd', '\xdd', + 0, 0, 0, 0, 27, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}}, + {"both extension and developer area", {InPlaceInit, { + '\xee', '\xee', '\xee', '\xdd', '\xdd', '\xdd', '\xdd', '\xdd', + 27, 0, 0, 0, 30, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}}, + {"empty extension area", {InPlaceInit, { + 27, 0, 0, 0, 0, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}}, + {"empty developer area", {InPlaceInit, { + 0, 0, 0, 0, 27, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}}, + {"empty extension and developer area", {InPlaceInit, { + 27, 0, 0, 0, 27, 0, 0, 0, + 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', '\0' + }}}, +}; + /* Shared among all plugins that implement data copying optimizations */ const struct { const char* name; @@ -183,6 +267,9 @@ TgaImporterTest::TgaImporterTest() { addTests({&TgaImporterTest::grayscale8, &TgaImporterTest::grayscale8Rle}); + addInstancedTests({&TgaImporterTest::tga2}, + Containers::arraySize(Tga2Data)); + addInstancedTests({&TgaImporterTest::openMemory}, Containers::arraySize(OpenMemoryData)); @@ -400,18 +487,31 @@ void TgaImporterTest::grayscale8() { void TgaImporterTest::grayscale8Rle() { Containers::Pointer importer = _manager.instantiate("TgaImporter"); - const char data[] = { - 0, 0, 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 8, 0, - /* 2 pixels as-is */ - '\x01', 1, 2, - /* 1 pixel 2x repeated */ - '\x81', 3, - /* 1 pixel as-is */ - '\x00', 5, - /* 1 pixel 1x repeated */ - '\x00', 6 - }; - CORRADE_VERIFY(importer->openData(data)); + CORRADE_VERIFY(importer->openData(Grayscale8Rle)); + + Containers::Optional image = importer->image2D(0); + CORRADE_VERIFY(image); + CORRADE_COMPARE(image->flags(), ImageFlags2D{}); + CORRADE_COMPARE(image->storage().alignment(), 1); + CORRADE_COMPARE(image->format(), PixelFormat::R8Unorm); + CORRADE_COMPARE(image->size(), Vector2i(2, 3)); + CORRADE_COMPARE_AS(image->data(), Containers::arrayView({ + 1, 2, + 3, 3, + 5, 6 + }), TestSuite::Compare::Container); +} + +void TgaImporterTest::tga2() { + auto&& data = Tga2Data[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + Containers::Pointer importer = _manager.instantiate("TgaImporter"); + + /* The actual image data is always the same, only the footer differs */ + CORRADE_VERIFY(importer->openData( + Containers::StringView{Containers::arrayView(Grayscale8Rle)} + + Containers::StringView{data.footer})); Containers::Optional image = importer->image2D(0); CORRADE_VERIFY(image); diff --git a/src/MagnumPlugins/TgaImporter/TgaHeader.h b/src/MagnumPlugins/TgaImporter/TgaHeader.h index d35453849..49ef76afc 100644 --- a/src/MagnumPlugins/TgaImporter/TgaHeader.h +++ b/src/MagnumPlugins/TgaImporter/TgaHeader.h @@ -49,6 +49,14 @@ struct TgaHeader { UnsignedByte bpp; /* Bits per pixel (8, 16, 24, 32) */ UnsignedByte descriptor; /* Image descriptor */ }; + +/* TGA 2 file footer (optional) + https://en.wikipedia.org/wiki/Truevision_TGA#File_footer_(optional) */ +struct TgaFooter { + UnsignedInt extensionOffset; + UnsignedInt developerAreaOffset; + char signature[18]; /* TRUEVISION-XFILE.\0 */ +}; #pragma pack() static_assert(sizeof(TgaHeader) == 18, "TgaHeader size is not 18 bytes"); diff --git a/src/MagnumPlugins/TgaImporter/TgaImporter.cpp b/src/MagnumPlugins/TgaImporter/TgaImporter.cpp index 1ae6ed39b..e43b734aa 100644 --- a/src/MagnumPlugins/TgaImporter/TgaImporter.cpp +++ b/src/MagnumPlugins/TgaImporter/TgaImporter.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include @@ -38,6 +39,8 @@ namespace Magnum { namespace Trade { +using namespace Containers::Literals; + TgaImporter::TgaImporter() = default; TgaImporter::TgaImporter(PluginManager::AbstractManager& manager, const Containers::StringView& plugin): AbstractImporter{manager, plugin} {} @@ -132,9 +135,58 @@ Containers::Optional TgaImporter::doImage2D(UnsignedInt, UnsignedIn const std::size_t pixelSize = header.bpp/8; const std::size_t outputSize = std::size_t(size.product())*pixelSize; + /* The source pixel data is implicitly the rest of the file. If there's a + TGA 2 header at the end, ignore the extension and developer areas. + https://en.wikipedia.org/wiki/Truevision_TGA#File_footer_(optional) */ + Containers::ArrayView srcPixels = _in.exceptPrefix(sizeof(Implementation::TgaHeader)); + if(Containers::StringView{_in}.hasSuffix("TRUEVISION-XFILE.\0"_s)) { + if(srcPixels.size() < sizeof(Implementation::TgaFooter)) { + Error{} << "Trade::TgaImporter::image2D(): TGA 2 file too short, expected at least" << sizeof(Implementation::TgaHeader) + sizeof(Implementation::TgaFooter) << "bytes but got" << _in.size(); + return {}; + } + + const auto& footer = *reinterpret_cast(srcPixels.end() - sizeof(Implementation::TgaFooter)); + const UnsignedInt extensionOffset = Utility::Endianness::littleEndian(footer.extensionOffset); + const UnsignedInt developerAreaOffset = Utility::Endianness::littleEndian(footer.developerAreaOffset); + + srcPixels = srcPixels.exceptSuffix(sizeof(Implementation::TgaFooter)); + + /* If the extension area is present, cut it from the pixel data */ + if(extensionOffset) { + if(extensionOffset < sizeof(Implementation::TgaHeader)) { + Error{} << "Trade::TgaImporter::image2D(): TGA 2 extension offset" << extensionOffset << "overlaps with file header"; + return {}; + } + if(extensionOffset > _in.size() - sizeof(Implementation::TgaFooter)) { + Error{} << "Trade::TgaImporter::image2D(): TGA 2 extension offset" << extensionOffset << "out of bounds for" << _in.size() << "bytes and a" << sizeof(Implementation::TgaFooter) << Debug::nospace << "-byte file footer"; + return {}; + } + + srcPixels = srcPixels.prefix(_in.data() + extensionOffset); + } + + /* If the developer area is present, cut it from the pixel data */ + if(developerAreaOffset) { + if(developerAreaOffset < sizeof(Implementation::TgaHeader)) { + Error{} << "Trade::TgaImporter::image2D(): TGA 2 developer area offset" << developerAreaOffset << "overlaps with file header"; + return {}; + } + if(developerAreaOffset > _in.size() - sizeof(Implementation::TgaFooter)) { + Error{} << "Trade::TgaImporter::image2D(): TGA 2 developer area offset" << developerAreaOffset << "out of bounds for" << _in.size() << "bytes and a" << sizeof(Implementation::TgaFooter) << Debug::nospace << "-byte file footer"; + return {}; + } + + if(!extensionOffset) + srcPixels = srcPixels.prefix(_in.data() + developerAreaOffset); + else if(developerAreaOffset < extensionOffset) { + Error{} << "Trade::TgaImporter::image2D(): TGA 2 developer area offset" << developerAreaOffset << "overlaps with extensions at" << extensionOffset << "bytes"; + return {}; + } + } + } + /* Copy data directly if not RLE */ Containers::Array data{outputSize}; - Containers::ArrayView srcPixels = _in.exceptPrefix(sizeof(Implementation::TgaHeader)); if(!rle) { /* Files that are larger are allowed in this case (but not for RLE) */ if(srcPixels.size() < outputSize) { diff --git a/src/MagnumPlugins/TgaImporter/TgaImporter.h b/src/MagnumPlugins/TgaImporter/TgaImporter.h index 8cbfbbf68..9ec01052b 100644 --- a/src/MagnumPlugins/TgaImporter/TgaImporter.h +++ b/src/MagnumPlugins/TgaImporter/TgaImporter.h @@ -98,6 +98,9 @@ are imported with default @ref PixelStorage parameters except for alignment, which may be changed to `1` if the data require it. RLE compression is supported, paletted images are not. + +If a TGA 2 footer is recognized in the file, the optional extension and +developer area blocks at the end of the file are ignored. */ class MAGNUM_TGAIMPORTER_EXPORT TgaImporter: public AbstractImporter { public: