diff --git a/renderdoc/common/formatting.h b/renderdoc/common/formatting.h index c1d738d54..323f6e67a 100644 --- a/renderdoc/common/formatting.h +++ b/renderdoc/common/formatting.h @@ -32,6 +32,8 @@ struct Args { virtual void reset() = 0; + virtual void error(const char *err) = 0; + virtual int get_int() = 0; virtual unsigned int get_uint() = 0; virtual double get_double() = 0; diff --git a/renderdoc/driver/vulkan/vk_shader_feedback.cpp b/renderdoc/driver/vulkan/vk_shader_feedback.cpp index 95de2d639..4bbab5882 100644 --- a/renderdoc/driver/vulkan/vk_shader_feedback.cpp +++ b/renderdoc/driver/vulkan/vk_shader_feedback.cpp @@ -53,7 +53,8 @@ struct feedbackData struct PrintfData { - rdcstr format; + rdcstr user_format; + rdcstr effective_format; // vectors are expanded so there's one for each component (as printf will expect) rdcarray argTypes; size_t payloadWords; @@ -72,6 +73,7 @@ public: m_Cur = m_Start; m_Idx = 0; } + void error(const char *err) override { m_Error = err; } int get_int() override { int32_t ret = *(int32_t *)m_Cur; @@ -125,11 +127,13 @@ public: } size_t get_size() override { return sizeof(size_t) == 8 ? (size_t)get_uint64() : get_uint(); } + rdcstr get_error() { return m_Error; } private: const uint32_t *m_Cur; const uint32_t *m_Start; size_t m_Idx; const PrintfData &m_Formats; + rdcstr m_Error; }; rdcstr PatchFormatString(rdcstr format) @@ -886,7 +890,8 @@ void AnnotateShader(const ShaderReflection &refl, const SPIRVPatchData &patchDat { rdcspv::OpString str(editor.GetID(rdcspv::Id::fromWord(extinst.params[0]))); - format.format = PatchFormatString(str.string); + format.user_format = str.string; + format.effective_format = PatchFormatString(str.string); } rdcarray packetWords; @@ -1848,7 +1853,10 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) RDCLOG("pixel %u, %u", msg.location.pixel.x, msg.location.pixel.y); } - msg.message = StringFormat::FmtArgs(fmt.format.c_str(), args); + msg.message = StringFormat::FmtArgs(fmt.effective_format.c_str(), args); + + if(!args.get_error().empty()) + msg.message = args.get_error() + " in \"" + fmt.user_format + "\""; result.messages.push_back(msg); } diff --git a/renderdoc/strings/utf8printf.cpp b/renderdoc/strings/utf8printf.cpp index a5a896d91..5517d3add 100644 --- a/renderdoc/strings/utf8printf.cpp +++ b/renderdoc/strings/utf8printf.cpp @@ -1401,11 +1401,6 @@ void formatargument(char type, void *rawarg, FormatterParams formatter, char *&o PrintFloat(argd, formatter, e, f, g, a, uppercaseDigits, output, actualsize, end); } - else - { - // Unrecognised format specifier - RDCDUMPMSG("Unrecognised % formatter"); - } } struct va_arg_getter @@ -1417,6 +1412,8 @@ struct va_arg_getter { return va_arg(list, T); } + + void error(const char *err) {} }; struct custom_arg_getter @@ -1425,6 +1422,8 @@ struct custom_arg_getter custom_arg_getter(StringFormat::Args &f) : formatter(f) {} template inline T get_next(); + + void error(const char *err) { formatter.error(err); } }; template <> @@ -1460,6 +1459,18 @@ inline size_t custom_arg_getter::get_next() } #endif +int utf8print_error(char *buf, size_t bufsize, const char *str) +{ + // copy the string with no formatting in the error case + size_t ret = strlen(str); + if(bufsize > 0) + { + memcpy(buf, str, RDCMIN(bufsize - 1, ret)); + buf[RDCMIN(bufsize - 1, ret)] = 0; + } + return int(ret); +} + template int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter args) { @@ -1481,7 +1492,10 @@ int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter ar iter++; if(*iter == 0) - RDCDUMPMSG("unterminated formatter (should be %% if you want a literal %)"); + { + args.error("unterminated formatter (should be %% if you want a literal %)"); + return utf8print_error(buf, bufsize, fmt); + } if(*iter == '%') // %% found, insert single % and continue copying { @@ -1556,7 +1570,10 @@ int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter ar // unterminated formatter if(*iter == 0) - RDCDUMPMSG("Unterminated % formatter found after width"); + { + args.error("Unterminated % formatter found after width"); + return utf8print_error(buf, bufsize, fmt); + } } else { @@ -1597,7 +1614,10 @@ int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter ar // unterminated formatter if(*iter == 0) - RDCDUMPMSG("Unterminated % formatter found after precision"); + { + args.error("Unterminated % formatter found after precision"); + return utf8print_error(buf, bufsize, fmt); + } } } else @@ -1688,7 +1708,8 @@ int utf8print_template(char *buf, size_t bufsize, const char *fmt, arg_getter ar } else { - RDCDUMPMSG("Unrecognised % formatter"); + args.error("Unrecognised % formatter"); + return utf8print_error(buf, bufsize, fmt); } formatargument(type, arg, formatter, output, actualsize, end); @@ -1915,6 +1936,24 @@ TEST_CASE("utf8printf buffer sizing", "[utf8printf]") }; }; +TEST_CASE("utf8printf error cases", "[utf8printf]") +{ + // unrecognised format type (covers most errors where an invalid character happens after optional + // precision stuff + CHECK(StringFormat::Fmt("%f foo %s: %q", 1.234f, "blah", 777) == "%f foo %s: %q"); + CHECK(StringFormat::Fmt("%f foo %s: %8q", 1.234f, "blah", 777) == "%f foo %s: %8q"); + CHECK(StringFormat::Fmt("%f foo %s: %1.8q", 1.234f, "blah", 777) == "%f foo %s: %1.8q"); + + // unterminated % at the end of the string + CHECK(StringFormat::Fmt("%f foo %s: %", 1.234f, "blah", 777) == "%f foo %s: %"); + CHECK(StringFormat::Fmt("%f foo %s: %8", 1.234f, "blah", 777) == "%f foo %s: %8"); + CHECK(StringFormat::Fmt("%f foo %s: %1.8", 1.234f, "blah", 777) == "%f foo %s: %1.8"); + CHECK(StringFormat::Fmt("%f foo %s: %1.", 1.234f, "blah", 777) == "%f foo %s: %1."); + + // precision as vararg + CHECK(StringFormat::Fmt("%f foo %*s", 1.234f, "blah", 777) == "%f foo %*s"); +}; + TEST_CASE("utf8printf standard string formatters", "[utf8printf]") { const wchar_t wc = L'\xe1';