From cf3d0b7dd3f3c3442339b0ccf46ac69abbed15e4 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sat, 16 Aug 2025 13:46:34 +0200 Subject: [PATCH 1/6] Use std::format in components/fx --- apps/openmw/mwlua/postprocessingbindings.cpp | 1 + components/fx/lexer.cpp | 10 +-- components/fx/pass.cpp | 2 +- components/fx/technique.cpp | 92 ++++++++++---------- components/fx/types.hpp | 40 ++++----- components/fx/widgets.hpp | 5 +- 6 files changed, 70 insertions(+), 80 deletions(-) diff --git a/apps/openmw/mwlua/postprocessingbindings.cpp b/apps/openmw/mwlua/postprocessingbindings.cpp index 6d0c33848a..8b3779d084 100644 --- a/apps/openmw/mwlua/postprocessingbindings.cpp +++ b/apps/openmw/mwlua/postprocessingbindings.cpp @@ -3,6 +3,7 @@ #include "MyGUI_LanguageManager.h" #include +#include #include "../mwbase/environment.hpp" #include "../mwbase/world.hpp" diff --git a/components/fx/lexer.cpp b/components/fx/lexer.cpp index cab59df314..4369febbcb 100644 --- a/components/fx/lexer.cpp +++ b/components/fx/lexer.cpp @@ -2,11 +2,7 @@ #include #include -#include -#include -#include - -#include +#include namespace Fx { @@ -126,7 +122,7 @@ namespace Fx [[noreturn]] void Lexer::error(const std::string& msg) { - throw LexerException(Misc::StringUtils::format("Line %zu Col %zu. %s", mLine + 1, mColumn, msg)); + throw LexerException(std::format("Line {} Col {}. {}", mLine + 1, mColumn, msg)); } void Lexer::advance() @@ -209,7 +205,7 @@ namespace Fx advance(); return { Comma{} }; default: - error(Misc::StringUtils::format("unexpected token <%c>", head())); + error(std::format("unexpected token <{}>", head())); } } diff --git a/components/fx/pass.cpp b/components/fx/pass.cpp index 85b420f4fd..eceffa188f 100644 --- a/components/fx/pass.cpp +++ b/components/fx/pass.cpp @@ -300,7 +300,7 @@ float omw_EstimateFogCoverageFromUV(vec2 uv) header.replace(pos, define.size(), value); for (const auto& target : mRenderTargets) - header.append("uniform sampler2D " + std::string(target) + ";"); + header.append("uniform sampler2D " + target + ";"); for (auto& uniform : technique.getUniformMap()) if (auto glsl = uniform->getGLSL()) diff --git a/components/fx/technique.cpp b/components/fx/technique.cpp index ee41ac4f12..b675e7ddad 100644 --- a/components/fx/technique.cpp +++ b/components/fx/technique.cpp @@ -1,6 +1,7 @@ #include "technique.hpp" #include +#include #include #include @@ -91,10 +92,7 @@ namespace Fx std::string Technique::getBlockWithLineDirective() { auto block = mLexer->getLastJumpBlock(); - std::string content = std::string(block.content); - - content = "\n#line " + std::to_string(block.line + 1) + "\n" + std::string(block.content) + "\n"; - return content; + return std::format("\n#line {}\n{}\n", block.line + 1, block.content); } Technique::UniformMap::iterator Technique::findUniform(const std::string& name) @@ -140,9 +138,9 @@ namespace Fx if (it == mPassMap.end()) error( - Misc::StringUtils::format("pass '%s' was found in the pass list, but there was no matching " - "'fragment', 'vertex' or 'compute' block", - std::string(name))); + std::format("pass '{}' was found in the pass list, but there was no matching 'fragment', " + "'vertex' or 'compute' block", + name)); if (mLastAppliedType != Pass::Type::None && mLastAppliedType != it->second->mType) { @@ -167,7 +165,7 @@ namespace Fx { auto rtIt = mRenderTargets.find(it->second->mTarget); if (rtIt == mRenderTargets.end()) - error(Misc::StringUtils::format("target '%s' not defined", std::string(it->second->mTarget))); + error(std::format("target '{}' not defined", it->second->mTarget)); } mPasses.emplace_back(it->second); @@ -212,7 +210,7 @@ namespace Fx void Technique::parseBlockImp() { if (!mLexer->jump()) - error(Misc::StringUtils::format("unterminated 'shared' block, expected closing brackets")); + error("unterminated 'shared' block, expected closing brackets"); if (!mShared.empty()) error("repeated 'shared' block, only one allowed per technique file"); @@ -255,7 +253,7 @@ namespace Fx else if (key == "glsl_profile") { expect(); - mGLSLProfile = std::string(std::get(mToken).value); + mGLSLProfile = std::get(mToken).value; } else if (key == "glsl_extensions") { @@ -265,7 +263,7 @@ namespace Fx else if (key == "dynamic") mDynamic = parseBool(); else - error(Misc::StringUtils::format("unexpected key '%s'", std::string{ key })); + error(std::format("unexpected key '{}'", key)); expect(); } @@ -278,7 +276,7 @@ namespace Fx void Technique::parseBlockImp() { if (mRenderTargets.count(mBlockName)) - error(Misc::StringUtils::format("redeclaration of render target '%s'", std::string(mBlockName))); + error(std::format("redeclaration of render target '{}'", mBlockName)); Fx::Types::RenderTarget rt; rt.mTarget->setTextureSize(mWidth, mHeight); @@ -324,7 +322,7 @@ namespace Fx else if (key == "clear_color") rt.mClearColor = parseVec(); else - error(Misc::StringUtils::format("unexpected key '%s'", std::string(key))); + error(std::format("unexpected key '{}'", key)); expect(); } @@ -336,7 +334,7 @@ namespace Fx void Technique::parseBlockImp() { if (!mLexer->jump()) - error(Misc::StringUtils::format("unterminated 'vertex' block, expected closing brackets")); + error("unterminated 'vertex' block, expected closing brackets"); auto& pass = mPassMap[mBlockName]; @@ -346,11 +344,11 @@ namespace Fx pass->mName = mBlockName; if (pass->mCompute) - error(Misc::StringUtils::format("'compute' block already defined. Usage is ambiguous.")); + error("'compute' block already defined. Usage is ambiguous."); else if (!pass->mVertex) pass->mVertex = new osg::Shader(osg::Shader::VERTEX, getBlockWithLineDirective()); else - error(Misc::StringUtils::format("duplicate vertex shader for block '%s'", std::string(mBlockName))); + error(std::format("duplicate vertex shader for block '{}'", mBlockName)); pass->mType = Pass::Type::Pixel; } @@ -359,7 +357,7 @@ namespace Fx void Technique::parseBlockImp() { if (!mLexer->jump()) - error(Misc::StringUtils::format("unterminated 'fragment' block, expected closing brackets")); + error("unterminated 'fragment' block, expected closing brackets"); auto& pass = mPassMap[mBlockName]; @@ -370,11 +368,11 @@ namespace Fx pass->mName = mBlockName; if (pass->mCompute) - error(Misc::StringUtils::format("'compute' block already defined. Usage is ambiguous.")); + error("'compute' block already defined. Usage is ambiguous."); else if (!pass->mFragment) pass->mFragment = new osg::Shader(osg::Shader::FRAGMENT, getBlockWithLineDirective()); else - error(Misc::StringUtils::format("duplicate vertex shader for block '%s'", std::string(mBlockName))); + error(std::format("duplicate vertex shader for block '{}'", mBlockName)); pass->mType = Pass::Type::Pixel; } @@ -383,7 +381,7 @@ namespace Fx void Technique::parseBlockImp() { if (!mLexer->jump()) - error(Misc::StringUtils::format("unterminated 'compute' block, expected closing brackets")); + error("unterminated 'compute' block, expected closing brackets"); auto& pass = mPassMap[mBlockName]; @@ -393,13 +391,13 @@ namespace Fx pass->mName = mBlockName; if (pass->mFragment) - error(Misc::StringUtils::format("'fragment' block already defined. Usage is ambiguous.")); + error("'fragment' block already defined. Usage is ambiguous."); else if (pass->mVertex) - error(Misc::StringUtils::format("'vertex' block already defined. Usage is ambiguous.")); + error("'vertex' block already defined. Usage is ambiguous."); else if (!pass->mFragment) pass->mCompute = new osg::Shader(osg::Shader::COMPUTE, getBlockWithLineDirective()); else - error(Misc::StringUtils::format("duplicate vertex shader for block '%s'", std::string(mBlockName))); + error(std::format("duplicate vertex shader for block '{}'", mBlockName)); pass->mType = Pass::Type::Compute; } @@ -408,7 +406,7 @@ namespace Fx void Technique::parseSampler() { if (findUniform(std::string(mBlockName)) != mDefinedUniforms.end()) - error(Misc::StringUtils::format("redeclaration of uniform '%s'", std::string(mBlockName))); + error(std::format("redeclaration of uniform '{}'", mBlockName)); ProxyTextureData proxy; osg::ref_ptr sampler; @@ -466,13 +464,12 @@ namespace Fx } } else - error(Misc::StringUtils::format("unexpected key '%s'", std::string{ key })); + error(std::format("unexpected key '{}'", key)); expect(); } if (!sampler) - error(Misc::StringUtils::format( - "%s '%s' requires a filename", std::string(T::repr), std::string{ mBlockName })); + error(std::format("{} '{}' requires a filename", T::repr, mBlockName)); if (!is1D) { @@ -497,7 +494,7 @@ namespace Fx std::shared_ptr uniform = std::make_shared(); uniform->mSamplerType = type; - uniform->mName = std::string(mBlockName); + uniform->mName = mBlockName; mDefinedUniforms.emplace_back(std::move(uniform)); } @@ -534,7 +531,7 @@ namespace Fx void Technique::parseUniform() { if (findUniform(std::string(mBlockName)) != mDefinedUniforms.end()) - error(Misc::StringUtils::format("redeclaration of uniform '%s'", std::string(mBlockName))); + error(std::format("redeclaration of uniform '{}'", mBlockName)); std::shared_ptr uniform = std::make_shared(); Types::Uniform data = Types::Uniform(); @@ -591,7 +588,7 @@ namespace Fx parseWidgetType(data); } else - error(Misc::StringUtils::format("unexpected key '%s'", std::string{ key })); + error(std::format("unexpected key '{}'", key)); expect(); } @@ -599,7 +596,7 @@ namespace Fx if (data.isArray()) uniform->mStatic = false; - uniform->mName = std::string(mBlockName); + uniform->mName = mBlockName; uniform->mData = data; uniform->mTechniqueName = mName; @@ -680,9 +677,9 @@ namespace Fx if (!std::holds_alternative(mToken)) { if (err.empty()) - error(Misc::StringUtils::format("Expected %s", std::string(T::repr))); + error(std::format("Expected {}", T::repr)); else - error(Misc::StringUtils::format("%s. Expected %s", err, std::string(T::repr))); + error(std::format("{}. Expected {}", err, T::repr)); } } @@ -693,10 +690,9 @@ namespace Fx if (!std::holds_alternative(mToken) && !std::holds_alternative(mToken)) { if (err.empty()) - error(Misc::StringUtils::format( - "%s. Expected %s or %s", err, std::string(T::repr), std::string(T2::repr))); + error(std::format("{}. Expected {} or {}", err, T::repr, T2::repr)); else - error(Misc::StringUtils::format("Expected %s or %s", std::string(T::repr), std::string(T2::repr))); + error(std::format("Expected {} or {}", T::repr, T2::repr)); } } @@ -856,14 +852,14 @@ namespace Fx pass->mBlendEq = blendEq; } else - error(Misc::StringUtils::format("unrecognized key '%s' in block header", std::string(key))); + error(std::format("unrecognized key '{}' in block header", key)); mToken = mLexer->next(); if (std::holds_alternative(mToken)) { if (std::holds_alternative(mLexer->peek())) - error(Misc::StringUtils::format("leading comma in '%s' is not allowed", std::string(mBlockName))); + error(std::format("leading comma in '{}' is not allowed", mBlockName)); else continue; } @@ -888,7 +884,7 @@ namespace Fx if (Misc::StringUtils::ciEqual(term, identifer)) return bit; } - error(Misc::StringUtils::format("unrecognized flag '%s'", std::string(term))); + error(std::format("unrecognized flag '{}'", term)); }; FlagsType flag = 0; @@ -908,7 +904,7 @@ namespace Fx return mode; } - error(Misc::StringUtils::format("unrecognized filter mode '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized filter mode '{}'", asLiteral())); } osg::Texture::WrapMode Technique::parseWrapMode() @@ -926,7 +922,7 @@ namespace Fx "unsupported wrap mode 'clamp'; 'clamp_to_edge' was likely intended, look for an updated shader or " "contact author"); - error(Misc::StringUtils::format("unrecognized wrap mode '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized wrap mode '{}'", asLiteral())); } osg::Texture::InternalFormatMode Technique::parseCompression() @@ -939,7 +935,7 @@ namespace Fx return mode; } - error(Misc::StringUtils::format("unrecognized compression '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized compression '{}'", asLiteral())); } int Technique::parseInternalFormat() @@ -952,7 +948,7 @@ namespace Fx return mode; } - error(Misc::StringUtils::format("unrecognized internal format '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized internal format '{}'", asLiteral())); } int Technique::parseSourceType() @@ -965,7 +961,7 @@ namespace Fx return mode; } - error(Misc::StringUtils::format("unrecognized source type '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized source type '{}'", asLiteral())); } int Technique::parseSourceFormat() @@ -978,7 +974,7 @@ namespace Fx return mode; } - error(Misc::StringUtils::format("unrecognized source format '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized source format '{}'", asLiteral())); } osg::BlendEquation::Equation Technique::parseBlendEquation() @@ -991,7 +987,7 @@ namespace Fx return mode; } - error(Misc::StringUtils::format("unrecognized blend equation '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized blend equation '{}'", asLiteral())); } osg::BlendFunc::BlendFuncMode Technique::parseBlendFuncMode() @@ -1004,7 +1000,7 @@ namespace Fx return mode; } - error(Misc::StringUtils::format("unrecognized blend function '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized blend function '{}'", asLiteral())); } template @@ -1057,7 +1053,7 @@ namespace Fx } else { - error(Misc::StringUtils::format("unrecognized widget type '%s'", std::string{ asLiteral() })); + error(std::format("unrecognized widget type '{}'", asLiteral())); } } diff --git a/components/fx/types.hpp b/components/fx/types.hpp index 440bc69470..7897156d7f 100644 --- a/components/fx/types.hpp +++ b/components/fx/types.hpp @@ -1,6 +1,7 @@ #ifndef OPENMW_COMPONENTS_FX_TYPES_HPP #define OPENMW_COMPONENTS_FX_TYPES_HPP +#include #include #include @@ -8,7 +9,6 @@ #include #include -#include #include #include @@ -236,11 +236,11 @@ namespace Fx switch (mSamplerType.value()) { case Texture_1D: - return Misc::StringUtils::format("uniform sampler1D %s;", mName); + return std::format("uniform sampler1D {};", mName); case Texture_2D: - return Misc::StringUtils::format("uniform sampler2D %s;", mName); + return std::format("uniform sampler2D {};", mName); case Texture_3D: - return Misc::StringUtils::format("uniform sampler3D %s;", mName); + return std::format("uniform sampler3D {};", mName); } } @@ -253,54 +253,52 @@ namespace Fx const bool useUniform = arg.isArray() || (Settings::ShaderManager::get().getMode() == Settings::ShaderManager::Mode::Debug || mStatic == false); - const std::string uname = arg.isArray() - ? Misc::StringUtils::format("%s[%zu]", mName, arg.getArray().size()) - : mName; + const std::string uname + = arg.isArray() ? std::format("{}[{}]", mName, arg.getArray().size()) : mName; if constexpr (std::is_same_v) { if (useUniform) - return Misc::StringUtils::format("uniform vec2 %s;", uname); + return std::format("uniform vec2 {};", uname); - return Misc::StringUtils::format("const vec2 %s=vec2(%f,%f);", mName, value[0], value[1]); + return std::format("const vec2 {}=vec2({},{});", mName, value[0], value[1]); } else if constexpr (std::is_same_v) { if (useUniform) - return Misc::StringUtils::format("uniform vec3 %s;", uname); + return std::format("uniform vec3 {};", uname); - return Misc::StringUtils::format( - "const vec3 %s=vec3(%f,%f,%f);", mName, value[0], value[1], value[2]); + return std::format("const vec3 {}=vec3({},{},{});", mName, value[0], value[1], value[2]); } else if constexpr (std::is_same_v) { if (useUniform) - return Misc::StringUtils::format("uniform vec4 %s;", uname); + return std::format("uniform vec4 {};", uname); - return Misc::StringUtils::format( - "const vec4 %s=vec4(%f,%f,%f,%f);", mName, value[0], value[1], value[2], value[3]); + return std::format( + "const vec4 {}=vec4({},{},{},{});", mName, value[0], value[1], value[2], value[3]); } else if constexpr (std::is_same_v) { if (useUniform) - return Misc::StringUtils::format("uniform float %s;", uname); + return std::format("uniform float {};", uname); - return Misc::StringUtils::format("const float %s=%f;", mName, value); + return std::format("const float {}={};", mName, value); } else if constexpr (std::is_same_v) { if (useUniform) - return Misc::StringUtils::format("uniform int %s;", uname); + return std::format("uniform int {};", uname); - return Misc::StringUtils::format("const int %s=%i;", mName, value); + return std::format("const int {}={};", mName, value); } else { static_assert(std::is_same_v, "Non-exhaustive visitor"); if (useUniform) - return Misc::StringUtils::format("uniform bool %s;", uname); + return std::format("uniform bool {};", uname); - return Misc::StringUtils::format("const bool %s=%s;", mName, value ? "true" : "false"); + return std::format("const bool {}={};", mName, value ? "true" : "false"); } }, mData); diff --git a/components/fx/widgets.hpp b/components/fx/widgets.hpp index c91fa01c4e..ca1b76f2e1 100644 --- a/components/fx/widgets.hpp +++ b/components/fx/widgets.hpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -18,8 +19,6 @@ #include #include -#include - #include "types.hpp" namespace Gui @@ -88,7 +87,7 @@ namespace Fx { mValue = value; if constexpr (std::is_floating_point_v) - mValueLabel->setCaption(Misc::StringUtils::format("%.3f", mValue)); + mValueLabel->setCaption(std::format("{:.3f}", mValue)); else mValueLabel->setCaption(std::to_string(mValue)); From 55c72ecb296b19fdc96c77af0573631ae4c0afd7 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sat, 16 Aug 2025 13:49:07 +0200 Subject: [PATCH 2/6] Use string_view in components/fx --- components/fx/lexer.cpp | 2 +- components/fx/lexer.hpp | 2 +- components/fx/technique.cpp | 14 +++++++------- components/fx/technique.hpp | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/components/fx/lexer.cpp b/components/fx/lexer.cpp index 4369febbcb..39cad9970a 100644 --- a/components/fx/lexer.cpp +++ b/components/fx/lexer.cpp @@ -120,7 +120,7 @@ namespace Fx return mLastJumpBlock; } - [[noreturn]] void Lexer::error(const std::string& msg) + [[noreturn]] void Lexer::error(std::string_view msg) { throw LexerException(std::format("Line {} Col {}. {}", mLine + 1, mColumn, msg)); } diff --git a/components/fx/lexer.hpp b/components/fx/lexer.hpp index 1b32298608..f49dbcd694 100644 --- a/components/fx/lexer.hpp +++ b/components/fx/lexer.hpp @@ -46,7 +46,7 @@ namespace Fx Block getLastJumpBlock() const; - [[noreturn]] void error(const std::string& msg); + [[noreturn]] void error(std::string_view msg); private: void drop(); diff --git a/components/fx/technique.cpp b/components/fx/technique.cpp index b675e7ddad..38db49bc7a 100644 --- a/components/fx/technique.cpp +++ b/components/fx/technique.cpp @@ -95,10 +95,10 @@ namespace Fx return std::format("\n#line {}\n{}\n", block.line + 1, block.content); } - Technique::UniformMap::iterator Technique::findUniform(const std::string& name) + Technique::UniformMap::iterator Technique::findUniform(std::string_view name) { return std::find_if(mDefinedUniforms.begin(), mDefinedUniforms.end(), - [&name](const auto& uniform) { return uniform->mName == name; }); + [=](const auto& uniform) { return uniform->mName == name; }); } bool Technique::compile() @@ -201,7 +201,7 @@ namespace Fx return isDirty; } - [[noreturn]] void Technique::error(const std::string& msg) + [[noreturn]] void Technique::error(std::string_view msg) { mLexer->error(msg); } @@ -405,7 +405,7 @@ namespace Fx template void Technique::parseSampler() { - if (findUniform(std::string(mBlockName)) != mDefinedUniforms.end()) + if (findUniform(mBlockName) != mDefinedUniforms.end()) error(std::format("redeclaration of uniform '{}'", mBlockName)); ProxyTextureData proxy; @@ -530,7 +530,7 @@ namespace Fx template void Technique::parseUniform() { - if (findUniform(std::string(mBlockName)) != mDefinedUniforms.end()) + if (findUniform(mBlockName) != mDefinedUniforms.end()) error(std::format("redeclaration of uniform '{}'", mBlockName)); std::shared_ptr uniform = std::make_shared(); @@ -671,7 +671,7 @@ namespace Fx } template - void Technique::expect(const std::string& err) + void Technique::expect(std::string_view err) { mToken = mLexer->next(); if (!std::holds_alternative(mToken)) @@ -684,7 +684,7 @@ namespace Fx } template - void Technique::expect(const std::string& err) + void Technique::expect(std::string_view err) { mToken = mLexer->next(); if (!std::holds_alternative(mToken) && !std::holds_alternative(mToken)) diff --git a/components/fx/technique.hpp b/components/fx/technique.hpp index ebf3fe30f5..3db46447dd 100644 --- a/components/fx/technique.hpp +++ b/components/fx/technique.hpp @@ -172,7 +172,7 @@ namespace Fx std::string getLastError() const { return mLastError; } - UniformMap::iterator findUniform(const std::string& name); + UniformMap::iterator findUniform(std::string_view name); bool getDynamic() const { return mDynamic; } @@ -183,17 +183,17 @@ namespace Fx bool getInternal() const { return mInternal; } private: - [[noreturn]] void error(const std::string& msg); + [[noreturn]] void error(std::string_view msg); void clear(); std::string_view asLiteral() const; template - void expect(const std::string& err = ""); + void expect(std::string_view err = {}); template - void expect(const std::string& err = ""); + void expect(std::string_view err = {}); template bool isNext(); From 86e40f5b6b260c97e5c56ef134c83f8072b08bef Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sat, 16 Aug 2025 14:01:21 +0200 Subject: [PATCH 3/6] Remove potential sources of UB --- components/fx/lexer.cpp | 20 +++++++------------- components/fx/lexer.hpp | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/components/fx/lexer.cpp b/components/fx/lexer.cpp index 39cad9970a..9844a6de6e 100644 --- a/components/fx/lexer.cpp +++ b/components/fx/lexer.cpp @@ -1,7 +1,7 @@ #include "lexer.hpp" #include -#include +#include #include namespace Fx @@ -132,7 +132,7 @@ namespace Fx mColumn++; } - char Lexer::head() + unsigned char Lexer::head() { return *mHead; } @@ -289,21 +289,15 @@ namespace Fx Token Lexer::scanNumber() { double buffer; - - char* endPtr; - buffer = std::strtod(mHead, &endPtr); - - if (endPtr == nullptr) + const auto [endPtr, ec] = std::from_chars(mHead, mTail, buffer); + if (ec != std::errc()) error("critical error while parsing number"); - const char* tmp = mHead; + std::string_view literal(mHead, endPtr); mHead = endPtr; - for (; tmp != endPtr; ++tmp) - { - if ((*tmp == '.')) - return Float{ static_cast(buffer) }; - } + if (literal.find('.') != std::string_view::npos) + return Float{ static_cast(buffer) }; return Integer{ static_cast(buffer) }; } diff --git a/components/fx/lexer.hpp b/components/fx/lexer.hpp index f49dbcd694..c9120698aa 100644 --- a/components/fx/lexer.hpp +++ b/components/fx/lexer.hpp @@ -51,7 +51,7 @@ namespace Fx private: void drop(); void advance(); - char head(); + unsigned char head(); bool peekChar(char c); Token scanToken(); From 5f3f3a7e1da6bfe651afa13546700d1a610f89c8 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sun, 17 Aug 2025 08:48:04 +0200 Subject: [PATCH 4/6] Address feedback --- components/fx/lexer.cpp | 2 +- components/fx/technique.cpp | 10 +++++----- components/fx/types.hpp | 20 ++++++++------------ 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/components/fx/lexer.cpp b/components/fx/lexer.cpp index 9844a6de6e..e2759e7af0 100644 --- a/components/fx/lexer.cpp +++ b/components/fx/lexer.cpp @@ -205,7 +205,7 @@ namespace Fx advance(); return { Comma{} }; default: - error(std::format("unexpected token <{}>", head())); + error(std::format("unexpected token <{:c}>", head())); } } diff --git a/components/fx/technique.cpp b/components/fx/technique.cpp index 38db49bc7a..ac55a16b6f 100644 --- a/components/fx/technique.cpp +++ b/components/fx/technique.cpp @@ -98,7 +98,7 @@ namespace Fx Technique::UniformMap::iterator Technique::findUniform(std::string_view name) { return std::find_if(mDefinedUniforms.begin(), mDefinedUniforms.end(), - [=](const auto& uniform) { return uniform->mName == name; }); + [name](const auto& uniform) { return uniform->mName == name; }); } bool Technique::compile() @@ -372,7 +372,7 @@ namespace Fx else if (!pass->mFragment) pass->mFragment = new osg::Shader(osg::Shader::FRAGMENT, getBlockWithLineDirective()); else - error(std::format("duplicate vertex shader for block '{}'", mBlockName)); + error(std::format("duplicate fragment shader for block '{}'", mBlockName)); pass->mType = Pass::Type::Pixel; } @@ -397,7 +397,7 @@ namespace Fx else if (!pass->mFragment) pass->mCompute = new osg::Shader(osg::Shader::COMPUTE, getBlockWithLineDirective()); else - error(std::format("duplicate vertex shader for block '{}'", mBlockName)); + error(std::format("duplicate compute shader for block '{}'", mBlockName)); pass->mType = Pass::Type::Compute; } @@ -690,9 +690,9 @@ namespace Fx if (!std::holds_alternative(mToken) && !std::holds_alternative(mToken)) { if (err.empty()) - error(std::format("{}. Expected {} or {}", err, T::repr, T2::repr)); - else error(std::format("Expected {} or {}", T::repr, T2::repr)); + else + error(std::format("{}. Expected {} or {}", err, T::repr, T2::repr)); } } diff --git a/components/fx/types.hpp b/components/fx/types.hpp index 7897156d7f..bdb46a76c1 100644 --- a/components/fx/types.hpp +++ b/components/fx/types.hpp @@ -112,12 +112,7 @@ namespace Fx size_t getNumElements() const { - return std::visit( - [&](auto&& arg) { - ; - return arg.isArray() ? arg.getArray().size() : 1; - }, - mData); + return std::visit([](auto&& arg) { return arg.isArray() ? arg.getArray().size() : 1; }, mData); } template @@ -261,29 +256,30 @@ namespace Fx if (useUniform) return std::format("uniform vec2 {};", uname); - return std::format("const vec2 {}=vec2({},{});", mName, value[0], value[1]); + return std::format("const vec2 {}=vec2({:f},{:f});", mName, value[0], value[1]); } else if constexpr (std::is_same_v) { if (useUniform) return std::format("uniform vec3 {};", uname); - return std::format("const vec3 {}=vec3({},{},{});", mName, value[0], value[1], value[2]); + return std::format( + "const vec3 {}=vec3({:f},{:f},{:f});", mName, value[0], value[1], value[2]); } else if constexpr (std::is_same_v) { if (useUniform) return std::format("uniform vec4 {};", uname); - return std::format( - "const vec4 {}=vec4({},{},{},{});", mName, value[0], value[1], value[2], value[3]); + return std::format("const vec4 {}=vec4({:f},{:f},{:f},{:f});", mName, value[0], value[1], + value[2], value[3]); } else if constexpr (std::is_same_v) { if (useUniform) return std::format("uniform float {};", uname); - return std::format("const float {}={};", mName, value); + return std::format("const float {}={:f};", mName, value); } else if constexpr (std::is_same_v) { @@ -298,7 +294,7 @@ namespace Fx if (useUniform) return std::format("uniform bool {};", uname); - return std::format("const bool {}={};", mName, value ? "true" : "false"); + return std::format("const bool {}={};", mName, value); } }, mData); From 3f2fd06514c5ecadf123e455bf9d7d5e193c605a Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Sun, 17 Aug 2025 11:26:30 +0200 Subject: [PATCH 5/6] Work around Apple Clang --- components/fx/lexer.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/components/fx/lexer.cpp b/components/fx/lexer.cpp index e2759e7af0..07d7f060a4 100644 --- a/components/fx/lexer.cpp +++ b/components/fx/lexer.cpp @@ -1,9 +1,14 @@ #include "lexer.hpp" #include -#include #include +#ifndef __cpp_lib_to_chars +#include +#else +#include +#endif + namespace Fx { namespace Lexer @@ -289,8 +294,14 @@ namespace Fx Token Lexer::scanNumber() { double buffer; +#ifndef __cpp_lib_to_chars + char* endPtr = nullptr; + buffer = std::strtod(mHead, &endPtr); + if (endPtr == nullptr || endPtr == mHead) +#else const auto [endPtr, ec] = std::from_chars(mHead, mTail, buffer); if (ec != std::errc()) +#endif error("critical error while parsing number"); std::string_view literal(mHead, endPtr); From a6c942b33aa3b3572ca92d2a85ef2d695a3e3ba7 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Mon, 18 Aug 2025 19:52:28 +0200 Subject: [PATCH 6/6] Account for numeric precision and infinities. Also pretend to be more like GLSL --- apps/components_tests/fx/lexer.cpp | 11 ++++ apps/components_tests/fx/technique.cpp | 91 ++++++++++++++++++++++---- components/fx/lexer.cpp | 43 ++++++++---- components/fx/technique.cpp | 2 +- 4 files changed, 121 insertions(+), 26 deletions(-) diff --git a/apps/components_tests/fx/lexer.cpp b/apps/components_tests/fx/lexer.cpp index 976f88d7a3..f8996118da 100644 --- a/apps/components_tests/fx/lexer.cpp +++ b/apps/components_tests/fx/lexer.cpp @@ -135,6 +135,17 @@ namespace EXPECT_EQ(std::get(token).value, 123); } + TEST(LexerTest, float_suffix_should_be_float) + { + Lexer lexer(R"( + 123f + )"); + + auto token = lexer.next(); + EXPECT_TRUE(std::holds_alternative(token)); + EXPECT_FLOAT_EQ(std::get(token).value, 123.f); + } + TEST(LexerTest, simple_string) { Lexer lexer(R"( diff --git a/apps/components_tests/fx/technique.cpp b/apps/components_tests/fx/technique.cpp index b04ff5c52a..63d65ef95f 100644 --- a/apps/components_tests/fx/technique.cpp +++ b/apps/components_tests/fx/technique.cpp @@ -90,6 +90,56 @@ namespace technique { passes = main; } )" }; + constexpr VFS::Path::NormalizedView invalidNumberInfPath("shaders/invalid_number_inf.omwfx"); + + TestingOpenMW::VFSTestFile invalidNumberInf{ R"( + uniform_vec4 uVec4 { + step = inf; + } + fragment main { } + technique { passes = main; } +)" }; + + constexpr VFS::Path::NormalizedView invalidNumberNegativeInfPath("shaders/invalid_number_negative_inf.omwfx"); + + TestingOpenMW::VFSTestFile invalidNumberNegativeInf{ R"( + uniform_vec4 uVec4 { + step = -inf; + } + fragment main { } + technique { passes = main; } +)" }; + + constexpr VFS::Path::NormalizedView invalidNumberUnsignedLongPath("shaders/invalid_number_ulong.omwfx"); + + TestingOpenMW::VFSTestFile invalidNumberUnsignedLong{ R"( + uniform_vec4 uVec4 { + step = 18446744073709551615; + } + fragment main { } + technique { passes = main; } +)" }; + + constexpr VFS::Path::NormalizedView invalidNumberHexFloatPath("shaders/invalid_number_hex.omwfx"); + + TestingOpenMW::VFSTestFile invalidNumberHexFloat{ R"( + uniform_vec4 uVec4 { + step = 0x1.fffffep+12; + } + fragment main { } + technique { passes = main; } +)" }; + + constexpr VFS::Path::NormalizedView invalidNumberDoublePath("shaders/invalid_number_double.omwfx"); + + TestingOpenMW::VFSTestFile invalidNumberDouble{ R"( + uniform_vec4 uVec4 { + step = 1.79769e+50; + } + fragment main { } + technique { passes = main; } +)" }; + using namespace testing; using namespace Fx; @@ -106,6 +156,11 @@ namespace { uniformPropertiesPath, &uniformProperties }, { missingSamplerSourcePath, &missingSamplerSource }, { repeatedSharedBlockPath, &repeatedSharedBlock }, + { invalidNumberInfPath, &invalidNumberInf }, + { invalidNumberNegativeInfPath, &invalidNumberNegativeInf }, + { invalidNumberUnsignedLongPath, &invalidNumberUnsignedLong }, + { invalidNumberHexFloatPath, &invalidNumberHexFloat }, + { invalidNumberDoublePath, &invalidNumberDouble }, })) , mImageManager(mVFS.get(), 0) { @@ -117,6 +172,17 @@ namespace *mVFS.get(), mImageManager, Technique::makeFileName(name), name, 1, 1, true, true); mTechnique->compile(); } + + void expectFailure(const std::string& name, std::string_view errorString) + { + internal::CaptureStdout(); + + compile(name); + + std::string output = internal::GetCapturedStdout(); + Log(Debug::Error) << output; + EXPECT_THAT(output, HasSubstr(errorString)); + } }; TEST_F(TechniqueTest, technique_properties) @@ -183,23 +249,24 @@ namespace TEST_F(TechniqueTest, fail_with_missing_source_for_sampler) { - internal::CaptureStdout(); - - compile("missing_sampler_source"); - - std::string output = internal::GetCapturedStdout(); - Log(Debug::Error) << output; - EXPECT_THAT(output, HasSubstr("sampler_1d 'mysampler1d' requires a filename")); + expectFailure("missing_sampler_source", "sampler_1d 'mysampler1d' requires a filename"); } TEST_F(TechniqueTest, fail_with_repeated_shared_block) { - internal::CaptureStdout(); + expectFailure("repeated_shared_block", "repeated 'shared' block"); + } - compile("repeated_shared_block"); + TEST_F(TechniqueTest, fail_with_invalid_float) + { + expectFailure("invalid_number_inf", "expected float value"); + expectFailure("invalid_number_negative_inf", "expected float value"); + expectFailure("invalid_number_hex", "expected float value"); + } - std::string output = internal::GetCapturedStdout(); - Log(Debug::Error) << output; - EXPECT_THAT(output, HasSubstr("repeated 'shared' block")); + TEST_F(TechniqueTest, fail_with_out_of_range) + { + expectFailure("invalid_number_ulong", "number out of range"); + expectFailure("invalid_number_double", "number out of range"); } } diff --git a/components/fx/lexer.cpp b/components/fx/lexer.cpp index 07d7f060a4..43ce17717f 100644 --- a/components/fx/lexer.cpp +++ b/components/fx/lexer.cpp @@ -1,13 +1,11 @@ #include "lexer.hpp" #include +#include +#include #include -#ifndef __cpp_lib_to_chars -#include -#else -#include -#endif +#include namespace Fx { @@ -294,23 +292,42 @@ namespace Fx Token Lexer::scanNumber() { double buffer; -#ifndef __cpp_lib_to_chars char* endPtr = nullptr; buffer = std::strtod(mHead, &endPtr); if (endPtr == nullptr || endPtr == mHead) -#else - const auto [endPtr, ec] = std::from_chars(mHead, mTail, buffer); - if (ec != std::errc()) -#endif error("critical error while parsing number"); + bool isDefinitelyAFloat = false; + // GLSL allows floats to end on f/F + if (endPtr != mTail && Misc::StringUtils::toLower(*endPtr) == 'f') + { + isDefinitelyAFloat = true; + ++endPtr; + } + std::string_view literal(mHead, endPtr); mHead = endPtr; - if (literal.find('.') != std::string_view::npos) - return Float{ static_cast(buffer) }; + // Disallow -inf, -nan, and values that cannot be represented as doubles + if (!std::isfinite(buffer)) + return Literal{ literal }; + // Disallow hex notation (not allowed in GLSL so confusing to partially allow) + if (Misc::StringUtils::ciStartsWith(literal, "0x") || Misc::StringUtils::ciStartsWith(literal, "-0x")) + return Literal{ literal }; - return Integer{ static_cast(buffer) }; + constexpr std::string_view floatCharacters = ".eE"; + if (!isDefinitelyAFloat && literal.find_first_of(floatCharacters) == std::string_view::npos) + { + // This is supposed to be an int, but that doesn't mean it can fit in one + int intValue = static_cast(buffer); + if (intValue != buffer) + error("number out of range"); + return Integer{ intValue }; + } + float floatValue = static_cast(buffer); + if (!std::isfinite(floatValue)) + error("number out of range"); + return Float{ floatValue }; } } } diff --git a/components/fx/technique.cpp b/components/fx/technique.cpp index ac55a16b6f..945a3cf1b1 100644 --- a/components/fx/technique.cpp +++ b/components/fx/technique.cpp @@ -758,7 +758,7 @@ namespace Fx if (named) { - expect("name is required for preceeding block decleration"); + expect("name is required for preceeding block declaration"); mBlockName = std::get(mToken).value;