cgltf icon indicating copy to clipboard operation
cgltf copied to clipboard

Broken text to floating point conversion on some locales

Open JulianGro opened this issue 1 year ago • 1 comments

The CGLTF_ATOF function seems to only work properly on systems with . as decimal separator. On German Linux systems for example , is used as decimal separator, which breaks the conversion. My understanding is that this is a problem with the C standard being decimal separator dependent in the first place.

In our C++ code, we work around this the following way:

float atof_locale_independent(char* str);

#define CGLTF_ATOF(str) atof_locale_independent(str)
#define CGLTF_IMPLEMENTATION
#include <cgltf.h>

float atof_locale_independent(char* str) {
    //TODO: Once we have C++17 we can use std::from_chars
    std::istringstream streamToParse(str);
    streamToParse.imbue(std::locale("C"));
    float value;
    if (!(streamToParse >> value)) {
        qDebug(modelformat) << "cgltf: Cannot parse float from string: " << str;
        return 0.0f;
    }
    return value;
}

Here is an example model which completely breaks on systems with , decimal separator: http://oaktown.pl/models/stopnie_2_1.gltf (model is CC0)

JulianGro avatar Dec 22 '24 22:12 JulianGro

Btw, to reproduce this you need to do setlocale(LC_ALL, ""); first (since the C standard declares that the locale always defaults to C).

After that printing the atof calls confirms that the decimals are truncated.

Another way of working around this similar to .imbue():

GNU does have the non-standard atof_l/strod_l (inspired by POSIX's locale_t extension for C's locale.h):

#define CGLTF_ATOF(str) strtod_l(str, NULL, newlocale(LC_ALL, "C", NULL))

It isn't really portable though since it requires gnuXX/gnu++XX (or the equivalent _GNU_SOURCE defined before including anything else with glibc), it is supported pretty widely but not everywhere (and Windows has _create_locale(LC_ALL,"C") instead of newlocale(LC_ALL, "C", NULL)).

Also, keep in mind that calling newlocale does allocate memory, although with glibc newlocale() will return the preallocated C locale if base is null or LC_ALL_MASK is used: https://github.com/bminor/glibc/blob/bb6496b96444dfd55d7105396780f6eba14b1cd9/locale/newlocale.c#L75

Afaict the only portable way to handle this is to re-implement a locale-independent strtod/atof.

Technically speaking you can also just convert the strings to the locale format before calling strtod, like what jansson does (which is problematic for the reasons stated in the code comment): https://github.com/akheron/jansson/blob/8b975abca1055d40637c90b1dc4585af1d7df76c/src/strconv.c#L14-L43

oreo639 avatar Feb 09 '25 12:02 oreo639