From d88ed3150376693000665b9016c0350d5d90e9e1 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Sat, 7 Mar 2015 02:01:45 +0100 Subject: [PATCH] Drop setting the locale altogether. 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 | 24 ------------------------ effect_chain_test.cpp | 20 ++++++++++++++------ util.cpp | 43 ++++++++++++++++++++++++++++++++----------- util.h | 4 ++++ ycbcr_input.cpp | 19 ++++--------------- 5 files changed, 54 insertions(+), 56 deletions(-) diff --git a/effect_chain.cpp b/effect_chain.cpp index 5d7e923..21c4a0d 100644 --- a/effect_chain.cpp +++ b/effect_chain.cpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -13,9 +12,6 @@ #include #include #include -#if defined(__APPLE__) -#include -#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) diff --git a/effect_chain_test.cpp b/effect_chain_test.cpp index 7b86d16..dd4ffa2 100644 --- a/effect_chain_test.cpp +++ b/effect_chain_test.cpp @@ -2,6 +2,10 @@ // // Note that this also contains the tests for some of the simpler effects. +#include +#include +#include + #include #include @@ -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 diff --git a/util.cpp b/util.cpp index b12ee80..e08a085 100644 --- a/util.cpp +++ b/util.cpp @@ -4,6 +4,9 @@ #include #include #include +#include +#include +#include #include #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 diff --git a/util.h b/util.h index e4474d3..f57af77 100644 --- 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); diff --git a/ycbcr_input.cpp b/ycbcr_input.cpp index dc5e4d9..22d689d 100644 --- a/ycbcr_input.cpp +++ b/ycbcr_input.cpp @@ -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; -- 2.39.2