Refactor origin function to a Slice factory and Rgba custom utility

Instead of a general templated routine, have a Slice factory function
and then a custom Rgba utility function to clarify and avoid missing
strides, etc. when dealing with slices

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

OpenEXR/IlmImf/ImfFrameBuffer.cpp

@@ -74,6 +74,88 @@ Slice::Slice (PixelType t,
     // empty
 }
 
+Slice
+Slice::Make (
+    PixelType                   type,
+    const void*                 ptr,
+    const IMATH_NAMESPACE::V2i& origin,
+    int64_t                     w,
+    int64_t                     h,
+    size_t                      xStride,
+    size_t                      yStride,
+    int                         xSampling,
+    int                         ySampling,
+    double                      fillValue,
+    bool                        xTileCoords,
+    bool                        yTileCoords)
+{
+    char* base = reinterpret_cast<char*> (const_cast<void *> (ptr));
+    if (xStride == 0)
+    {
+        switch (type)
+        {
+            case UINT: xStride = sizeof (uint32_t); break;
+            case HALF: xStride = sizeof (uint16_t); break;
+            case FLOAT: xStride = sizeof (float); break;
+            case NUM_PIXELTYPES:
+                THROW (IEX_NAMESPACE::ArgExc, "Invalid pixel type.");
+        }
+    }
+    if (yStride == 0)
+        yStride = static_cast<size_t> (w / xSampling) * xStride;
+
+    // data window is an int, so force promote to higher type to avoid
+    // overflow for off y (degenerate size checks should be in
+    // ImfHeader::sanityCheck, but offset can be large-ish)
+    int64_t offx = (static_cast<int64_t> (origin.x) /
+                    static_cast<int64_t> (xSampling));
+    offx *= static_cast<int64_t> (xStride);
+
+    int64_t offy = (static_cast<int64_t> (origin.y) /
+                    static_cast<int64_t> (ySampling));
+    offy *= static_cast<int64_t> (yStride);
+
+    return Slice (
+        type,
+        base - offx - offy,
+        xStride,
+        yStride,
+        xSampling,
+        ySampling,
+        fillValue,
+        xTileCoords,
+        yTileCoords);
+}
+
+Slice
+Slice::Make (
+    PixelType                     type,
+    const void*                   ptr,
+    const IMATH_NAMESPACE::Box2i& dataWindow,
+    size_t                        xStride,
+    size_t                        yStride,
+    int                           xSampling,
+    int                           ySampling,
+    double                        fillValue,
+    bool                          xTileCoords,
+    bool                          yTileCoords)
+{
+    return Make (
+        type,
+        ptr,
+        dataWindow.min,
+        static_cast<int64_t> (dataWindow.max.x) -
+            static_cast<int64_t> (dataWindow.min.x) + 1,
+        static_cast<int64_t> (dataWindow.max.y) -
+            static_cast<int64_t> (dataWindow.min.y) + 1,
+        xStride,
+        yStride,
+        xSampling,
+        ySampling,
+        fillValue,
+        xTileCoords,
+        yTileCoords);
+}
 
 void
 FrameBuffer::insert (const char name[], const Slice &slice)

OpenEXR/IlmImf/ImfFrameBuffer.h

@@ -52,34 +52,11 @@
 
 #include <map>
 #include <string>
+#include <cstdint>
 
 
 OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_ENTER
 
-//-------------------------------------------------------
-// Utility to compute the origin-based pointer address
-//
-// With large offsets for the data window, the naive code
-// can wrap around, especially on 32-bit machines.
-// This can be used to avoid that
-//-------------------------------------------------------
-
-template <typename T>
-inline T *
-ComputeOriginPointer( T *p, const IMATH_NAMESPACE::Box2i &dw, int xSamp = 1, int ySamp = 1 )
-{
-    // data window is an int, so force promote to higher type to avoid
-    // overflow for off y (degenerate size checks should be in
-    // ImfHeader::sanityCheck, but offset can be large-ish)
-    long long basex = static_cast<long long>( dw.min.x );
-    long long w = static_cast<long long>( dw.max.x ) - basex + 1;
-    long long offy = ( static_cast<long long>( dw.min.y ) /
-                       static_cast<long long>( ySamp ) );
-    offy *= w / static_cast<long long>( xSamp );
-
-    return p - offy - ( basex / static_cast<long long>( xSamp ) );
-}
-
 //-------------------------------------------------------
 // Description of a single slice of the frame buffer:
 //
@@ -172,6 +149,36 @@ struct Slice
            double fillValue = 0.0,
            bool xTileCoords = false,
            bool yTileCoords = false);
+
+    // Does the heavy lifting of computing the base pointer for a slice,
+    // avoiding overflow issues with large origin offsets
+    //
+    // if xStride == 0, assumes sizeof(pixeltype)
+    // if yStride == 0, assumes xStride * ( w / xSampling )
+    static Slice Make(PixelType type,
+                      const void *ptr,
+                      const IMATH_NAMESPACE::V2i &origin,
+                      int64_t w,
+                      int64_t h,
+                      size_t xStride = 0,
+                      size_t yStride = 0,
+                      int xSampling = 1,
+                      int ySampling = 1,
+                      double fillValue = 0.0,
+                      bool xTileCoords = false,
+                      bool yTileCoords = false);
+    // same as above, just computes w and h for you
+    // from a data window
+    static Slice Make(PixelType type,
+                      const void *ptr,
+                      const IMATH_NAMESPACE::Box2i &dataWindow,
+                      size_t xStride = 0,
+                      size_t yStride = 0,
+                      int xSampling = 1,
+                      int ySampling = 1,
+                      double fillValue = 0.0,
+                      bool xTileCoords = false,
+                      bool yTileCoords = false);
 };
 
 

OpenEXR/IlmImf/ImfRgbaFile.h

@@ -60,6 +60,65 @@
 
 OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_ENTER
 
+//-------------------------------------------------------
+// Utility to compute the origin-based pointer address
+//
+// With large offsets for the data window, the naive code
+// can wrap around, especially on 32-bit machines.
+// This can be used to avoid that
+//-------------------------------------------------------
+
+inline const Rgba *
+ComputeBasePointer (
+    const Rgba*                 ptr,
+    const IMATH_NAMESPACE::V2i& origin,
+    int64_t                     w,
+    size_t                      xStride = 1,
+    size_t                      yStride = 0)
+{
+    if (yStride == 0)
+        yStride = w;
+    int64_t offx = static_cast<int64_t> (origin.x);
+    offx *= xStride;
+    int64_t offy = static_cast<int64_t> (origin.y);
+    offy *= yStride;
+    return ptr - offx - offy;
+}
+
+inline const Rgba *
+ComputeBasePointer (const Rgba* ptr, const IMATH_NAMESPACE::Box2i& dataWindow)
+{
+    return ComputeBasePointer (ptr, dataWindow.min,
+                         static_cast<int64_t> (dataWindow.max.x) -
+                          static_cast<int64_t> (dataWindow.min.x) + 1);
+}
+
+inline Rgba*
+ComputeBasePointer (
+    Rgba*                       ptr,
+    const IMATH_NAMESPACE::V2i& origin,
+    int64_t                     w,
+    size_t                      xStride = 1,
+    size_t                      yStride = 0)
+{
+    if (yStride == 0)
+        yStride = w;
+    int64_t offx = static_cast<int64_t> (origin.x);
+    offx *= xStride;
+    int64_t offy = static_cast<int64_t> (origin.y);
+    offy *= yStride;
+    return ptr - offx - offy;
+}
+
+inline Rgba*
+ComputeBasePointer (Rgba* ptr, const IMATH_NAMESPACE::Box2i& dataWindow)
+{
+    return ComputeBasePointer (
+        ptr,
+        dataWindow.min,
+        static_cast<int64_t> (dataWindow.max.x) -
+            static_cast<int64_t> (dataWindow.min.x) + 1);
+}
 
 //
 // RGBA output file.

OpenEXR/exr2aces/main.cpp

@@ -43,6 +43,7 @@
 
 #include <ImfAcesFile.h>
 #include <ImfArray.h>
+#include <ImfRgbaFile.h>
 #include <iostream>
 #include <exception>
 #include <string>
@@ -124,7 +125,7 @@ exr2aces (const char inFileName[],
 	height = dw.max.y - dw.min.y + 1;
 	p.resizeErase (height, width);
 
-	in.setFrameBuffer (ComputeOriginPointer (&p[0][0], dw), 1, width);
+	in.setFrameBuffer (ComputeBasePointer (&p[0][0], dw), 1, width);
 	in.readPixels (dw.min.y, dw.max.y);
     }
 
@@ -147,7 +148,7 @@ exr2aces (const char inFileName[],
 
 	AcesOutputFile out (outFileName, h, ch);
 
-	out.setFrameBuffer (ComputeOriginPointer (&p[0][0], dw), 1, width);
+	out.setFrameBuffer (ComputeBasePointer (&p[0][0], dw), 1, width);
 	out.writePixels (height);
     }
 }

OpenEXR/exrenvmap/readInputImage.cpp

@@ -194,7 +194,7 @@ readSixImages (const char inFileName[],
                    "from the data window of other cube faces.");
         }
 
-        in.setFrameBuffer (ComputeOriginPointer (pixels, dw), 1, w);
+        in.setFrameBuffer (ComputeBasePointer (pixels, dw), 1, w);
         in.readPixels (dw.min.y, dw.max.y);
 
         pixels += w * h;

OpenEXR/exrmakepreview/makePreview.cpp

@@ -110,7 +110,7 @@ generatePreview (const char inFileName[],
     int h = dw.max.y - dw.min.y + 1;
 
     Array2D <Rgba> pixels (h, w);
-    in.setFrameBuffer (ComputeOriginPointer (&pixels[0][0], dw), 1, w);
+    in.setFrameBuffer (ComputeBasePointer (&pixels[0][0], dw), 1, w);
     in.readPixels (dw.min.y, dw.max.y);
 
     //

OpenEXR/exrmaketiled/Image.h

@@ -191,10 +191,11 @@ TypedImageChannel<T>::slice () const
 {
     const IMATH_NAMESPACE::Box2i &dw = image().dataWindow();
 
-    return OPENEXR_IMF_INTERNAL_NAMESPACE::Slice (
+    return OPENEXR_IMF_INTERNAL_NAMESPACE::Slice::Make (
         pixelType(),
-        (char *) ComputeOriginPointer (&_pixels[0][0], dw),
-        sizeof (T), w * sizeof (T));
+        &_pixels[0][0],
+        dw,
+        sizeof (T));
 }
 
 

OpenEXR/exrmultiview/Image.h

@@ -203,13 +203,14 @@ TypedImageChannel<T>::slice () const
     const IMATH_NAMESPACE::Box2i &dw = image().dataWindow();
     int w = dw.max.x - dw.min.x + 1;
 
-    return IMF::Slice (pixelType(),
-                       (char *) IMF::ComputeOriginPointer(
-                           &_pixels[0][0], dw, _xSampling, _ySampling ),
-		       sizeof (T),
-		       (w / _xSampling) * sizeof (T),
-		       _xSampling,
-		       _ySampling);
+    return IMF::Slice::Make (
+        pixelType(),
+        &_pixels[0][0],
+        dw,
+        sizeof(T),
+        (w / _xSampling) * sizeof (T),
+        _xSampling,
+        _ySampling);
 }