TgaImageConverter: use RLE only if uncompressed output isn't smaller.

Makes sense, especially for LUTs and other things that don't have any
repeated pixels at all.

src/MagnumPlugins/TgaImageConverter/Test/TgaImageConverterTest.cpp

@@ -61,6 +61,7 @@ struct TgaImageConverterTest: TestSuite::Tester {
     void rleRgb();
     void rleRgba();
     void rleDisabled();
+    void rleFallbackIfLarger();
 
     void unsupportedMetadata();
 
@@ -245,6 +246,37 @@ const struct {
         }}, 128 + 31 + 3, {}},
 };
 
+const struct {
+    const char* name;
+    char data[2];
+    Containers::Array<UnsignedByte> expected;
+    Containers::Optional<bool> rle;
+    Containers::Optional<bool> rleFallbackIfLarger;
+    ImageConverterFlags flags;
+    const char* message;
+} RleFallbackIfLargerData[]{
+    {"RLE smaller, verbose", {7, 7}, {InPlaceInit, {
+            /* well, not smaller but not larger either, so we pick what's less
+               work (which is to not discard all the already-done RLE work) */
+            0x80|1, 7
+        }}, {}, {}, ImageConverterFlag::Verbose, ""},
+    {"RLE smaller, RLE disabled, verbose", {7, 7}, {InPlaceInit, {
+            7, 7
+        }}, false, {}, ImageConverterFlag::Verbose, ""},
+    {"uncompressed smaller", {7, 13}, {InPlaceInit, {
+            7, 13
+        }}, {}, {}, {}, ""},
+    {"uncompressed smaller, verbose", {7, 13}, {InPlaceInit, {
+            7, 13
+        }}, {}, {}, ImageConverterFlag::Verbose, "Trade::TgaImageConverter::convertToData(): RLE output 1 bytes larger than uncompressed, falling back to uncompressed\n"},
+    {"uncompressed smaller, fallback disabled, verbose", {7, 13}, {InPlaceInit, {
+            0x00|1, 7, 13
+        }}, {}, false, ImageConverterFlag::Verbose, ""},
+    {"uncompressed smaller, RLE disabled, verbose", {7, 13}, {InPlaceInit, {
+            7, 13
+        }}, false, false, ImageConverterFlag::Verbose, ""},
+};
+
 const struct {
     const char* name;
     ImageFlags2D flags;
@@ -271,6 +303,9 @@ TgaImageConverterTest::TgaImageConverterTest() {
               &TgaImageConverterTest::rleRgba,
               &TgaImageConverterTest::rleDisabled});
 
+    addInstancedTests({&TgaImageConverterTest::rleFallbackIfLarger},
+        Containers::arraySize(RleFallbackIfLargerData));
+
     addInstancedTests({&TgaImageConverterTest::unsupportedMetadata},
         Containers::arraySize(UnsupportedMetadataData));
 
@@ -435,9 +470,11 @@ void TgaImageConverterTest::rle() {
     ImageView2D image{PixelStorage{}.setAlignment(1), PixelFormat::R8Unorm, size, data.data};
 
     Containers::Pointer<AbstractImageConverter> converter = _converterManager.instantiate("TgaImageConverter");
-
     if(data.rleAcrossScanlines)
         converter->configuration().setValue("rleAcrossScanlines", *data.rleAcrossScanlines);
+    /* Force RLE to be used even if larger than uncompressed. This behavior is
+       tested in rleFallbackIfLarger() instead. */
+    converter->configuration().setValue("rleFallbackIfLarger", false);
 
     Containers::Optional<Containers::Array<char>> array = converter->convertToData(image);
     CORRADE_VERIFY(array);
@@ -566,6 +603,49 @@ void TgaImageConverterTest::rleDisabled() {
     /* No need to verify a roundtrip, that's tested enough in uncompressed*() */
 }
 
+void TgaImageConverterTest::rleFallbackIfLarger() {
+    auto&& data = RleFallbackIfLargerData[testCaseInstanceId()];
+    setTestCaseDescription(data.name);
+
+    /* Skip/alignment handling tested in rleRgb() */
+    ImageView2D image{PixelStorage{}.setAlignment(1), PixelFormat::R8Unorm, {2, 1}, data.data};
+
+    Containers::Pointer<AbstractImageConverter> converter = _converterManager.instantiate("TgaImageConverter");
+    converter->setFlags(data.flags);
+    if(data.rle)
+        converter->configuration().setValue("rle", *data.rle);
+    if(data.rleFallbackIfLarger)
+        converter->configuration().setValue("rleFallbackIfLarger", *data.rleFallbackIfLarger);
+
+    std::ostringstream out;
+    Containers::Optional<Containers::Array<char>> array;
+    {
+        Debug redirectOutput{&out};
+        array = converter->convertToData(image);
+    }
+    CORRADE_VERIFY(array);
+    CORRADE_COMPARE_AS(
+        Containers::arrayCast<const UnsignedByte>(*array)
+            .exceptPrefix(sizeof(Implementation::TgaHeader)),
+        data.expected,
+        TestSuite::Compare::Container);
+    CORRADE_COMPARE(out.str(), data.message);
+
+    if(!(_importerManager.loadState("TgaImporter") & PluginManager::LoadState::Loaded))
+        CORRADE_SKIP("TgaImporter plugin not enabled, can't test the result");
+
+    Containers::Pointer<AbstractImporter> importer = _importerManager.instantiate("TgaImporter");
+    CORRADE_VERIFY(importer->openData(*array));
+    Containers::Optional<Trade::ImageData2D> converted = importer->image2D(0);
+    CORRADE_VERIFY(converted);
+
+    CORRADE_COMPARE(converted->size(), (Vector2i{2, 1}));
+    CORRADE_COMPARE(converted->format(), PixelFormat::R8Unorm);
+    CORRADE_COMPARE_AS(converted->data(),
+        Containers::arrayView(data.data),
+        TestSuite::Compare::Container);
+}
+
 void TgaImageConverterTest::unsupportedMetadata() {
     auto&& data = UnsupportedMetadataData[testCaseInstanceId()];
     setTestCaseDescription(data.name);

src/MagnumPlugins/TgaImageConverter/TgaImageConverter.conf

@@ -3,6 +3,10 @@
 # Run-length encode the data for smaller file size
 rle=true
 
+# Fall back to uncompressed output if the RLE output is larger. Disable to
+# always produce a RLE output.
+rleFallbackIfLarger=true
+
 # Allow RLE to go across scanlines. Can result in even smaller files but
 # considered invalid in the TGA 2.0 specification and thus may cause issues
 # in certain importers.

src/MagnumPlugins/TgaImageConverter/TgaImageConverter.cpp

@@ -220,12 +220,13 @@ Containers::Optional<Containers::Array<char>> TgaImageConverter::doConvertToData
        not then allocate exactly the amount of bytes so we don't need to copy
        after. */
     const auto pixelSize = UnsignedByte(image.pixelSize());
+    const std::size_t uncompressedSize = sizeof(Implementation::TgaHeader) + pixelSize*image.size().product();
     const bool rle = configuration().value<bool>("rle");
     Containers::Array<char> data;
     if(rle) {
         arrayAppend(data, NoInit, sizeof(Implementation::TgaHeader));
     } else {
-        data = Containers::Array<char>{NoInit, sizeof(Implementation::TgaHeader) + pixelSize*image.size().product()};
+        data = Containers::Array<char>{NoInit, uncompressedSize};
     }
 
     /* Clear the header and fill non-zero values */
@@ -270,14 +271,24 @@ Containers::Optional<Containers::Array<char>> TgaImageConverter::doConvertToData
                 break;
             default: CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
         }
+    }
+
+    /* If RLE wasn't used or if a RLE output is larger than uncompressed
+       output, write an uncompressed output instead */
+    if(!rle || (data.size() > uncompressedSize && configuration().value<bool>("rleFallbackIfLarger"))) {
+        if(rle) {
+            if(flags() & ImageConverterFlag::Verbose)
+                Debug{} << "Trade::TgaImageConverter::convertToData(): RLE output" << data.size() - uncompressedSize << "bytes larger than uncompressed, falling back to uncompressed";
 
-        /* Turn the array back into a non-growable to avoid a dangling deleter
-           on plugin unload */
-        arrayShrink(data);
+            /* Resize the array to exactly the uncompressed size (this will
+               always shrink, never grow) */
+            arrayResize(data, NoInit, uncompressedSize);
+
+            /* Remove the RLE bit from the header. Can't use the header
+               variable from above as it may be dangling at this point. */
+            reinterpret_cast<Implementation::TgaHeader*>(data.begin())->imageType &= ~8;
+        }
 
-    /* Otherwise directly dopy the pixels into output, dropping padding (if
-       any), and do the BGR(A) conversion on the copied data */
-    } else {
         const Containers::ArrayView<char> pixels = data.exceptPrefix(sizeof(Implementation::TgaHeader));
         Utility::copy(image.pixels(), Containers::StridedArrayView3D<char>{pixels,
             {std::size_t(image.size().y()), std::size_t(image.size().x()), pixelSize}});
@@ -291,6 +302,10 @@ Containers::Optional<Containers::Array<char>> TgaImageConverter::doConvertToData
         }
     }
 
+    /* If we started with a RLE-encoded file, turn the array back into a
+       non-growable to avoid a dangling deleter on plugin unload */
+    if(rle) arrayShrink(data);
+
     /* GCC 4.8 needs extra help here */
     return Containers::optional(std::move(data));
 }

src/MagnumPlugins/TgaImageConverter/TgaImageConverter.h

@@ -91,10 +91,12 @@ information.
 
 @section Trade-TgaImageConverter-behavior Behavior and limitations
 
-The output is RLE-compressed by default, you can produce uncompressed files by
-disabling the @cb{.ini} rle @ce @ref Trade-TgaImageConverter-configuration "configuration option".
-Enabling @cb{.ini} rleAcrossScanlines @ce will result in even smaller files
-but [such files are considered invalid in the TGA 2.0 spec](https://en.wikipedia.org/wiki/Truevision_TGA#Specification_discrepancies)
+The output is RLE-compressed by default unless the uncompressed output is
+smaller than RLE. You can always produce uncompressed files by disabling the
+@cb{.ini} rle @ce @ref Trade-TgaImageConverter-configuration "configuration option", and always RLE files by disabling the
+@cb{.ini} rleFallbackIfLarger @ce option. Enabling
+@cb{.ini} rleAcrossScanlines @ce can result in even smaller files but
+[such files are considered invalid in the TGA 2.0 spec](https://en.wikipedia.org/wiki/Truevision_TGA#Specification_discrepancies)
 and thus may cause issues in certain importers.
 
 The TGA file format doesn't have a way to distinguish between 2D and 1D array