From 4b741987372255c62fab74e68bfa5ebe166a3b9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Ke=C3=9Fler?= Date: Mon, 29 Nov 2021 13:38:40 +0100 Subject: [PATCH] Fixed bug in serdes of Named color; ColorU8 was read into ColorU16 Replaced some calls to imbue with an appropriate call to an template function. Added some imbues to serdesstreams. --- src/core/control/latex/LatexGenerator.cpp | 4 +- src/core/gui/toolbarMenubar/FontButton.cpp | 5 +- .../gui/toolbarMenubar/model/ColorPalette.cpp | 68 +++++++++---------- src/core/pdf/base/XojCairoPdfExport.cpp | 4 +- src/util/NamedColor.cpp | 18 ++--- src/util/include/util/serdesstream.h | 15 ++++ 6 files changed, 62 insertions(+), 52 deletions(-) create mode 100644 src/util/include/util/serdesstream.h diff --git a/src/core/control/latex/LatexGenerator.cpp b/src/core/control/latex/LatexGenerator.cpp index 6da639fabf89..19cef4baa782 100644 --- a/src/core/control/latex/LatexGenerator.cpp +++ b/src/core/control/latex/LatexGenerator.cpp @@ -9,6 +9,7 @@ #include "util/PathUtil.h" #include "util/i18n.h" +#include "util/serdesstream.h" LatexGenerator::LatexGenerator(const LatexSettings& settings): settings(settings) {} @@ -26,8 +27,7 @@ auto LatexGenerator::templateSub(const std::string& input, const std::string& te if (matchStr == "TOOL_INPUT") { repl = input; } else if (matchStr == "TEXT_COLOR") { - std::ostringstream s; - s.imbue(std::locale::classic()); + auto s = serdes_stream(); auto tmp_color = textColor; tmp_color.alpha = 0; s << std::hex << std::setfill('0') << std::setw(6) << std::right << tmp_color; diff --git a/src/core/gui/toolbarMenubar/FontButton.cpp b/src/core/gui/toolbarMenubar/FontButton.cpp index fbba519d275a..72eaba33cb26 100644 --- a/src/core/gui/toolbarMenubar/FontButton.cpp +++ b/src/core/gui/toolbarMenubar/FontButton.cpp @@ -7,6 +7,7 @@ #include #include "util/i18n.h" +#include "util/serdesstream.h" using std::string; @@ -33,9 +34,7 @@ void FontButton::activated(GdkEvent* event, GtkMenuItem* menuitem, GtkToolButton void FontButton::setFontFontButton(GtkWidget* fontButton, XojFont& font) { GtkFontButton* button = GTK_FONT_BUTTON(fontButton); - // Fixing locale to make format of font-size string independent of localization setting - std::stringstream fontSizeStream; - fontSizeStream.imbue(std::locale("C")); + auto fontSizeStream = serdes_stream(); fontSizeStream << font.getSize(); string name = font.getName() + " " + fontSizeStream.str(); gtk_font_button_set_font_name(button, name.c_str()); diff --git a/src/core/gui/toolbarMenubar/model/ColorPalette.cpp b/src/core/gui/toolbarMenubar/model/ColorPalette.cpp index ec37eed6c756..625e92773dc2 100644 --- a/src/core/gui/toolbarMenubar/model/ColorPalette.cpp +++ b/src/core/gui/toolbarMenubar/model/ColorPalette.cpp @@ -8,9 +8,10 @@ #include "util/StringUtils.h" #include "util/i18n.h" +#include "util/serdesstream.h" -Palette::Palette(fs::path path): filepath{std::move(path)}, header{}, namedColors{} {} +Palette::Palette(fs::path path): filepath{std::move(path)}, namedColors{}, header{} {} void Palette::load() { if (!fs::exists(this->filepath)) @@ -20,7 +21,8 @@ void Palette::load() { header.clear(); namedColors.clear(); - std::ifstream gplFile{filepath}; + + auto gplFile = serdes_stream(filepath); std::string line; if (gplFile.is_open()) { @@ -79,12 +81,7 @@ auto Palette::parseColorLine(const std::string& line) -> bool { return false; } -auto Palette::parseCommentLine(const std::string& line) const -> bool { - if (line.front() == '#') - return true; - else - return false; -} +auto Palette::parseCommentLine(const std::string& line) const -> bool { return line.front() == '#'; } auto Palette::parseLineFallback(int lineNumber) const -> const bool { throw std::invalid_argument(FS(FORMAT_STR("The line {1} is malformed.") % lineNumber)); @@ -104,38 +101,37 @@ auto Palette::size() const -> size_t { return namedColors.size(); } auto Palette::default_palette() -> const std::string { - std::stringstream d{}; - d << "GIMP Palette" << std::endl; - d << "Name: Xournal Default Palette" << std::endl; - d << "#" << std::endl; - d << 0 << " " << 0 << " " << 0 << " " - << "Black" << std::endl; - d << 0 << " " << 128 << " " << 0 << " " - << "Green" << std::endl; - d << 0 << " " << 192 << " " << 255 << " " - << "Light Blue" << std::endl; - d << 0 << " " << 255 << " " << 0 << " " - << "Light Green" << std::endl; - d << 51 << " " << 51 << " " << 204 << " " - << "Blue" << std::endl; - d << 128 << " " << 128 << " " << 128 << " " - << "Gray" << std::endl; - d << 255 << " " << 0 << " " << 0 << " " - << "Red" << std::endl; - d << 255 << " " << 0 << " " << 255 << " " - << "Magenta" << std::endl; - d << 255 << " " << 128 << " " << 0 << " " - << "Orange" << std::endl; - d << 255 << " " << 255 << " " << 0 << " " - << "Yellow" << std::endl; - d << 255 << " " << 255 << " " << 255 << " " + auto d = serdes_stream(); + d << "GIMP Palette\n" + << "Name: Xournal Default Palette\n" + << "#\n" + << 0 << " " << 0 << " " << 0 << " " + << "Black\n" + << 0 << " " << 128 << " " << 0 << " " + << "Green\n" + << 0 << " " << 192 << " " << 255 << " " + << "Light Blue\n" + << 0 << " " << 255 << " " << 0 << " " + << "Light Green\n" + << 51 << " " << 51 << " " << 204 << " " + << "Blue\n" + << 128 << " " << 128 << " " << 128 << " " + << "Gray\n" + << 255 << " " << 0 << " " << 0 << " " + << "Red\n" + << 255 << " " << 0 << " " << 255 << " " + << "Magenta\n" + << 255 << " " << 128 << " " << 0 << " " + << "Orange\n" + << 255 << " " << 255 << " " << 0 << " " + << "Yellow\n" + << 255 << " " << 255 << " " << 255 << " " << "White" << std::endl; return d.str(); } void Palette::create_default(fs::path filepath) { - std::ofstream myfile{filepath}; - myfile.imbue(std::locale::classic()); + auto myfile = serdes_stream(filepath); myfile << default_palette(); } @@ -188,5 +184,5 @@ auto Palette::parseErrorDialog(const std::exception& e) const -> void { gtk_dialog_run(GTK_DIALOG(dialog)); gtk_widget_destroy(dialog); - g_warning(msg_stream.str().c_str()); + g_warning("%s", msg_stream.str().c_str()); } diff --git a/src/core/pdf/base/XojCairoPdfExport.cpp b/src/core/pdf/base/XojCairoPdfExport.cpp index c9378957f3c0..8b50df2b4a64 100644 --- a/src/core/pdf/base/XojCairoPdfExport.cpp +++ b/src/core/pdf/base/XojCairoPdfExport.cpp @@ -9,6 +9,7 @@ #include "util/Util.h" #include "util/i18n.h" +#include "util/serdesstream.h" #include "view/DocumentView.h" #include "filesystem.h" @@ -69,8 +70,7 @@ void XojCairoPdfExport::populatePdfOutline(GtkTreeModel* tocModel) { auto pdfBgPage = dest->getPdfPage(); // Link destination in original background PDF auto pageDest = pdfBgPage == npos ? npos : doc->findPdfPage(pdfBgPage); // Destination in document if (pageDest != npos) { - std::ostringstream linkAttrBuf; - linkAttrBuf.imbue(std::locale::classic()); + auto linkAttrBuf = serdes_stream(); linkAttrBuf << "page=" << pageDest + 1; if (dest->shouldChangeLeft() && dest->shouldChangeTop()) { linkAttrBuf << " pos=[" << dest->getLeft() << " " << dest->getTop() << "]"; diff --git a/src/util/NamedColor.cpp b/src/util/NamedColor.cpp index fd25f9345260..dd93c7391748 100644 --- a/src/util/NamedColor.cpp +++ b/src/util/NamedColor.cpp @@ -1,8 +1,11 @@ #include "util/NamedColor.h" +#include #include #include "util/StringUtils.h" +#include "util/serdesstream.h" + NamedColor::NamedColor(): paletteIndex{0}, name{"Custom Color"}, colorU16{ColorU16{}}, color{Color(0u)}, isPaletteColor{false} {} @@ -25,18 +28,15 @@ auto operator>>(std::istream& str, NamedColor& namedColor) -> std::istream& { std::string line; NamedColor tmp; if (std::getline(str, line)) { - std::istringstream iss{line}; - /** - * Some locales have a white space as a thousand separator, leading the following parsing to fail. - * We avoid that by parsing in the classic locale. - */ - iss.imbue(std::locale::classic()); - if (iss >> tmp.colorU16.red >> tmp.colorU16.green >> tmp.colorU16.blue && std::getline(iss, tmp.name)) { - if (tmp.colorU16.red > 255 || tmp.colorU16.green > 255 || tmp.colorU16.blue > 255) { + auto iss = serdes_stream(line); + uint16_t r{}, g{}, b{}; + if (iss >> r >> g >> b && std::getline(iss, tmp.name)) { + if (r > 255 || g > 255 || b > 255) { throw std::invalid_argument("RGB values bigger than 255 are not supported."); } + tmp.color = Color(uint8_t(r), uint8_t(g), uint8_t(b)); tmp.name = StringUtils::trim(tmp.name); - tmp.color = Util::colorU16_to_argb(tmp.colorU16); + tmp.colorU16 = Util::argb_to_ColorU16(tmp.color); tmp.isPaletteColor = true; tmp.paletteIndex = namedColor.paletteIndex; // All read operations worked diff --git a/src/util/include/util/serdesstream.h b/src/util/include/util/serdesstream.h new file mode 100644 index 000000000000..b58f832273a8 --- /dev/null +++ b/src/util/include/util/serdesstream.h @@ -0,0 +1,15 @@ +#pragma once + +#include +#include + +/** + * @brief Serdes always requires the same locale. Independently of the local / C++ global locale. + */ + +template +StdStream serdes_stream(Args&&... args) { + StdStream ret(std::forward(args)...); + ret.imbue(std::locale::classic()); + return ret; +} \ No newline at end of file