Address CR comments.

Signed-off-by: Aaron Colwell <[email protected]>

src/libOpenImageIO/imagebuf.cpp

@@ -1419,19 +1419,22 @@ ImageBuf::write(ImageOutput* out, ProgressCallback progress_callback,
                               1024);
             std::unique_ptr<char[]> tmp(new char[chunk * slsize]);
 
+            // Special handling for flipped vertical scanline order. Right now, OpenEXR
+            // is the only format that allows it, so we special case it by name. For
+            // just one format, trying to be more general just seems even more awkward.
             const bool isDecreasingY = !strcmp(out->format_name(), "openexr")
                                        && outspec.get_string_attribute(
                                               "openexr:lineOrder")
                                               == "decreasingY";
-            const int yStart = isDecreasingY
-                                   ? ((outspec.height - 1) / chunk) * chunk
-                                   : 0;
-            const int yDelta = isDecreasingY ? -chunk : chunk;
+            const int numChunks  = outspec.height > 0
+                                       ? 1 + ((outspec.height - 1) / chunk)
+                                       : 0;
+            const int yLoopStart = isDecreasingY ? (numChunks - 1) * chunk : 0;
+            const int yDelta     = isDecreasingY ? -chunk : chunk;
+            const int yLoopEnd   = yLoopStart + numChunks * yDelta;
+
             for (int z = 0; z < outspec.depth; ++z) {
-                for (int y = yStart; ((isDecreasingY && y >= 0)
-                                      || (!isDecreasingY && y < outspec.height))
-                                     && ok;
-                     y += yDelta) {
+                for (int y = yLoopStart; y != yLoopEnd && ok; y += yDelta) {
                     int yend = std::min(y + outspec.y + chunk,
                                         outspec.y + outspec.height);
                     ok &= get_pixels(ROI(outspec.x, outspec.x + outspec.width,

src/libOpenImageIO/imageoutput.cpp

@@ -519,19 +519,22 @@ ImageOutput::write_image(TypeDesc format, const void* data, stride_t xstride,
         int chunk = std::max(1, (1 << 26) / int(m_spec.scanline_bytes(true)));
         chunk     = round_to_multiple(chunk, rps);
 
+        // Special handling for flipped vertical scanline order. Right now, OpenEXR
+        // is the only format that allows it, so we special case it by name. For
+        // just one format, trying to be more general just seems even more awkward.
         const bool isDecreasingY = !strcmp(format_name(), "openexr")
                                    && m_spec.get_string_attribute(
                                           "openexr:lineOrder")
                                           == "decreasingY";
-        const int yStart = isDecreasingY ? ((m_spec.height - 1) / chunk) * chunk
-                                         : 0;
-        const int yDelta = isDecreasingY ? -chunk : chunk;
+        const int numChunks  = m_spec.height > 0
+                                   ? 1 + ((m_spec.height - 1) / chunk)
+                                   : 0;
+        const int yLoopStart = isDecreasingY ? (numChunks - 1) * chunk : 0;
+        const int yDelta     = isDecreasingY ? -chunk : chunk;
+        const int yLoopEnd   = yLoopStart + numChunks * yDelta;
 
         for (int z = 0; z < m_spec.depth; ++z)
-            for (int y = yStart; ((isDecreasingY && y >= 0)
-                                  || (!isDecreasingY && y < m_spec.height))
-                                 && ok;
-                 y += yDelta) {
+            for (int y = yLoopStart; y != yLoopEnd && ok; y += yDelta) {
                 int yend      = std::min(y + m_spec.y + chunk,
                                          m_spec.y + m_spec.height);
                 const char* d = (const char*)data + z * zstride + y * ystride;

src/openexr.imageio/exroutput.cpp

@@ -1488,31 +1488,30 @@ OpenEXROutput::write_scanlines(int ybegin, int yend, int z, TypeDesc format,
     bool ok                  = true;
     const bool isDecreasingY = m_spec.get_string_attribute("openexr:lineOrder")
                                == "decreasingY";
-
-    int yChunkStart = isDecreasingY ? ybegin + ((yend - ybegin) / chunk) * chunk
-                                    : ybegin;
-    const int yDelta = isDecreasingY ? -chunk : chunk;
-    for (; ok
-           && ((isDecreasingY && yChunkStart >= ybegin)
-               || (!isDecreasingY && yChunkStart < yend));
-         yChunkStart += yDelta) {
-        int y1         = std::min(yChunkStart + chunk, yend);
-        int nscanlines = y1 - yChunkStart;
-
-        const void* dataStart = (const char*)data
-                                + (yChunkStart - ybegin) * ystride;
+    const int nAvailableScanLines = yend - ybegin;
+    const int numChunks           = nAvailableScanLines > 0
+                                        ? 1 + ((nAvailableScanLines - 1) / chunk)
+                                        : 0;
+    const int yLoopStart = isDecreasingY ? ybegin + (numChunks - 1) * chunk
+                                         : ybegin;
+    const int yDelta     = isDecreasingY ? -chunk : chunk;
+    const int yLoopEnd   = yLoopStart + numChunks * yDelta;
+    for (int y = yLoopStart; ok && y != yLoopEnd; y += yDelta) {
+        int y1         = std::min(y + chunk, yend);
+        int nscanlines = y1 - y;
+
+        const void* dataStart = (const char*)data + (y - ybegin) * ystride;
         const void* d = to_native_rectangle(m_spec.x, m_spec.x + m_spec.width,
-                                            yChunkStart, y1, z, z + 1, format,
-                                            dataStart, xstride, ystride,
-                                            zstride, m_scratch);
+                                            y, y1, z, z + 1, format, dataStart,
+                                            xstride, ystride, zstride,
+                                            m_scratch);
 
         // Compute where OpenEXR needs to think the full buffers starts.
         // OpenImageIO requires that 'data' points to where client stored
         // the bytes to be written, but OpenEXR's frameBuffer.insert() wants
         // where the address of the "virtual framebuffer" for the whole
         // image.
-        char* buf = (char*)d - m_spec.x * pixel_bytes
-                    - yChunkStart * scanlinebytes;
+        char* buf = (char*)d - m_spec.x * pixel_bytes - y * scanlinebytes;
         try {
             Imf::FrameBuffer frameBuffer;
             size_t chanoffset = 0;