Drop setting the locale altogether.
authorSteinar H. Gunderson <sgunderson@bigfoot.com>
Sat, 7 Mar 2015 01:01:45 +0000 (02:01 +0100)
committerSteinar H. Gunderson <sgunderson@bigfoot.com>
Sat, 7 Mar 2015 01:01:45 +0000 (02:01 +0100)
Trying to use sprintf and floats right in a portable manner is seemingly
impossible (MinGW doesn't support the per-thread locale stuff), so simply
do it a different way; stop sprintf-ing floats and use std::stringstream
instead. I dislike the iostream interface a lot, but it can do per-stream
locales, which is exactly what we want here.

effect_chain.cpp
effect_chain_test.cpp
util.cpp
util.h
ycbcr_input.cpp

index 5d7e923..21c4a0d 100644 (file)
@@ -2,7 +2,6 @@
 
 #include <epoxy/gl.h>
 #include <assert.h>
-#include <locale.h>
 #include <math.h>
 #include <stddef.h>
 #include <stdio.h>
@@ -13,9 +12,6 @@
 #include <stack>
 #include <utility>
 #include <vector>
-#if defined(__APPLE__)
-#include <xlocale.h>
-#endif
 
 #include "alpha_division_effect.h"
 #include "alpha_multiplication_effect.h"
@@ -1333,19 +1329,6 @@ Node *EffectChain::find_output_node()
 
 void EffectChain::finalize()
 {
-       // Save the current locale, and set it to C, so that we can output decimal
-       // numbers with printf and be sure to get them in the format mandated by GLSL.
-#if defined(__MINGW32__)
-       // Note that the OpenGL driver might call setlocale() behind-the-scenes,
-       // and that might corrupt the returned pointer, so we need to take our own
-       // copy of it here.
-       char *saved_locale = strdup(setlocale(LC_NUMERIC, NULL));
-       setlocale(LC_NUMERIC, "C");
-#else
-       locale_t c_locale = newlocale(LC_NUMERIC_MASK, "C", (locale_t)0);
-       locale_t saved_locale = uselocale(c_locale);
-#endif
-
        // Output the graph as it is before we do any conversions on it.
        output_dot("step0-start.dot");
 
@@ -1408,13 +1391,6 @@ void EffectChain::finalize()
        assert(phases[0]->inputs.empty());
        
        finalized = true;
-#if defined(__MINGW32__)
-       setlocale(LC_NUMERIC, saved_locale);
-       free(saved_locale);
-#else
-       uselocale(saved_locale);
-       freelocale(c_locale);
-#endif
 }
 
 void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height)
index 7b86d16..dd4ffa2 100644 (file)
@@ -2,6 +2,10 @@
 //
 // Note that this also contains the tests for some of the simpler effects.
 
+#include <locale>
+#include <sstream>
+#include <string>
+
 #include <epoxy/gl.h>
 #include <assert.h>
 
@@ -1047,14 +1051,17 @@ public:
        PrintfingBlueEffect() {}
        virtual string effect_type_id() const { return "PrintfingBlueEffect"; }
        string output_fragment_shader() {
-               char buf[256];
-               snprintf(buf, sizeof(buf), "vec4 FUNCNAME(vec2 tc) { return vec4(%f, %f, %f, %f); }\n",
-                       0.0f, 0.0f, 1.0f, 1.0f);
-               return buf;
+               stringstream ss;
+               ss.imbue(locale("C"));
+               ss.precision(8);
+               ss << "vec4 FUNCNAME(vec2 tc) { return vec4("
+                  << 0.0f << ", " << 0.0f << ", "
+                  << 0.5f << ", " << 1.0f << "); }\n";
+               return ss.str();
        }
 };
 
-TEST(EffectChainTest, LocaleIsIgnoredDuringFinalize) {
+TEST(EffectChainTest, StringStreamLocalesWork) {
        // An example of a locale with comma instead of period as decimal separator.
        // Obviously, if you run on a machine without this locale available,
        // the test will always succeed. Note that the OpenGL driver might call
@@ -1065,7 +1072,7 @@ TEST(EffectChainTest, LocaleIsIgnoredDuringFinalize) {
                0.0f, 0.0f, 0.0f, 0.0f,
        };
        float expected_data[] = {
-               0.0f, 0.0f, 1.0f, 1.0f,
+               0.0f, 0.0f, 0.5f, 1.0f,
        };
        float out_data[4];
        EffectChainTester tester(data, 1, 1, FORMAT_RGBA_PREMULTIPLIED_ALPHA, COLORSPACE_sRGB, GAMMA_LINEAR);
@@ -1078,4 +1085,5 @@ TEST(EffectChainTest, LocaleIsIgnoredDuringFinalize) {
        free(saved_locale);
 }
 
+
 }  // namespace movit
index b12ee80..e08a085 100644 (file)
--- a/util.cpp
+++ b/util.cpp
@@ -4,6 +4,9 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <locale>
+#include <sstream>
+#include <string>
 #include <Eigen/Core>
 
 #include "fp16.h"
@@ -145,17 +148,35 @@ void print_3x3_matrix(const Eigen::Matrix3d& m)
 
 string output_glsl_mat3(const string &name, const Eigen::Matrix3d &m)
 {
-       char buf[1024];
-       sprintf(buf,
-               "const mat3 %s = mat3(\n"
-               "    %.8f, %.8f, %.8f,\n"
-               "    %.8f, %.8f, %.8f,\n"
-               "    %.8f, %.8f, %.8f);\n\n",
-               name.c_str(),
-               m(0,0), m(1,0), m(2,0),
-               m(0,1), m(1,1), m(2,1),
-               m(0,2), m(1,2), m(2,2));
-       return buf;
+       // Use stringstream to be independent of the current locale in a thread-safe manner.
+       stringstream ss;
+       ss.imbue(locale("C"));
+       ss.precision(8);
+       ss << "const mat3 " << name << " = mat3(\n";
+       ss << "    " << m(0,0) << ", " << m(1,0) << ", " << m(2,0) << ",\n";
+       ss << "    " << m(0,1) << ", " << m(1,1) << ", " << m(2,1) << ",\n";
+       ss << "    " << m(0,2) << ", " << m(1,2) << ", " << m(2,2) << ");\n\n";
+       return ss.str();
+}
+
+string output_glsl_vec2(const string &name, float x, float y)
+{
+       // Use stringstream to be independent of the current locale in a thread-safe manner.
+       stringstream ss;
+       ss.imbue(locale("C"));
+       ss.precision(8);
+       ss << "const vec2 " << name << " = vec2(" << x << ", " << y << ");\n";
+       return ss.str();
+}
+
+string output_glsl_vec3(const string &name, float x, float y, float z)
+{
+       // Use stringstream to be independent of the current locale in a thread-safe manner.
+       stringstream ss;
+       ss.imbue(locale("C"));
+       ss.precision(8);
+       ss << "const vec3 " << name << " = vec3(" << x << ", " << y << ", " << z << ");\n";
+       return ss.str();
 }
 
 template<class DestFloat>
diff --git a/util.h b/util.h
index e4474d3..f57af77 100644 (file)
--- a/util.h
+++ b/util.h
@@ -38,6 +38,10 @@ void print_3x3_matrix(const Eigen::Matrix3d &m);
 // Output a GLSL 3x3 matrix declaration.
 std::string output_glsl_mat3(const std::string &name, const Eigen::Matrix3d &m);
 
+// Output GLSL 2-length and 3-length vector declarations.
+std::string output_glsl_vec2(const std::string &name, float x, float y);
+std::string output_glsl_vec3(const std::string &name, float x, float y, float z);
+
 // Calculate a / b, rounding up. Does not handle overflow correctly.
 unsigned div_round_up(unsigned a, unsigned b);
 
index dc5e4d9..22d689d 100644 (file)
@@ -212,31 +212,20 @@ string YCbCrInput::output_fragment_shader()
        string frag_shader;
 
        frag_shader = output_glsl_mat3("PREFIX(inv_ycbcr_matrix)", ycbcr_to_rgb);
-
-       char buf[256];
-       sprintf(buf, "const vec3 PREFIX(offset) = vec3(%.8f, %.8f, %.8f);\n",
-               offset[0], offset[1], offset[2]);
-       frag_shader += buf;
-
-       sprintf(buf, "const vec3 PREFIX(scale) = vec3(%.8f, %.8f, %.8f);\n",
-               scale[0], scale[1], scale[2]);
-       frag_shader += buf;
+       frag_shader += output_glsl_vec3("PREFIX(offset)", offset[0], offset[1], offset[2]);
+       frag_shader += output_glsl_vec3("PREFIX(scale)", scale[0], scale[1], scale[2]);
 
        float cb_offset_x = compute_chroma_offset(
                ycbcr_format.cb_x_position, ycbcr_format.chroma_subsampling_x, widths[1]);
        float cb_offset_y = compute_chroma_offset(
                ycbcr_format.cb_y_position, ycbcr_format.chroma_subsampling_y, heights[1]);
-       sprintf(buf, "const vec2 PREFIX(cb_offset) = vec2(%.8f, %.8f);\n",
-               cb_offset_x, cb_offset_y);
-       frag_shader += buf;
+       frag_shader += output_glsl_vec2("PREFIX(cb_offset)", cb_offset_x, cb_offset_y);
 
        float cr_offset_x = compute_chroma_offset(
                ycbcr_format.cr_x_position, ycbcr_format.chroma_subsampling_x, widths[2]);
        float cr_offset_y = compute_chroma_offset(
                ycbcr_format.cr_y_position, ycbcr_format.chroma_subsampling_y, heights[2]);
-       sprintf(buf, "const vec2 PREFIX(cr_offset) = vec2(%.8f, %.8f);\n",
-               cr_offset_x, cr_offset_y);
-       frag_shader += buf;
+       frag_shader += output_glsl_vec2("PREFIX(cr_offset)", cr_offset_x, cr_offset_y);
 
        frag_shader += read_file("ycbcr_input.frag");
        return frag_shader;