Skip to content

MinGW: Work around lack of strtof_l for locale independent parsing#1697

Merged
doug-walker merged 2 commits into
AcademySoftwareFoundation:mainfrom
amyspark:amyspark/locale-2
Oct 31, 2022
Merged

MinGW: Work around lack of strtof_l for locale independent parsing#1697
doug-walker merged 2 commits into
AcademySoftwareFoundation:mainfrom
amyspark:amyspark/locale-2

Conversation

@amyspark
Copy link
Copy Markdown
Contributor

@amyspark amyspark commented Oct 2, 2022

👋

This PR is to catch a slipup in #1496 I've only found yesterday. MinGW-based compilers lack ucrtbase's definition of _strtof_l completely, and strtod_l is also missing from GCC, so neither are viable alternatives to achieve locale-independent floating point number parsing.

In this patch, I propose to apply a workaround similar to the one existing for macOS, that covers only this flavor of compilers.

Comment thread src/utils/NumberUtils.h

float
#ifdef _WIN32
#if defined(__MINGW32__) || defined(__MINGW64__)
Copy link
Copy Markdown
Contributor Author

@amyspark amyspark Oct 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, should I make the function existence check in the CMakeLists file, so that this is safely reverted to _strtof_l once the mingw-w64 platform updates their SDK?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally it would be a test directly in the source code if possible? Can be done later in any case I think given the current compilation issue.

@remia
Copy link
Copy Markdown
Collaborator

remia commented Oct 24, 2022

Just checking @amyspark, does that mean latest OCIO releases do not compile on MinGW?

@remia remia linked an issue Oct 24, 2022 that may be closed by this pull request
@amyspark
Copy link
Copy Markdown
Contributor Author

@remia No, they do not. I had not noticed because our build was still running the old patch (which did not rely on the affected function).

Signed-off-by: L. E. Segovia <amy@amyspark.me>
@doug-walker doug-walker merged commit 1e6b01c into AcademySoftwareFoundation:main Oct 31, 2022
@amyspark amyspark deleted the amyspark/locale-2 branch October 31, 2022 21:13
@amyspark
Copy link
Copy Markdown
Contributor Author

amyspark commented Oct 31, 2022

Thanks @doug-walker, @remia and @cedrik-fuoco-adsk ! Please don't forget to cherry-pick to 2.1 as well.

cedrik-fuoco-adsk pushed a commit that referenced this pull request Jan 5, 2023
…1697)

Signed-off-by: L. E. Segovia <amy@amyspark.me>

Signed-off-by: L. E. Segovia <amy@amyspark.me>
Co-authored-by: Doug Walker <doug.walker@autodesk.com>
(cherry picked from commit 1e6b01c)
cedrik-fuoco-adsk pushed a commit that referenced this pull request Jan 5, 2023
…1697)

Signed-off-by: L. E. Segovia <amy@amyspark.me>

Signed-off-by: L. E. Segovia <amy@amyspark.me>
Co-authored-by: Doug Walker <doug.walker@autodesk.com>
(cherry picked from commit 1e6b01c)
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
doug-walker added a commit that referenced this pull request Jan 6, 2023
* Changing version release type for 2.1.3 official release.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* MinGW: Work around lack of strtof_l for locale independent parsing (#1697)

Signed-off-by: L. E. Segovia <amy@amyspark.me>

Signed-off-by: L. E. Segovia <amy@amyspark.me>
Co-authored-by: Doug Walker <doug.walker@autodesk.com>
(cherry picked from commit 1e6b01c)
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Fix atan2 argument order for HLSL (#1712)

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

Signed-off-by: Doug Walker <doug.walker@autodesk.com>
(cherry picked from commit 8a93946)
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* replace texture2D function with texture for GLSL 1.3 (#1723)

Signed-off-by: Bart Styczen <bart.styczen@cine.dev>

Signed-off-by: Bart Styczen <bart.styczen@cine.dev>
Co-authored-by: Doug Walker <doug.walker@autodesk.com>
(cherry picked from commit d5cedbf)
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Adsk contrib - Processor cache does not detect changes in cccid (#1726)

* For Looks that has a FileTransform and for Colorspace with FileTransfrom, add the CCCID to the processor's cache key for that transform.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Removing the workaround in the related unit tests and fixing the issue by adding the environment variable to the context using setStringVar. The processor's cache key is using the context cache ID which has all the context variables taken into account.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Now using addStringVars and creating a new context instead of reusing the one used for the filename.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Adding cccid to the context when there are no context variable.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Adding a few unit tests to test that the processor is different when changing the FileTransform's CCCID.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Using setStringVar to set CCNUM context variable in unit test.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Adding a test in FileTransform to test CollectContextVariables directly.

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Minor tweaks for the unit test

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Co-authored-by: Doug Walker <doug.walker@autodesk.com>
(cherry picked from commit 4fa7750)
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

* Fix inverse Lut1D optimization bug (#1743)

Signed-off-by: Doug Walker <doug.walker@autodesk.com>

Signed-off-by: Doug Walker <doug.walker@autodesk.com>
(cherry picked from commit 5152635)
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>

Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: L. E. Segovia <amy@amyspark.me>
Signed-off-by: Doug Walker <doug.walker@autodesk.com>
Signed-off-by: Bart Styczen <bart.styczen@cine.dev>
Co-authored-by: amyspark <amy@amyspark.me>
Co-authored-by: Doug Walker <doug.walker@autodesk.com>
Co-authored-by: bartstyczen <93516126+bartstyczen@users.noreply.github.com>
@doug-walker
Copy link
Copy Markdown
Collaborator

@amyspark , this fix is now in OCIO 2.1.3, per your request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

building fails on windows with mingw-w64

4 participants