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

@@ -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

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<unsigned char> 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,39 +192,39 @@ 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;
     }
 
     if (is_palette()
         && (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);
         }

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