From 2004751048e8535354ca2beadeba1cf8d7f7c6d5 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sun, 23 Jan 2022 23:09:38 -0800 Subject: [PATCH] Targa improvements: attribs, unassociated alpha handling, and more (#3279) The main event: * Assume that for a TGA 1.0 file (too old to have the header field that says if alpha is associated, unassociated, or meaningless), if the alpha is zero everywhere, it means that it's meaningless, not unassociated, so in that case we should skip the auto-premultiplication. (See issue 3277) Public-ish stuff: * Be more consistent with Targa-specific attributes all being called "targa:foo" (most were, but one was "tga:alpha_type"). * Document all the targa-specific attributes (a few were not). * Add "targa:version" to reveal whether the file was a TGA 1.0 or 2.0 version of the format. Internal stuff, opportunistic: * Some more minor conversions of string fomatting to fmt style. * Convert some std::cerr debugging code (commented out) to the DBG macro we use anywhere. --- src/doc/builtinplugins.rst | 35 +++++++++++++++ src/targa.imageio/targainput.cpp | 74 ++++++++++++++++++++------------ testsuite/targa/ref/out.txt | 19 ++++++-- 3 files changed, 96 insertions(+), 32 deletions(-) diff --git a/src/doc/builtinplugins.rst b/src/doc/builtinplugins.rst index b0c7d120c3..b5c22b8105 100644 --- a/src/doc/builtinplugins.rst +++ b/src/doc/builtinplugins.rst @@ -1995,9 +1995,20 @@ http://www.dca.fee.unicamp.br/~martino/disciplinas/ea978/tgaffs.pdf - string - values of ``none`` and ``rle`` are supported. The writer will use RLE compression if any unknown compression methods are requested. + * - ``targa:alpha_type`` + - int + - Meaning of any alpha channel (0 = none; 1 = undefined, ignore; + 2 = undefined, preserve; 3 = useful unassociated alpha; + 4 = useful associated alpha / premultiplied color). * - ``targa:ImageID`` - string - Image ID + * - ``targa:JobTime`` + - string + - Job time + * - ``targa:version`` + - int + - TGA file format version (1 or 2) * - ``PixelAspectRatio`` - float - pixel aspect ratio @@ -2017,6 +2028,30 @@ attributes ``"thumbnail_width"``, ``"thumbnail_height"``, and retrievable via `ImageInput::get_thumbnail()` or `ImageBuf::thumbnail()` or `ImageCache::get_thumbnail()`. +**Configuration settings for Targa input** + +When opening an Targa ImageInput with a *configuration* (see +Section :ref:`sec-input-with-config`), the following special configuration +attributes are supported: + +.. list-table:: + :widths: 30 10 65 + :header-rows: 1 + + * - Input Configuration Attribute + - Type + - Meaning + * - ``oiio:ioproxy`` + - ptr + - Pointer to a ``Filesystem::IOProxy`` that will handle the I/O, for + example by reading from memory rather than the file system. + * - ``oiio:UnassociatedAlpha`` + - int + - If nonzero, and the file contains unassociated alpha, this will + cause the reader to leave alpha unassociated (versus the default of + premultiplying color channels by alpha if the alpha channel is + unassociated). + **Configuration settings for Targa output** When opening a Targa ImageOutput, the following special metadata tokens diff --git a/src/targa.imageio/targainput.cpp b/src/targa.imageio/targainput.cpp index 3afaab24b0..60bda4b14b 100644 --- a/src/targa.imageio/targainput.cpp +++ b/src/targa.imageio/targainput.cpp @@ -59,6 +59,7 @@ class TGAInput final : public ImageInput { int64_t m_ofs_palette = 0; ///< Offset of palette int64_t m_ofs_colcorr_tbl = 0; ///< Offset to colour correction table bool m_keep_unassociated_alpha; ///< Do not convert unassociated alpha + short m_tga_version = 1; // TGA version (1 or 2) std::vector m_buf; ///< Buffer the image pixels // Is this a palette image, i.e. it has a color map? @@ -75,6 +76,7 @@ class TGAInput final : public ImageInput { m_ofs_colcorr_tbl = 0; m_alpha_type = TGA_ALPHA_NONE; m_keep_unassociated_alpha = false; + m_tga_version = 1; } /// Helper function: read the image. @@ -190,24 +192,24 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) return false; } if (m_tga.type == TYPE_NODATA) { - errorf("Image with no data"); + errorfmt("Image with no data"); return false; } if (m_tga.type != TYPE_PALETTED && m_tga.type != TYPE_RGB && m_tga.type != TYPE_GRAY && m_tga.type != TYPE_PALETTED_RLE && m_tga.type != TYPE_RGB_RLE && m_tga.type != TYPE_GRAY_RLE) { - errorf("Illegal image type: %d", m_tga.type); + errorfmt("Illegal image type: {}", m_tga.type); return false; } if (m_tga.bpp != 8 && m_tga.bpp != 15 && m_tga.bpp != 16 && m_tga.bpp != 24 && m_tga.bpp != 32) { - errorf("Illegal pixel size: %d bits per pixel", m_tga.bpp); + errorfmt("Illegal pixel size: {} bits per pixel", m_tga.bpp); return false; } if ((m_tga.type == TYPE_PALETTED || m_tga.type == TYPE_PALETTED_RLE) && !is_palette()) { - errorf("Palette image with no palette"); + errorfmt("Palette image with no palette"); return false; } @@ -215,14 +217,14 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) && (m_tga.type == TYPE_GRAY || m_tga.type == TYPE_GRAY_RLE)) { // it should be an error for TYPE_RGB* as well, but apparently some // *very* old TGAs can be this way, so we'll hack around it - errorf("Palette defined for grayscale image"); + errorfmt("Palette defined for grayscale image"); return false; } if (is_palette() && (m_tga.cmap_size != 15 && m_tga.cmap_size != 16 && m_tga.cmap_size != 24 && m_tga.cmap_size != 32)) { - errorf("Illegal palette entry size: %d bits", m_tga.cmap_size); + errorfmt("Illegal palette entry size: {} bits", m_tga.cmap_size); return false; } @@ -285,6 +287,8 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) && fread(&m_foot.signature, sizeof(m_foot.signature), 1) && !strncmp(m_foot.signature, "TRUEVISION-XFILE.", 17)) { //std::cerr << "[tga] this is a TGA 2.0 file\n"; + m_spec.attribute("targa:version", 2); + m_tga_version = 2; // read the extension area if (!fseek(m_foot.ofs_ext)) { @@ -410,8 +414,7 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) m_spec.attribute("oiio:ColorSpace", "linear"); } else { m_spec.attribute("oiio:ColorSpace", - Strutil::sprintf("GammaCorrected%.2g", - gamma)); + Strutil::fmt::format("Gamma{:.2g}", gamma)); m_spec.attribute("oiio:Gamma", gamma); } } @@ -440,7 +443,7 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) return false; m_alpha_type = (tga_alpha_type)buf.c[0]; if (m_alpha_type) - m_spec.attribute("tga:alpha_type", m_alpha_type); + m_spec.attribute("targa:alpha_type", m_alpha_type); // Check for presence of a thumbnail and set the metadata that // says its dimensions, but don't read and decode it unless @@ -462,11 +465,14 @@ TGAInput::open(const std::string& name, ImageSpec& newspec) // FIXME: provide access to the developer area; according to Larry, // it's probably safe to ignore it altogether until someone complains // that it's missing :) + } else { + m_tga_version = 1; + m_spec.attribute("targa:version", 1); } - if (m_spec.alpha_channel != -1 && m_alpha_type == TGA_ALPHA_USEFUL) - if (m_keep_unassociated_alpha) - m_spec.attribute("oiio:UnassociatedAlpha", 1); + if (m_spec.alpha_channel != -1 && m_alpha_type == TGA_ALPHA_USEFUL + && m_keep_unassociated_alpha) + m_spec.attribute("oiio:UnassociatedAlpha", 1); // Reposition back to where the palette starts if (!fseek(ofs)) { @@ -687,7 +693,6 @@ associateAlpha(T* data, int64_t size, int channels, int alpha_channel, bool TGAInput::readimg() { - DBG("TGA readimg {}\n", m_filename); // how many bytes we actually read // for 15-bit read 2 bytes and ignore the 16th bit int bytespp = (m_tga.bpp == 15) ? 2 : (m_tga.bpp / 8); @@ -696,10 +701,8 @@ TGAInput::readimg() if (alphabits == 0 && m_tga.bpp == 32) alphabits = 8; - /*std::cerr << "[tga] bytespp = " << bytespp - << " palbytespp = " << palbytespp - << " alphabits = " << alphabits - << "\n";*/ + DBG("TGA readimg {}, bytespp = {} palbytespp = {} alphabits = {}\n", + m_filename, bytespp, palbytespp, alphabits); try { DBG("TGA {} allocating for {}x{} {}-chan image = {}\n", m_filename, @@ -720,9 +723,10 @@ TGAInput::readimg() return false; } - unsigned char pixel[4]; + unsigned char pixel[4] = { 0, 0, 0, 0 }; if (m_tga.type < TYPE_PALETTED_RLE) { // uncompressed image data + DBG("TGA readimg, reading uncompressed image data\n"); unsigned char in[4]; for (int64_t y = m_spec.height - 1; y >= 0; y--) { for (int64_t x = 0; x < m_spec.width; x++) { @@ -737,6 +741,7 @@ TGAInput::readimg() } else { // Run Length Encoded image unsigned char in[5]; + DBG("TGA readimg, reading RLE image data\n"); int packet_size; for (int64_t y = m_spec.height - 1; y >= 0; y--) { for (int64_t x = 0; x < m_spec.width; x++) { @@ -747,8 +752,8 @@ TGAInput::readimg() packet_size = 1 + (in[0] & 0x7f); decode_pixel(&in[1], pixel, palette.get(), bytespp, palbytespp); if (in[0] & 0x80) { // run length packet - // std::cerr << "[tga] run length packet " - // << packet_size << "\n"; + // DBG("[tga] run length packet size {} @ ({},{})\n", + // packet_size, x, y); for (int i = 0; i < packet_size; i++) { memcpy(&m_buf[y * m_spec.width * m_spec.nchannels + x * m_spec.nchannels], @@ -766,8 +771,9 @@ TGAInput::readimg() } } } else { // non-rle packet - // std::cerr << "[tga] non-run length packet " - // << packet_size << "\n"; + // DBG("[tga] non-run length packet size {} @ ({},{}): [{:d} {:d} {:d} {:d}]\n", + // packet_size, x, y, pixel[0], pixel[1], pixel[2], + // pixel[3]); for (int i = 0; i < packet_size; i++) { memcpy(&m_buf[y * m_spec.width * m_spec.nchannels + x * m_spec.nchannels], @@ -789,6 +795,8 @@ TGAInput::readimg() } decode_pixel(&in[1], pixel, palette.get(), bytespp, palbytespp); + // DBG("\t\t@ ({},{}): [{:d} {:d} {:d} {:d}]\n", x, y, + // pixel[0], pixel[1], pixel[2], pixel[3]); } } } @@ -833,12 +841,22 @@ TGAInput::readimg() } } - if (m_alpha_type != TGA_ALPHA_PREMULTIPLIED) { - // Convert to associated unless we were requested not to do so - if (m_spec.alpha_channel != -1 && !m_keep_unassociated_alpha) { - int64_t size = m_spec.image_pixels(); - float gamma = m_spec.get_float_attribute("oiio:Gamma", 1.0f); - + // Convert to associated unless we were requested not to do so. + if (m_spec.alpha_channel != -1 && !m_keep_unassociated_alpha + && m_alpha_type != TGA_ALPHA_PREMULTIPLIED) { + // TGA 1.0 files don't have a way to indicate that the alpha is not + // premultiplied. We presume unpremultiplied, but if alpha is zero + // everywhere, ugh, it's probably meaningless. + bool alpha0_everywhere = (m_tga_version == 1); + int64_t size = m_spec.image_pixels(); + for (int64_t i = 0; i < size; ++i) { + if (m_buf[i * m_spec.nchannels + m_spec.alpha_channel]) { + alpha0_everywhere = false; + break; + } + } + if (!alpha0_everywhere) { + float gamma = m_spec.get_float_attribute("oiio:Gamma", 1.0f); associateAlpha((unsigned char*)m_buf.data(), size, m_spec.nchannels, m_spec.alpha_channel, gamma); } diff --git a/testsuite/targa/ref/out.txt b/testsuite/targa/ref/out.txt index 7205b14353..69c9f22f47 100644 --- a/testsuite/targa/ref/out.txt +++ b/testsuite/targa/ref/out.txt @@ -13,6 +13,7 @@ Reading ../oiio-images/targa/CBW8.TGA thumbnail_width: 64 oiio:BitsPerSample: 8 targa:ImageID: "Truevision(R) Sample Image" + targa:version: 2 Comparing "../oiio-images/targa/CBW8.TGA" and "CBW8.TGA" PASS Reading ../oiio-images/targa/CCM8.TGA @@ -30,6 +31,7 @@ Reading ../oiio-images/targa/CCM8.TGA thumbnail_width: 64 oiio:BitsPerSample: 2 targa:ImageID: "Truevision(R) Sample Image" + targa:version: 2 Comparing "../oiio-images/targa/CCM8.TGA" and "CCM8.TGA" PASS Reading ../oiio-images/targa/CTC16.TGA @@ -46,8 +48,9 @@ Reading ../oiio-images/targa/CTC16.TGA thumbnail_nchannels: 3 thumbnail_width: 64 oiio:BitsPerSample: 5 + targa:alpha_type: 2 targa:ImageID: "Truevision(R) Sample Image" - tga:alpha_type: 2 + targa:version: 2 Comparing "../oiio-images/targa/CTC16.TGA" and "CTC16.TGA" PASS Reading ../oiio-images/targa/CTC24.TGA @@ -65,6 +68,7 @@ Reading ../oiio-images/targa/CTC24.TGA thumbnail_width: 64 oiio:BitsPerSample: 8 targa:ImageID: "Truevision(R) Sample Image" + targa:version: 2 Comparing "../oiio-images/targa/CTC24.TGA" and "CTC24.TGA" PASS Reading ../oiio-images/targa/CTC32.TGA @@ -81,8 +85,9 @@ Reading ../oiio-images/targa/CTC32.TGA thumbnail_nchannels: 4 thumbnail_width: 64 oiio:BitsPerSample: 8 + targa:alpha_type: 2 targa:ImageID: "Truevision(R) Sample Image" - tga:alpha_type: 2 + targa:version: 2 Comparing "../oiio-images/targa/CTC32.TGA" and "CTC32.TGA" PASS Reading ../oiio-images/targa/UBW8.TGA @@ -99,6 +104,7 @@ Reading ../oiio-images/targa/UBW8.TGA thumbnail_width: 64 oiio:BitsPerSample: 8 targa:ImageID: "Truevision(R) Sample Image" + targa:version: 2 Comparing "../oiio-images/targa/UBW8.TGA" and "UBW8.TGA" PASS Reading ../oiio-images/targa/UCM8.TGA @@ -115,6 +121,7 @@ Reading ../oiio-images/targa/UCM8.TGA thumbnail_width: 64 oiio:BitsPerSample: 2 targa:ImageID: "Truevision(R) Sample Image" + targa:version: 2 Comparing "../oiio-images/targa/UCM8.TGA" and "UCM8.TGA" PASS Reading ../oiio-images/targa/UTC16.TGA @@ -130,8 +137,9 @@ Reading ../oiio-images/targa/UTC16.TGA thumbnail_nchannels: 3 thumbnail_width: 64 oiio:BitsPerSample: 5 + targa:alpha_type: 2 targa:ImageID: "Truevision(R) Sample Image" - tga:alpha_type: 2 + targa:version: 2 Comparing "../oiio-images/targa/UTC16.TGA" and "UTC16.TGA" PASS Reading ../oiio-images/targa/UTC24.TGA @@ -148,6 +156,7 @@ Reading ../oiio-images/targa/UTC24.TGA thumbnail_width: 64 oiio:BitsPerSample: 8 targa:ImageID: "Truevision(R) Sample Image" + targa:version: 2 Comparing "../oiio-images/targa/UTC24.TGA" and "UTC24.TGA" PASS Reading ../oiio-images/targa/UTC32.TGA @@ -163,8 +172,9 @@ Reading ../oiio-images/targa/UTC32.TGA thumbnail_nchannels: 4 thumbnail_width: 64 oiio:BitsPerSample: 8 + targa:alpha_type: 2 targa:ImageID: "Truevision(R) Sample Image" - tga:alpha_type: 2 + targa:version: 2 Comparing "../oiio-images/targa/UTC32.TGA" and "UTC32.TGA" PASS Reading ../oiio-images/targa/round_grill.tga @@ -173,6 +183,7 @@ Reading ../oiio-images/targa/round_grill.tga channel list: R, G, B, A Software: "COREL 0.0" oiio:BitsPerSample: 8 + targa:version: 2 Comparing "../oiio-images/targa/round_grill.tga" and "round_grill.tga" PASS Converting src/crash1.tga to crash1.exr