Decimal mark dependency

[moresascha]

Currently the compile time string to binary number parsing depends on the current active locale.

#version 450 core
layout(set = 1, binding = 0) uniform sampler2D tex;
layout(location = 0) in vec2 texcoord;
layout(location = 0) out vec4 outColor;

const float OFFSET_X = 0.5f;

void main()  {
    outColor = texture(tex, vec2(texcoord.x + OFFSET_X, texcoord.y));
}

Decimal mark set to "," (e.g. setlocale(LC_ALL, "German_Germany")) produces:

...
23:    6(float) Constant 0
...

Default Decimal mark "." produces the correct output:

...
23:    6(float) Constant 1056964608
...

[xorgy]

Very good catch, wow.

[johnkslang]

Both strtod() and atod() are in use, each once, and each (annoyingly) takes locale into account.

These are not trivial functions to replicate, and locale is typically a process-wide setting. Bringing in streamstring stuff to imbue it with a stream-specific locale also feels heavyweight for this purpose, but perhaps not? Any other ideas?

[Robert42]

I ran into the same issue. I am using the glslang static library directly, so calling setlocale(LC_ALL, "C"); at the start of my application was a simple workaround which did the job for me.

[xorgy]

@johnkslang For what it's worth, we have a full number parser in the preprocessor. Though it is not the prettiest code; it is straightforward.

[ratchetfreak]

strtod() has a variant that takes the locale to use explicitly in both windows (_strtod_l) and linux (strtod_l):

Looking around a bit I found a gist with an example to create a strtod that always uses the C locale without affecting the application's locale: https://gist.github.com/tknopp/9271244

[xorgy]

@johnkslang I was writing a standalone number parser, so I looked at the specification to make sure. In looking at the GLSL specification, I think that GLSL's definition of floating-point literals is more narrow than that of a full fledged strtod or atof, so I don't think it's as ambitious as writing a real strtod. For example, The OpenGL Shading Language 4.6 Revision 3 makes no mention of [-]1.#INF in §4.1.4. I think in the case of #INF it's just an oversight from when it was added for HLSL. (Update: PR for this: #1122)

I'm confident I can write (and exhaustively test) a compact standalone parser which accepts only what's specified, but otherwise behaves like a correct locale-free positive-only decimal-only atof, just wanna make sure I'm not missing something.

This could also be a good opportunity to properly handle double constants (seemingly not supported currently due to ambiguity between l/L in integers, and lf/LF in float constants, since the floatingPointChar test function relies on reading a single character without lookahead).

[johnkslang]

Basically agreed, and agreed about the accidental inclusion of the HLSL syntax.

Otherwise, I think the glslang parser is validating against the spec, it just uses the library functions to create the actual number.

[johnkslang]

It turns out this is a quite complex problem. To completely correctly both parse and print double values is ~1000 of lines of code, so I prefer to use library functions. Yet, library functions for these are not completely portable. istringstream is different than strtod, and handles infinities differently, and all functions vary across platforms, and even the ways of controlling them are not standardized, because the C++ standard is not IEEE based. Further, istringstream is very slow.

(General side note: to get even close to correct rounding requires slow algorithms, whose performance is dependent on the actual values being translated. These are available in istringstream, and it is worth using them for some values.)

I believe, now, I have a combination of

• an explicit (means totally portable) fast path that handles most values
• istringstream that handles the remaining values
• an explicit detection of infinities, both directions
• not printing to full precision, to get portable testing
• high performance by caching the istringstream and only using it when the fast path might not get the correct result

[johnkslang]

#1385 has a series of three commits that

1. increase testing in this area
2. changes macro processing to not need to reparse floating-point tokens
3. uses a combination of istringstream and a fast path for parsing of floating-point tokens

These should be locale dependent. If not, it should now be an easy fix. See the pull request for more details.