pbm: Fix for incorrect inverting

We didn't have thorough tests of pbm/pgm/ppm files, add some. That immediately uncovered a problem with pbp files. By spec, pbm (1 bit) files use 1 to mean black, 0 for white. Confusing, because pgm (grey) and ppm (rgb) use 0 for black.
AcademySoftwareFoundation · Dec 27, 2022 · 6626cf3 · 6626cf3
1 parent 070790e
commit 6626cf3
Show file tree

Hide file tree

Showing 10 changed files with 4,433 additions and 10 deletions.
diff --git a/src/pnm.imageio/pnminput.cpp b/src/pnm.imageio/pnminput.cpp
@@ -71,7 +71,8 @@ class PNMInput final : public ImageInput {
         return Strutil::parse_value(m_remaining, val);
     }
 
-    template<class T> bool ascii_to_raw(T* write, imagesize_t nvals, T max);
+    template<class T>
+    bool ascii_to_raw(T* write, imagesize_t nvals, T max, bool invert = false);
 };
 
 
@@ -111,19 +112,23 @@ invert(const T* read, T* write, imagesize_t nvals)
 
 template<class T>
 bool
-PNMInput::ascii_to_raw(T* write, imagesize_t nvals, T max)
+PNMInput::ascii_to_raw(T* write, imagesize_t nvals, T max, bool invert)
 {
-    if (max)
+    if (max) {
         for (imagesize_t i = 0; i < nvals; i++) {
             int tmp;
             if (!nextVal(tmp))
                 return false;
             write[i] = std::min((int)max, tmp) * std::numeric_limits<T>::max()
                        / max;
         }
-    else
+        if (invert)
+            for (imagesize_t i = 0; i < nvals; i++)
+                write[i] = std::numeric_limits<T>::max() - write[i];
+    } else {
         for (imagesize_t i = 0; i < nvals; i++)
             write[i] = std::numeric_limits<T>::max();
+    }
     return true;
 }
 
@@ -221,8 +226,7 @@ PNMInput::read_file_scanline(void* data, int y)
         //Ascii
         case P1:
             good &= ascii_to_raw((unsigned char*)data, nsamples,
-                                 (unsigned char)m_max_val);
-            invert((unsigned char*)data, (unsigned char*)data, nsamples);
+                                 (unsigned char)m_max_val, true);
             break;
         case P2:
         case P3:
@@ -306,8 +310,9 @@ PNMInput::read_file_header()
                                              : TypeDesc::UINT8);
         m_spec.attribute("pnm:binary",
                          (m_pnm_type >= P1 && m_pnm_type <= P3) ? 0 : 1);
-        m_spec.attribute("oiio:BitsPerSample",
-                         ceilf(logf(m_max_val + 1) / logf(2)));
+        int bps = int(ceilf(logf(m_max_val + 1) / logf(2)));
+        if (bps < 8)
+            m_spec.attribute("oiio:BitsPerSample", bps);
     } else {
         //Read scaling factor
         if (!nextVal(m_scaling_factor))

diff --git a/src/pnm.imageio/pnmoutput.cpp b/src/pnm.imageio/pnmoutput.cpp
@@ -68,7 +68,7 @@ bool
 PNMOutput::write_ascii_binary(const unsigned char* data, const stride_t stride)
 {
     for (int x = 0; x < m_spec.width; x++)
-        if (!iowritefmt("{}\n", data[x * stride] ? '1' : '0'))
+        if (!iowritefmt("{}\n", data[x * stride] ? '0' : '1'))
             return false;
     return true;
 }
@@ -81,7 +81,7 @@ PNMOutput::write_raw_binary(const unsigned char* data, const stride_t stride)
     for (int x = 0; x < m_spec.width;) {
         unsigned char val = 0;
         for (int bit = 7; bit >= 0 && x < m_spec.width; x++, bit--)
-            val += (data[x * stride] ? (1 << bit) : 0);
+            val += (data[x * stride] ? 0 : (1 << bit));
         if (!iowrite(&val, sizeof(val)))
             return false;
     }

diff --git a/testsuite/pnm/ref/out.txt b/testsuite/pnm/ref/out.txt
@@ -1,3 +1,53 @@
+Reading src/bw-ascii.pbm
+src/bw-ascii.pbm     :   16 x   16, 1 channel, uint1 pnm
+    SHA-1: 73AF19248E1B09BA9DDA7AC88B8A2E045645BA16
+    channel list: Y
+    oiio:BitsPerSample: 1
+    oiio:ColorSpace: "Rec709"
+    pnm:binary: 0
+Comparing "src/bw-ascii.pbm" and "bw-ascii.pbm"
+PASS
+Reading src/bw-binary.pbm
+src/bw-binary.pbm    :   32 x   32, 1 channel, uint1 pnm
+    SHA-1: 9760D82E4D9D6B8AD0C7FEB55DE02B710F95D954
+    channel list: Y
+    oiio:BitsPerSample: 1
+    oiio:ColorSpace: "Rec709"
+    pnm:binary: 1
+Comparing "src/bw-binary.pbm" and "bw-binary.pbm"
+PASS
+Reading src/grey-ascii.pgm
+src/grey-ascii.pgm   :   16 x   16, 1 channel, uint8 pnm
+    SHA-1: 87E59F779B4AD2157ED2FE5A278EBCE3604DE4CD
+    channel list: Y
+    oiio:ColorSpace: "Rec709"
+    pnm:binary: 0
+Comparing "src/grey-ascii.pgm" and "grey-ascii.pgm"
+PASS
+Reading src/grey-binary.pgm
+src/grey-binary.pgm  :   32 x   32, 1 channel, uint8 pnm
+    SHA-1: 8244B6CFD7C6FF251D9AC84A7F01CE6F69B0C1E8
+    channel list: Y
+    oiio:ColorSpace: "Rec709"
+    pnm:binary: 1
+Comparing "src/grey-binary.pgm" and "grey-binary.pgm"
+PASS
+Reading src/rgb-ascii.ppm
+src/rgb-ascii.ppm    :   16 x   16, 3 channel, uint8 pnm
+    SHA-1: 42CFBAACB3650C1FAD753A27DAE934DD96640ECC
+    channel list: R, G, B
+    oiio:ColorSpace: "Rec709"
+    pnm:binary: 0
+Comparing "src/rgb-ascii.ppm" and "rgb-ascii.ppm"
+PASS
+Reading src/rgb-binary.ppm
+src/rgb-binary.ppm   :   32 x   32, 3 channel, uint8 pnm
+    SHA-1: 38F36B1237AF6A57059F5E0DB167D4D786F252C8
+    channel list: R, G, B
+    oiio:ColorSpace: "Rec709"
+    pnm:binary: 0
+Comparing "src/rgb-binary.ppm" and "rgb-binary.ppm"
+PASS
 Reading ../oiio-images/pnm/test-1.pfm
 ../oiio-images/pnm/test-1.pfm :   64 x   64, 3 channel, float pnm
     SHA-1: ACEB5CA4B88F78E3344D79E7C8E16200FF434085

diff --git a/testsuite/pnm/run.py b/testsuite/pnm/run.py
@@ -2,6 +2,11 @@
 
 imagedir = OIIO_TESTSUITE_IMAGEDIR + "/pnm"
 
+for f in [ "bw-ascii.pbm", "bw-binary.pbm",
+           "grey-ascii.pgm", "grey-binary.pgm",
+           "rgb-ascii.ppm", "rgb-binary.ppm" ] :
+    command += rw_command ("src", f)
+
 # We can't yet write PFM files, so just get the hashes and call it a day
 files = [ "test-1.pfm", "test-2.pfm", "test-3.pfm" ]
 for f in files: