Skip to content

Commit

Permalink
Fixed bug in serdes of Named color;
Browse files Browse the repository at this point in the history
ColorU8 was read into ColorU16
Replaced some calls to imbue with an appropriate call to an template function.
Added some imbues to serdesstreams.
  • Loading branch information
Febbe committed Nov 29, 2021
1 parent 9670b86 commit 4b74198
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 52 deletions.
4 changes: 2 additions & 2 deletions src/core/control/latex/LatexGenerator.cpp
Expand Up @@ -9,6 +9,7 @@

#include "util/PathUtil.h"
#include "util/i18n.h"
#include "util/serdesstream.h"

LatexGenerator::LatexGenerator(const LatexSettings& settings): settings(settings) {}

Expand All @@ -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<std::ostringstream>();
auto tmp_color = textColor;
tmp_color.alpha = 0;
s << std::hex << std::setfill('0') << std::setw(6) << std::right << tmp_color;
Expand Down
5 changes: 2 additions & 3 deletions src/core/gui/toolbarMenubar/FontButton.cpp
Expand Up @@ -7,6 +7,7 @@
#include <config.h>

#include "util/i18n.h"
#include "util/serdesstream.h"

using std::string;

Expand All @@ -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<std::stringstream>();
fontSizeStream << font.getSize();
string name = font.getName() + " " + fontSizeStream.str();
gtk_font_button_set_font_name(button, name.c_str());
Expand Down
68 changes: 32 additions & 36 deletions src/core/gui/toolbarMenubar/model/ColorPalette.cpp
Expand Up @@ -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))
Expand All @@ -20,7 +21,8 @@ void Palette::load() {
header.clear();
namedColors.clear();

std::ifstream gplFile{filepath};

auto gplFile = serdes_stream<std::ifstream>(filepath);
std::string line;

if (gplFile.is_open()) {
Expand Down Expand Up @@ -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));
Expand All @@ -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<std::stringstream>();
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<std::ofstream>(filepath);
myfile << default_palette();
}

Expand Down Expand Up @@ -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());
}
4 changes: 2 additions & 2 deletions src/core/pdf/base/XojCairoPdfExport.cpp
Expand Up @@ -9,6 +9,7 @@

#include "util/Util.h"
#include "util/i18n.h"
#include "util/serdesstream.h"
#include "view/DocumentView.h"

#include "filesystem.h"
Expand Down Expand Up @@ -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<std::ostringstream>();
linkAttrBuf << "page=" << pageDest + 1;
if (dest->shouldChangeLeft() && dest->shouldChangeTop()) {
linkAttrBuf << " pos=[" << dest->getLeft() << " " << dest->getTop() << "]";
Expand Down
18 changes: 9 additions & 9 deletions src/util/NamedColor.cpp
@@ -1,8 +1,11 @@
#include "util/NamedColor.h"

#include <cstdint>
#include <sstream>

#include "util/StringUtils.h"
#include "util/serdesstream.h"


NamedColor::NamedColor():
paletteIndex{0}, name{"Custom Color"}, colorU16{ColorU16{}}, color{Color(0u)}, isPaletteColor{false} {}
Expand All @@ -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<std::istringstream>(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
Expand Down
15 changes: 15 additions & 0 deletions src/util/include/util/serdesstream.h
@@ -0,0 +1,15 @@
#pragma once

#include <locale>
#include <utility>

/**
* @brief Serdes always requires the same locale. Independently of the local / C++ global locale.
*/

template <typename StdStream, typename... Args>
StdStream serdes_stream(Args&&... args) {
StdStream ret(std::forward<Args>(args)...);
ret.imbue(std::locale::classic());
return ret;
}

0 comments on commit 4b74198

Please sign in to comment.