more thorough test and some fixes

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Author: lgritz

src/include/OpenImageIO/image_span.h

@@ -205,7 +205,8 @@ template<typename T, size_t Rank = 4> class image_span {
     /// un-typed memory.
     image_span<const std::byte> as_bytes_image_span() const noexcept
     {
-        return image_span<const std::byte>(static_cast<const std::byte*>(m_data),
+        return image_span<const std::byte>(reinterpret_cast<const std::byte*>(
+                                               m_data),
                                            nchannels(), width(), height(),
                                            depth(), chanstride(), xstride(),
                                            ystride(), zstride(), m_chansize);
@@ -216,7 +217,7 @@ template<typename T, size_t Rank = 4> class image_span {
     /// Note that this will not work (be a compiler error) if T a const type.
     image_span<std::byte> as_writable_bytes_image_span() const noexcept
     {
-        return image_span<std::byte>(static_cast<std::byte*>(m_data),
+        return image_span<std::byte>(reinterpret_cast<std::byte*>(m_data),
                                      nchannels(), width(), height(), depth(),
                                      chanstride(), xstride(), ystride(),
                                      zstride(), m_chansize);

src/libOpenImageIO/image_span_test.cpp

@@ -93,28 +93,72 @@ test_image_span()
 
 
 
+template<typename T>
+T
+testvalue(int x, int y, int z, int c)
+{
+    return T(x + y + z + c);
+}
+
+
+// Fill an image span in a characteristic way
+template<typename T>
+void
+fill_image_span(image_span<T> img)
+{
+    // Fill the image with a constant value
+    for (uint32_t z = 0; z < img.depth(); ++z) {
+        for (uint32_t y = 0; y < img.height(); ++y) {
+            for (uint32_t x = 0; x < img.width(); ++x) {
+                for (uint32_t c = 0; c < img.nchannels(); ++c) {
+                    img(x, y, z)[c] = testvalue<T>(x, y, z, c);
+                }
+            }
+        }
+    }
+}
+
+
+// Check that an image span in the characteristic way
+template<typename T>
+bool
+check_image_span(image_span<T> img, int xoff = 0, int yoff = 0, int zoff = 0)
+{
+    // Fill the image with a constant value
+    for (uint32_t z = 0; z < img.depth(); ++z) {
+        for (uint32_t y = 0; y < img.height(); ++y) {
+            for (uint32_t x = 0; x < img.width(); ++x) {
+                for (uint32_t c = 0; c < img.nchannels(); ++c) {
+                    auto v = testvalue<T>(x + xoff, y + yoff, z + zoff, c);
+                    OIIO_CHECK_EQUAL(img(x, y, z)[c], v);
+                    if (img(x, y, z)[c] != v) {
+                        print("\tError at ({}, {}, {})[{}]\n", x, y, z, c);
+                        return false;
+                    }
+                }
+            }
+        }
+    }
+    return true;
+}
+
+
+
 template<typename T = float>
 void
-benchmark_image_span_copy_image()
+test_image_span_copy_image()
 {
-    // Benchmark old (ptr) versus new (span) copy_image functions
-    Benchmarker bench;
-    bench.units(Benchmarker::Unit::us);
     const int xres = 2048, yres = 1536, nchans = 4;
-    std::vector<T> sbuf(xres * yres * nchans);
-    std::vector<T> dbuf(xres * yres * nchans);
     const size_t chansize = sizeof(T);
-
-    print("Benchmarking copy_image {} (total {} MB):\n",
-          TypeDescFromC<T>::value(),
+    print("\nTesting copy_image {} (total {} MB):\n", TypeDescFromC<T>::value(),
           xres * yres * nchans * chansize * 3 / 4 / 1024 / 1024);
 
     // We test different amounts of contiguity. Each test copies 3/4 of the
     // total image, to keep the total number of bytes copied identical.
     const stride_t src_xstride(chansize * nchans);
     const stride_t src_ystride(src_xstride * xres);
     for (int i = 0; i < 3; ++i) {
-        size_t nc(nchans), pixsize(chansize * nchans), w(xres), h(yres);
+        size_t nc(nchans), w(xres), h(yres);
         std::string label;
         if (i == 0) {
             // Fully contiguous region -- copy 3/4 of the image.
@@ -129,14 +173,31 @@ benchmark_image_span_copy_image()
             label = "contig pixels";
             nc    = nc * 3 / 4;
         }
-        bench(Strutil::format("  copy_image image_span {}", label), [&]() {
-            copy_image(image_span<T>(dbuf.data(), nc, w, h, 1, chansize,
-                                     src_xstride, src_ystride, AutoStride,
-                                     chansize),
-                       image_span<const T>(sbuf.data(), nc, w, h, 1));
-        });
-        bench(Strutil::format("  copy_image raw ptrs   {}", label), [&]() {
-            copy_image(nc, w, h, 1, sbuf.data(), pixsize, src_xstride,
+
+        print("  test image_span copy_image {}\n", label);
+        std::vector<T> sbuf(xres * yres * nchans);
+        std::vector<T> dbuf(w * h * nc);
+
+        // Spans for src and dst -- src has the "original" strides, dst
+        // has contiguous strides.
+        image_span<T> sispan(sbuf.data(), nc, w, h, 1, chansize, src_xstride,
+                             src_ystride, AutoStride);
+        image_span<T> dispan(dbuf.data(), nc, w, h, 1);
+
+        // Test correctness
+        fill_image_span(sispan);
+        copy_image(dispan, sispan);
+        OIIO_CHECK_ASSERT(check_image_span(dispan));
+
+        // Benchmark old (ptr) versus new (span) copy_image functions
+        Benchmarker bench;
+        bench.units(Benchmarker::Unit::us);
+
+        bench(Strutil::format("    copy_image image_span {}", label),
+              [&]() { copy_image(dispan, sispan); });
+        // Test equivalent version with pointers
+        bench(Strutil::format("    copy_image raw ptrs   {}", label), [&]() {
+            copy_image(nc, w, h, 1, sbuf.data(), nc * chansize, src_xstride,
                        src_ystride, AutoStride, dbuf.data(), AutoStride,
                        AutoStride, AutoStride);
         });
@@ -147,23 +208,19 @@ benchmark_image_span_copy_image()
 
 template<typename T = float>
 void
-benchmark_image_span_contiguize()
+test_image_span_contiguize()
 {
     // Benchmark old (ptr) versus new (span) contiguize functions
     using pvt::contiguize;
 
-    // Benchmark old (ptr) versus new (span) contiguize functions
-    Benchmarker bench;
-    bench.units(Benchmarker::Unit::us);
     const int xres = 2048, yres = 1536, nchans = 4;
-    std::vector<T> sbuf(xres * yres * nchans);
-    std::vector<T> dbuf(xres * yres * nchans);
     const size_t chansize = sizeof(T);
-
-    print("Benchmarking contiguize {} (total {} MB):\n",
-          TypeDescFromC<T>::value(),
+    print("\nTesting contiguize {} (total {} MB):\n", TypeDescFromC<T>::value(),
           xres * yres * nchans * chansize * 3 / 4 / 1024 / 1024);
 
+    // std::vector<T> sbuf(xres * yres * nchans);
+    // std::vector<T> dbuf(xres * yres * nchans);
+
     // We test different amounts of contiguity. Each test copies 3/4 of the
     // total image, to keep the total number of bytes copied identical.
     const stride_t src_xstride(chansize * nchans);
@@ -184,15 +241,35 @@ benchmark_image_span_contiguize()
             label = "contig pixels";
             nc    = nc * 3 / 4;
         }
-        bench(Strutil::format("  contiguize image_span {}", label), [&]() {
-            auto r = contiguize(image_span(reinterpret_cast<const std::byte*>(
-                                               sbuf.data()),
-                                           nc, w, h, 1, chansize, src_xstride,
-                                           src_ystride, AutoStride, chansize),
+
+        print("  test image_span contiguize {}\n", label);
+        std::vector<T> sbuf(xres * yres * nchans, T(100));
+        std::vector<T> dbuf(w * h * nc, T(100));
+
+        // Spans for src and dst -- src has the "original" strides, dst
+        // has contiguous strides.
+        image_span<T> sispan(sbuf.data(), nc, w, h, 1, chansize, src_xstride,
+                             src_ystride, AutoStride);
+        image_span<T> dispan(dbuf.data(), nc, w, h, 1);
+
+        // Test correctness
+        fill_image_span(sispan);
+        auto rspan = contiguize(sispan.as_bytes_image_span(),
+                                as_writable_bytes(make_span(dbuf)));
+        OIIO_CHECK_ASSERT(check_image_span(
+            image_span<const T>(reinterpret_cast<const T*>(rspan.data()), nc, w,
+                                h, 1)));
+
+        // Benchmark old (ptr) versus new (span) contiguize functions
+        Benchmarker bench;
+        bench.units(Benchmarker::Unit::us);
+
+        bench(Strutil::format("    contiguize image_span {}", label), [&]() {
+            auto r = contiguize(sispan.as_writable_bytes_image_span(),
                                 as_writable_bytes(make_span(dbuf)));
             OIIO_ASSERT(r.size_bytes() == nc * w * h * sizeof(T));
         });
-        bench(Strutil::format("  contiguize raw ptrs   {}", label), [&]() {
+        bench(Strutil::format("    contiguize raw ptrs   {}", label), [&]() {
             contiguize(sbuf.data(), nc, src_xstride, src_ystride,
                        src_ystride * h, dbuf.data(), w, h, 1,
                        TypeDescFromC<T>::value());
@@ -212,13 +289,13 @@ main(int /*argc*/, char* /*argv*/[])
     test_image_span<uint8_t>();
     test_image_span<const uint8_t>();
 
-    benchmark_image_span_copy_image<float>();
-    benchmark_image_span_copy_image<uint16_t>();
-    benchmark_image_span_copy_image<uint8_t>();
+    test_image_span_copy_image<float>();
+    test_image_span_copy_image<uint16_t>();
+    test_image_span_copy_image<uint8_t>();
 
-    benchmark_image_span_contiguize<float>();
-    benchmark_image_span_contiguize<uint16_t>();
-    benchmark_image_span_contiguize<uint8_t>();
+    test_image_span_contiguize<float>();
+    test_image_span_contiguize<uint16_t>();
+    test_image_span_contiguize<uint8_t>();
 
     return unit_test_failures;
 }

src/libOpenImageIO/imageio.cpp

@@ -750,6 +750,7 @@ pvt::contiguize(image_span<const std::byte> src, span<std::byte> dst)
         image_span dstspan(dst.data(), src.nchannels(), src.width(),
                            src.height(), src.depth(), AutoStride, AutoStride,
                            AutoStride, AutoStride, src.chansize());
+        OIIO_DASSERT(dstspan.size_bytes() == src.size_bytes());
         copy_image(dstspan, src);
         return make_cspan(dst.data(), src.size_bytes());
     }