Skip to content

Commit acf2625

Browse files
author
Tim Foley
authored
Fix floating-point literal emit to be locale-independent. (shader-slang#315)
There was a bug that arose in the context of Falcor, because: - Slang uses `sprintf` to format floating-point values when outputting HLSL/GLSL source - Falcor (or a library it uses) does the equivalent of `setlocale(LC_ALL, "")` to set the global locale for the process based on the user's environment variables This led to a floating-point literal of `0.5` getting printed as `0,5f`, with a comma for the decimal point. This then gets consumed by `glslang` which (luckily for us) complains that `5f` is not a valid floating-point literal in their language (since it has neither decimal point nor exponent). The most expedient fix in this case was to switch from using C `sprintf` for formatting floating-point numbers over to using the C++ `<stdio>` implementation, which allows the locale to be set per-stream so that we don't have to rely on (or potentially disrupt) the global locale set by an application using Slang. Longer term if we have the time/resources to do the Right Thing, it would be best to implement our own printing logic for floating-point numbers, since we would eventually need/want to support arbitrary-precision numbers for literals.
1 parent 393e25f commit acf2625

File tree

1 file changed

+36
-4
lines changed

1 file changed

+36
-4
lines changed

source/slang/emit.cpp

+36-4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@
1313

1414
#include <assert.h>
1515

16+
// Note: using C++ stdio just to get a locale-independent
17+
// way to format floating-point values.
18+
//
19+
// TODO: Go ahead and implement teh Dragon4 algorithm so
20+
// that we can print floating-point values to arbitrary
21+
// precision as needed.
22+
#include <sstream>
23+
1624
#ifdef _WIN32
1725
#include <d3dcompiler.h>
1826
#pragma warning(disable:4996)
@@ -546,10 +554,34 @@ struct EmitVisitor
546554

547555
void Emit(double value)
548556
{
549-
// TODO(tfoley): need to print things in a way that can round-trip
550-
char buffer[128];
551-
sprintf(buffer, "%.20ff", value);
552-
Emit(buffer);
557+
// There are a few different requirements here that we need to deal with:
558+
//
559+
// 1) We need to print somethign that is valid syntax in the target language
560+
// (this means that hex floats are off the table for now)
561+
//
562+
// 2) We need our printing to be independent of the current global locale in C,
563+
// so that we don't depend on the application leaving it as the default,
564+
// and we also don't revert any changes they make.
565+
// (this means that `sprintf` and friends are off the table)
566+
//
567+
// 3) We need to be sure that floating-point literals specified by the user will
568+
// "round-trip" and turn into the same value when parsed back in. This means
569+
// that we need to print a reasonable number of digits of precision.
570+
//
571+
// For right now, the easiest option that can balance these is to use
572+
// the C++ standard library `iostream`s, because they support an explicit locale,
573+
// and can (hopefully) print floating-point numbers accurately.
574+
//
575+
// Eventually, the right move here would be to implement proper floating-point
576+
// number formatting ourselves, but that would require extensive testing to
577+
// make sure we get it right.
578+
579+
std::ostringstream stream;
580+
stream.imbue(std::locale::classic());
581+
stream.precision(20);
582+
stream << value;
583+
584+
Emit(stream.str().c_str());
553585
}
554586

555587

0 commit comments

Comments
 (0)