Fixed regression in Imath::succf() and Imath::predf() when negative values are given (for 2.4)

[christianaguilera-foundry]

The issue was introduced in OpenEXR 2.4.0 as part of c8a7f6a#diff-f09c17d3ab2fe5564e547c8461f1a43a6fe1109ebaf663a8463d1a9643c20ee0, where the type of a member of a union was changed from signed to unsigned, breaking comparison against > 0 a few lines below.

Since OpenEXR 2.4.0, Imath::predf(-2) returns -1.9999998807907104 instead of -2.000000238418579.

Test coverage has been added for this regression.

Fixes #999 (for the 2.4 line).

---

EDIT:

This PR is going to be abandoned. See #1013 for v2.4 and AcademySoftwareFoundation/Imath#140 for v3. (Another PR will have to be created for v2.5.)

[linux-foundation-easycla]

• ❌ - Christian Aguilera The commit (a6ab492 ,6ad4eb4abc2882dccae8aa7784d6bcd3466fa920) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[cary-ilm]

Good catch, and thanks for the test, it's a bit surprising there wasn't already a test to catch this.

We do require a signed CLA, although we're in the process of switching the EasyCLA system to a new form, so the automated process will lead to the outdated form. The correct form is here:
https://github.com/AcademySoftwareFoundation/openexr/blob/master/ASWF/legal/OpenEXR%20Corporate%20Contributor%20License%20Agreement.md

We will also want to patch this into v2.5 and 3.0.

[christianaguilera-foundry]

We do require a signed CLA, although we're in the process of switching the EasyCLA system to a new form, so the automated process will lead to the outdated form. The correct form is here:
https://github.com/AcademySoftwareFoundation/openexr/blob/master/ASWF/legal/OpenEXR%20Corporate%20Contributor%20License%20Agreement.md

Foundry is looking at the CLA matter. I'll get back to you when I have more details.

We will also want to patch this into v2.5 and 3.0.

I was waiting for this PR to be reviewed to create another PR for 2.5.

In regards to 3.0, that now lives in a different project, a fresh patch would be created. (Since the .clang-format file has changed, the patch would be the same, except for the indentation, that would be slightly different.) Does this sound correct?

[cary-ilm]

Yes, that sounds correct, for 3.0 this file is in https://github.com/AcademySoftwareFoundation/Imath/blob/master/src/Imath/ImathFun.cpp, and the formatting has indeed changed. Thanks.

[cary-ilm]

LGTM

[cary-ilm]

@christianaguilera-foundry, any update on the CLA?

[christianaguilera-foundry]

@christianaguilera-foundry, any update on the CLA?

No news. Let me try to speed it up.

[cary-ilm]

Checking in again, any progress on the CLA? If this is an obstacle, I can recreate the fix myself, let me know. Thanks.

[cary-ilm]

Fix is replicated in #1013.

[christianaguilera-foundry]

This PR is going to be abandoned. See #1013 for v2.4 and AcademySoftwareFoundation/Imath#140 for v3. (Another PR will have to be created for v2.5.)