Fix out-of-bounds reads when using OpenEXR decreasingY lineOrder.

[acolwell]

Description

This change fixes out-of-bounds reads and incorrect scanline ordering when OpenEXR's decreasingY lineOrder mode is used.

OpenEXR expects to process scanlines in decreasing order when the decreasingY lineOrder option is enabled. This change fixes the code that provides the chunks of scanlines to OpenEXR so that the proper scanlines are available when it accesses the FrameBuffer OIIO created. The old code was providing incorrect scanlines AND setting up the FrameBuffer with incorrect pointers so out-of-bounds reads were occurring.

The key things to keep in mind are:

• Chunks of scanlines need to be sent to OpenEXROutput::write_scanlines() from the bottom of the image to the top when decreasingY is enabled. If the chunk's scanline range does not contain the scanlines OpenEXR is expecting to fetch, then it can cause out-of-bound reads because the wrong information is used to construct the pointers for the Slices created when constructing a FrameBuffer.
• OpenEXROutput::write_scanlines() does its own chunking of scanlines and this logic must also properly start from the bottom of the chunk data passed into this function when decreasingY is enabled.
• The to_native_rectangle() call in OpenEXROutput::write_scanlines() may return a pointer to m_scratch if a format conversion is needed. This means the pointers passed to OpenEXR, in this situation, WILL NOT be a full frame buffer. This caused OpenEXR to do out-of-bounds reads because the computations used to create the Slices were using the wrong scanline number to adjust the pointer.
• The progress callback computation needed to be modified to handle traversing scanlines in decreasing order.

Tests

I added an openexr-decreasingY test to verify that the code no longer crashes because of the out-of-bound reads and it also verifies that the decreasingY images match the increasingY images.

Checklist:

• I have read the contribution guidelines.
• I have updated the documentation, if applicable.
• I have ensured that the change is tested somewhere in the testsuite
  (adding new test cases if necessary).
• If I added or modified a C++ API call, I have also amended the
  corresponding Python bindings (and if altering ImageBufAlgo functions, also
  exposed the new functionality as oiiotool options).
• My code follows the prevailing code style of this project. If I haven't
  already run clang-format before submitting, I definitely will look at the CI
  test that runs clang-format and fix anything that it highlights as being
  nonconforming.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: acolwell / name: Aaron Colwell (3a07454, 01d1b9f, 40a14bb, f43c19a)

[lgritz]

LGTM pending the few comments I had about how to make the for loop iteration maybe a little less confusing. Other than that, looks good. Thanks for adding a careful test as well.

[lgritz]

same comment as above regarding ...; y != yEnd; ... would be simpler, with yEnd = yStart + yDelta

[acolwell]

added similar fix here.

[acolwell]

Addressed all comments. Thanks for the review.

[acolwell]

added similar fix here.

[acolwell]

I'm not really sure what to do about the 1 failed builder. It doesn't appear to be related to my change. Unfortunately, I'm not familiar with fmt or the related cmake magic that appears to be including it so I don't have a suggested fix.

[lgritz]

That's not related to this PR. There is one build we do that uses the current top-of-tree of several major dependencies. It often fails because one of those dependencies has checked a bug into their own tree.

[lgritz]

LGTM, thanks for the patch!