Fix computation of chunk height (#2423)

* remove errant include

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* add test for combo of start line, lines per compression, y samples

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* fix compute of sampled height

When there are strange y_sampling, the number of lines per chunk was not
being computed correctly depending on how the start position lines up
with the sampling

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>

* Apply suggestion from @cary-ilm

remove debugging printf

Signed-off-by: Cary Phillips <cary@ilm.com>

* Apply suggestion from @cary-ilm

remove debugging printf

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Co-authored-by: Cary Phillips <cary@ilm.com>

Author: kdt3rd

src/lib/OpenEXRCore/internal_util.h

@@ -6,6 +6,7 @@
 #ifndef OPENEXR_PRIVATE_UTIL_H
 #define OPENEXR_PRIVATE_UTIL_H
 
+#include <stdio.h>
 #include <stdint.h>
 
 static inline int
@@ -19,25 +20,27 @@ compute_sampled_height (int height, int y_sampling, int start_y)
         nlines = (start_y % y_sampling) == 0 ? 1 : 0;
     else
     {
-        int start, end;
+        int off, tmph;
 
         /* computed the number of times y % ysampling == 0, by
          * computing interval based on first and last time that occurs
          * on the given range
          */
-        start = start_y % y_sampling;
-        if (start != 0)
-            start = start_y + (y_sampling - start);
+        if (start_y < 0)
+        {
+            off = -start_y % y_sampling;
+        }
         else
-            start = start_y;
+        {
+            off = start_y % y_sampling;
+            if (off != 0)
+                off = (y_sampling - off);
+        }
 
-        end = start_y + height - 1;
-        end -= (end < 0 ? -end : end) % y_sampling;
-
-        if (start > end)
-            nlines = start == start_y ? 1 : 0;
-        else
-            nlines = (end - start) / y_sampling + 1;
+        tmph = height - off;
+        if (tmph == 0) return 0;
+        --tmph;
+        nlines = tmph / y_sampling + 1;
     }
 
     return nlines;

src/lib/OpenEXRCore/parse_header.c

@@ -748,7 +748,7 @@ extract_attr_tiledesc (
 }
 
 /**************************************/
-#include <assert.h>
+
 static exr_result_t
 extract_attr_bytes(
     exr_context_t ctxt,

src/test/OpenEXRCoreTest/CMakeLists.txt

@@ -96,6 +96,7 @@ define_openexrcore_tests(
  testReadMultiPart
  testReadDeep
  testReadUnpack
+ testSamplingCalcs
 
  testWriteBadArgs
  testWriteBadFiles

src/test/OpenEXRCoreTest/main.cpp

@@ -178,6 +178,7 @@ main (int argc, char* argv[])
     TEST (testReadMultiPart, "core_read");
     TEST (testReadDeep, "core_read");
     TEST (testReadUnpack, "core_read");
+    TEST (testSamplingCalcs, "core_read");
 
     TEST (testWriteBadArgs, "core_write");
     TEST (testWriteBadFiles, "core_write");

src/test/OpenEXRCoreTest/read.cpp

@@ -616,3 +616,121 @@ testReadUnpack (const std::string& tempdir)
 
     exr_finish (&f);
 }
+
+#include "../../lib/OpenEXRCore/internal_util.h"
+
+static inline int
+compute_sampled_height_p (int height, int y_sampling, int start_y)
+{
+    int nlines;
+
+    if (y_sampling <= 1) return height;
+
+    if (height == 1)
+        nlines = (start_y % y_sampling) == 0 ? 1 : 0;
+    else
+    {
+        int off, tmph;
+
+        /* computed the number of times y % ysampling == 0, by
+         * computing interval based on first and last time that occurs
+         * on the given range
+         */
+        if (start_y < 0)
+        {
+            off = -start_y % y_sampling;
+        }
+        else
+        {
+            off = start_y % y_sampling;
+            if (off != 0)
+                off = (y_sampling - off);
+        }
+
+        tmph = height - off;
+        if (tmph == 0) return 0;
+        --tmph;
+        nlines = tmph / y_sampling + 1;
+    }
+
+    return nlines;
+}
+
+static inline int hardway_height (int height, int y_sampling, int start_y)
+{
+    int nlines = 0;
+    int end = start_y + height;
+
+    if (y_sampling <= 1) return height;
+
+    if (height == 1)
+        return (start_y % y_sampling) == 0 ? 1 : 0;
+
+    for ( int y = start_y; y < end; ++y )
+    {
+        if (y % y_sampling != 0) continue;
+        ++nlines;
+    }
+    return nlines;
+}
+
+static inline int hardway_height_p (int height, int y_sampling, int start_y)
+{
+    int nlines = 0;
+    int end = start_y + height;
+    int off = 0;
+
+    if (y_sampling <= 1) return height;
+
+    if (height == 1)
+        return (start_y % y_sampling) == 0 ? 1 : 0;
+
+    for ( int y = start_y; y < end; ++y )
+    {
+        if (y % y_sampling != 0)
+        {
+            if (nlines == 0) ++off;
+            continue;
+        }
+        ++nlines;
+    }
+    return nlines;
+}
+
+static void test_lpc( int samp, int h, int lpc )
+{
+    for ( int y = -100; y < 100; ++y )
+    {
+        int sy = y * samp;
+        int ey = sy + h - 1;
+        //printf( "============= %d - %d ============\n", sy, ey);
+        for ( int ty = sy; ty < ey; ty += lpc )
+        {
+            int th = ty + lpc;
+            if (th > ey) th = ey - ty + 1;
+            else th = lpc;
+
+            int nl = compute_sampled_height(th, samp, ty);
+            int hnl = hardway_height(th, samp, ty);
+            if (nl != hnl)
+            {
+                compute_sampled_height_p(th, samp, ty);
+                hardway_height_p(th, samp, ty);
+                printf( "ty %d h %d (%d) samp %d lpc %d => nl %d hnl %d\n",
+                        ty, th, h, samp, lpc, nl, hnl );
+                EXRCORE_TEST(nl == hnl);
+            }
+        }
+    }
+}
+
+void testSamplingCalcs (const std::string& tempdir)
+{
+    int lines[] = { 1, 16, 32, 256 };
+
+    for ( int s = 2; s < 11; ++s )
+    {
+        for ( int lpc = 0; lpc < 4; ++lpc )
+            test_lpc( s, 1000, lines[lpc] );
+    }
+}

src/test/OpenEXRCoreTest/read.h

@@ -22,4 +22,6 @@ void testReadMultiPart (const std::string& tempdir);
 
 void testReadUnpack (const std::string& tempdir);
 
+void testSamplingCalcs (const std::string& tempdir);
+
 #endif // OPENEXR_CORE_TEST_READ_H