Add HioImage unit test, and fix issues it exposed.

(Internal change: 2331014)

pxr/imaging/hio/CMakeLists.txt

@@ -40,8 +40,8 @@ pxr_library(hio
 
     CPPFILES
         OpenEXRImage.cpp
-        stbImage.cpp
         OpenEXR/openexr-c.c
+        stbImage.cpp
 
     RESOURCE_FILES
         plugInfo.json
@@ -50,3 +50,17 @@ pxr_library(hio
         overview.dox
 )
 
+pxr_build_test(testHioImage
+    LIBRARIES
+        ar
+        hio
+        plug
+        tf
+
+   CPPFILES
+        testenv/testHioImage.cpp
+)
+
+pxr_register_test(testHioImage
+    COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testHioImage"
+)

pxr/imaging/hio/OpenEXR/OpenEXRCore/chunk.c

@@ -681,7 +681,7 @@ compute_chunk_unpack_size (
             chansz *= (uint64_t) width;
             if (curc->x_sampling > 1) chansz /= ((uint64_t) curc->x_sampling);
             chansz *=
-                (uint64_t) compute_sampled_lines (height, curc->y_sampling, y);
+                (uint64_t) compute_sampled_height (height, curc->y_sampling, y);
             unpacksize += chansz;
         }
     }
@@ -1723,7 +1723,7 @@ exr_write_scanline_chunk_info (
     cinfo->data_offset              = 0;
     cinfo->packed_size              = 0;
     cinfo->unpacked_size =
-        compute_chunk_unpack_size (y, cinfo->width, cinfo->height, lpc, part);
+        compute_chunk_unpack_size (dw.min.y, cinfo->width, cinfo->height, lpc, part);
 
     return EXR_UNLOCK_AND_RETURN_PCTXT (EXR_ERR_SUCCESS);
 }

pxr/imaging/hio/OpenEXR/OpenEXRCore/coding.c

@@ -41,13 +41,10 @@ internal_coding_fill_channel_info (
 
         decc->channel_name = curc->name.str;
 
-        decc->height = compute_sampled_lines (
+        decc->height = compute_sampled_height (
             cinfo->height, curc->y_sampling, cinfo->start_y);
-
-        if (curc->x_sampling > 1)
-            decc->width = cinfo->width / curc->x_sampling;
-        else
-            decc->width = cinfo->width;
+        decc->width = compute_sampled_width (
+            cinfo->width, curc->x_sampling, cinfo->start_x);
 
         decc->x_samples         = curc->x_sampling;
         decc->y_samples         = curc->y_sampling;
@@ -99,13 +96,11 @@ internal_coding_update_channel_info (
 
         ccic->channel_name = curc->name.str;
 
-        ccic->height = compute_sampled_lines (
+        ccic->height = compute_sampled_height (
             cinfo->height, curc->y_sampling, cinfo->start_y);
+        ccic->width = compute_sampled_width (
+            cinfo->width, curc->x_sampling, cinfo->start_x);
 
-        if (curc->x_sampling > 1)
-            ccic->width = cinfo->width / curc->x_sampling;
-        else
-            ccic->width = cinfo->width;
         ccic->x_samples = curc->x_sampling;
         ccic->y_samples = curc->y_sampling;
 

pxr/imaging/hio/OpenEXR/OpenEXRCore/internal_util.h

@@ -9,7 +9,7 @@
 #include <stdint.h>
 
 static inline int
-compute_sampled_lines (int height, int y_sampling, int start_y)
+compute_sampled_height (int height, int y_sampling, int start_y)
 {
     int nlines;
 
@@ -31,7 +31,7 @@ compute_sampled_lines (int height, int y_sampling, int start_y)
         else
             start = start_y;
         end = start_y + height - 1;
-        end -= (end % y_sampling);
+        end -= (end < 0) ? (-end % y_sampling) : (end % y_sampling);
 
         if (start > end)
             nlines = 0;
@@ -42,4 +42,19 @@ compute_sampled_lines (int height, int y_sampling, int start_y)
     return nlines;
 }
 
+static inline int
+compute_sampled_width (int width, int x_sampling, int start_x)
+{
+    /*
+     * we require that the start_x % x_sampling == 0 and for tiled images (and for deep),
+     * x_sampling must be 1, so this can simplify the math compared to the y case
+     * where when we are reading scanline images, we always are reading the entire
+     * width. If this changes, can look like the above call for the lines, but
+     * for now can be simpler math
+     */
+    if (x_sampling <= 1) return width;
+
+    return (width == 1) ? 1 : (width / x_sampling);
+}
+
 #endif /* OPENEXR_PRIVATE_UTIL_H */

pxr/imaging/hio/OpenEXR/OpenEXRCore/parse_header.c

@@ -2552,9 +2552,9 @@ internal_exr_parse_header (struct _internal_exr_context* ctxt)
 {
     struct _internal_exr_seq_scratch scratch;
     struct _internal_exr_part*       curpart;
-    uint32_t                         flags;
+    uint32_t                         flags = 0;
     uint64_t                         initpos;
-    uint8_t                          next_byte;
+    uint8_t                          next_byte = 0;
     exr_result_t                     rv = EXR_ERR_UNKNOWN;
 
     if (ctxt->silent_header)

pxr/imaging/hio/OpenEXR/OpenEXRCore/part_attr.c

@@ -71,7 +71,7 @@ exr_get_attribute_by_name (
     const char*             name,
     const exr_attribute_t** outattr)
 {
-    exr_attribute_t* tmpptr;
+    exr_attribute_t* tmpptr = NULL;
     exr_result_t     rv;
     EXR_PROMOTE_CONST_CONTEXT_AND_PART_OR_ERROR (ctxt, part_index);
 

pxr/imaging/hio/OpenEXR/openexr-c.c

@@ -136,9 +136,11 @@ bool nanoexr_Gaussian_resample(const nanoexr_ImageData_t* src,
     // again for height
     radius = ceilf(sqrtf(-2.0f * sigma_h * sigma_h * logf(1.0f - support)));
     int filterSize_h = (int)radius;
-    if (!filterSize_h)
+    if (!filterSize_h) {
+        free(filter_w);
         return false;
-
+    }
+
     float* filter_h = (float*) malloc(sizeof(float) * (1 + filterSize_h) * 2);
     sum = 0.0f;
     for (int i = 0; i <= filterSize_h; i++) {
@@ -498,31 +500,31 @@ exr_result_t nanoexr_write_exr(
             encoder.channels[c].user_pixel_stride = alphaPixelStride;
             encoder.channels[c].user_line_stride  = alphaLineStride;
             encoder.channels[c].height            = scansperchunk; // chunk height
-            encoder.channels[c].width             = dataw.max.x - dataw.min.y + 1;
+            encoder.channels[c].width             = dataw.max.x - dataw.min.x + 1;
             encoder.channels[c].encode_from_ptr   = pAlpha;
             ++c;
         }
         if (blue) {
             encoder.channels[c].user_pixel_stride = bluePixelStride;
             encoder.channels[c].user_line_stride  = blueLineStride;
             encoder.channels[c].height            = scansperchunk; // chunk height
-            encoder.channels[c].width             = dataw.max.x - dataw.min.y + 1;
+            encoder.channels[c].width             = dataw.max.x - dataw.min.x + 1;
             encoder.channels[c].encode_from_ptr   = pBlue;
             ++c;
         }
         if (green) {
             encoder.channels[c].user_pixel_stride = greenPixelStride;
             encoder.channels[c].user_line_stride  = greenLineStride;
             encoder.channels[c].height            = scansperchunk; // chunk height
-            encoder.channels[c].width             = dataw.max.x - dataw.min.y + 1;
+            encoder.channels[c].width             = dataw.max.x - dataw.min.x + 1;
             encoder.channels[c].encode_from_ptr   = pGreen;
             ++c;
         }
         if (red) {
             encoder.channels[c].user_pixel_stride = redPixelStride;
             encoder.channels[c].user_line_stride  = redLineStride;
             encoder.channels[c].height            = scansperchunk; // chunk height
-            encoder.channels[c].width             = dataw.max.x - dataw.min.y + 1;
+            encoder.channels[c].width             = dataw.max.x - dataw.min.x + 1;
             encoder.channels[c].encode_from_ptr   = pRed;
             ++c;
         }
@@ -574,81 +576,42 @@ int nanoexr_getPixelTypeSize(exr_pixel_type_t t)
     }
 }
 
-static bool strIsRed(const char* layerName, const char* str) {
-    if (layerName && (strncmp(layerName, str, strlen(layerName)) != 0))
-        return false;
-
-    // check if the case folded string is R or RED, or ends in .R or .RED
-    char* folded = strdup(str);
-    for (int i = 0; folded[i]; ++i) {
-        folded[i] = tolower(folded[i]);
-    }
-    if (strcmp(folded, "y") == 0 || strcmp(folded, "r") == 0 ||
-        strcmp(folded, "red") == 0)
-        return true;
-    size_t l = strlen(folded);
-    if ((l > 2) && (folded[l - 2] == '.') && (folded[l - 1] == 'r'))
-        return true;
-    if (l < 4)
-        return false;
-    return strcmp(folded + l - 4, ".red");
-}
+// strcasecmp is not available in the MSVC stdlib.
+#ifdef _MSC_VER
+#define strcasecmp _stricmp
+#endif
 
-static bool strIsGreen(const char* layerName, const char* str) {
-    if (layerName && (strncmp(layerName, str, strlen(layerName)) != 0))
+static bool strIsComponent(const char* layerName, 
+                           const char* str, 
+                           const char* component)
+{
+    if (!str || !component)
         return false;
 
-    // check if the case folded string is G or GREEN, or ends in .G or .GREEN
-    char* folded = strdup(str);
-    for (int i = 0; folded[i]; ++i) {
-        folded[i] = tolower(folded[i]);
+    // if the layername is specified, it must match the beginning of the string.
+    // if layername was specified, move the string pointer past it
+    if (layerName) {
+        if (strncmp(layerName, str, strlen(layerName)) != 0)
+            return false;
+        str += strlen(layerName);
     }
-    if (strcmp(folded, "g") == 0 || strcmp(folded, "green") == 0)
-        return true;
-    size_t l = strlen(folded);
-    if ((l > 2) && (folded[l - 2] == '.') && (folded[l - 1] == 'g'))
-        return true;
-    if (l < 6)
-        return false;
-    return strcmp(folded + l - 6, ".green");
-}
-
-static bool strIsBlue(const char* layerName, const char* str) {
-    if (layerName && (strncmp(layerName, str, strlen(layerName)) != 0))
-        return false;
 
-    // check if the case folded string is B or BLUE, or ends in .B or .BLUE
-    char* folded = strdup(str);
-    for (int i = 0; folded[i]; ++i) {
-        folded[i] = tolower(folded[i]);
+    // search backwards in str for a dot. If found, move the string pointer
+    // past it
+    const char* dot = strrchr(str, '.');
+    if (dot) {
+        str = dot + 1;
     }
-    if (strcmp(folded, "b") == 0 || strcmp(folded, "blue") == 0)
-        return true;
-    size_t l = strlen(folded);
-    if ((l > 2) && (folded[l - 2] == '.') && (folded[l - 1] == 'b'))
-        return true;
-    if (l < 5)
-        return false;
-    return strcmp(folded + l - 5, ".blue");
-}
 
-static bool strIsAlpha(const char* layerName, const char* str) {
-    if (layerName && (strncmp(layerName, str, strlen(layerName)) != 0))
-        return false;
-
-    // check if the case folded string is A or ALPHA, or ends in .A or .ALPHA
-    char* folded = strdup(str);
-    for (int i = 0; folded[i]; ++i) {
-        folded[i] = tolower(folded[i]);
+    // if one character is left, then do a case insensitive comparison with the
+    // first character of the component.
+    if (strlen(str) == 1) {
+        return tolower(str[0]) == tolower(component[0]);
     }
-    if (strcmp(folded, "a") == 0 || strcmp(folded, "alpha") == 0)
-        return true;
-    size_t l = strlen(folded);
-    if ((l > 2) && (folded[l - 2] == '.') && (folded[l - 1] == 'a'))
-        return true;
-    if (l < 6)
-        return false;
-    return strcmp(folded + l - 6, ".alpha");
+
+    // The final check is to return true if a case insensitive comparison of
+    // the string with the component is true.
+    return strcasecmp(str, component) == 0;
 }
 
 void nanoexr_release_image_data(nanoexr_ImageData_t* imageData)
@@ -694,19 +657,19 @@ static exr_result_t _nanoexr_rgba_decoding_initialize(
     for (int c = 0; c < decoder->channel_count; ++c) {
         int channelIndex = -1;
         decoder->channels[c].decode_to_ptr = NULL;
-        if (strIsRed(layerName, decoder->channels[c].channel_name)) {
+        if (strIsComponent(layerName, decoder->channels[c].channel_name, "red")) {
             rgba[0] = c;
             channelIndex = 0;
         }
-        else if (strIsGreen(layerName, decoder->channels[c].channel_name)) {
+        else if (strIsComponent(layerName, decoder->channels[c].channel_name, "green")) {
             rgba[1] = c;
             channelIndex = 1;
         }
-        else if (strIsBlue(layerName, decoder->channels[c].channel_name)) {
+        else if (strIsComponent(layerName, decoder->channels[c].channel_name, "blue")) {
             rgba[2] = c;
             channelIndex = 2;
         }
-        else if (strIsAlpha(layerName, decoder->channels[c].channel_name)) {
+        else if (strIsComponent(layerName, decoder->channels[c].channel_name, "alpha")) {
             rgba[3] = c;
             channelIndex = 3;
         }
@@ -1018,7 +981,7 @@ exr_result_t nanoexr_read_exr(const char* filename,
     exr_compression_t compression;
     rv = exr_get_compression(exr, partIndex, &compression);
     if (rv != EXR_ERR_SUCCESS) {
-        fprintf(stderr, "nanoexr compression error: %s\n", 
+        fprintf(stderr, "nanoexr compression error: %s\n",
                 exr_get_default_error_message(rv));
         exr_finish(&exr);
         return rv;
@@ -1068,8 +1031,8 @@ exr_result_t nanoexr_read_exr(const char* filename,
     img->height = height;
     img->dataSize = width * height * img->channelCount * bytesPerChannel;
     img->pixelType = pixelType;
-    img->dataWindowMinY = (datawin.min.y + 1) >> mipLevel;
-    img->dataWindowMaxY = (datawin.max.y + 1) >> mipLevel;
+    img->dataWindowMinY = datawin.min.y >> mipLevel;
+    img->dataWindowMaxY = (datawin.max.y+1) >> mipLevel;
     img->data = (unsigned char*) malloc(img->dataSize);
     if (img->data == NULL) {
         fprintf(stderr, "nanoexr error: could not allocate memory for image data\n");