[gimp] Issue #3493: GIMP changes R-channel, when it should not.



commit b090bc521323d6a834c9c1e98d3c5dbd601f7d37
Author: Jehan <jehan girinstud io>
Date:   Tue Jun 11 15:33:15 2019 +0200

    Issue #3493: GIMP changes R-channel, when it should not.
    
    Our TIFF loading code was not taking into account the case when extra
    channels were stored in the TIFF file while ExtraSamples field is not
    set. Yet as a side effect of a later channel count, we were setting
    `alpha` to TRUE while `save_transp_pixels` was left uninitialized (hence
    undefined behavior).
    
    For now let's make sure we have no undefined behavior. When the
    ExtraSamples field is missing and at least one extra channel is stored,
    we will consider the first extra channel as non-premultiplied alpha
    (this is also what we were doing when ExtraSamples was set to
    "Unspecified data" and apparently according to Massimo, it would be a
    common behavior in other software).
    
    Note that it is an improvement from previous code (no undefined
    behaviour anymore, instead we handle explicitly the TIFF error). Yet
    this is not perfect yet. Ideally we should pop-up a dialog asking what
    to do with this extra channel: either open as a channel (no alpha), or
    as premultiplied or non-premultiplied alpha.

 plug-ins/file-tiff/file-tiff-io.c   | 10 ++++++++
 plug-ins/file-tiff/file-tiff-load.c | 46 +++++++++++++++++++++++++++----------
 2 files changed, 44 insertions(+), 12 deletions(-)
---
diff --git a/plug-ins/file-tiff/file-tiff-io.c b/plug-ins/file-tiff/file-tiff-io.c
index 62b991b84f..2405a8970e 100644
--- a/plug-ins/file-tiff/file-tiff-io.c
+++ b/plug-ins/file-tiff/file-tiff-io.c
@@ -176,6 +176,16 @@ tiff_io_warning (const gchar *module,
       g_printerr ("%s\n", msg);
       g_free (msg);
 
+      return;
+    }
+  else if (! strcmp (module, "TIFFReadDirectory") &&
+           ! strcmp (fmt,
+                     "Sum of Photometric type-related color channels and ExtraSamples doesn't match 
SamplesPerPixel."
+                     " Defining non-color channels as ExtraSamples."))
+    {
+      /* We will process this issue in our code. Just report to stderr. */
+      g_printerr ("%s: [%s] %s\n", G_STRFUNC, module, fmt);
+
       return;
     }
 
diff --git a/plug-ins/file-tiff/file-tiff-load.c b/plug-ins/file-tiff/file-tiff-load.c
index 4cf49688b5..d1cc064b09 100644
--- a/plug-ins/file-tiff/file-tiff-load.c
+++ b/plug-ins/file-tiff/file-tiff-load.c
@@ -448,21 +448,43 @@ load_image (GFile              *file,
           tsvals.save_transp_pixels = TRUE;
           extra--;
         }
-      else
+      else /* extra == 0 */
         {
-          alpha = FALSE;
+          /* ExtraSamples field not set, yet we have more channels than
+           * the PhotometricInterpretation field suggests.
+           * This should not happen as the spec clearly says "This field
+           * must be present if there are extra samples". So the files
+           * can be considered non-conformant. For now, let's consider
+           * the first extra field as alpha non-premultiplied.
+           *
+           * TODO: we should rather pop-up a dialog asking what to do:
+           * 1/ open as channel;
+           * 2/ use as premultiplied alpha;
+           * 3/ use as non-premultiplied alpha.
+           */
+          if ((photomet == PHOTOMETRIC_RGB && spp > 3) ||
+              /* All other color space expect 1 channel (grayscale,
+               * palette, mask). */
+              (photomet != PHOTOMETRIC_RGB && spp > 1))
+            {
+              /* assuming unassociated alpha if unspecified */
+              g_message ("File '%s' does not conform to TIFF specification: "
+                         "ExtraSamples field is not set while extra channels are present. "
+                         "Assuming the first extra channel as non-premultiplied alpha.",
+                         gimp_file_get_utf8_name (file));
+              alpha = TRUE;
+              tsvals.save_transp_pixels = TRUE;
+            }
+          else
+            {
+              alpha = FALSE;
+            }
         }
 
-      if (photomet == PHOTOMETRIC_RGB && spp > 3 + extra)
-        {
-          alpha = TRUE;
-          extra = spp - 4;
-        }
-      else if (photomet != PHOTOMETRIC_RGB && spp > 1 + extra)
-        {
-          alpha = TRUE;
-          extra = spp - 2;
-        }
+      if (photomet == PHOTOMETRIC_RGB && spp > 3 + (alpha ? 1 : 0) + extra)
+        extra = spp - 3 - (alpha ? 1 : 0);
+      else if (photomet != PHOTOMETRIC_RGB && spp > 1 + (alpha ? 1 : 0) + extra)
+        extra = spp - 1 - (alpha ? 1 : 0);
 
       is_bw = FALSE;
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]