gz-rendering icon indicating copy to clipboard operation
gz-rendering copied to clipboard

contentScalingFactor uses locale-dependent conversion to string

Open peci1 opened this issue 3 years ago • 9 comments

https://github.com/ignitionrobotics/ign-rendering/blob/e22fd80e25aa89454a2be1d69891c83c7900f4c0/ogre/src/OgreRenderEngine.cc#L651

https://github.com/ignitionrobotics/ign-rendering/blob/e22fd80e25aa89454a2be1d69891c83c7900f4c0/ogre2/src/Ogre2RenderEngine.cc#L715

std::to_string respects locale, so if you run ign rendering on a system with e.g. LC_NUMERIC=cs_CZ.UTF-8, it fails passing correct parameters to OGRE. See the decimal comma (not dot) in contentScalingFactor:

17:01:29: GL3PlusRenderSystem::_createRenderWindow "OgreWindow(0)_0", 1x1 windowed  miscParams: FSAA=0 border=none contentScalingFactor=1,000000 currentGLContext=true externalGLControl=true gamma=true stereoMode=Frame Sequential

OGRE parses the value with

mContentScalingFactor = StringConverter::parseReal(opt->second);

which is

Real StringConverter::parseReal(const String& val, Real defaultValue) { StringStream str(val); if (msUseLocale) str.imbue(msLocale); Real ret = defaultValue; if( !(str >> ret) ) return defaultValue; return ret; }

The behavior of OGRE thus depends on whether StringUtils::setUseLocale(true) has been called or not, and I haven't found a reference to this function neither in OGRE nor in ign-rendering. Thus I assume in this use-case, OGRE parses the value in C locale.

The ign-rendering code should either use https://en.cppreference.com/w/cpp/utility/to_chars (if C++17 is available), or boost::lexical_cast. Or set the locale before the call, but that might be unwanted in other parts...

It'd be best to add a utility function for float->string and string->float conversions to ign-common/StringUtils, so that people can easily get the correct behavior...

peci1 avatar Sep 08 '20 19:09 peci1