From 7d0f9283f61ebf81b13182c85e7b5eb6f0c64691 Mon Sep 17 00:00:00 2001 From: tattsan Date: Sat, 21 Oct 2023 21:27:33 +0900 Subject: [PATCH 01/14] Fix long text being truncated in LaTeX tool --- resources/default_template.tex | 9 ++++++++- resources/legacy_template.tex | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/resources/default_template.tex b/resources/default_template.tex index f19a0ca55a91..17011824a13f 100644 --- a/resources/default_template.tex +++ b/resources/default_template.tex @@ -1,5 +1,11 @@ % This template uses the scontents package, which is only available on relatively recent TeX distributions. In case it is not available on your system, use the legacy_template.tex -\documentclass[varwidth=true, crop, border=5pt]{standalone} +\documentclass[varwidth=0.999\maxdimen, crop, border=5pt]{standalone} +% The upper limit value of 'varwidth' can be referenced by \hsize. +\newcommand*{\setTextWidthReference}{% + \setlength{\textwidth}{345.0pt}% Same value when you use 'varwidth=true'. + \setlength{\linewidth}{\textwidth}% + \setlength{\columnwidth}{\textwidth}% +} % Packages \usepackage{amsmath} @@ -25,6 +31,7 @@ \end{scontents} \begin{document} + \setTextWidthReference % Check if the formula is empty \settoheight{\pheight}{\getstored[1]{preview}}% \ifthenelse{\pheight=0}{\GenericError{}{xournalpp:blankformula}{}{}} diff --git a/resources/legacy_template.tex b/resources/legacy_template.tex index a18561ad148a..dc510236aa7f 100644 --- a/resources/legacy_template.tex +++ b/resources/legacy_template.tex @@ -1,5 +1,11 @@ % This template avoids the scontents package, which is only available on relatively recent TeX distributions -\documentclass[varwidth=true, crop, border=5pt]{standalone} +\documentclass[varwidth=0.999\maxdimen, crop, border=5pt]{standalone} +% The upper limit value of 'varwidth' can be referenced by \hsize. +\newcommand*{\setTextWidthReference}{% + \setlength{\textwidth}{345.0pt}% Same value when you use 'varwidth=true'. + \setlength{\linewidth}{\textwidth}% + \setlength{\columnwidth}{\textwidth}% +} % Packages \usepackage{amsmath} @@ -21,6 +27,7 @@ } \begin{document} + \setTextWidthReference % Check if the formula is empty \settoheight{\pheight}{\preview} \ifthenelse{\pheight=0}{\GenericError{}{xournalpp:blankformula}{}{}} From dc4b82d29aa2a0a5cadd21726a27fd2cba905b25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20L=C3=B6tscher?= Date: Mon, 30 Oct 2023 17:03:22 +0100 Subject: [PATCH 02/14] Fix Util::rgb_to_hex_string --- src/util/Color.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/util/Color.cpp b/src/util/Color.cpp index 0a9d0d3b257f..755c4921cfd3 100644 --- a/src/util/Color.cpp +++ b/src/util/Color.cpp @@ -2,9 +2,11 @@ #include // for max, min #include // for assert -#include // for snprintf +#include // for operator<<, setfill, setw #include // for operator<<, stringstream, basic_ostream, char_t... +#include "util/serdesstream.h" // for serdes_stream + float Util::as_grayscale_color(Color color) { GdkRGBA components = rgb_to_GdkRGBA(color); float componentAvg = static_cast(components.red + components.green + components.blue) / 3.0f; @@ -20,12 +22,10 @@ float Util::get_color_contrast(Color color1, Color color2) { } auto Util::rgb_to_hex_string(Color rgb) -> std::string { - char resultHex[7]; - - // 06: Disregard alpha channel and pad with zeroes to a length of 6. - assert(std::snprintf(resultHex, 7, "%06x", uint32_t(rgb) & 0xffffff) > 0); + auto s = serdes_stream(); + auto tmp_color = rgb; + tmp_color.alpha = 0; + s << "#" << std::hex << std::setfill('0') << std::setw(6) << std::right << tmp_color; - std::stringstream result; - result << "#" << resultHex; - return result.str(); + return s.str(); } From c899cec918f2d92454cce6282327aa0536b67ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20L=C3=B6tscher?= Date: Mon, 30 Oct 2023 22:54:26 +0100 Subject: [PATCH 03/14] Use Util::rgb_to_hex_string everywhere --- src/core/control/latex/LatexGenerator.cpp | 11 ++--------- src/core/control/settings/PageTemplateSettings.cpp | 9 ++------- src/core/gui/toolbarMenubar/ColorToolItem.cpp | 1 - 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/core/control/latex/LatexGenerator.cpp b/src/core/control/latex/LatexGenerator.cpp index 6e63382d839c..aae7848f59f6 100644 --- a/src/core/control/latex/LatexGenerator.cpp +++ b/src/core/control/latex/LatexGenerator.cpp @@ -1,8 +1,6 @@ #include "LatexGenerator.h" -#include // for operator<<, setfill, setw -#include // for smatch, sregex_iterator -#include // for ostringstream, basic_ost... +#include // for smatch, sregex_iterator #include // for GError, gchar, g_error_free #include // for g_object_unref @@ -15,7 +13,6 @@ #include "util/raii/GLibGuards.h" // for GErrorGuard, GStrvGuard #include "util/raii/GObjectSPtr.h" // for GObjectSptr #include "util/safe_casts.h" // for as_signed -#include "util/serdesstream.h" // for serdes_stream using namespace xoj::util; @@ -36,11 +33,7 @@ auto LatexGenerator::templateSub(const std::string& input, const std::string& te if (matchStr == "TOOL_INPUT") { repl = input; } else if (matchStr == "TEXT_COLOR") { - 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; - repl = s.str(); + repl = Util::rgb_to_hex_string(textColor).substr(1); } output.append(templ, templatePos, as_unsigned(match.position()) - templatePos); output.append(repl); diff --git a/src/core/control/settings/PageTemplateSettings.cpp b/src/core/control/settings/PageTemplateSettings.cpp index d7de85478050..9259f04bae45 100644 --- a/src/core/control/settings/PageTemplateSettings.cpp +++ b/src/core/control/settings/PageTemplateSettings.cpp @@ -1,9 +1,6 @@ #include "PageTemplateSettings.h" -#include // for PRIx32 -#include // for uint32_t -#include // for snprintf, size_t -#include // for basic_istream, strings... +#include // for basic_istream, strings... #include "control/pagetype/PageTypeHandler.h" // for PageTypeHandler @@ -116,9 +113,7 @@ auto PageTemplateSettings::toString() const -> string { str += string("backgroundTypeConfig=") + backgroundType.config + "\n"; } - char buffer[64]; - snprintf(buffer, sizeof(buffer), "#%06" PRIx32, uint32_t{this->backgroundColor}); - str += string("backgroundColor=") + buffer + "\n"; + str += string("backgroundColor=") + Util::rgb_to_hex_string(this->backgroundColor) + "\n"; return str; } diff --git a/src/core/gui/toolbarMenubar/ColorToolItem.cpp b/src/core/gui/toolbarMenubar/ColorToolItem.cpp index 6fcd1b682085..0d3bff769392 100644 --- a/src/core/gui/toolbarMenubar/ColorToolItem.cpp +++ b/src/core/gui/toolbarMenubar/ColorToolItem.cpp @@ -1,7 +1,6 @@ #include "ColorToolItem.h" #include // for array -#include // for snprintf, size_t #include // for unique_ptr #include // for ostringstream #include // for move From d2f5b34d8a9a73a8efcce80c400addcd27f0efe2 Mon Sep 17 00:00:00 2001 From: Thomas Moerschell <144177788+tmoerschell@users.noreply.github.com> Date: Sun, 29 Oct 2023 17:47:10 +0100 Subject: [PATCH 04/14] Fix window maximized callback and save setting --- src/core/control/settings/Settings.cpp | 8 +++++++- src/core/gui/MainWindow.cpp | 11 +++++++---- src/core/gui/MainWindow.h | 4 ++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/core/control/settings/Settings.cpp b/src/core/control/settings/Settings.cpp index f1444e03437a..e9f3045b72d8 100644 --- a/src/core/control/settings/Settings.cpp +++ b/src/core/control/settings/Settings.cpp @@ -1899,7 +1899,13 @@ auto Settings::getMainWndHeight() const -> int { return this->mainWndHeight; } auto Settings::isMainWndMaximized() const -> bool { return this->maximized; } -void Settings::setMainWndMaximized(bool max) { this->maximized = max; } +void Settings::setMainWndMaximized(bool max) { + if (this->maximized == max) { + return; + } + this->maximized = max; + save(); +} void Settings::setSelectedToolbar(const string& name) { if (this->selectedToolbar == name) { diff --git a/src/core/gui/MainWindow.cpp b/src/core/gui/MainWindow.cpp index f468335543f7..68338ed4e236 100644 --- a/src/core/gui/MainWindow.cpp +++ b/src/core/gui/MainWindow.cpp @@ -79,8 +79,13 @@ MainWindow::MainWindow(GladeSearchpath* gladeSearchPath, Control* control, GtkAp // Window handler g_signal_connect(this->window, "delete-event", xoj::util::wrap_for_g_callback_v, this->control); - g_signal_connect(this->window, "window_state_event", xoj::util::wrap_for_g_callback_v, +#if GTK_MAJOR_VERSION == 3 + g_signal_connect(this->window, "notify::is-maximized", xoj::util::wrap_for_g_callback_v, this); +#else + g_signal_connect(this->window, "notify::maximized", xoj::util::wrap_for_g_callback_v, + this); +#endif g_signal_connect(get("buttonCloseSidebar"), "clicked", xoj::util::wrap_for_g_callback_v, this); @@ -535,10 +540,8 @@ auto MainWindow::isMaximized() const -> bool { return this->maximized; } auto MainWindow::getXournal() const -> XournalView* { return xournal.get(); } -auto MainWindow::windowStateEventCallback(GtkWidget* window, GdkEventWindowState* event, MainWindow* win) -> bool { +auto MainWindow::windowMaximizedCallback(GObject* window, GParamSpec*, MainWindow* win) -> void { win->setMaximized(gtk_window_is_maximized(GTK_WINDOW(window))); - - return false; } void MainWindow::toolbarSelected(const std::string& id) { diff --git a/src/core/gui/MainWindow.h b/src/core/gui/MainWindow.h index 6e9024e1fd29..a2eefe6e059e 100644 --- a/src/core/gui/MainWindow.h +++ b/src/core/gui/MainWindow.h @@ -165,9 +165,9 @@ class MainWindow: public GladeGui, public LayerCtrlListener { static bool deleteEventCallback(GtkWidget* widget, GdkEvent* event, Control* control); /** - * Callback fro window states, we ned to know if the window is fullscreen + * Window is maximized/minimized */ - static bool windowStateEventCallback(GtkWidget* window, GdkEventWindowState* event, MainWindow* win); + static void windowMaximizedCallback(GObject* window, GParamSpec*, MainWindow* win); /** * Callback for drag & drop files From 16e9778da965d7e6f60518c151ab83639958eba0 Mon Sep 17 00:00:00 2001 From: Thomas Moerschell <144177788+tmoerschell@users.noreply.github.com> Date: Mon, 6 Nov 2023 09:31:31 +0100 Subject: [PATCH 05/14] Allow pulling spline tangents outside of page (#5298) --- src/core/control/ToolHandler.cpp | 8 ++++++++ src/core/control/ToolHandler.h | 6 ++++++ src/core/gui/inputdevices/PenInputHandler.cpp | 8 +++++--- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/core/control/ToolHandler.cpp b/src/core/control/ToolHandler.cpp index 5c01fbce1abc..631c09b601ed 100644 --- a/src/core/control/ToolHandler.cpp +++ b/src/core/control/ToolHandler.cpp @@ -583,6 +583,14 @@ auto ToolHandler::isSinglePageTool() const -> bool { toolType == TOOL_SELECT_PDF_TEXT_RECT; } +auto ToolHandler::acceptsOutOfPageEvents() const -> bool { + ToolType toolType = this->getToolType(); + DrawingType drawingType = this->getDrawingType(); + + return ((toolType == TOOL_PEN || toolType == TOOL_HIGHLIGHTER) && (drawingType == DRAWING_TYPE_SPLINE)) || + (toolType == TOOL_DRAW_SPLINE); +} + auto ToolHandler::supportsTapFilter() const -> bool { ToolType toolType = this->getToolType(); diff --git a/src/core/control/ToolHandler.h b/src/core/control/ToolHandler.h index d5b8fecf654e..29209ee7b473 100644 --- a/src/core/control/ToolHandler.h +++ b/src/core/control/ToolHandler.h @@ -345,6 +345,12 @@ class ToolHandler { */ bool isSinglePageTool() const; + /** + * Returns whether the current tool accepts events from outside the page input started in (i.e. events + * should not be clamped to the page) + */ + bool acceptsOutOfPageEvents() const; + /** * @brief Whether the tool supports short taps filtering (for floating toolbox or selection) * see Preferences->Drawing Area->Action on Tool tap diff --git a/src/core/gui/inputdevices/PenInputHandler.cpp b/src/core/gui/inputdevices/PenInputHandler.cpp index 514f5fd1f5b2..1151ecc65769 100644 --- a/src/core/gui/inputdevices/PenInputHandler.cpp +++ b/src/core/gui/inputdevices/PenInputHandler.cpp @@ -317,9 +317,11 @@ auto PenInputHandler::actionMotion(InputEvent const& event) -> bool { // Relay the event to the page PositionInputData pos = getInputDataRelativeToCurrentPage(sequenceStartPage, event); - // Enforce input to stay within page - pos.x = std::clamp(pos.x, 0.0, static_cast(sequenceStartPage->getDisplayWidth())); - pos.y = std::clamp(pos.y, 0.0, static_cast(sequenceStartPage->getDisplayHeight())); + if (!toolHandler->acceptsOutOfPageEvents()) { + // Enforce input to stay within page + pos.x = std::clamp(pos.x, 0.0, static_cast(sequenceStartPage->getDisplayWidth())); + pos.y = std::clamp(pos.y, 0.0, static_cast(sequenceStartPage->getDisplayHeight())); + } pos.pressure = this->filterPressure(pos, sequenceStartPage); From 77e296c9851c7af481284eef8989d558d3ce420c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20L=C3=B6tscher?= Date: Sun, 5 Nov 2023 14:57:46 +0100 Subject: [PATCH 06/14] Fix PROJECT_HOMEPAGE_URL Instead of Xournal++ document - see https://xournalpp.github.io/ we had Xournal++ document - see written into all xopp-files --- src/config.h.in | 2 +- src/core/control/xojfile/SaveHandler.cpp | 2 +- src/core/control/xojfile/XojExportHandler.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config.h.in b/src/config.h.in index 974ae91026dd..49e0c8ee3c0e 100644 --- a/src/config.h.in +++ b/src/config.h.in @@ -36,7 +36,7 @@ constexpr auto PROJECT_STRING = "@PROJECT_NAME@ @PROJECT_VERSION@"; /** * Project URL */ -constexpr auto PROJECT_URL = "@PROJECT_URL@"; +constexpr auto PROJECT_HOMEPAGE_URL = "@PROJECT_HOMEPAGE_URL@"; /** * File format version diff --git a/src/core/control/xojfile/SaveHandler.cpp b/src/core/control/xojfile/SaveHandler.cpp index d7d6d662ab8d..f16a52ccdb42 100644 --- a/src/core/control/xojfile/SaveHandler.cpp +++ b/src/core/control/xojfile/SaveHandler.cpp @@ -78,7 +78,7 @@ void SaveHandler::prepareSave(Document* doc) { void SaveHandler::writeHeader() { this->root->setAttrib("creator", PROJECT_STRING); this->root->setAttrib("fileversion", FILE_FORMAT_VERSION); - this->root->addChild(new XmlTextNode("title", std::string{"Xournal++ document - see "} + PROJECT_URL)); + this->root->addChild(new XmlTextNode("title", std::string{"Xournal++ document - see "} + PROJECT_HOMEPAGE_URL)); } auto SaveHandler::getColorStr(Color c, unsigned char alpha) -> std::string { diff --git a/src/core/control/xojfile/XojExportHandler.cpp b/src/core/control/xojfile/XojExportHandler.cpp index b9a5e7005741..3a392299a4d1 100644 --- a/src/core/control/xojfile/XojExportHandler.cpp +++ b/src/core/control/xojfile/XojExportHandler.cpp @@ -33,7 +33,7 @@ void XojExportHandler::writeHeader() { // Keep this version on 2, as this is anyway not read by Xournal this->root->setAttrib("fileversion", "2"); this->root->addChild( - new XmlTextNode("title", std::string{"Xournal document (Compatibility) - see "} + PROJECT_URL)); + new XmlTextNode("title", std::string{"Xournal document (Compatibility) - see "} + PROJECT_HOMEPAGE_URL)); } void XojExportHandler::writeSolidBackground(XmlNode* background, PageRef p) { From 0d97d7145fce1aab0d9bf95ff27d00d77576ede4 Mon Sep 17 00:00:00 2001 From: Thomas Moerschell <144177788+tmoerschell@users.noreply.github.com> Date: Thu, 2 Nov 2023 17:38:58 +0100 Subject: [PATCH 07/14] Improve backtrace --- src/core/control/CrashHandlerUnix.h | 2 +- src/util/CMakeLists.txt | 2 +- src/util/Stacktrace.cpp | 21 ++++++++++++----- src/util/include/util/safe_casts.h | 35 +++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/core/control/CrashHandlerUnix.h b/src/core/control/CrashHandlerUnix.h index 09d6eaf15ad5..dda77773a459 100644 --- a/src/core/control/CrashHandlerUnix.h +++ b/src/core/control/CrashHandlerUnix.h @@ -150,7 +150,7 @@ static void crashHandler(int sig) { free(messages); - fp << "\n\nTry to get a better stracktrace...\n"; + fp << "\n\nTry to get a better stacktrace...\n"; Stacktrace::printStracktrace(fp); diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 52dc2e8e579b..c497be6f9d05 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -8,7 +8,7 @@ file(GLOB_RECURSE util_SOURCES *.cpp *.h) add_library(util STATIC ${util_SOURCES}) add_dependencies(util std::filesystem) target_link_libraries(util - PUBLIC xoj::defaults xoj::external_modules std::filesystem cxx17 + PUBLIC xoj::defaults xoj::external_modules std::filesystem cxx17 ${CMAKE_DL_LIBS} ) target_compile_features(util PUBLIC ${PROJECT_CXX_FEATURES}) target_include_directories(util PUBLIC $) diff --git a/src/util/Stacktrace.cpp b/src/util/Stacktrace.cpp index 0cfff327fe05..c33b7f2cd739 100644 --- a/src/util/Stacktrace.cpp +++ b/src/util/Stacktrace.cpp @@ -2,14 +2,18 @@ #include // for max #include // for array +#include // for uintptr_t #include // for fgets, pclose, popen, snprintf, FILE #include // for operator<<, basic_ostream, basic_ostream::... #include // for string +#include "util/safe_casts.h" // for bit_cast + #ifdef _WIN32 #include #else +#include // for dladdr #include // for backtrace, backtrace_symbols #include // for readlink, ssize_t @@ -90,21 +94,28 @@ void Stacktrace::printStracktrace(std::ostream& stream) { int trace_size = backtrace(trace.data(), trace.size()); char** messages = backtrace_symbols(trace.data(), trace_size); - std::string exeName = getExePath(); - // skip first stack frame (points here) - for (int i = 1; i < trace_size; ++i) { - stream << "[bt] #" << i << " " << messages[i] << endl; + for (unsigned int i = 1; i < trace_size; ++i) { + stream << "[bt] #" << i - 1 << " " << messages[i] << endl; + + Dl_info info; // NOLINT(cppcoreguidelines-init-variables) + dladdr(trace[i], &info); std::array syscom{}; - snprintf(syscom.data(), syscom.size(), "addr2line %p -e %s", trace[i], exeName.c_str()); + // Todo (cpp20): use std::format instead of snprintf + snprintf(syscom.data(), syscom.size(), "addr2line %lx -fCpe \"%s\"", + xoj::util::bit_cast(trace[i]) - xoj::util::bit_cast(info.dli_fbase), + info.dli_fname); + FILE* fProc = popen(syscom.data(), "r"); while (fgets(buff.data(), buff.size(), fProc) != nullptr) { stream << buff.data(); } pclose(fProc); } + + free(messages); } #endif diff --git a/src/util/include/util/safe_casts.h b/src/util/include/util/safe_casts.h index fe09566a5e3e..6df79b59db26 100644 --- a/src/util/include/util/safe_casts.h +++ b/src/util/include/util/safe_casts.h @@ -35,6 +35,7 @@ #include #include + template [[maybe_unused]] [[nodiscard]] constexpr auto is_safely_castable(From from) -> bool { using Big = decltype(from * std::declval()); @@ -99,6 +100,40 @@ inline auto floor_cast(Float f) -> Integral { return rv1; } +#if defined __has_include && !defined(XOJ_USE_STD_BIT_CAST) +#if __has_include() +#include +#if defined(__cpp_lib_bit_cast) && __cpp_lib_bit_cast >= 201806L +#define XOJ_USE_STD_BIT_CAST 1 +#endif +#endif +#endif + +#if defined(XOJ_USE_STD_BIT_CAST) && XOJ_USE_STD_BIT_CAST == 1 +namespace xoj::util { +using std::bit_cast; +} // namespace xoj::util +#else +#include + +namespace xoj::util { +/** + * A backport from C++20 of std::bit_cast() + */ +template +inline std::enable_if_t< + sizeof(To) == sizeof(From) && std::is_trivially_copyable_v && std::is_trivially_copyable_v, To> + // constexpr support needs compiler magic + bit_cast(const From& src) noexcept { + static_assert(std::is_trivially_constructible_v, + "backport of bit_cast requires destination type to be trivially constructible"); + To dst; + std::memcpy(&dst, &src, sizeof(To)); + return dst; +} +} // namespace xoj::util +#endif + // static_assert(is_safely_castable(std::numeric_limits::max()) == false); // static_assert(is_safely_castable(std::numeric_limits::min()) == false); // static_assert(is_safely_castable(std::numeric_limits::min()) == false); From 1cbc67fe9531fca871b347c99d48b9b0bad6f7e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20L=C3=B6tscher?= Date: Thu, 16 Nov 2023 17:22:00 +0100 Subject: [PATCH 08/14] Make .deb package build in RelWithDebInfo --- debian/rules | 3 +++ 1 file changed, 3 insertions(+) diff --git a/debian/rules b/debian/rules index 2280e9512184..6a3753b21013 100755 --- a/debian/rules +++ b/debian/rules @@ -10,3 +10,6 @@ override_dh_auto_build: override_dh_dwz: dh_dwz --no-dwz-multifile + +override_dh_auto_configure: + dh_auto_configure -- -DCMAKE_BUILD_TYPE=RelWithDebInfo From a5497b1892b53018f4ef990e8771f13990a154cf Mon Sep 17 00:00:00 2001 From: Benjamin Hennion <17818041+bhennion@users.noreply.github.com> Date: Fri, 17 Nov 2023 12:03:41 +0100 Subject: [PATCH 09/14] Ensure repainted region is non-empty in PdfElemSelection --- src/core/control/tools/PdfElemSelection.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/control/tools/PdfElemSelection.cpp b/src/core/control/tools/PdfElemSelection.cpp index a0475fdd7463..621231f2dc2b 100644 --- a/src/core/control/tools/PdfElemSelection.cpp +++ b/src/core/control/tools/PdfElemSelection.cpp @@ -47,7 +47,9 @@ auto PdfElemSelection::finalizeSelectionAndRepaint(XojPdfPageSelectionStyle styl Range rg = getRegionBbox(); bool result = this->finalizeSelection(style); rg = rg.unite(getRegionBbox()); - this->viewPool->dispatch(xoj::view::PdfElementSelectionView::FLAG_DIRTY_REGION_REQUEST, rg); + if (!rg.empty()) { + this->viewPool->dispatch(xoj::view::PdfElementSelectionView::FLAG_DIRTY_REGION_REQUEST, rg); + } return result; } @@ -110,7 +112,9 @@ void PdfElemSelection::currentPos(double x, double y, XojPdfPageSelectionStyle s g_assert(this->selectedTextRegion); rg = rg.unite(getRegionBbox()); - this->viewPool->dispatch(xoj::view::PdfElementSelectionView::FLAG_DIRTY_REGION_REQUEST, rg); + if (!rg.empty()) { + this->viewPool->dispatch(xoj::view::PdfElementSelectionView::FLAG_DIRTY_REGION_REQUEST, rg); + } } auto PdfElemSelection::contains(double x, double y) -> bool { From a90a7be85bdd9f740e2082c57806acf3350db1b7 Mon Sep 17 00:00:00 2001 From: Thomas Moerschell <144177788+tmoerschell@users.noreply.github.com> Date: Sat, 25 Nov 2023 08:11:23 +0100 Subject: [PATCH 10/14] Fix stroke replacement --- src/core/view/overlays/StrokeToolView.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/view/overlays/StrokeToolView.cpp b/src/core/view/overlays/StrokeToolView.cpp index efaca1d84019..4451f5c23bac 100644 --- a/src/core/view/overlays/StrokeToolView.cpp +++ b/src/core/view/overlays/StrokeToolView.cpp @@ -103,7 +103,10 @@ void StrokeToolView::deleteOn(StrokeToolView::CancellationRequest, const Range& } void StrokeToolView::on(StrokeToolView::StrokeReplacementRequest, const Stroke& newStroke) { - this->mask.wipe(); + if (this->mask.isInitialized()) { + // only wipe mask it actually exists (the view has already been drawn at least once) + this->mask.wipe(); + } this->pointBuffer = newStroke.getPointVector(); this->dashOffset = 0; this->strokeWidth = newStroke.getWidth(); From bd167e29c9d48a16fc4c48c28c688e98ecc04124 Mon Sep 17 00:00:00 2001 From: Benjamin Hennion <17818041+bhennion@users.noreply.github.com> Date: Fri, 24 Nov 2023 14:45:03 +0100 Subject: [PATCH 11/14] Add error checks for saving operations --- src/core/control/Control.cpp | 68 +++++++---------------- src/core/control/Control.h | 3 +- src/core/control/jobs/AutosaveJob.cpp | 24 ++++++-- src/core/control/xml/XmlStrokeNode.cpp | 70 ------------------------ src/core/control/xml/XmlStrokeNode.h | 37 ------------- src/core/control/xojfile/SaveHandler.cpp | 8 ++- src/util/OutputStream.cpp | 44 +++++++++++++-- src/util/include/util/OutputStream.h | 12 ++-- 8 files changed, 87 insertions(+), 179 deletions(-) delete mode 100644 src/core/control/xml/XmlStrokeNode.cpp delete mode 100644 src/core/control/xml/XmlStrokeNode.h diff --git a/src/core/control/Control.cpp b/src/core/control/Control.cpp index 4c8cc3e87b90..5d9f93b38096 100644 --- a/src/core/control/Control.cpp +++ b/src/core/control/Control.cpp @@ -175,7 +175,7 @@ Control::~Control() { g_source_remove(this->changeTimout); this->enableAutosave(false); - deleteLastAutosaveFile(""); + deleteLastAutosaveFile(); this->scheduler->stop(); this->changedPages.clear(); // can be removed, will be done by implicit destructor @@ -219,55 +219,29 @@ Control::~Control() { this->fullscreenHandler = nullptr; } - -void Control::renameLastAutosaveFile() { - if (this->lastAutosaveFilename.empty()) { - return; - } - - auto const& filename = this->lastAutosaveFilename; - auto renamed = Util::getAutosaveFilepath(); - Util::clearExtensions(renamed); - if (!filename.empty() && filename.string().front() != '.') { - // This file must be a fresh, unsaved document. Since this file is - // already in the autosave directory, we need to change the renamed filename. - renamed += ".old.autosave.xopp"; - } else { - // The file is a saved document with the form "..autosave.xopp" - renamed += filename.filename(); - } - - g_message("%s", FS(_F("Autosave renamed from {1} to {2}") % this->lastAutosaveFilename.string() % renamed.string()) - .c_str()); - - if (!fs::exists(filename)) { - this->save(false); - } - - std::vector errors; +void Control::setLastAutosaveFile(fs::path newAutosaveFile) { try { - Util::safeRenameFile(filename, renamed); + if (!fs::equivalent(newAutosaveFile, this->lastAutosaveFilename) && fs::exists(newAutosaveFile)) { + deleteLastAutosaveFile(); + } } catch (const fs::filesystem_error& e) { - auto fmtstr = _F("Could not rename autosave file from \"{1}\" to \"{2}\": {3}"); - errors.emplace_back(FS(fmtstr % filename.u8string() % renamed.u8string() % e.what())); - } - - - if (!errors.empty()) { - string error = std::accumulate(errors.begin() + 1, errors.end(), *errors.begin(), - [](const string& e1, const string& e2) { return e1 + "\n" + e2; }); - Util::execInUiThread([=]() { - string msg = FS(_F("Autosave failed with an error: {1}") % error); - XojMsgBox::showErrorToUser(getGtkWindow(), msg); - }); + auto fmtstr = FS(_F("Filesystem error: {2}") % e.what()); + Util::execInUiThread([fmtstr, win = getGtkWindow()]() { XojMsgBox::showErrorToUser(win, fmtstr); }); } + this->lastAutosaveFilename = std::move(newAutosaveFile); } -void Control::setLastAutosaveFile(fs::path newAutosaveFile) { this->lastAutosaveFilename = std::move(newAutosaveFile); } - -void Control::deleteLastAutosaveFile(fs::path newAutosaveFile) { - fs::remove(this->lastAutosaveFilename); - this->lastAutosaveFilename = std::move(newAutosaveFile); +void Control::deleteLastAutosaveFile() { + try { + if (fs::exists(this->lastAutosaveFilename)) { + fs::remove(this->lastAutosaveFilename); + } + } catch (const fs::filesystem_error& e) { + auto fmtstr = FS(_F("Could not remove old autosave file \"{1}\": {2}") % this->lastAutosaveFilename.string() % + e.what()); + Util::execInUiThread([fmtstr, win = getGtkWindow()]() { XojMsgBox::showErrorToUser(win, fmtstr); }); + } + this->lastAutosaveFilename.clear(); } auto Control::checkChangedDocument(Control* control) -> bool { @@ -361,10 +335,6 @@ auto Control::autosaveCallback(Control* control) -> bool { return true; } - - g_message("Info: autosave document..."); - - auto* job = new AutosaveJob(control); control->scheduler->addJob(job, JOB_PRIORITY_NONE); job->unref(); diff --git a/src/core/control/Control.h b/src/core/control/Control.h index 1c8553f1ca43..ac3c5ab4f8e4 100644 --- a/src/core/control/Control.h +++ b/src/core/control/Control.h @@ -252,9 +252,8 @@ class Control: void block(const std::string& name); void unblock(); - void renameLastAutosaveFile(); void setLastAutosaveFile(fs::path newAutosaveFile); - void deleteLastAutosaveFile(fs::path newAutosaveFile); + void deleteLastAutosaveFile(); void setClipboardHandlerSelection(EditSelection* selection); void addChangedDocumentListener(DocumentListener* dl); diff --git a/src/core/control/jobs/AutosaveJob.cpp b/src/core/control/jobs/AutosaveJob.cpp index 3cc396d33b20..07cc57aca530 100644 --- a/src/core/control/jobs/AutosaveJob.cpp +++ b/src/core/control/jobs/AutosaveJob.cpp @@ -43,18 +43,32 @@ void AutosaveJob::run() { Util::clearExtensions(filepath); filepath += ".autosave.xopp"; - control->renameLastAutosaveFile(); - g_message("%s", FS(_F("Autosaving to {1}") % filepath.string()).c_str()); - handler.saveTo(filepath); + fs::path tempfile = filepath; + tempfile += u8"~"; + handler.saveTo(tempfile); this->error = handler.getErrorMessage(); if (!this->error.empty()) { callAfterRun(); } else { - // control->deleteLastAutosaveFile(filepath); - control->setLastAutosaveFile(filepath); + try { + if (fs::exists(filepath)) { + fs::path swaptmpfile = filepath; + swaptmpfile += u8".swap"; + Util::safeRenameFile(filepath, swaptmpfile); + Util::safeRenameFile(tempfile, filepath); + // All went well, we can delete the old autosave file + fs::remove(swaptmpfile); + } else { + Util::safeRenameFile(tempfile, filepath); + } + control->setLastAutosaveFile(filepath); + } catch (const fs::filesystem_error& e) { + auto fmtstr = _F("Could not rename autosave file from \"{1}\" to \"{2}\": {3}"); + this->error = FS(fmtstr % tempfile.u8string() % filepath.u8string() % e.what()); + } } } diff --git a/src/core/control/xml/XmlStrokeNode.cpp b/src/core/control/xml/XmlStrokeNode.cpp deleted file mode 100644 index ce2f690097f2..000000000000 --- a/src/core/control/xml/XmlStrokeNode.cpp +++ /dev/null @@ -1,70 +0,0 @@ -#include "XmlStrokeNode.h" - -#include // for g_ascii_formatd, G_ASCII_DTOSTR_BUF... - -#include "control/xml/XmlNode.h" // for XmlNode -#include "model/Point.h" // for Point -#include "util/OutputStream.h" // for OutputStream -#include "util/Util.h" // for writeCoordinateString, PRECISION_FO... - -XmlStrokeNode::XmlStrokeNode(const char* tag): XmlNode(tag) { - this->points = nullptr; - this->pointsLength = 0; - this->width = 0; - this->widths = nullptr; - this->widthsLength = 0; -} - -XmlStrokeNode::~XmlStrokeNode() { - delete[] this->points; - delete[] this->widths; -} - -void XmlStrokeNode::setWidth(double width, const double* widths, int widthsLength) { - this->width = width; - - - delete[] this->widths; - - this->widths = new double[widthsLength]; - for (int i = 0; i < widthsLength; i++) { this->widths[i] = widths[i]; } - this->widthsLength = widthsLength; -} - -void XmlStrokeNode::writeOut(OutputStream* out) { - out->write("<"); - out->write(tag); - writeAttributes(out); - - out->write(" width=\""); - - char widthStr[G_ASCII_DTOSTR_BUF_SIZE]; - // g_ascii_ version uses C locale always. - g_ascii_formatd(widthStr, G_ASCII_DTOSTR_BUF_SIZE, Util::PRECISION_FORMAT_STRING, width); - out->write(widthStr); - - for (int i = 0; i < widthsLength; i++) { - g_ascii_formatd(widthStr, G_ASCII_DTOSTR_BUF_SIZE, Util::PRECISION_FORMAT_STRING, widths[i]); - out->write(" "); - out->write(widthStr); - } - - out->write("\""); - - if (this->pointsLength == 0) { - out->write("/>"); - } else { - out->write(">"); - - Util::writeCoordinateString(out, points[0].x, points[0].y); - - for (int i = 1; i < this->pointsLength; i++) { - out->write(" "); - Util::writeCoordinateString(out, points[i].x, points[i].y); - } - - out->write("write(tag); - out->write(">\n"); - } -} diff --git a/src/core/control/xml/XmlStrokeNode.h b/src/core/control/xml/XmlStrokeNode.h deleted file mode 100644 index 225760966241..000000000000 --- a/src/core/control/xml/XmlStrokeNode.h +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Xournal++ - * - * XML Writer helper class - * - * @author Xournal++ Team - * https://github.com/xournalpp/xournalpp - * - * @license GNU GPLv2 or later - */ - -#pragma once - -#include "XmlNode.h" // for XmlNode - -class OutputStream; -class Point; - -class XmlStrokeNode: public XmlNode { -public: - XmlStrokeNode(const char* tag); - virtual ~XmlStrokeNode(); - -public: - void setWidth(double width, const double* widths, int widthsLength); - - void writeOut(OutputStream* out) override; - -private: - Point* points; - int pointsLength; - - double width; - - double* widths; - int widthsLength; -}; diff --git a/src/core/control/xojfile/SaveHandler.cpp b/src/core/control/xojfile/SaveHandler.cpp index f16a52ccdb42..6c8d15bd11d3 100644 --- a/src/core/control/xojfile/SaveHandler.cpp +++ b/src/core/control/xojfile/SaveHandler.cpp @@ -89,9 +89,11 @@ auto SaveHandler::getColorStr(Color c, unsigned char alpha) -> std::string { } void SaveHandler::writeTimestamp(AudioElement* audioElement, XmlAudioNode* xmlAudioNode) { - /** set stroke timestamp value to the XmlPointNode */ - xmlAudioNode->setAttrib("ts", audioElement->getTimestamp()); - xmlAudioNode->setAttrib("fn", audioElement->getAudioFilename().u8string()); + if (!audioElement->getAudioFilename().empty()) { + /** set stroke timestamp value to the XmlPointNode */ + xmlAudioNode->setAttrib("ts", audioElement->getTimestamp()); + xmlAudioNode->setAttrib("fn", audioElement->getAudioFilename().u8string()); + } } void SaveHandler::visitStroke(XmlPointNode* stroke, Stroke* s) { diff --git a/src/util/OutputStream.cpp b/src/util/OutputStream.cpp index e67e66b4f835..4e93cc8656bb 100644 --- a/src/util/OutputStream.cpp +++ b/src/util/OutputStream.cpp @@ -1,10 +1,13 @@ #include "util/OutputStream.h" +#include +#include #include // for strlen #include // for move #include "util/GzUtil.h" // for GzUtil #include "util/i18n.h" // for FS, _F +#include "util/safe_casts.h" OutputStream::OutputStream() = default; @@ -12,7 +15,7 @@ OutputStream::~OutputStream() = default; void OutputStream::write(const std::string& str) { write(str.c_str(), str.length()); } -void OutputStream::write(const char* str) { write(str, strlen(str)); } +void OutputStream::write(const char* str) { write(str, std::strlen(str)); } //////////////////////////////////////////////////////// /// GzOutputStream ///////////////////////////////////// @@ -22,6 +25,7 @@ GzOutputStream::GzOutputStream(fs::path file): file(std::move(file)) { this->fp = GzUtil::openPath(this->file, "w"); if (this->fp == nullptr) { this->error = FS(_F("Error opening file: \"{1}\"") % this->file.u8string()); + this->error = this->error + "\n" + std::strerror(errno); } } @@ -32,13 +36,41 @@ GzOutputStream::~GzOutputStream() { this->fp = nullptr; } -auto GzOutputStream::getLastError() -> std::string& { return this->error; } +auto GzOutputStream::getLastError() const -> const std::string& { return this->error; } -void GzOutputStream::write(const char* data, int len) { gzwrite(this->fp, data, len); } +void GzOutputStream::write(const char* data, size_t len) { + assert(len != 0 && this->fp); + auto written = gzwrite(this->fp, data, strict_cast(len)); + if (as_unsigned(written) != len) { + int errnum = 0; + const char* error = gzerror(this->fp, &errnum); + if (errnum != Z_OK) { + this->error = FS(_F("Error writing data to file: \"{1}\"") % this->file.u8string()); + this->error += "\n" + FS(_F("Error code {1}. Message:") % errnum) + "\n"; + if (errnum == Z_ERRNO) { + // fs error. Fetch the precise message + this->error += std::strerror(errno); + } else { + this->error += error; + } + } + } +} void GzOutputStream::close() { - if (this->fp) { - gzclose(this->fp); - this->fp = nullptr; + if (!this->fp) { + return; + } + + auto errnum = gzclose(this->fp); + this->fp = nullptr; + + if (errnum != Z_OK) { + this->error = FS(_F("Error occurred while closing file: \"{1}\"") % this->file.u8string()); + this->error += "\n" + FS(_F("Error code {1}") % errnum); + if (errnum == Z_ERRNO) { + // fs error. Fetch the precise message + this->error = this->error + "\n" + std::strerror(errno); + } } } diff --git a/src/util/include/util/OutputStream.h b/src/util/include/util/OutputStream.h index ebf3679ce96b..9c2a2f14333d 100644 --- a/src/util/include/util/OutputStream.h +++ b/src/util/include/util/OutputStream.h @@ -23,10 +23,10 @@ class OutputStream { virtual ~OutputStream(); public: - virtual void write(const char* str); - virtual void write(const char* data, int len) = 0; - virtual void write(const std::string& str); + void write(const char* str); + void write(const std::string& str); + virtual void write(const char* data, size_t len) = 0; virtual void close() = 0; }; @@ -36,17 +36,15 @@ class GzOutputStream: public OutputStream { ~GzOutputStream() override; public: - void write(const char* data, int len) override; + void write(const char* data, size_t len) override; void close() override; - std::string& getLastError(); + const std::string& getLastError() const; private: gzFile fp = nullptr; std::string error; - - std::string target; fs::path file; }; From 7c315dd874e161f181439f44d14bfb2b252f3be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20L=C3=B6tscher?= Date: Thu, 7 Dec 2023 14:42:05 +0100 Subject: [PATCH 12/14] Ensure non-empty repaint range in SearchResultView --- src/core/view/overlays/SearchResultView.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/view/overlays/SearchResultView.cpp b/src/core/view/overlays/SearchResultView.cpp index db6f1f139c30..e33550ba2d69 100644 --- a/src/core/view/overlays/SearchResultView.cpp +++ b/src/core/view/overlays/SearchResultView.cpp @@ -31,5 +31,8 @@ void SearchResultView::draw(cairo_t* cr) const { bool SearchResultView::isViewOf(const OverlayBase* overlay) const { return overlay == this->searchControl; } void SearchResultView::on(SearchResultView::SearchChangedNotification) { - this->parent->flagDirtyRegion(this->parent->getVisiblePart()); + Range rg = this->parent->getVisiblePart(); + if (!rg.empty()) { + this->parent->flagDirtyRegion(this->parent->getVisiblePart()); + } } From ac00297cc6e4f30c76d0a9fb81e7e2a6ba6ceea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20L=C3=B6tscher?= Date: Thu, 7 Dec 2023 15:48:41 +0100 Subject: [PATCH 13/14] Ensure valid repaint range in VerticalToolView --- src/core/view/overlays/VerticalToolView.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/view/overlays/VerticalToolView.cpp b/src/core/view/overlays/VerticalToolView.cpp index 15b12cfe4bfb..9ad1d864c08b 100644 --- a/src/core/view/overlays/VerticalToolView.cpp +++ b/src/core/view/overlays/VerticalToolView.cpp @@ -63,9 +63,9 @@ void VerticalToolView::on(VerticalToolView::SetVerticalShiftRequest, double shif // Padding for taking into account the drawing aid line width const double padding = 0.5 * BORDER_WIDTH_IN_PIXELS / this->parent->getZoom(); if (side == VerticalToolHandler::Side::Above) { - rg.maxY = std::min(std::max(shift, this->lastShift) + padding, rg.maxY); + rg.maxY = std::clamp(std::max(shift, this->lastShift) + padding, rg.minY, rg.maxY); } else { - rg.minY = std::max(std::min(shift, this->lastShift) - padding, rg.minY); + rg.minY = std::clamp(std::min(shift, this->lastShift) - padding, rg.minY, rg.maxY); } this->parent->flagDirtyRegion(rg); this->lastShift = shift; From 592b2763e944fb5401771b636593d6efe8e24f6b Mon Sep 17 00:00:00 2001 From: Thomas Moerschell <144177788+tmoerschell@users.noreply.github.com> Date: Mon, 11 Dec 2023 18:38:24 +0100 Subject: [PATCH 14/14] Fix wrongly sized loading pages --- src/core/gui/PageView.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/core/gui/PageView.cpp b/src/core/gui/PageView.cpp index a8ef341dce14..a18c8e372fdd 100644 --- a/src/core/gui/PageView.cpp +++ b/src/core/gui/PageView.cpp @@ -871,16 +871,10 @@ auto XojPageView::actionDelete() -> bool { void XojPageView::drawLoadingPage(cairo_t* cr) { static const string txtLoading = _("Loading..."); - double zoom = xournal->getZoom(); - int dispWidth = getDisplayWidth(); - int dispHeight = getDisplayHeight(); - cairo_set_source_rgb(cr, 1, 1, 1); - cairo_rectangle(cr, 0, 0, dispWidth, dispHeight); + cairo_rectangle(cr, 0, 0, page->getWidth(), page->getHeight()); cairo_fill(cr); - cairo_scale(cr, zoom, zoom); - cairo_set_source_rgb(cr, 0.5, 0.5, 0.5); cairo_select_font_face(cr, "Sans", CAIRO_FONT_SLANT_NORMAL, CAIRO_FONT_WEIGHT_BOLD); cairo_set_font_size(cr, 32.0);